Remove wait for ini-load when layout a4a (#19738)

* remove wait for ini-load * expr * fix presubmit * address comment
ampproject · Dec 14, 2018 · 9940a5f · 9940a5f
1 parent 303ded9
commit 9940a5f
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 8 deletions.
diff --git a/extensions/amp-a4a/0.1/amp-a4a.js b/extensions/amp-a4a/0.1/amp-a4a.js
@@ -1408,14 +1408,18 @@ export class AmpA4A extends AMP.BaseElement {
         dev().error(TAG, this.element.getAttribute('type'),
             'Error executing onCreativeRender', err);
       })(creativeMetaData, friendlyIframeEmbed.whenWindowLoaded());
-      // It's enough to wait for "ini-load" signal because in a FIE case
-      // we know that the embed no longer consumes significant resources
-      // after the initial load.
-      return friendlyIframeEmbed.whenIniLoaded();
-    }).then(() => {
-      checkStillCurrent();
-      // Capture ini-load ping.
-      this.maybeTriggerAnalyticsEvent_('friendlyIframeIniLoad');
+      const iniLoadPromise = friendlyIframeEmbed.whenIniLoaded().then(() => {
+        checkStillCurrent();
+        this.maybeTriggerAnalyticsEvent_('friendlyIframeIniLoad');
+      });
+      const isIniLoadFixExpr = !!frameDoc.querySelector(
+          'meta[name="amp-experiments-opt-in"][content*="fie_ini_load_fix"]');
+      if (!isIniLoadFixExpr) {
+        return iniLoadPromise;
+      }
+
+      // There's no need to wait for all resources to load.
+      // StartRender is enough
     });
   }
 

diff --git a/extensions/amp-a4a/0.1/test/test-amp-a4a.js b/extensions/amp-a4a/0.1/test/test-amp-a4a.js
@@ -422,6 +422,7 @@ describe('amp-a4a', () => {
       });
     });
 
+    // TODO: Remove after launch fie_ini_load_fix to 100%
     it('for A4A FIE should wait for initial layout', () => {
       let iniLoadResolver;
       const iniLoadPromise = new Promise(resolve => {
@@ -448,6 +449,25 @@ describe('amp-a4a', () => {
       });
     });
 
+    it('for A4A layout should resolve once FIE is created', () => {
+      a4a.buildCallback();
+      a4a.onLayoutMeasure();
+
+      // Never resolve
+      sandbox.stub/*OK*/(FriendlyIframeEmbed.prototype,'whenIniLoaded')
+          .callsFake(() => {return new Promise(() => {});});
+      let creativeString = buildCreativeString();
+      // TODO: Remove after launch fie_ini_load_fix to 100%
+      creativeString = creativeString.replace('<body>',
+          '<body>' +
+          '<meta name=amp-experiments-opt-in content=a,fie_ini_load_fix,b>');
+      const metaData = a4a.getAmpAdMetadata(creativeString);
+      return a4a.renderAmpCreative_(metaData).then(() => {
+        expect(a4a.friendlyIframeEmbed_).to.exist;
+        expect(a4a.friendlyIframeEmbed_.host).to.equal(a4a.element);
+      });
+    });
+
     it('should fire amp-analytics triggers for lifecycle events', () => {
       let iniLoadResolver;
       const iniLoadPromise = new Promise(resolve => {
@@ -506,6 +526,8 @@ describe('amp-a4a', () => {
           sandbox.spy(analytics, 'triggerAnalyticsEvent');
       a4a.buildCallback();
       a4a.onLayoutMeasure();
+      sandbox.stub/*OK*/(FriendlyIframeEmbed.prototype, 'whenIniLoaded')
+          .callsFake(() => Promise.resolve());
       return a4a.layoutCallback().then(() => {
         verifyA4aAnalyticsTriggersWereFired(a4a, triggerAnalyticsEventSpy);
       });