Creative API: resize - attempt 2 (ampproject#4236)

* Changed <amp-ad> to allow nested frames to request resize using embed-size messages. Added a test using a nested frame to verify that these messages are received correctly.

* Fixed function name in a test.

* Made linter happy.

* Refactored a bit around sendEmbedContext.

* OK lint, you win again.

* Begone lint errors!

* Switched to use the returned Promise from attemptChangeSize.

extensions/amp-ad/0.1/amp-ad-api-handler.js

@@ -20,9 +20,8 @@ import {
   SubscriptionApi,
   listenFor,
   listenForOnce,
-  postMessage,
+  postMessageToWindows,
 } from '../../../src/iframe-helper';
-import {parseUrl} from '../../../src/url';
 import {IntersectionObserver} from '../../../src/intersection-observer';
 import {viewerFor} from '../../../src/viewer';
 import {user} from '../../../src/log';
@@ -95,24 +94,28 @@ export class AmpAdApiHandler {
     listenForOnce(this.iframe_, 'entity-id', info => {
       this.element_.creativeId = info.id;
     }, this.is3p_);
-    this.unlisteners_.push(listenFor(this.iframe_, 'embed-size', data => {
-      let newHeight, newWidth;
-      if (data.width !== undefined) {
-        newWidth = Math.max(this.element_./*OK*/offsetWidth +
-            data.width - this.iframe_./*OK*/offsetWidth, data.width);
-        this.iframe_.width = newWidth;
-        this.element_.setAttribute('width', newWidth);
-      }
-      if (data.height !== undefined) {
-        newHeight = Math.max(this.element_./*OK*/offsetHeight +
-            data.height - this.iframe_./*OK*/offsetHeight, data.height);
-        this.iframe_.height = newHeight;
-        this.element_.setAttribute('height', newHeight);
-      }
-      if (newHeight !== undefined || newWidth !== undefined) {
-        this.updateSize_(newHeight, newWidth);
-      }
-    }, this.is3p_));
+
+    // Install iframe resize API.
+    this.unlisteners_.push(listenFor(this.iframe_, 'embed-size',
+        (data, source, origin) => {
+          let newHeight, newWidth;
+          if (data.width !== undefined) {
+            newWidth = Math.max(this.element_./*OK*/offsetWidth +
+                data.width - this.iframe_./*OK*/offsetWidth, data.width);
+            this.iframe_.width = newWidth;
+            this.element_.setAttribute('width', newWidth);
+          }
+          if (data.height !== undefined) {
+            newHeight = Math.max(this.element_./*OK*/offsetHeight +
+                data.height - this.iframe_./*OK*/offsetHeight, data.height);
+            this.iframe_.height = newHeight;
+            this.element_.setAttribute('height', newHeight);
+          }
+          if (newHeight !== undefined || newWidth !== undefined) {
+            this.updateSize_(newHeight, newWidth, source, origin);
+          }
+        }, this.is3p_, this.is3p_));
+
     if (!opt_defaultVisible) {
       // NOTE(tdrl,keithwrightbos): This will not work for A4A with an AMP
       // creative as it will not expect having to send the render-start message.
@@ -151,29 +154,39 @@ export class AmpAdApiHandler {
 
   /**
    * Updates the element's dimensions to accommodate the iframe's
-   *    requested dimensions.
+   * requested dimensions. Notifies the window that request the resize
+   * of success or failure.
    * @param {number|undefined} height
    * @param {number|undefined} width
+   * @param {!Window} source
+   * @param {string} origin
    * @private
    */
-  updateSize_(height, width) {
-    const targetOrigin =
-          this.iframe_.src ? parseUrl(this.iframe_.src).origin : '*';
-    this.baseInstance_.attemptChangeSize(height, width).then(() => {
-      postMessage(
-          this.iframe_,
-          'embed-size-changed',
-          {requestedHeight: height, requestedWidth: width},
-          targetOrigin,
-          this.is3p_);
-    }, () => {
-      postMessage(
-          this.iframe_,
-          'embed-size-denied',
-          {requestedHeight: height, requestedWidth: width},
-          targetOrigin,
-          this.is3p_);
-    });
+  updateSize_(height, width, source, origin) {
+    this.baseInstance_.attemptChangeSize(height, width).then(
+        () => this.sendEmbedSizeResponse_(
+            true /* success */, width, height, source, origin),
+        () => this.sendEmbedSizeResponse_(
+            false /* success */, width, height, source, origin));
+  }
+
+  /**
+   * Sends a response to the window which requested a resize.
+   * @param {boolean} success
+   * @param {number} requestedWidth
+   * @param {number} requestedHeight
+   * @param {!Window} source
+   * @param {string} origin
+   * @private
+   */
+  sendEmbedSizeResponse_(
+      success, requestedWidth, requestedHeight, source, origin) {
+    postMessageToWindows(
+        this.iframe_,
+        [{win: source, origin}],
+        success ? 'embed-size-changed' : 'embed-size-denied',
+        {requestedWidth, requestedHeight},
+        this.is3p_);
   }
 
   /**

extensions/amp-ad/0.1/test/test-amp-ad-3p-impl.js

@@ -129,6 +129,43 @@ function tests(name) {
         });
       });
 
+      it('should listen for resize events from nested frames', () => {
+        const iframeSrc = 'http://ads.localhost:' + location.port +
+            '/base/test/fixtures/served/iframe-resize-outer.html';
+        return getAd({
+          width: 100,
+          height: 100,
+          type: '_ping_',
+          src: 'testsrc',
+          resizable: '',
+        }, 'https://schema.org').then(element => {
+          return new Promise((resolve, unusedReject) => {
+            const impl = element.implementation_;
+            impl.layoutCallback();
+            impl.apiHandler_.updateSize_ = (newHeight, newWidth) => {
+              expect(newHeight).to.equal(217);
+              expect(newWidth).to.equal(114);
+              resolve(impl);
+            };
+            impl.iframe_.onload = function() {
+              impl.iframe_.contentWindow.frames[0].postMessage({
+                sentinel: 'amp-test',
+                type: 'requestHeight',
+                is3p: true,
+                height: 217,
+                width: 114,
+                amp3pSentinel:
+                    impl.iframe_.getAttribute('data-amp-3p-sentinel'),
+              }, '*');
+            };
+            impl.iframe_.src = iframeSrc;
+          });
+        }).then(impl => {
+          expect(impl.iframe_.height).to.equal('217');
+          expect(impl.iframe_.width).to.equal('114');
+        });
+      });
+
       it('should resize height only', () => {
         const iframeSrc = 'http://ads.localhost:' + location.port +
             '/base/test/fixtures/served/iframe.html';

test/fixtures/served/iframe-resize-outer.html

@@ -0,0 +1,4 @@
+<!doctype html>
+<body style="background-color: red">
+<iframe src="iframe.html"></iframe>
+</body>

test/fixtures/served/iframe.html

@@ -4,11 +4,29 @@
 <script>
 parent.parent./*OK*/postMessage('loaded-iframe', '*');
 
+function getAmpWindow(sentinel) {
+  var match = sentinel.match(/^(\d+)-\d+$/);
+  // If we have a proper 3P sentinel then use it to find the AMP window.
+  if (match) {
+    // Depth is measured from window.top.
+    var depth = Number(match[1]);
+    var ancestors = [];
+    for (var win = window; win && win != win.parent; win = win.parent) {
+      // Add window keeping the top-most one at the front.
+      ancestors.unshift(win.parent);
+    }
+    return ancestors[depth];
+  } else {
+    // Assume our parent is AMP.
+    return parent;
+  }
+}
+
 window.addEventListener('message', function(event) {
   if (event.data && event.data.sentinel == 'amp-test') {
     if (event.data.type == 'requestHeight') {
       var sentinel = event.data.is3p ? event.data.amp3pSentinel : 'amp';
-      parent./*OK*/postMessage(
+      getAmpWindow(sentinel)./*OK*/postMessage(
           {
             sentinel: sentinel,
             type: 'embed-size',
@@ -17,7 +35,7 @@
           }, '*');
     } else if (event.data.type == 'subscribeToEmbedState') {
       var sentinel = event.data.is3p ? event.data.amp3pSentinel : 'amp';
-      parent./*OK*/postMessage(
+      getAmpWindow(sentinel)./*OK*/postMessage(
           {
             sentinel: sentinel,
             type: 'send-embed-state',