Cleanup fie-resources experiment (#32226)

ampproject · Jan 29, 2021 · 4b96724 · 4b96724
1 parent 7b09938
commit 4b96724
Show file tree

Hide file tree

Showing 6 changed files with 6 additions and 130 deletions.
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
@@ -23,7 +23,6 @@
 import {AdsenseSharedState} from './adsense-shared-state';
 import {AmpA4A} from '../../amp-a4a/0.1/amp-a4a';
 import {CONSENT_POLICY_STATE} from '../../../src/consent-state';
-import {FIE_RESOURCES_EXP} from '../../../src/experiments/fie-resources-exp';
 import {INTERSECT_RESOURCES_EXP} from '../../../src/experiments/intersect-resources-exp';
 import {Navigation} from '../../../src/service/navigation';
 import {
@@ -255,13 +254,6 @@ export class AmpAdNetworkAdsenseImpl extends AmpA4A {
     if (intersectResourcesExpId) {
       addExperimentIdToElement(intersectResourcesExpId, this.element);
     }
-    const fieResourcesExpId = getExperimentBranch(
-      this.win,
-      FIE_RESOURCES_EXP.id
-    );
-    if (fieResourcesExpId) {
-      addExperimentIdToElement(fieResourcesExpId, this.element);
-    }
     const ssrExpIds = this.getSsrExpIds_();
     for (let i = 0; i < ssrExpIds.length; i++) {
       addAmpExperimentIdToElement(ssrExpIds[i], this.element);

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
@@ -51,7 +51,6 @@ import {
 } from '../../../ads/google/a4a/utils';
 import {CONSENT_POLICY_STATE} from '../../../src/consent-state';
 import {Deferred} from '../../../src/utils/promise';
-import {FIE_RESOURCES_EXP} from '../../../src/experiments/fie-resources-exp';
 import {
   FlexibleAdSlotDataTypeDef,
   getFlexibleAdSlotData,
@@ -511,14 +510,6 @@ export class AmpAdNetworkDoubleclickImpl extends AmpA4A {
       this.experimentIds.push(intersectResourcesExpId);
     }
 
-    const fieResourcesExpId = getExperimentBranch(
-      this.win,
-      FIE_RESOURCES_EXP.id
-    );
-    if (fieResourcesExpId) {
-      this.experimentIds.push(fieResourcesExpId);
-    }
-
     const ssrExpIds = this.getSsrExpIds_();
     for (let i = 0; i < ssrExpIds.length; i++) {
       addAmpExperimentIdToElement(ssrExpIds[i], this.element);

diff --git a/src/experiments/fie-resources-exp.js b/src/experiments/fie-resources-exp.js
diff --git a/src/friendly-iframe-embed.js b/src/friendly-iframe-embed.js
@@ -17,7 +17,6 @@
 import {CommonSignals} from './common-signals';
 import {Deferred} from './utils/promise';
 import {FIE_EMBED_PROP} from './iframe-helper';
-import {FIE_RESOURCES_EXP} from './experiments/fie-resources-exp';
 import {Services} from './services';
 import {Signals} from './utils/signals';
 import {VisibilityState} from './visibility-state';
@@ -29,7 +28,6 @@ import {
   setParentWindow,
 } from './service';
 import {escapeHtml} from './dom';
-import {getExperimentBranch} from './experiments';
 import {install as installAbortController} from './polyfills/abort-controller';
 import {installAmpdocServicesForEmbed} from './service/core-services';
 import {install as installCustomElements} from './polyfills/custom-elements';
@@ -390,8 +388,6 @@ export class FriendlyIframeEmbed {
    * Ensures that all resources from this iframe have been released.
    */
   destroy() {
-    // TODO(#31246): remove when the fie-resources experiment is cleaned up.
-    this.removeResources_();
     disposeServicesForEmbed(this.win);
     if (this.ampdoc) {
       this.ampdoc.dispose();
@@ -521,18 +517,9 @@ export class FriendlyIframeEmbed {
         this.win./*OK*/ innerHeight
       );
     }
-    const fieResourcesOn =
-      this.ampdoc &&
-      this.ampdoc.getParent() &&
-      getExperimentBranch(this.ampdoc.getParent().win, FIE_RESOURCES_EXP.id) ===
-        FIE_RESOURCES_EXP.experiment;
     Promise.all([
       this.whenRenderComplete(),
-      whenContentIniLoad(
-        fieResourcesOn ? this.ampdoc : this.iframe,
-        this.win,
-        rect
-      ),
+      whenContentIniLoad(this.ampdoc, this.win, rect),
     ]).then(() => {
       this.signals_.signal(CommonSignals.INI_LOAD);
     });
@@ -559,16 +546,6 @@ export class FriendlyIframeEmbed {
     );
   }
 
-  /**
-   * @return {!./service/resources-interface.ResourcesInterface}
-   * @private
-   */
-  getResources_() {
-    const host =
-      this.host && !this.iframe.isConnected ? this.host : this.iframe;
-    return Services.resourcesForDoc(host);
-  }
-
   /**
    * @return {!./service/mutator-interface.MutatorInterface}
    * @private
@@ -592,21 +569,6 @@ export class FriendlyIframeEmbed {
     );
   }
 
-  /**
-   * Removes all resources belonging to the FIE window.
-   * @private
-   */
-  removeResources_() {
-    const resources = this.getResources_();
-    const toRemove = resources
-      .get()
-      .filter((resource) => resource.hostWin == this.win);
-    toRemove.forEach((resource) => {
-      resources.remove(resource.element);
-      resource.disconnect();
-    });
-  }
-
   /**
    * @return {!Promise}
    */

diff --git a/src/service/core-services.js b/src/service/core-services.js
@@ -14,13 +14,8 @@
  * limitations under the License.
  */
 
-import {
-  FIE_RESOURCES_EXP,
-  divertFieResources,
-} from '../experiments/fie-resources-exp';
 import {adoptServiceForEmbedDoc} from '../service';
 import {devAssert} from '../log';
-import {getExperimentBranch} from '../experiments';
 import {installActionServiceForDoc} from './action-impl';
 import {installBatchedXhrService} from './batched-xhr-impl';
 import {installCidService} from './cid-impl';
@@ -130,31 +125,11 @@ function installAmpdocServicesInternal(ampdoc, isEmbedded) {
     ? adoptServiceForEmbedDoc(ampdoc, 'history')
     : installHistoryServiceForDoc(ampdoc);
 
-  // fie-resources experiment.
-  if (ampdoc.isSingleDoc()) {
-    divertFieResources(ampdoc.win);
-  }
-  const fieResourcesOn =
-    ampdoc.getParent() &&
-    getExperimentBranch(ampdoc.getParent().win, FIE_RESOURCES_EXP.id) ===
-      FIE_RESOURCES_EXP.experiment;
-  if (fieResourcesOn) {
-    isEmbedded
-      ? installInaboxResourcesServiceForDoc(ampdoc)
-      : installResourcesServiceForDoc(ampdoc);
-    installOwnersServiceForDoc(ampdoc);
-    installMutatorServiceForDoc(ampdoc);
-  } else {
-    isEmbedded
-      ? adoptServiceForEmbedDoc(ampdoc, 'resources')
-      : installResourcesServiceForDoc(ampdoc);
-    isEmbedded
-      ? adoptServiceForEmbedDoc(ampdoc, 'owners')
-      : installOwnersServiceForDoc(ampdoc);
-    isEmbedded
-      ? adoptServiceForEmbedDoc(ampdoc, 'mutator')
-      : installMutatorServiceForDoc(ampdoc);
-  }
+  isEmbedded
+    ? installInaboxResourcesServiceForDoc(ampdoc)
+    : installResourcesServiceForDoc(ampdoc);
+  installOwnersServiceForDoc(ampdoc);
+  installMutatorServiceForDoc(ampdoc);
 
   isEmbedded
     ? adoptServiceForEmbedDoc(ampdoc, 'url-replace')

diff --git a/tools/experiments/experiments-config.js b/tools/experiments/experiments-config.js
@@ -210,11 +210,6 @@ export const EXPERIMENTS = [
     name: 'Proxy for TCF PostMessageAPI to send TCData to 3p iframes',
     spec: 'https://github.com/ampproject/amphtml/issues/30385',
   },
-  {
-    id: 'fie-resources',
-    name: 'Separate FIE resource manager from the main doc',
-    spec: 'https://github.com/ampproject/amphtml/issues/31246',
-  },
   {
     id: 'dfp-render-on-idle-cwv-exp',
     name: 'To measure the CWV impact of ads idle rendering',