Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resources: remove intersect-resources experiment #33262

Merged
merged 3 commits into from
Mar 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion build-system/global-configs/canary-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
"dfp-render-on-idle-cwv-exp": 1,
"expand-json-targeting": 1,
"flexAdSlots": 0.05,
"intersect-resources": 0,
"ios-fixed-no-transfer": 1,
"visibility-trigger-improvements": 1,
"ads-initialIntersection": 1,
Expand Down
1 change: 0 additions & 1 deletion build-system/global-configs/prod-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
"doubleclickSraReportExcludedBlock": 0.1,
"expand-json-targeting": 1,
"flexAdSlots": 0.05,
"intersect-resources": 0,
"ios-fixed-no-transfer": 0,
"visibility-trigger-improvements": 1,
"layout-aspect-ratio-css": 0,
Expand Down
5 changes: 0 additions & 5 deletions src/inabox/inabox-resources.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,11 +201,6 @@ export class InaboxResources {
return this.firstPassDone_.promise;
}

/** @override */
isIntersectionExperimentOn() {
return false;
}

/**
* @private
*/
Expand Down
37 changes: 10 additions & 27 deletions src/service/mutator-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {Services} from '../services';
import {areMarginsChanged} from '../layout-rect';
import {closest} from '../dom';
import {computedStyle} from '../style';
import {dev, devAssert} from '../log';
import {dev} from '../log';
import {isExperimentOn} from '../experiments';
import {registerServiceBuilderForDoc} from '../service';

Expand Down Expand Up @@ -58,9 +58,6 @@ export class MutatorImpl {
this.activeHistory_.onFocus((element) => {
this.checkPendingChangeSize_(element);
});

/** @private @const {boolean} */
this.intersect_ = this.resources_.isIntersectionExperimentOn();
}

