From 606350c7dd8fe8a397e787d510a5539df75a0891 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Thu, 6 Jul 2017 10:44:12 -0700 Subject: [PATCH] Wait until doc-ready before scheduling font-wait. (#10279) The 3s timeout counts from the time the font is requested which cannot be done before the doc is ready to be styled. Follow up for #10195 and #10267 --- src/service/resources-impl.js | 30 ++++++++++++------------- test/functional/test-resources.js | 37 ++++++++++++++++++++----------- testing/fake-dom.js | 5 ++++- 3 files changed, 43 insertions(+), 29 deletions(-) diff --git a/src/service/resources-impl.js b/src/service/resources-impl.js index 651856d8a433..fded8f01e1f5 100644 --- a/src/service/resources-impl.js +++ b/src/service/resources-impl.js @@ -257,22 +257,22 @@ export class Resources { } this.schedulePass(); this.monitorInput_(); - }); - // Safari 10 and under incorrectly estimates font spacing for `@font-face` - // fonts. This leads to wild measurement errors. The best course of action - // is to remeasure everything on window.onload or font timeout (3s), - // whichever is earlier. This has to be done on the global window because - // this is where the fonts are always added. Unfortunately, - // `document.fonts.ready` cannot be used here due to - // https://bugs.webkit.org/show_bug.cgi?id=174030. - // See https://bugs.webkit.org/show_bug.cgi?id=174031 for more details. - Promise.race([ - loadPromise(this.win), - timerFor(this.win).promise(3100), - ]).then(() => { - this.relayoutAll_ = true; - this.schedulePass(); + // Safari 10 and under incorrectly estimates font spacing for + // `@font-face` fonts. This leads to wild measurement errors. The best + // course of action is to remeasure everything on window.onload or font + // timeout (3s), whichever is earlier. This has to be done on the global + // window because this is where the fonts are always added. + // Unfortunately, `document.fonts.ready` cannot be used here due to + // https://bugs.webkit.org/show_bug.cgi?id=174030. + // See https://bugs.webkit.org/show_bug.cgi?id=174031 for more details. + Promise.race([ + loadPromise(this.win), + timerFor(this.win).promise(3100), + ]).then(() => { + this.relayoutAll_ = true; + this.schedulePass(); + }); }); } diff --git a/test/functional/test-resources.js b/test/functional/test-resources.js index a3f6beb5ac9c..5bb13ec376f0 100644 --- a/test/functional/test-resources.js +++ b/test/functional/test-resources.js @@ -564,25 +564,36 @@ describes.fakeWin('Resources startup', { expect(schedulePassStub).to.not.be.called; win.readyState = 'complete'; win.eventListeners.fire({type: 'load'}); - return loadPromise(win).then(() => { - // Skip a microtask. - return Promise.race([Promise.resolve()]); + win.document.eventListeners.fire({type: 'readystatechange'}); + return resources.ampdoc.whenReady().then(() => { + expect(resources.relayoutAll_).to.be.true; + // Reset to make sure it is set again on load. + resources.relayoutAll_ = false; + return loadPromise(win).then(() => { + // Skip a microtask. + return Promise.race([Promise.resolve()]); + }); }).then(() => { expect(resources.relayoutAll_).to.be.true; - expect(schedulePassStub).to.be.calledOnce; + expect(schedulePassStub).to.be.calledTwice; }); }); it('should run a full reload pass on fonts timeout', () => { - expect(resources.relayoutAll_).to.be.false; - expect(schedulePassStub).to.not.be.called; - clock.tick(4000); - // Skip a microtask. - return Promise.resolve().then(() => { - return Promise.race([Promise.resolve()]); - }).then(() => { - expect(resources.relayoutAll_).to.be.true; - expect(schedulePassStub).to.be.calledOnce; + win.readyState = 'complete'; + win.document.eventListeners.fire({type: 'readystatechange'}); + return resources.ampdoc.whenReady().then(() => { + return; + expect(resources.relayoutAll_).to.be.false; + expect(schedulePassStub).to.not.be.called; + clock.tick(4000); + // Skip a microtask. + return Promise.resolve().then(() => { + return Promise.race([Promise.resolve()]); + }).then(() => { + expect(resources.relayoutAll_).to.be.true; + expect(schedulePassStub).to.be.calledOnce; + }); }); }); }); diff --git a/testing/fake-dom.js b/testing/fake-dom.js index 833ecbcaf2a8..29b124dee64b 100644 --- a/testing/fake-dom.js +++ b/testing/fake-dom.js @@ -41,7 +41,10 @@ export class FakeWindow { const spec = opt_spec || {}; - /** @type {string} */ + /** + * This value is reflected on this.document.readyState. + * @type {string} + */ this.readyState = spec.readyState || 'complete'; // Passthrough.