Convert ad iframes to get initialIntersection async (ampproject#31753)

* getContextMetadata --> Promise * ad iframes gather initialIntersection async * use experiments * remove log / fix test * revert test change now that under expeirment. * fix test-amp-ad-custom * caleb updates * toggle intersection needs win
PetrBlaha · Jan 28, 2021 · d27129e · d27129e
1 parent 02917e1
commit d27129e
Show file tree

Hide file tree

Showing 14 changed files with 169 additions and 124 deletions.
diff --git a/3p/3d-gltf/viewer.js b/3p/3d-gltf/viewer.js
@@ -58,8 +58,7 @@ export default class GltfViewer {
     this.ampPlay_ = true;
 
     /** @private */
-    this.ampInViewport_ =
-      options['initialIntersection']['intersectionRatio'] > 0;
+    this.ampInViewport_ = false;
 
     /** @private */
     this.setSize_ = this.setupSize_();

diff --git a/build-system/global-configs/canary-config.json b/build-system/global-configs/canary-config.json
@@ -16,6 +16,7 @@
   "build-in-chunks": 1,
   "visibility-trigger-improvements": 1,
   "fie-resources": 1,
+  "ads-initialIntersection": 1,
   "amp-cid-backup": 1,
   "sticky-ad-transition": 0.1
 }
diff --git a/extensions/amp-a4a/0.1/amp-a4a.js b/extensions/amp-a4a/0.1/amp-a4a.js
@@ -55,14 +55,18 @@ import {
   getConsentPolicyState,
 } from '../../../src/consent';
 import {getContextMetadata} from '../../../src/iframe-attributes';
-import {getExperimentBranch} from '../../../src/experiments';
+import {getExperimentBranch, isExperimentOn} from '../../../src/experiments';
 import {getMode} from '../../../src/mode';
 import {insertAnalyticsElement} from '../../../src/extension-analytics';
 import {
   installFriendlyIframeEmbed,
   isSrcdocSupported,
 } from '../../../src/friendly-iframe-embed';
 import {installUrlReplacementsForEmbed} from '../../../src/service/url-replacements-impl';
+import {
+  intersectionEntryToJson,
+  measureIntersection,
+} from '../../../src/utils/intersection';
 import {isAdPositionAllowed} from '../../../src/ad-helper';
 import {isArray, isEnumValue, isObject} from '../../../src/types';
 import {listenOnce} from '../../../src/event-helper';
@@ -2042,14 +2046,30 @@ export class AmpA4A extends AMP.BaseElement {
    */
   renderViaIframeGet_(adUrl) {
     this.maybeTriggerAnalyticsEvent_('renderCrossDomainStart');
-    return this.iframeRenderHelper_(
-      dict({
-        'src': Services.xhrFor(this.win).getCorsUrl(this.win, adUrl),
-        'name': JSON.stringify(
-          getContextMetadata(this.win, this.element, this.sentinel)
-        ),
-      })
+    const contextMetadata = getContextMetadata(
+      this.win,
+      this.element,
+      this.sentinel
     );
+    const asyncIntersection = isExperimentOn(
+      this.win,
+      'ads-initialIntersection'
+    );
+    const intersectionPromise = asyncIntersection
+      ? measureIntersection(this.element)
+      : Promise.resolve(this.element.getIntersectionChangeEntry());
+
+    return intersectionPromise.then((intersection) => {
+      contextMetadata['_context'][
+        'initialIntersection'
+      ] = intersectionEntryToJson(intersection);
+      return this.iframeRenderHelper_(
+        dict({
+          'src': Services.xhrFor(this.win).getCorsUrl(this.win, adUrl),
+          'name': JSON.stringify(contextMetadata),
+        })
+      );
+    });
   }
 
   /**
@@ -2115,17 +2135,30 @@ export class AmpA4A extends AMP.BaseElement {
         this.sentinel,
         this.getAdditionalContextMetadata(method == XORIGIN_MODE.SAFEFRAME)
       );
-      // TODO(bradfrizzell) Clean up name assigning.
-      if (method == XORIGIN_MODE.NAMEFRAME) {
-        contextMetadata['creative'] = creative;
-        name = JSON.stringify(contextMetadata);
-      } else if (method == XORIGIN_MODE.SAFEFRAME) {
-        contextMetadata = JSON.stringify(contextMetadata);
-        name =
-          `${this.safeframeVersion};${creative.length};${creative}` +
-          `${contextMetadata}`;
-      }
-      return this.iframeRenderHelper_(dict({'src': srcPath, 'name': name}));
+
+      const asyncIntersection = isExperimentOn(
+        this.win,
+        'ads-initialIntersection'
+      );
+      const intersectionPromise = asyncIntersection
+        ? measureIntersection(this.element)
+        : Promise.resolve(this.element.getIntersectionChangeEntry());
+      return intersectionPromise.then((intersection) => {
+        contextMetadata['initialIntersection'] = intersectionEntryToJson(
+          intersection
+        );
+        if (method == XORIGIN_MODE.NAMEFRAME) {
+          contextMetadata['creative'] = creative;
+          name = JSON.stringify(contextMetadata);
+        } else if (method == XORIGIN_MODE.SAFEFRAME) {
+          contextMetadata = JSON.stringify(contextMetadata);
+          name =
+            `${this.safeframeVersion};${creative.length};${creative}` +
+            `${contextMetadata}`;
+        }
+
+        return this.iframeRenderHelper_(dict({'src': srcPath, 'name': name}));
+      });
     });
   }
 

diff --git a/extensions/amp-a4a/0.1/name-frame-renderer.js b/extensions/amp-a4a/0.1/name-frame-renderer.js
@@ -19,6 +19,11 @@ import {createElementWithAttributes} from '../../../src/dom';
 import {dict} from '../../../src/utils/object';
 import {getContextMetadata} from '../../../src/iframe-attributes';
 import {getDefaultBootstrapBaseUrl} from '../../../src/3p-frame';
+import {
+  intersectionEntryToJson,
+  measureIntersection,
+} from '../../../src/utils/intersection';
+import {isExperimentOn} from '../../../src/experiments';
 import {utf8Decode} from '../../../src/utils/bytes';
 
 /**
@@ -50,28 +55,39 @@ export class NameFrameRenderer extends Renderer {
       crossDomainData.additionalContextMetadata
     );
     contextMetadata['creative'] = creative;
-    const attributes = dict({
-      'src': srcPath,
-      'name': JSON.stringify(contextMetadata),
-      'height': context.size.height,
-      'width': context.size.width,
-      'frameborder': '0',
-      'allowfullscreen': '',
-      'allowtransparency': '',
-      'scrolling': 'no',
-      'marginwidth': '0',
-      'marginheight': '0',
-    });
-    if (crossDomainData.sentinel) {
-      attributes['data-amp-3p-sentinel'] = crossDomainData.sentinel;
-    }
-    const iframe = createElementWithAttributes(
-      /** @type {!Document} */ (element.ownerDocument),
-      'iframe',
-      /** @type {!JsonObject} */ (attributes)
+    const asyncIntersection = isExperimentOn(
+      element.ownerDocument.defaultView,
+      'ads-initialIntersection'
     );
-    // TODO(glevitzky): Ensure that applyFillContent or equivalent is called.
-    element.appendChild(iframe);
-    return Promise.resolve();
+    const intersectionPromise = asyncIntersection
+      ? measureIntersection(element)
+      : Promise.resolve(element.getIntersectionChangeEntry());
+    return intersectionPromise.then((intersection) => {
+      contextMetadata['_context'][
+        'initialIntersection'
+      ] = intersectionEntryToJson(intersection);
+      const attributes = dict({
+        'src': srcPath,
+        'name': JSON.stringify(contextMetadata),
+        'height': context.size.height,
+        'width': context.size.width,
+        'frameborder': '0',
+        'allowfullscreen': '',
+        'allowtransparency': '',
+        'scrolling': 'no',
+        'marginwidth': '0',
+        'marginheight': '0',
+      });
+      if (crossDomainData.sentinel) {
+        attributes['data-amp-3p-sentinel'] = crossDomainData.sentinel;
+      }
+      const iframe = createElementWithAttributes(
+        /** @type {!Document} */ (element.ownerDocument),
+        'iframe',
+        /** @type {!JsonObject} */ (attributes)
+      );
+      // TODO(glevitzky): Ensure that applyFillContent or equivalent is called.
+      element.appendChild(iframe);
+    });
   }
 }
diff --git a/extensions/amp-a4a/0.1/test/test-amp-a4a.js b/extensions/amp-a4a/0.1/test/test-amp-a4a.js
@@ -294,7 +294,12 @@ describe('amp-a4a', () => {
     element.getLayoutBox = () => layoutBox;
     element.getLayoutSize = () => layoutSizeFromRect(layoutBox);
     element.getIntersectionChangeEntry = () => {
-      return null;
+      return {
+        time: null,
+        boundingClientRect: {},
+        rootBounds: {},
+        intersectionRect: {},
+      };
     };
     const signals = new Signals();
     element.signals = () => signals;

diff --git a/extensions/amp-a4a/0.1/test/test-name-frame-renderer.js b/extensions/amp-a4a/0.1/test/test-name-frame-renderer.js
@@ -31,7 +31,7 @@ describes.realWin('NameFrameRenderer', realWinConfig, (env) => {
   let context;
   let creativeData;
 
-  beforeEach(() => {
+  beforeEach(async () => {
     context = {
       size: {width: '320', height: '50'},
       requestUrl: 'http://www.google.com',
@@ -48,10 +48,19 @@ describes.realWin('NameFrameRenderer', realWinConfig, (env) => {
     containerElement = env.win.document.createElement('div');
     containerElement.setAttribute('height', 50);
     containerElement.setAttribute('width', 320);
-    containerElement.getIntersectionChangeEntry = () => ({});
+    containerElement.getIntersectionChangeEntry = () => ({
+      time: null,
+      boundingClientRect: {},
+      rootBounds: {},
+      intersectionRect: {},
+    });
     env.win.document.body.appendChild(containerElement);
 
-    new NameFrameRenderer().render(context, containerElement, creativeData);
+    await new NameFrameRenderer().render(
+      context,
+      containerElement,
+      creativeData
+    );
   });
 
   it('should append iframe child', () => {

diff --git a/extensions/amp-ad-custom/0.1/test/test-amp-ad-custom.js b/extensions/amp-ad-custom/0.1/test/test-amp-ad-custom.js
@@ -55,7 +55,11 @@ describes.realWin('TemplateRenderer', realWinConfig, (env) => {
       width: 0,
       height: 0,
     });
-    containerElement.getIntersectionChangeEntry = () => ({});
+    containerElement.getIntersectionChangeEntry = () => ({
+      rootBounds: {},
+      intersectionRect: {},
+      boundingClientRect: {},
+    });
     containerElement.isInViewport = () => true;
     containerElement.getAmpDoc = () => env.ampdoc;
     doc.body.appendChild(containerElement);

diff --git a/extensions/amp-ad/0.1/amp-ad-3p-impl.js b/extensions/amp-ad/0.1/amp-ad-3p-impl.js
@@ -48,6 +48,11 @@ import {
   getConsentPolicyState,
 } from '../../../src/consent';
 import {getIframe, preloadBootstrap} from '../../../src/3p-frame';
+import {
+  intersectionEntryToJson,
+  measureIntersection,
+} from '../../../src/utils/intersection';
+import {isExperimentOn} from '../../../src/experiments';
 import {moveLayoutRect} from '../../../src/layout-rect';
 import {
   observeWithSharedInOb,
@@ -407,16 +412,28 @@ export class AmpAd3PImpl extends AMP.BaseElement {
         // because both happen inside a cross-domain iframe.  Separating them
         // here, though, allows us to measure the impact of ad throttling via
         // incrementLoadingAds().
-        const iframe = getIframe(
-          toWin(this.element.ownerDocument.defaultView),
-          this.element,
-          this.type_,
-          opt_context,
-          {disallowCustom: this.config.remoteHTMLDisabled}
+        const asyncIntersection = isExperimentOn(
+          this.win,
+          'ads-initialIntersection'
         );
-        iframe.title = this.element.title || 'Advertisement';
-        this.xOriginIframeHandler_ = new AmpAdXOriginIframeHandler(this);
-        return this.xOriginIframeHandler_.init(iframe);
+        const intersectionPromise = asyncIntersection
+          ? measureIntersection(this.element)
+          : Promise.resolve(this.element.getIntersectionChangeEntry());
+        return intersectionPromise.then((intersection) => {
+          const iframe = getIframe(
+            toWin(this.element.ownerDocument.defaultView),
+            this.element,
+            this.type_,
+            opt_context,
+            {
+              disallowCustom: this.config.remoteHTMLDisabled,
+              initialIntersection: intersectionEntryToJson(intersection),
+            }
+          );
+          iframe.title = this.element.title || 'Advertisement';
+          this.xOriginIframeHandler_ = new AmpAdXOriginIframeHandler(this);
+          return this.xOriginIframeHandler_.init(iframe);
+        });
       })
       .then(() => {
         observeWithSharedInOb(this.element, (inViewport) =>

diff --git a/extensions/amp-ad/0.1/legacy-ad-intersection-observer-host.js b/extensions/amp-ad/0.1/legacy-ad-intersection-observer-host.js
@@ -19,6 +19,7 @@ import {Services} from '../../../src/services';
 import {SubscriptionApi} from '../../../src/iframe-helper';
 import {devAssert} from '../../../src/log';
 import {dict} from '../../../src/utils/object';
+import {intersectionEntryToJson} from '../../../src/utils/intersection';
 import {
   layoutRectLtwh,
   moveLayoutRect,
@@ -319,34 +320,3 @@ export class LegacyAdIntersectionObserverHost {
     this.postMessageApi_.destroy();
   }
 }
-
-/**
- * Convert a DOMRect to a regular object to make it serializable.
- *
- * @param {!DOMRect} domRect
- * @return {!DOMRect}
- */
-function domRectToJson(domRect) {
-  if (domRect == null) {
-    return domRect;
-  }
-
-  const {x, y, width, height, top, right, bottom, left} = domRect;
-  return {x, y, width, height, top, right, bottom, left};
-}
-
-/**
- * Convert an IntersectionObserverEntry to a regular object to make it serializable.
- *
- * @param {!IntersectionObserverEntry} entry
- * @return {!IntersectionObserverEntry}
- */
-function intersectionEntryToJson(entry) {
-  return {
-    time: entry.time,
-    rootBounds: domRectToJson(entry.rootBounds),
-    boundingClientRect: domRectToJson(entry.boundingClientRect),
-    intersectionRect: domRectToJson(entry.intersectionRect),
-    intersectionRatio: entry.intersectionRatio,
-  };
-}
diff --git a/extensions/amp-ad/0.1/test/test-legacy-ad-intersection-observer-host.js b/extensions/amp-ad/0.1/test/test-legacy-ad-intersection-observer-host.js
@@ -461,7 +461,7 @@ describes.sandboxed('IntersectionObserverHostForAd', {}, () => {
     );
     stubFireInOb(ioInstance);
     insert(testIframe);
-    ioInstance.onViewportCallback_(true);
+    ioInstance.onViewportCallback_(getInObEntry());
     expect(sendElementIntersectionSpy).to.be.calledOnce;
     expect(onScrollSpy).to.be.calledOnce;
     expect(onChangeSpy).to.be.calledOnce;
@@ -479,7 +479,7 @@ describes.sandboxed('IntersectionObserverHostForAd', {}, () => {
     );
     insert(testIframe);
     ioInstance.onViewportCallback_(getInObEntry());
-    ioInstance.onViewportCallback_({intersectionRatio: false});
+    ioInstance.onViewportCallback_({...getInObEntry(), intersectionRatio: 0});
     expect(sendElementIntersectionSpy).to.have.callCount(2);
     expect(ioInstance.unlistenViewportChanges_).to.be.null;
   });

diff --git a/src/3p-frame.js b/src/3p-frame.js
@@ -68,6 +68,7 @@ function getFrameAttributes(parentWindow, element, opt_type, opt_context) {
  * @param {{
  *   disallowCustom: (boolean|undefined),
  *   allowFullscreen: (boolean|undefined),
+ *   initialIntersection: (IntersectionObserverEntry|undefined),
  * }=} options Options for the created iframe.
  * @return {!HTMLIFrameElement} The iframe.
  */
@@ -78,7 +79,11 @@ export function getIframe(
   opt_context,
   options = {}
 ) {
-  const {disallowCustom = false, allowFullscreen = false} = options;
+  const {
+    disallowCustom = false,
+    allowFullscreen = false,
+    initialIntersection,
+  } = options;
   // Check that the parentElement is already in DOM. This code uses a new and
   // fast `isConnected` API and thus only used when it's available.
   devAssert(
@@ -92,6 +97,10 @@ export function getIframe(
     opt_type,
     opt_context
   );
+  if (initialIntersection) {
+    attributes['_context']['initialIntersection'] = initialIntersection;
+  }
+
   const iframe = /** @type {!HTMLIFrameElement} */ (parentWindow.document.createElement(
     'iframe'
   ));