Schedule pass after DOM mutation done by render delaying services. (a…

…mpproject#5148)

build-system/tasks/presubmit-checks.js

@@ -424,6 +424,12 @@ var forbiddenTerms = {
       'src/log.js',
     ],
   },
+  '\\.schedulePass\\(': {
+    message: 'schedulePass is heavy, thinking twice before using it',
+    whitelist: [
+      'src/service/resources-impl.js',
+    ],
+  },
   '(win|Win)(dow)?(\\(\\))?\\.open\\W': {
     message: 'Use dom.openWindowDialog',
     whitelist: [

src/render-delaying-services.js

@@ -41,7 +41,8 @@ const LOAD_TIMEOUT = 3000;
  * Detects any render delaying services that are required on the page,
  * and returns a promise with a timeout.
  * @param {!Window} win
- * @return {!Promise}
+ * @return {!Promise<!Array<*>>} resolves to an Array that has the same length as
+ *     the detected render delaying services
  */
 export function waitForServices(win) {
   const promises = includedServices(win).map(service => {

src/service/resources-impl.js

@@ -695,9 +695,13 @@ export class Resources {
   /**
    * Schedules the work pass at the latest with the specified delay.
    * @param {number=} opt_delay
+   * @param {boolean=} opt_relayoutAll
    * @return {boolean}
    */
-  schedulePass(opt_delay) {
+  schedulePass(opt_delay, opt_relayoutAll) {
+    if (opt_relayoutAll) {
+      this.relayoutAll_ = true;
+    }
     return this.pass_.schedule(opt_delay);
   }
 

src/style-installer.js

@@ -20,6 +20,7 @@ import {performanceFor} from './performance';
 import {platformFor} from './platform';
 import {setStyles} from './style';
 import {waitForServices} from './render-delaying-services';
+import {resourcesForDoc} from './resources';
 
 
 const bodyVisibleSentinel = '__AMP_BODY_VISIBLE';
@@ -128,14 +129,17 @@ export function makeBodyVisible(doc, opt_waitForServices) {
     }
   };
   const win = doc.defaultView;
-  const docState = documentStateFor(win);
-  docState.onBodyAvailable(() => {
+  documentStateFor(win).onBodyAvailable(() => {
     if (win[bodyVisibleSentinel]) {
       return;
     }
     win[bodyVisibleSentinel] = true;
     if (opt_waitForServices) {
-      waitForServices(win).then(set, set).then(() => {
+      waitForServices(win).catch(() => []).then(services => {
+        set();
+        if (services.length > 0) {
+          resourcesForDoc(doc)./*OK*/schedulePass(1, /* relayoutAll */ true);
+        }
         try {
           const perf = performanceFor(win);
           perf.tick('mbv');

test/functional/test-render-delaying-services.js

@@ -51,7 +51,7 @@ describe('waitForServices', () => {
   it('should resolve if no blocking services is presented', () => {
     // <script custom-element="amp-experiment"> should not block
     addExtensionScript(win, 'amp-experiment');
-    return expect(waitForServices(win)).to.eventually.be.fulfilled;
+    return expect(waitForServices(win)).to.eventually.have.lengthOf(0);
   });
 
   it('should timeout if some blocking services are missing', () => {
@@ -79,7 +79,7 @@ describe('waitForServices', () => {
     dynamicCssResolve();
     variantResolve(); // this unblocks 'amp-experiment'
 
-    return expect(promise).to.eventually.be.fulfilled;
+    return expect(promise).to.eventually.have.lengthOf(3);
   });
 });
 

test/functional/test-style-installer.js

@@ -15,91 +15,117 @@
  */
 
 import {getStyle} from '../../src/style';
-import {waitForBodyPromise} from '../../src/dom';
-import {waitForServices} from '../../src/render-delaying-services';
+import * as rds from '../../src/render-delaying-services';
 import {installPerformanceService} from '../../src/service/performance-impl';
-import {resetServiceForTesting} from '../../src/service';
+import {createIframePromise} from '../../testing/iframe';
+import {installResourcesServiceForDoc} from '../../src/service/resources-impl';
 import * as sinon from 'sinon';
 import * as styles from '../../src/style-installer';
 
 
 describe('Styles', () => {
+
   let sandbox;
-  let clock;
-  let perf;
-  const bodyVisibleSentinel = '__AMP_BODY_VISIBLE';
+  let win;
+  let doc;
+  let tickSpy;
+  let schedulePassSpy;
+  let waitForServicesStub;
 
   beforeEach(() => {
-    sandbox = sinon.sandbox.create();
-    clock = sandbox.useFakeTimers();
-    perf = installPerformanceService(document.defaultView);
+    return createIframePromise().then(iframe => {
+      sandbox = sinon.sandbox.create();
+      win = iframe.win;
+      doc = win.document;
+      const perf = installPerformanceService(doc.defaultView);
+      tickSpy = sandbox.spy(perf, 'tick');
+
+      const resources = installResourcesServiceForDoc(doc);
+      schedulePassSpy = sandbox.spy(resources, 'schedulePass');
+      waitForServicesStub = sandbox.stub(rds, 'waitForServices');
+    });
   });
 
   afterEach(() => {
-    resetServiceForTesting(document.defaultView, 'performance');
-    delete document.defaultView[bodyVisibleSentinel];
     sandbox.restore();
   });
 
-  it('makeBodyVisible', () => {
-    styles.makeBodyVisible(document);
-    clock.tick(1000);
-    expect(document.body).to.exist;
-    expect(getStyle(document.body, 'opacity')).to.equal('1');
-    expect(getStyle(document.body, 'visibility')).to.equal('visible');
-    expect(getStyle(document.body, 'animation')).to.equal('none');
-  });
+  describe('makeBodyVisible', () => {
 
-  it('should install runtime styles', () => {
-    const cssText = '/*amp-runtime*/';
-    return new Promise(resolve => {
-      styles.installStyles(document, cssText, () => {
-        resolve();
-      }, true);
-    }).then(() => {
-      const styleEl = document.head.querySelector('style');
-      expect(styleEl.hasAttribute('amp-runtime')).to.be.true;
-      expect(styleEl.textContent).to.equal(cssText);
-      document.head.removeChild(styleEl);
+    it('should work with waitForService=false', () => {
+      expect(getStyle(doc.body, 'opacity')).to.equal('');
+      expect(getStyle(doc.body, 'visibility')).to.equal('');
+      expect(getStyle(doc.body, 'animation')).to.equal('');
+
+      styles.makeBodyVisible(doc);
+      expect(doc.body).to.exist;
+      expect(getStyle(doc.body, 'opacity')).to.equal('1');
+      expect(getStyle(doc.body, 'visibility')).to.equal('visible');
+      expect(getStyle(doc.body, 'animation')).to.equal('none');
     });
-  });
 
-  it('should install extension styles after runtime', () => {
-    const runtimeCssText = '/*amp-runtime*/';
-    const extCssText = '/*amp-ext1*/';
-    styles.installStyles(document, runtimeCssText, () => {}, true);
-    return new Promise(resolve => {
-      styles.installStyles(document, extCssText, () => {
-        resolve();
-      }, false, 'amp-ext1');
-    }).then(() => {
-      const styleEls = document.head.querySelectorAll('style');
-      expect(styleEls[0].hasAttribute('amp-runtime')).to.be.true;
-      expect(styleEls[1].getAttribute('amp-extension')).to.equal('amp-ext1');
-      expect(styleEls[1].textContent).to.equal(extCssText);
-      document.head.removeChild(styleEls[0]);
-      document.head.removeChild(styleEls[1]);
+    it('should wait for render delaying services', done => {
+      expect(getStyle(doc.body, 'opacity')).to.equal('');
+      expect(getStyle(doc.body, 'visibility')).to.equal('');
+      expect(getStyle(doc.body, 'animation')).to.equal('');
+
+      waitForServicesStub.withArgs(win)
+          .returns(Promise.resolve(['service1', 'service2']));
+      styles.makeBodyVisible(doc, true);
+      styles.makeBodyVisible(doc, true); // 2nd call should make no difference
+      setTimeout(() => {
+        expect(getStyle(doc.body, 'opacity')).to.equal('1');
+        expect(getStyle(doc.body, 'visibility')).to.equal('visible');
+        expect(getStyle(doc.body, 'animation')).to.equal('none');
+        expect(tickSpy.withArgs('mbv')).to.be.calledOnce;
+        expect(schedulePassSpy.withArgs(1, true)).to.be.calledOnce;
+        done();
+      }, 0);
+    });
+
+    it('should skip schedulePass if no render delaying services', done => {
+      waitForServicesStub.withArgs(win).returns(Promise.resolve([]));
+      styles.makeBodyVisible(doc, true);
+      setTimeout(() => {
+        expect(tickSpy.withArgs('mbv')).to.be.calledOnce;
+        expect(schedulePassSpy).to.not.be.calledWith(sinon.match.number, true);
+        done();
+      }, 0);
     });
   });
 
-  it('should only set body to be visible only once per document', () => {
-    const tickSpy = sandbox.spy(perf, 'tick');
-    expect(tickSpy.callCount).to.equal(0);
-    expect(document.defaultView[bodyVisibleSentinel]).to.be.undefined;
-    styles.makeBodyVisible(document, true);
-    // mbv = make body visible
-    return waitForBodyPromise(document).then(() => {
-      return waitForServices(document.defaultView).then(() => {
-        expect(tickSpy.lastCall.args[0]).to.equal('mbv');
-        expect(tickSpy.callCount).to.equal(2);
-        expect(document.defaultView[bodyVisibleSentinel]).to.be.true;
-        styles.makeBodyVisible(document, true);
-        return waitForBodyPromise(document).then(() => {
-          return waitForServices(document.defaultView);
-        }).then(() => {
-          expect(tickSpy.callCount).to.equal(2);
-          expect(document.defaultView[bodyVisibleSentinel]).to.be.true;
-        });
+  describe('installStyles', () => {
+
+    it('should install runtime styles', () => {
+      const cssText = '/*amp-runtime*/';
+      return new Promise(resolve => {
+        styles.installStyles(doc, cssText, () => {
+          resolve();
+        }, true);
+      }).then(() => {
+        const styleEl = doc.head.querySelector('style');
+        expect(styleEl.hasAttribute('amp-runtime')).to.be.true;
+        expect(styleEl.textContent).to.equal(cssText);
+        doc.head.removeChild(styleEl);
+      });
+    });
+
+    it('should install extension styles after runtime', () => {
+      const runtimeCssText = '/*amp-runtime*/';
+      const extCssText = '/*amp-ext1*/';
+      styles.installStyles(doc, runtimeCssText, () => {
+      }, true);
+      return new Promise(resolve => {
+        styles.installStyles(doc, extCssText, () => {
+          resolve();
+        }, false, 'amp-ext1');
+      }).then(() => {
+        const styleEls = doc.head.querySelectorAll('style');
+        expect(styleEls[0].hasAttribute('amp-runtime')).to.be.true;
+        expect(styleEls[1].getAttribute('amp-extension')).to.equal('amp-ext1');
+        expect(styleEls[1].textContent).to.equal(extCssText);
+        doc.head.removeChild(styleEls[0]);
+        doc.head.removeChild(styleEls[1]);
       });
     });
   });