intersect-resources: Refactor to a lower risk integration (#27497)

* 1vp/3vp observers, make 'force build' less confusing.

Add comment about first intersection callback in Chrome 82.

Only prerender 1 viewport (if prerenderSize > 0).

Create 3vp observer after first 1vp observer callback.

Wait until visible to start 3vp observer.

First firstCallback.

Avoid double-handling by early-out when isIntersecting didn't change.

Add comment.

Simplify handling of first intersection callback.

Tweak comment.

Use ampdoc instead of viewer.

Add comments.

Don't handle prerenderSize_ == 0.

Refactor stuff.

Less efficient but much more simple.

Missing ivar init.

Fix types.

Remove logs for apply sizes/media query.

Remove commented out code.

Refactor to use one observer and calculate 1vp.

Support 25% larger 'visible rect'.

Move log, fix types.

Fix missing experiment guard.

Add assert in resource.measure, early out if resource was removed.

* First pass at pass.

* Tidy up a bit.

* Fix lint.

* Fix case where measure precedes build and resource is never laid out.

* Placeholder handling of resource/element build race.

* Clean up for PR.

* Fix types, add comments.

* Revert ignoreQuota in phase 1.

* Fix lint for new prettier version.

* Fix test-resource.js.

src/service/mutator-impl.js

@@ -221,18 +221,8 @@ export class MutatorImpl {
       mutate: () => {
         mutator();
 
-        // TODO(willchou): IntersectionObserver won't catch size changes,
-        // which means layout boxes may be stale. However, always requesting
-        // measure after any mutation is overkill and probably expensive.
-        // Instead, survey measureMutateElement() callers to determine which
-        // should explicitly call requestMeasure() to fix this.
-
-        // With IntersectionObserver, no need to remeasure and set relayout
-        // on element size changes since enter/exit viewport will be detected.
-        if (this.intersect_) {
-          this.resources_.maybeHeightChanged();
-          return;
-        }
+        // TODO(willchou): toggleLoading() mutate causes a lot of unnecessary
+        // remeasures. Add affordance to mutateElement() to disable remeasures.
 
         if (element.classList.contains('i-amphtml-element')) {
           const r = Resource.forElement(element);
@@ -243,11 +233,17 @@ export class MutatorImpl {
           const r = Resource.forElement(ampElements[i]);
           r.requestMeasure();
         }
+        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);
         }
-        this.resources_.schedulePass(FOUR_FRAME_DELAY_);
-
         // Need to measure again in case the element has become visible or
         // shifted.
         this.vsync_.measure(() => {
@@ -433,10 +429,9 @@ export class MutatorImpl {
         callback: opt_callback,
       }
     );
-    // With IntersectionObserver, remeasuring after size changes are no longer needed.
-    if (!this.intersect_) {
-      this.resources_.schedulePassVsync();
-    }
+    // With IntersectionObserver, we still want to schedule a pass to execute
+    // the requested measures of the newly resized element(s).
+    this.resources_.schedulePassVsync();
   }
 }
 

src/service/owners-impl.js

@@ -200,14 +200,14 @@ export class OwnersImpl {
         resource.whenBuilt().then(() => {
           this.measureAndTryScheduleLayout_(
             resource,
-            !layout,
+            /* isPreload */ !layout,
             parentResource.getLayoutPriority()
           );
         });
       } else {
         this.measureAndTryScheduleLayout_(
           resource,
-          !layout,
+          /* isPreload */ !layout,
           parentResource.getLayoutPriority()
         );
       }
@@ -228,7 +228,7 @@ export class OwnersImpl {
     ) {
       this.resources_.scheduleLayoutOrPreload(
         resource,
-        !isPreload,
+        /* layout */ !isPreload,
         opt_parentPriority
       );
     }

src/service/resource.js

@@ -217,6 +217,15 @@ export class Resource {
 
     /** @private {?Function} */
     this.loadPromiseResolve_ = deferred.resolve;
+
+    /** @const @private {boolean} */
+    this.intersect_ = resources.isIntersectionExperimentOn();
+
+    /**
+     * A client rect that was "premeasured" by an IntersectionObserver.
+     * @private {?ClientRect}
+     */
+    this.premeasuredRect_ = null;
   }
 
   /**
@@ -327,7 +336,15 @@ export class Resource {
     return this.element.build().then(
       () => {
         this.isBuilding_ = false;
-        this.state_ = ResourceState.NOT_LAID_OUT;
+        // With IntersectionObserver, measure can happen before build
+        // so check if we're "ready for layout" (measured and built) here.
+        if (this.intersect_) {
+          this.state_ = this.hasBeenMeasured()
+            ? ResourceState.READY_FOR_LAYOUT
+            : ResourceState.NOT_LAID_OUT;
+        } else {
+          this.state_ = ResourceState.NOT_LAID_OUT;
+        }
         // TODO(dvoytenko): merge with the standard BUILT signal.
         this.element.signals().signal('res-built');
       },
@@ -420,13 +437,23 @@ 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;
+  }
+
   /**
    * Measures the resource's boundaries. An upgraded element will be
    * transitioned to the "ready for layout" state.
-   * @param {!ClientRect=} opt_premeasuredRect If provided, use this
-   *    premeasured ClientRect instead of calling getBoundingClientRect.
+   * @param {boolean} usePremeasuredRect If true, consumes the previously
+   *    premeasured ClientRect instead of calling getBoundingClientRect().
    */
-  measure(opt_premeasuredRect) {
+  measure(usePremeasuredRect = false) {
     // 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
@@ -457,7 +484,12 @@ export class Resource {
     this.isMeasureRequested_ = false;
 
     const oldBox = this.layoutBox_;
-    this.computeMeasurements_(opt_premeasuredRect);
+    if (usePremeasuredRect) {
+      this.computeMeasurements_(devAssert(this.premeasuredRect_));
+      this.premeasuredRect_ = null;
+    } else {
+      this.computeMeasurements_();
+    }
     const newBox = this.layoutBox_;
 
     // Note that "left" doesn't affect readiness for the layout.
@@ -570,6 +602,14 @@ 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.
    */