Remove sync measurement from ad iframes: experiment (#32437)

* ads-initialIntersection experiment * turn up for canary * forgot the file * fix 1 test * lint * toggleExperiment --> forceExperimentBranch * Force exp branch in better location, also use canonical ID
ampproject · Feb 10, 2021 · aeac31e · aeac31e
1 parent e61d652
commit aeac31e
Show file tree

Hide file tree

Showing 8 changed files with 71 additions and 22 deletions.
diff --git a/build-system/global-configs/canary-config.json b/build-system/global-configs/canary-config.json
@@ -16,7 +16,7 @@
   "build-in-chunks": 1,
   "visibility-trigger-improvements": 1,
   "fie-resources": 1,
-  "ads-initialIntersection": 0,
+  "ads-initialIntersection": 1,
   "amp-cid-backup": 1,
   "sticky-ad-transition": 0.1,
   "layout-aspect-ratio-css": 1

diff --git a/extensions/amp-a4a/0.1/amp-a4a.js b/extensions/amp-a4a/0.1/amp-a4a.js
@@ -15,6 +15,7 @@
  */
 
 import {A4AVariableSource} from './a4a-variable-source';
+import {ADS_INITIAL_INTERSECTION_EXP} from '../../../src/experiments/ads-initial-intersection-exp';
 import {CONSENT_POLICY_STATE} from '../../../src/consent-state';
 import {Deferred, tryResolve} from '../../../src/utils/promise';
 import {DetachedDomStream} from '../../../src/utils/detached-dom-stream';
@@ -55,7 +56,7 @@ import {
   getConsentPolicyState,
 } from '../../../src/consent';
 import {getContextMetadata} from '../../../src/iframe-attributes';
-import {getExperimentBranch, isExperimentOn} from '../../../src/experiments';
+import {getExperimentBranch} from '../../../src/experiments';
 import {getMode} from '../../../src/mode';
 import {insertAnalyticsElement} from '../../../src/extension-analytics';
 import {
@@ -2068,10 +2069,10 @@ export class AmpA4A extends AMP.BaseElement {
       this.element,
       this.sentinel
     );
-    const asyncIntersection = isExperimentOn(
-      this.win,
-      'ads-initialIntersection'
-    );
+
+    const asyncIntersection =
+      getExperimentBranch(this.win, ADS_INITIAL_INTERSECTION_EXP.id) ===
+      ADS_INITIAL_INTERSECTION_EXP.experiment;
     const intersectionPromise = asyncIntersection
       ? measureIntersection(this.element)
       : Promise.resolve(this.element.getIntersectionChangeEntry());
@@ -2153,10 +2154,9 @@ export class AmpA4A extends AMP.BaseElement {
         this.getAdditionalContextMetadata(method == XORIGIN_MODE.SAFEFRAME)
       );
 
-      const asyncIntersection = isExperimentOn(
-        this.win,
-        'ads-initialIntersection'
-      );
+      const asyncIntersection =
+        getExperimentBranch(this.win, ADS_INITIAL_INTERSECTION_EXP.id) ===
+        ADS_INITIAL_INTERSECTION_EXP.experiment;
       const intersectionPromise = asyncIntersection
         ? measureIntersection(this.element)
         : Promise.resolve(this.element.getIntersectionChangeEntry());

diff --git a/extensions/amp-a4a/0.1/name-frame-renderer.js b/extensions/amp-a4a/0.1/name-frame-renderer.js
@@ -14,16 +14,17 @@
  * limitations under the License.
  */
 
+import {ADS_INITIAL_INTERSECTION_EXP} from '../../../src/experiments/ads-initial-intersection-exp';
 import {Renderer} from './amp-ad-type-defs';
 import {createElementWithAttributes} from '../../../src/dom';
 import {dict} from '../../../src/utils/object';
 import {getContextMetadata} from '../../../src/iframe-attributes';
 import {getDefaultBootstrapBaseUrl} from '../../../src/3p-frame';
+import {getExperimentBranch} from '../../../src/experiments';
 import {
   intersectionEntryToJson,
   measureIntersection,
 } from '../../../src/utils/intersection';
-import {isExperimentOn} from '../../../src/experiments';
 import {utf8Decode} from '../../../src/utils/bytes';
 
 /**
@@ -55,10 +56,12 @@ export class NameFrameRenderer extends Renderer {
       crossDomainData.additionalContextMetadata
     );
     contextMetadata['creative'] = creative;
-    const asyncIntersection = isExperimentOn(
-      element.ownerDocument.defaultView,
-      'ads-initialIntersection'
-    );
+
+    const asyncIntersection =
+      getExperimentBranch(
+        element.ownerDocument.defaultView,
+        ADS_INITIAL_INTERSECTION_EXP.id
+      ) === ADS_INITIAL_INTERSECTION_EXP.experiment;
     const intersectionPromise = asyncIntersection
       ? measureIntersection(element)
       : Promise.resolve(element.getIntersectionChangeEntry());

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
@@ -20,6 +20,7 @@
 // Most other ad networks will want to put their A4A code entirely in the
 // extensions/amp-ad-network-${NETWORK_NAME}-impl directory.
 
+import {ADS_INITIAL_INTERSECTION_EXP} from '../../../src/experiments/ads-initial-intersection-exp';
 import {AMP_SIGNATURE_HEADER} from '../../amp-a4a/0.1/signature-verifier';
 import {AdsenseSharedState} from './adsense-shared-state';
 import {AmpA4A} from '../../amp-a4a/0.1/amp-a4a';
@@ -236,6 +237,14 @@ export class AmpAdNetworkAdsenseImpl extends AmpA4A {
           STICKY_AD_TRANSITION_EXP.experiment,
         ],
       },
+      {
+        experimentId: ADS_INITIAL_INTERSECTION_EXP.id,
+        isTrafficEligible: () => true,
+        branches: [
+          ADS_INITIAL_INTERSECTION_EXP.control,
+          ADS_INITIAL_INTERSECTION_EXP.experiment,
+        ],
+      },
     ]);
     const setExps = randomlySelectUnsetExperiments(
       this.win,

diff --git a/extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js b/extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js
@@ -21,6 +21,7 @@
 // extensions/amp-ad-network-${NETWORK_NAME}-impl directory.
 
 import '../../../src/service/real-time-config/real-time-config-impl';
+import {ADS_INITIAL_INTERSECTION_EXP} from '../../../src/experiments/ads-initial-intersection-exp';
 import {
   AmpA4A,
   ConsentTupleDef,
@@ -475,6 +476,14 @@ export class AmpAdNetworkDoubleclickImpl extends AmpA4A {
         isTrafficEligible: () => true,
         branches: Object.values(ZINDEX_EXP_BRANCHES),
       },
+      {
+        experimentId: ADS_INITIAL_INTERSECTION_EXP.id,
+        isTrafficEligible: () => true,
+        branches: [
+          ADS_INITIAL_INTERSECTION_EXP.control,
+          ADS_INITIAL_INTERSECTION_EXP.experiment,
+        ],
+      },
       {
         experimentId: IDLE_CWV_EXP,
         isTrafficEligible: () => {

diff --git a/extensions/amp-ad/0.1/amp-ad-3p-impl.js b/extensions/amp-ad/0.1/amp-ad-3p-impl.js
@@ -18,6 +18,7 @@ import {
   ADSENSE_MCRSPV_TAG,
   getMatchedContentResponsiveHeightAndUpdatePubParams,
 } from '../../../ads/google/utils';
+import {ADS_INITIAL_INTERSECTION_EXP} from '../../../src/experiments/ads-initial-intersection-exp';
 import {AmpAdUIHandler} from './amp-ad-ui';
 import {AmpAdXOriginIframeHandler} from './amp-ad-xorigin-iframe-handler';
 import {
@@ -47,12 +48,12 @@ import {
   getConsentPolicySharedData,
   getConsentPolicyState,
 } from '../../../src/consent';
+import {getExperimentBranch} from '../../../src/experiments';
 import {getIframe, preloadBootstrap} from '../../../src/3p-frame';
 import {
   intersectionEntryToJson,
   measureIntersection,
 } from '../../../src/utils/intersection';
-import {isExperimentOn} from '../../../src/experiments';
 import {moveLayoutRect} from '../../../src/layout-rect';
 import {
   observeWithSharedInOb,
@@ -412,10 +413,10 @@ export class AmpAd3PImpl extends AMP.BaseElement {
         // because both happen inside a cross-domain iframe.  Separating them
         // here, though, allows us to measure the impact of ad throttling via
         // incrementLoadingAds().
-        const asyncIntersection = isExperimentOn(
-          this.win,
-          'ads-initialIntersection'
-        );
+
+        const asyncIntersection =
+          getExperimentBranch(this.win, ADS_INITIAL_INTERSECTION_EXP.id) ===
+          ADS_INITIAL_INTERSECTION_EXP.experiment;
         const intersectionPromise = asyncIntersection
           ? measureIntersection(this.element)
           : Promise.resolve(this.element.getIntersectionChangeEntry());

diff --git a/src/experiments/ads-initial-intersection-exp.js b/src/experiments/ads-initial-intersection-exp.js
@@ -0,0 +1,22 @@
+/**
+ * Copyright 2021 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.
+ */
+
+/** @const {!{id: string, control: string, experiment: string}} */
+export const ADS_INITIAL_INTERSECTION_EXP = {
+  id: 'ads-initialIntersection',
+  control: '31060065',
+  experiment: '31060066',
+};
diff --git a/test/integration/test-amp-ad-3p.js b/test/integration/test-amp-ad-3p.js
@@ -14,12 +14,13 @@
  * limitations under the License.
  */
 
+import {ADS_INITIAL_INTERSECTION_EXP} from '../../src/experiments/ads-initial-intersection-exp';
 import {Services} from '../../src/services';
 import {createCustomEvent} from '../../src/event-helper';
 import {createFixtureIframe, poll} from '../../testing/iframe';
+import {forceExperimentBranch} from '../../src/experiments';
 import {installPlatformService} from '../../src/service/platform-impl';
 import {layoutRectLtwh} from '../../src/layout-rect';
-import {toggleExperiment} from '../../src/experiments';
 
 const IFRAME_HEIGHT = 3000;
 function createFixture() {
@@ -37,11 +38,15 @@ describe('amp-ad 3P', () => {
     return createFixture().then((f) => {
       fixture = f;
       installPlatformService(fixture.win);
+      forceExperimentBranch(
+        fixture.win,
+        ADS_INITIAL_INTERSECTION_EXP.id,
+        ADS_INITIAL_INTERSECTION_EXP.experiment
+      );
     });
   });
 
   it('create an iframe with APIs', async function () {
-    toggleExperiment(window, 'ads-initialIntersection');
     this.timeout(20000);
     let iframe;
     let lastIO = null;