Disable Fast Fetch for all ads when remote.html is used. (#7906)

ampproject · Mar 1, 2017 · 694af90 · 694af90
1 parent 30d0eba
commit 694af90
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 37 deletions.
diff --git a/extensions/amp-ad-network-doubleclick-impl/0.1/doubleclick-a4a-config.js b/extensions/amp-ad-network-doubleclick-impl/0.1/doubleclick-a4a-config.js
@@ -78,9 +78,6 @@ export const BETA_ATTRIBUTE = 'data-use-beta-a4a-implementation';
  * @returns {boolean}
  */
 export function doubleclickIsA4AEnabled(win, element) {
-  if (!!win.document.querySelector('meta[name=amp-3p-iframe-src]')) {
-    return false;
-  }
   const a4aRequested = element.hasAttribute(BETA_ATTRIBUTE);
   // Note: Under this logic, a4aRequested shortcuts googleAdsIsA4AEnabled and,
   // therefore, carves out of the experiment branches.  Any publisher using this

diff --git a/extensions/amp-ad-network-doubleclick-impl/0.1/test/test-doubleclick-a4a-config.js b/extensions/amp-ad-network-doubleclick-impl/0.1/test/test-doubleclick-a4a-config.js
@@ -82,21 +82,6 @@ describe('doubleclick-a4a-config', () => {
       expect(elem.getAttribute(EXPERIMENT_ATTRIBUTE)).to.not.be.ok;
     });
 
-    it('should carve out remote.html, in spite of experiment override', () => {
-      const doc = testFixture.doc;
-      mockWin.location = parseUrl(
-          'https://cdn.ampproject.org/some/path/to/content.html');
-      mockWin.document.querySelector = doc.querySelector.bind(doc);
-      const remoteTag = doc.createElement('meta');
-      remoteTag.setAttribute('name', 'amp-3p-iframe-src');
-      doc.head.appendChild(remoteTag);
-      const elem = doc.createElement('div');
-      elem.setAttribute(BETA_ATTRIBUTE, 'true');
-      doc.body.appendChild(elem);
-      expect(doubleclickIsA4AEnabled(mockWin, elem)).to.be.false;
-      expect(elem.getAttribute(EXPERIMENT_ATTRIBUTE)).to.not.be.ok;
-    });
-
     [-1, 0, 1, 2].forEach(expFlagValue => {
       it(`exp flag=${expFlagValue} should set eid attribute`, () => {
         mockWin.location = parseUrl(
@@ -115,21 +100,6 @@ describe('doubleclick-a4a-config', () => {
         }
       });
 
-      it(`should carve out remote.html, in spite of exp flag=${expFlagValue}`,
-          () => {
-            const doc = testFixture.doc;
-            mockWin.location = parseUrl(
-                'https://cdn.ampproject.org/some/path/to/content.html?exp=a4a:' +
-                String(expFlagValue));
-            mockWin.document.querySelector = doc.querySelector.bind(doc);
-            const remoteTag = doc.createElement('meta');
-            remoteTag.setAttribute('name', 'amp-3p-iframe-src');
-            doc.head.appendChild(remoteTag);
-            const elem = doc.createElement('div');
-            doc.body.appendChild(elem);
-            expect(doubleclickIsA4AEnabled(mockWin, elem)).to.be.false;
-            expect(elem.getAttribute(EXPERIMENT_ATTRIBUTE)).to.not.be.ok;
-          });
     });
 
     [0, 1, 2].forEach(expFlagValue => {

diff --git a/extensions/amp-ad/0.1/amp-ad.js b/extensions/amp-ad/0.1/amp-ad.js
@@ -69,10 +69,12 @@ export class AmpAd extends AMP.BaseElement {
 
       // TODO(tdrl): Check amp-ad registry to see if they have this already.
       if (!a4aRegistry[type] ||
-          !a4aRegistry[type](this.win, this.element)) {
-        // Network either has not provided any A4A implementation or the
-        // implementation exists, but has explicitly chosen not to handle this
-        // tag as A4A.  Fall back to the 3p implementation.
+          !a4aRegistry[type](this.win, this.element) ||
+          this.win.document.querySelector('meta[name=amp-3p-iframe-src]')) {
+        // Either this ad network doesn't support Fast Fetch, its Fast Fetch
+        // implementation has explicitly opted not to handle this tag, or this
+        // page uses remote.html which is inherently incompatible with Fast
+        // Fetch. Fall back to Delayed Fetch.
         return new AmpAd3PImpl(this.element);
       }
 

diff --git a/extensions/amp-ad/0.1/test/test-amp-ad.js b/extensions/amp-ad/0.1/test/test-amp-ad.js
@@ -135,6 +135,19 @@ describe('Ad loader', () => {
         });
       });
 
+      it('falls back to Delayed Fetch if remote.html is used', () => {
+        return iframePromise.then(({doc}) => {
+          const meta = doc.createElement('meta');
+          meta.setAttribute('name', 'amp-3p-iframe-src');
+          meta.setAttribute('content', 'https://example.com/remote.html');
+          doc.head.append(meta);
+          a4aRegistry['zort'] = () => true;
+          ampAdElement.setAttribute('type', 'zort');
+          const upgraded = new AmpAd(ampAdElement).upgradeCallback();
+          return expect(upgraded).to.eventually.be.instanceof(AmpAd3PImpl);
+        });
+      });
+
       it('upgrades to registered, A4A type network-specific element', () => {
         return iframePromise.then(fixture => {
           a4aRegistry['zort'] = function() {