Revert "Launch no center css for ad container (#28551)" (#28844)

This reverts commit f2843ea.
ampproject · Jun 12, 2020 · f9426d3 · f9426d3
1 parent 0ccf6f4
commit f9426d3
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 17 deletions.
diff --git a/ads/google/a4a/utils.js b/ads/google/a4a/utils.js
@@ -47,6 +47,13 @@ const AmpAdImplementation = {
   AMP_AD_IFRAME_GET: '5',
 };
 
+/** @const {!{id: string, control: string, experiment: string}} */
+export const AMP_AD_NO_CENTER_CSS_EXP = {
+  id: 'amp-ad-no-center-css',
+  control: '21065897',
+  experiment: '21065898',
+};
+
 /** @const {!{id: string, control: string, experiment: string}} */
 export const RENDER_ON_IDLE_FIX_EXP = {
   id: 'render-on-idle-fix',

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
@@ -21,12 +21,8 @@
 // extensions/amp-ad-network-${NETWORK_NAME}-impl directory.
 
 import {EXPERIMENT_INFO_MAP as AMPDOC_FIE_EXPERIMENT_INFO_MAP} from '../../../src/ampdoc-fie';
-import {AdsenseSharedState} from './adsense-shared-state';
-import {AmpA4A, NO_SIGNING_EXP} from '../../amp-a4a/0.1/amp-a4a';
-import {CONSENT_POLICY_STATE} from '../../../src/consent-state';
-import {FIE_INIT_CHUNKING_EXP} from '../../../src/friendly-iframe-embed';
-import {Navigation} from '../../../src/service/navigation';
 import {
+  AMP_AD_NO_CENTER_CSS_EXP,
   QQID_HEADER,
   RENDER_ON_IDLE_FIX_EXP,
   SANDBOX_HEADER,
@@ -43,6 +39,11 @@ import {
   isReportingEnabled,
   maybeAppendErrorParameter,
 } from '../../../ads/google/a4a/utils';
+import {AdsenseSharedState} from './adsense-shared-state';
+import {AmpA4A, NO_SIGNING_EXP} from '../../amp-a4a/0.1/amp-a4a';
+import {CONSENT_POLICY_STATE} from '../../../src/consent-state';
+import {FIE_INIT_CHUNKING_EXP} from '../../../src/friendly-iframe-embed';
+import {Navigation} from '../../../src/service/navigation';
 import {ResponsiveState} from './responsive-state';
 import {Services} from '../../../src/services';
 import {
@@ -57,6 +58,7 @@ import {getData} from '../../../src/event-helper';
 import {getDefaultBootstrapBaseUrl} from '../../../src/3p-frame';
 import {
   getExperimentBranch,
+  isExperimentOn,
   randomlySelectUnsetExperiments,
 } from '../../../src/experiments';
 import {getMode} from '../../../src/mode';
@@ -225,6 +227,13 @@ export class AmpAdNetworkAdsenseImpl extends AmpA4A {
           Number(this.element.getAttribute('height')) > 0,
         branches: ['21062003', '21062004'],
       },
+      [AMP_AD_NO_CENTER_CSS_EXP.id]: {
+        isTrafficEligible: () => true,
+        branches: [
+          AMP_AD_NO_CENTER_CSS_EXP.control,
+          AMP_AD_NO_CENTER_CSS_EXP.experiment,
+        ],
+      },
       [[FIE_INIT_CHUNKING_EXP.id]]: {
         isTrafficEligible: () => true,
         branches: [
@@ -528,6 +537,18 @@ export class AmpAdNetworkAdsenseImpl extends AmpA4A {
       height: `${this.size_.height}px`,
     });
 
+    // Set the centering CSS if the experiment is off
+    if (
+      !isExperimentOn(this.win, 'amp-ad-no-center-css') ||
+      getExperimentBranch(this.win, AMP_AD_NO_CENTER_CSS_EXP.id) ===
+        AMP_AD_NO_CENTER_CSS_EXP.control
+    ) {
+      setStyles(dev().assertElement(this.iframe), {
+        top: '50%',
+        left: '50%',
+        transform: 'translate(-50%, -50%)',
+      });
+    }
     if (this.qqid_) {
       this.element.setAttribute('data-google-query-id', this.qqid_);
     }

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
@@ -23,14 +23,7 @@
 import '../../amp-a4a/0.1/real-time-config-manager';
 import {EXPERIMENT_INFO_MAP as AMPDOC_FIE_EXPERIMENT_INFO_MAP} from '../../../src/ampdoc-fie';
 import {
-  AmpA4A,
-  ConsentTupleDef,
-  DEFAULT_SAFEFRAME_VERSION,
-  NO_SIGNING_EXP,
-  XORIGIN_MODE,
-  assignAdUrlToError,
-} from '../../amp-a4a/0.1/amp-a4a';
-import {
+  AMP_AD_NO_CENTER_CSS_EXP,
   AmpAnalyticsConfigDef,
   QQID_HEADER,
   RENDER_ON_IDLE_FIX_EXP,
@@ -51,6 +44,14 @@ import {
   maybeAppendErrorParameter,
   truncAndTimeUrl,
 } from '../../../ads/google/a4a/utils';
+import {
+  AmpA4A,
+  ConsentTupleDef,
+  DEFAULT_SAFEFRAME_VERSION,
+  NO_SIGNING_EXP,
+  XORIGIN_MODE,
+  assignAdUrlToError,
+} from '../../amp-a4a/0.1/amp-a4a';
 import {CONSENT_POLICY_STATE} from '../../../src/consent-state';
 import {Deferred} from '../../../src/utils/promise';
 import {FIE_INIT_CHUNKING_EXP} from '../../../src/friendly-iframe-embed';
@@ -99,15 +100,15 @@ import {
   waitFor3pThrottle,
 } from '../../amp-ad/0.1/concurrent-load';
 import {getCryptoRandomBytesArray, utf8Decode} from '../../../src/utils/bytes';
-import {getMode} from '../../../src/mode';
-import {getMultiSizeDimensions} from '../../../ads/google/utils';
-import {getOrCreateAdCid} from '../../../src/ad-cid';
-
 import {
   getExperimentBranch,
   isExperimentOn,
   randomlySelectUnsetExperiments,
 } from '../../../src/experiments';
+import {getMode} from '../../../src/mode';
+import {getMultiSizeDimensions} from '../../../ads/google/utils';
+import {getOrCreateAdCid} from '../../../src/ad-cid';
+
 import {insertAnalyticsElement} from '../../../src/extension-analytics';
 import {isArray} from '../../../src/types';
 import {isCancellation} from '../../../src/error';
@@ -460,6 +461,13 @@ export class AmpAdNetworkDoubleclickImpl extends AmpA4A {
           RANDOM_SUBDOMAIN_SAFEFRAME_BRANCHES.EXPERIMENT,
         ],
       },
+      [AMP_AD_NO_CENTER_CSS_EXP.id]: {
+        isTrafficEligible: () => true,
+        branches: [
+          [AMP_AD_NO_CENTER_CSS_EXP.control],
+          [AMP_AD_NO_CENTER_CSS_EXP.experiment],
+        ],
+      },
       [[FIE_INIT_CHUNKING_EXP.id]]: {
         isTrafficEligible: () => true,
         branches: [
@@ -1307,6 +1315,19 @@ export class AmpAdNetworkDoubleclickImpl extends AmpA4A {
       position: isMultiSizeFluid ? 'relative' : null,
     });
 
+    // Set the centering CSS if the experiment is off
+    if (
+      !isExperimentOn(this.win, 'amp-ad-no-center-css') ||
+      getExperimentBranch(this.win, AMP_AD_NO_CENTER_CSS_EXP.id) ===
+        AMP_AD_NO_CENTER_CSS_EXP.control
+    ) {
+      setStyles(dev().assertElement(this.iframe), {
+        top: '50%',
+        left: '50%',
+        transform: 'translate(-50%, -50%)',
+      });
+    }
+
     if (this.qqid_) {
       this.element.setAttribute('data-google-query-id', this.qqid_);
     }

diff --git a/tools/experiments/experiments-config.js b/tools/experiments/experiments-config.js
@@ -313,6 +313,11 @@ export const EXPERIMENTS = [
     spec: 'https://github.com/ampproject/amphtml/issues/28435',
     cleanupIssue: 'https://github.com/ampproject/amphtml/issues/28435',
   },
+  {
+    id: 'amp-ad-no-center-css',
+    name: 'Removing the centering css rule for amp-ad',
+    spec: 'https://github.com/ampproject/amphtml/issues/27095',
+  },
   {
     id: 'fie-init-chunking',
     name: 'More chunking for friendly iframe initialization',