amp-ad 3p: remove viewportCallback (#30995)

ampproject · Nov 4, 2020 · 82e3872 · 82e3872
1 parent 291e713
commit 82e3872
Showing 1 changed file with 45 additions and 34 deletions.
diff --git a/extensions/amp-ad/0.1/amp-ad-3p-impl.js b/extensions/amp-ad/0.1/amp-ad-3p-impl.js
@@ -51,6 +51,10 @@ import {
 import {getIframe, preloadBootstrap} from '../../../src/3p-frame';
 import {listen} from '../../../src/event-helper';
 import {moveLayoutRect} from '../../../src/layout-rect';
+import {
+  observeWithSharedInOb,
+  unobserveWithSharedInOb,
+} from '../../../src/viewport-observer';
 import {toWin} from '../../../src/types';
 
 /** @const {string} Tag name for 3P AD implementation. */
@@ -424,48 +428,54 @@ export class AmpAd3PImpl extends AMP.BaseElement {
       consentStringPromise,
       consentMetadataPromise,
       scrollPromise,
-    ]).then((consents) => {
-      // Use JsonObject to preserve field names so that ampContext can access
-      // values with name
-      // ampcontext.js and this file are compiled in different compilation unit
-
-      // Note: Field names can by perserved by using JsonObject, or by adding
-      // perserved name to extern. We are doing both right now.
-      // Please also add new introduced variable
-      // name to the extern list.
-      const opt_context = dict({
-        'clientId': consents[0] || null,
-        'container': this.container_,
-        'initialConsentState': consents[1],
-        'consentSharedData': consents[2],
-        'initialConsentValue': consents[3],
-        'initialConsentMetadata': consents[4],
-      });
+    ])
+      .then((consents) => {
+        // Use JsonObject to preserve field names so that ampContext can access
+        // values with name
+        // ampcontext.js and this file are compiled in different compilation unit
+
+        // Note: Field names can by perserved by using JsonObject, or by adding
+        // perserved name to extern. We are doing both right now.
+        // Please also add new introduced variable
+        // name to the extern list.
+        const opt_context = dict({
+          'clientId': consents[0] || null,
+          'container': this.container_,
+          'initialConsentState': consents[1],
+          'consentSharedData': consents[2],
+          'initialConsentValue': consents[3],
+          'initialConsentMetadata': consents[4],
+        });
 
-      // In this path, the request and render start events are entangled,
-      // 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}
-      );
-      iframe.title = this.element.title || 'Advertisement';
-      this.xOriginIframeHandler_ = new AmpAdXOriginIframeHandler(this);
-      return this.xOriginIframeHandler_.init(iframe);
-    });
+        // In this path, the request and render start events are entangled,
+        // 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}
+        );
+        iframe.title = this.element.title || 'Advertisement';
+        this.xOriginIframeHandler_ = new AmpAdXOriginIframeHandler(this);
+        return this.xOriginIframeHandler_.init(iframe);
+      })
+      .then(() => {
+        observeWithSharedInOb(this.element, (inViewport) =>
+          this.viewportCallback_(inViewport)
+        );
+      });
     incrementLoadingAds(this.win, this.layoutPromise_);
     return this.layoutPromise_;
   }
 
   /**
    * @param {boolean} inViewport
-   * @override
+   * @private
    */
-  viewportCallback(inViewport) {
+  viewportCallback_(inViewport) {
     if (this.xOriginIframeHandler_) {
       this.xOriginIframeHandler_.viewportCallback(inViewport);
     }
@@ -480,6 +490,7 @@ export class AmpAd3PImpl extends AMP.BaseElement {
   unlayoutCallback() {
     this.unlisteners_.forEach((unlisten) => unlisten());
     this.unlisteners_.length = 0;
+    unobserveWithSharedInOb(this.element);
 
     this.layoutPromise_ = null;
     this.uiHandler.applyUnlayoutUI();