/** @override */
Expand Down Expand Up @@ -129,16 +126,12 @@ export class MutatorImpl {

/** @override */
collapseElement(element) {
// With IntersectionObserver, no need to relayout or remeasure
// due to a single element collapse (similar to "relayout top").
if (!this.intersect_) {
const box = this.viewport_.getLayoutRect(element);
if (box.width != 0 && box.height != 0) {
if (isExperimentOn(this.win, 'dirty-collapse-element')) {
this.dirtyElement(element);
} else {
this.resources_.setRelayoutTop(box.top);
}
const box = this.viewport_.getLayoutRect(element);
if (box.width != 0 && box.height != 0) {
if (isExperimentOn(this.win, 'dirty-collapse-element')) {
this.dirtyElement(element);
} else {
this.resources_.setRelayoutTop(box.top);
}
}

Expand All @@ -147,9 +140,7 @@ export class MutatorImpl {

// Unlike completeExpand(), there's no requestMeasure() call here that
// requires another pass (with IntersectionObserver).
if (!this.intersect_) {
this.resources_.schedulePass(FOUR_FRAME_DELAY_);
}
this.resources_.schedulePass(FOUR_FRAME_DELAY_);
}

/** @override */
Expand Down Expand Up @@ -217,9 +208,8 @@ export class MutatorImpl {
if (measurer) {
measurer();
}
// With IntersectionObserver, "relayout top" is no longer needed since
// relative positional changes won't affect correctness.
if (!this.intersect_ && !skipRemeasure) {

if (!skipRemeasure) {
relayoutTop = calcRelayoutTop();
}
},
Expand All @@ -243,12 +233,6 @@ export class MutatorImpl {
}
this.resources_.schedulePass(FOUR_FRAME_DELAY_);

// With IntersectionObserver, no need to set relayout top.
if (this.intersect_) {
this.resources_.maybeHeightChanged();
return;
}

if (relayoutTop != -1) {
this.resources_.setRelayoutTop(relayoutTop);
}
Expand Down Expand Up @@ -279,7 +263,6 @@ export class MutatorImpl {
* @param {!Element} element
*/
dirtyElement(element) {
devAssert(!this.intersect_);
let relayoutAll = false;
const isAmpElement = element.classList.contains('i-amphtml-element');
if (isAmpElement) {
Expand Down
91 changes: 11 additions & 80 deletions src/service/resource.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,18 +222,9 @@ export class Resource {
/** @private {?Function} */
this.loadPromiseResolve_ = deferred.resolve;

/** @const @private {boolean} */
this.intersect_ = resources.isIntersectionExperimentOn();

// TODO(#30620): remove isInViewport_ and whenWithinViewport.
/** @const @private {boolean} */
this.isInViewport_ = false;

/**
* A client rect that was "premeasured" by an IntersectionObserver.
* @private {?ClientRect}
*/
this.premeasuredRect_ = null;
}

/**
Expand Down Expand Up @@ -344,20 +335,7 @@ export class Resource {
return this.element.buildInternal().then(
() => {
this.isBuilding_ = false;
// With IntersectionObserver, measure can happen before build
// so check if we're "ready for layout" (measured and built) here.
if (this.intersect_ && this.hasBeenMeasured()) {
this.state_ = ResourceState.READY_FOR_LAYOUT;
// The InOb premeasurement couldn't account for fixed position since
// implementation wasn't loaded yet. Perform a remeasure now that we know it
// is fixed.
if (this.element.isAlwaysFixed() && !this.isFixed_) {
this.requestMeasure();
}
this.element.onMeasure(/* sizeChanged */ true);
} else {
this.state_ = ResourceState.NOT_LAID_OUT;
}
this.state_ = ResourceState.NOT_LAID_OUT;
// TODO(dvoytenko): merge with the standard BUILT signal.
this.element.signals().signal('res-built');
},
Expand Down Expand Up @@ -443,29 +421,11 @@ export class Resource {
return this.element.getUpgradeDelayMs();
}

/**
* Stores a client rect that was "premeasured" by an IntersectionObserver.
* Should only be used in IntersectionObserver mode.
* @param {!ClientRect} clientRect
*/
premeasure(clientRect) {
devAssert(this.intersect_);
this.premeasuredRect_ = clientRect;
}

/** Removes the premeasured rect, likely forcing a manual measure. */
invalidatePremeasurementAndRequestMeasure() {
this.premeasuredRect_ = null;
this.requestMeasure();
}

/**
* Measures the resource's boundaries. An upgraded element will be
* transitioned to the "ready for layout" state.
* @param {boolean} usePremeasuredRect If true, consumes the previously
* premeasured ClientRect instead of calling getBoundingClientRect().
*/
measure(usePremeasuredRect = false) {
measure() {
// Check if the element is ready to be measured.
// Placeholders are special. They are technically "owned" by parent AMP
// elements, sized by parents, but laid out independently. This means
Expand Down Expand Up @@ -496,12 +456,7 @@ export class Resource {
this.isMeasureRequested_ = false;

const oldBox = this.layoutBox_;
if (usePremeasuredRect) {
this.computeMeasurements_(devAssert(this.premeasuredRect_));
} else {
this.computeMeasurements_();
}
this.premeasuredRect_ = null;
this.computeMeasurements_();
const newBox = this.layoutBox_;

// Note that "left" doesn't affect readiness for the layout.
Expand Down Expand Up @@ -547,12 +502,11 @@ export class Resource {

/**
* Computes the current layout box and position-fixed state of the element.
* @param {!ClientRect=} opt_premeasuredRect
* @private
*/
computeMeasurements_(opt_premeasuredRect) {
computeMeasurements_() {
const viewport = Services.viewportForDoc(this.element);
this.layoutBox_ = viewport.getLayoutRect(this.element, opt_premeasuredRect);
this.layoutBox_ = viewport.getLayoutRect(this.element);

// Calculate whether the element is currently is or in `position:fixed`.
let isFixed = false;
Expand Down Expand Up @@ -631,14 +585,6 @@ export class Resource {
return !!this.initialLayoutBox_;
}

/**
* @return {boolean}
*/
hasBeenPremeasured() {
devAssert(this.intersect_);
return !!this.premeasuredRect_;
}

/**
* Requests the element to be remeasured on the next pass.
*/
Expand Down Expand Up @@ -689,21 +635,16 @@ export class Resource {
/**
* Whether the resource is displayed, i.e. if it has non-zero width and
* height.
* @param {boolean} usePremeasuredRect If true and a premeasured rect is
* available, use it. Otherwise, use the cached layout box.
* @return {boolean}
*/
isDisplayed(usePremeasuredRect = false) {
isDisplayed() {
const isConnected =
this.element.ownerDocument && this.element.ownerDocument.defaultView;
if (!isConnected) {
return false;
}
devAssert(!usePremeasuredRect || this.intersect_);
const isFluid = this.element.getLayout() == Layout.FLUID;
const box = usePremeasuredRect
? devAssert(this.premeasuredRect_)
: this.getLayoutBox();
const box = this.getLayoutBox();
const hasNonZeroSize = box.height > 0 && box.width > 0;
return isFluid || hasNonZeroSize;
}
Expand Down Expand Up @@ -885,13 +826,9 @@ export class Resource {
* Undoes `layoutScheduled`.
*/
layoutCanceled() {
if (this.intersect_) {
this.state_ = ResourceState.READY_FOR_LAYOUT;
} else {
this.state_ = this.hasBeenMeasured()
? ResourceState.READY_FOR_LAYOUT
: ResourceState.NOT_LAID_OUT;
}
this.state_ = this.hasBeenMeasured()
? ResourceState.READY_FOR_LAYOUT
: ResourceState.NOT_LAID_OUT;
}

/**
Expand Down Expand Up @@ -1064,13 +1001,7 @@ export class Resource {
this.setInViewport(false);
if (this.element.unlayoutCallback()) {
this.element.togglePlaceholder(true);
// With IntersectionObserver, the element won't receive another
// measurement if/when the document becomes active again.
// Therefore, its post-unlayout state must be READY_FOR_LAYOUT
// (built and measured) to become eligible for relayout later.
this.state_ = this.intersect_
? ResourceState.READY_FOR_LAYOUT
: ResourceState.NOT_LAID_OUT;
this.state_ = ResourceState.NOT_LAID_OUT;
this.layoutCount_ = 0;
this.layoutPromise_ = null;
}
Expand Down
Loading