AMP Ad Refresh

ads/google/a4a/test/test-utils.js

@@ -234,36 +234,6 @@ describe('Google A4A utils', () => {
       sandbox = sinon.sandbox.create();
     });
 
-    it('should have the correct ifi numbers', function() {
-      // When ran locally, this test tends to exceed 2000ms timeout.
-      this.timeout(5000);
-      // Reset counter for purpose of this test.
-      delete window['ampAdGoogleIfiCounter'];
-      return createIframePromise().then(fixture => {
-        setupForAdTesting(fixture);
-        const doc = fixture.doc;
-        doc.win = window;
-        const elem = createElementWithAttributes(doc, 'amp-a4a', {
-          'type': 'adsense',
-          'width': '320',
-          'height': '50',
-        });
-        const impl = new MockA4AImpl(elem);
-        noopMethods(impl, doc, sandbox);
-        return fixture.addElement(elem).then(() => {
-          return googleAdUrl(impl, '', 0, [], [], []).then(url1 => {
-            expect(url1).to.match(/ifi=1/);
-            return googleAdUrl(impl, '', 0, [], [], []).then(url2 => {
-              expect(url2).to.match(/ifi=2/);
-              return googleAdUrl(impl, '', 0, [], [], []).then(url3 => {
-                expect(url3).to.match(/ifi=3/);
-              });
-            });
-          });
-        });
-      });
-    });
-
     it('should set ad position', function() {
       // When ran locally, this test tends to exceed 2000ms timeout.
       this.timeout(5000);

ads/google/a4a/utils.js

@@ -130,38 +130,25 @@ export function isReportingEnabled(ampElement) {
 export function googleBlockParameters(a4a, opt_experimentIds) {
   const adElement = a4a.element;
   const win = a4a.win;
-  win['ampAdGoogleIfiCounter'] = win['ampAdGoogleIfiCounter'] || 1;
   const slotRect = a4a.getPageLayoutBox();
   const iframeDepth = iframeNestingDepth(win);
-  // Detect container types.
-  const containerTypeSet = {};
-  for (let el = adElement.parentElement, counter = 0;
-      el && counter < 20; el = el.parentElement, counter++) {
-    const tagName = el.tagName.toUpperCase();
-    if (ValidAdContainerTypes[tagName]) {
-      containerTypeSet[ValidAdContainerTypes[tagName]] = true;
-    }
-  }
-  const pfx =
-      (containerTypeSet[ValidAdContainerTypes['AMP-FX-FLYING-CARPET']]
-       || containerTypeSet[ValidAdContainerTypes['AMP-STICKY-AD']])
-      ? '1' : '0';
+  const enclosingContainers = getEnclosingContainerTypes(adElement);
+  const pfx = enclosingContainers.includes(
+      ValidAdContainerTypes['AMP-FX-FLYING-CARPET']) ||
+      enclosingContainers.includes(ValidAdContainerTypes['AMP-STICKY-AD']);
   let eids = adElement.getAttribute('data-experiment-id');
   if (opt_experimentIds) {
     eids = mergeExperimentIds(opt_experimentIds, eids);
   }
-  const containerTypeArray = Object.keys(containerTypeSet);
   return {
-    'ifi': win['ampAdGoogleIfiCounter']++,
     'adf': domFingerprint(adElement),
     'nhd': iframeDepth,
     'eid': eids,
     'adx': slotRect.left,
     'ady': slotRect.top,
     'oid': '2',
-    pfx,
-    'rc': a4a.fromResumeCallback ? 1 : null,
-    'act': containerTypeArray.length ? containerTypeArray.join() : null,
+    'pfx': pfx ? '1' : '0',
+    'act': enclosingContainers.length ? enclosingContainers.join() : null,
   };
 }
 
@@ -535,3 +522,22 @@ export function addCsiSignalsToAmpAnalyticsConfig(win, element, config,
   config['triggers']['continuousVisible']['request'].push('visibilityCsi');
   return config;
 }
+
+/**
+ * Returns an array of two-letter codes representing the amp-ad containers
+ * enclosing the given ad element.
+ *
+ * @param {!Element} adElement
+ * @return {!Array<string>}
+ */
+export function getEnclosingContainerTypes(adElement) {
+  const containerTypeSet = {};
+  for (let el = adElement.parentElement, counter = 0;
+      el && counter < 20; el = el.parentElement, counter++) {
+    const tagName = el.tagName.toUpperCase();
+    if (ValidAdContainerTypes[tagName]) {
+      containerTypeSet[ValidAdContainerTypes[tagName]] = true;
+    }
+  }
+  return Object.keys(containerTypeSet);
+}

ads/google/doubleclick.js

@@ -67,24 +67,20 @@ export function doubleclick(global, data) {
  * @param {!string} url
  */
 function doubleClickWithGpt(global, data, gladeExperiment, url) {
-  const dimensions = [[
-    parseInt(data.overrideWidth || data.width, 10),
-    parseInt(data.overrideHeight || data.height, 10),
-  ]];
-
   // Handle multi-size data parsing, validation, and inclusion into dimensions.
   const multiSizeDataStr = data.multiSize || null;
-  if (multiSizeDataStr) {
-    const primarySize = dimensions[0];
-    const primaryWidth = primarySize[0];
-    const primaryHeight = primarySize[1];
-
-    getMultiSizeDimensions(
-        multiSizeDataStr,
-        primaryWidth,
-        primaryHeight,
-        (data.multiSizeValidation || 'true') == 'true',
-        dimensions);
+  const primaryWidth = parseInt(data.overrideWidth || data.width, 10);
+  const primaryHeight = parseInt(data.overrideHeight || data.height, 10);
+  let dimensions;
+  if (multiSizeDataStr && (dimensions = getMultiSizeDimensions(
+      multiSizeDataStr,
+      primaryWidth,
+      primaryHeight,
+      (data.multiSizeValidation || 'true') == 'true',
+      true))) {
+    dimensions.unshift([primaryWidth, primaryHeight]);
+  } else {
+    dimensions = [[primaryWidth, primaryHeight]];
   }
 
   loadScript(global, url, () => {

ads/google/test/test-utils.js

@@ -0,0 +1,94 @@
+/**
+ * Copyright 2017 The AMP HTML Authors. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS-IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import {getMultiSizeDimensions} from '../utils';
+
+describe('#getMultiSizeDimensions', () => {
+
+  const multiSizes = [
+    [300, 300],
+    [300, 250],
+    [250, 250],
+    [250, 200],
+    [150, 50],
+  ];
+
+  const multiSizeDataStr = '300x300,300x250,250x250,250x200,150x50';
+
+  function verifyArray(actual, lower, upper) {
+    expect(multiSizes.filter((size, index) => index < upper && index >= lower))
+        .to.deep.equal(actual);
+  }
+
+  it('should return all sizes', () => {
+    const actual = getMultiSizeDimensions(multiSizeDataStr, 300, 300,
+        /* Ignore lowerbound */ false,
+        /* Don't drop entire string on error */ false);
+    verifyArray(actual, 0, multiSizes.length);
+  });
+
+  it('should return a smaller array', () => {
+    const actual = getMultiSizeDimensions(multiSizeDataStr, 300, 250,
+        /* Ignore lowerbound */ false,
+        /* Don't drop entire string on error */ false);
+    verifyArray(actual, 1, multiSizes.length);
+  });
+
+  it('should return an even smaller array', () => {
+    const actual = getMultiSizeDimensions(multiSizeDataStr, 250, 250,
+        /* Ignore lowerbound */ false,
+        /* Don't drop entire string on error */ false);
+    verifyArray(actual, 2, multiSizes.length);
+  });
+
+  it('should return an empty array', () => {
+    const actual = getMultiSizeDimensions(multiSizeDataStr, 100, 50,
+        /* Ignore lowerbound */ false,
+        /* Don't drop entire string on error */ false);
+    verifyArray(actual, 0, 0);
+  });
+
+  it('should return a smaller array due to lowerbound', () => {
+    const actual = getMultiSizeDimensions(multiSizeDataStr, 300, 300,
+        /* Use lowerbound */ true,
+        /* Don't drop entire string on error */ false);
+
+    verifyArray(actual, 0, multiSizes.length - 1);
+  });
+
+  it('should return a smaller array due to lowerbound + smaller primary size',
+      () => {
+        const actual = getMultiSizeDimensions(multiSizeDataStr, 300, 250,
+            /* Use lowerbound */ true,
+            /* Don't drop entire string on error */ false);
+        verifyArray(actual, 1, multiSizes.length - 1);
+      });
+
+  it('should return null', () => {
+    const actual = getMultiSizeDimensions(multiSizeDataStr, 300, 250,
+        /* Ignore lowerbound */ false,
+        /* Drop entire string on error */ true);
+    expect(actual).to.be.null;
+  });
+
+  it('should return null due to non-positive argument', () => {
+    const actual = getMultiSizeDimensions(
+        '-1x300,' + multiSizeDataStr, 300, 300,
+        /* Ignore lowerbound */ false,
+        /* Drop entire string on error */ true);
+    expect(actual).to.be.null;
+  });
+});