AdSense Fast Fetch delay 3 viewport experiment - respect loading stra…

…tegy (ampproject#20866) * initial commit * modify dep-check * truly fix dep issue
estherkim · Feb 14, 2019 · a751cc7 · a751cc7
1 parent 806db59
commit a751cc7
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 2 deletions.
diff --git a/build-system/dep-check-config.js b/build-system/dep-check-config.js
@@ -235,6 +235,7 @@ exports.rules = [
       'extensions/amp-ad/0.1/amp-ad-xorigin-iframe-handler.js->extensions/amp-ad-network-adsense-impl/0.1/adsense-a4a-config.js',
 
       // Ads needs concurrent loading
+      'extensions/amp-ad-network-adsense-impl/0.1/amp-ad-network-adsense-impl.js->extensions/amp-ad/0.1/concurrent-load.js',
       'extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js->extensions/amp-ad/0.1/concurrent-load.js',
       'extensions/amp-a4a/0.1/amp-a4a.js->extensions/amp-ad/0.1/concurrent-load.js',
 

diff --git a/extensions/amp-ad-network-adsense-impl/0.1/amp-ad-network-adsense-impl.js b/extensions/amp-ad-network-adsense-impl/0.1/amp-ad-network-adsense-impl.js
@@ -62,6 +62,7 @@ import {domFingerprintPlain} from '../../../src/utils/dom-fingerprint';
 import {
   getAdSenseAmpAutoAdsResponsiveExperimentBranch,
 } from '../../../ads/google/adsense-amp-auto-ads-responsive';
+import {getAmpAdRenderOutsideViewport} from '../../amp-ad/0.1/concurrent-load';
 import {getDefaultBootstrapBaseUrl} from '../../../src/3p-frame';
 import {
   getExperimentBranch,
@@ -235,8 +236,10 @@ export class AmpAdNetworkAdsenseImpl extends AmpA4A {
 
   /** @override */
   delayAdRequestEnabled() {
-    return getExperimentBranch(
-        this.win, DELAY_NUMBER_EXP) == '21063207' ? 3 : true;
+    if (getExperimentBranch(this.win, DELAY_NUMBER_EXP) != '21063207') {
+      return true;
+    }
+    return getAmpAdRenderOutsideViewport(this.element) || 3;
   }
 
   /** @override */

diff --git a/extensions/amp-ad-network-adsense-impl/0.1/test/test-amp-ad-network-adsense-impl.js b/extensions/amp-ad-network-adsense-impl/0.1/test/test-amp-ad-network-adsense-impl.js
@@ -981,6 +981,14 @@ describes.realWin('amp-ad-network-adsense-impl', {
       impl.divertExperiments();
       expect(impl.delayAdRequestEnabled()).to.equal(3);
     });
+
+    it('should respect loading strategy in experiment', () => {
+      impl.element.setAttribute(
+          'data-loading-strategy', 'prefer-viewability-over-views');
+      forceExperimentBranch(impl.win, 'adsense-ff-number-delay', '21063207');
+      impl.divertExperiments();
+      expect(impl.delayAdRequestEnabled()).to.equal(1.25);
+    });
   });
 
   describe('#preconnect', () => {