From 4fb7b8827a514ce60217a4a7bb0abfc4843609ed Mon Sep 17 00:00:00 2001 From: Manuel Puyol Date: Tue, 28 Jun 2022 15:18:23 -0500 Subject: [PATCH 1/6] wait for CSS to load before rendering --- src/core/drive/page_renderer.ts | 19 +++++++++++++++---- src/core/view.ts | 6 +++--- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/core/drive/page_renderer.ts b/src/core/drive/page_renderer.ts index 0d320a033..7b841e149 100644 --- a/src/core/drive/page_renderer.ts +++ b/src/core/drive/page_renderer.ts @@ -21,8 +21,8 @@ export class PageRenderer extends Renderer { } } - prepareToRender() { - this.mergeHead() + async prepareToRender() { + await this.mergeHead() } async render() { @@ -50,8 +50,8 @@ export class PageRenderer extends Renderer { return this.newSnapshot.element } - mergeHead() { - this.copyNewHeadStylesheetElements() + async mergeHead() { + await this.copyNewHeadStylesheetElements() this.copyNewHeadScriptElements() this.removeCurrentHeadProvisionalElements() this.copyNewHeadProvisionalElements() @@ -69,9 +69,20 @@ export class PageRenderer extends Renderer { } copyNewHeadStylesheetElements() { + const loading = [] + for (const element of this.newHeadStylesheetElements) { + const promise = new Promise((resolve) => { + ;(element as HTMLLinkElement).onload = () => resolve(null) + ;(element as HTMLLinkElement).onerror = () => resolve(null) + }) + + loading.push(promise) + document.head.appendChild(element) } + + return Promise.all(loading) } copyNewHeadScriptElements() { diff --git a/src/core/view.ts b/src/core/view.ts index db2774dab..d1a6fddd1 100644 --- a/src/core/view.ts +++ b/src/core/view.ts @@ -82,7 +82,7 @@ export abstract class View< try { this.renderPromise = new Promise((resolve) => (this.resolveRenderPromise = resolve)) this.renderer = renderer - this.prepareToRenderSnapshot(renderer) + await this.prepareToRenderSnapshot(renderer) const renderInterception = new Promise((resolve) => (this.resolveInterceptionPromise = resolve)) const immediateRender = this.delegate.allowsImmediateRender(snapshot, this.resolveInterceptionPromise) @@ -106,9 +106,9 @@ export abstract class View< this.delegate.viewInvalidated(reason) } - prepareToRenderSnapshot(renderer: R) { + async prepareToRenderSnapshot(renderer: R) { this.markAsPreview(renderer.isPreview) - renderer.prepareToRender() + await renderer.prepareToRender() } markAsPreview(isPreview: boolean) { From 3480ea58a9cfc8246176c64b78de65bfba84ed76 Mon Sep 17 00:00:00 2001 From: Manuel Puyol Date: Tue, 28 Jun 2022 16:48:44 -0500 Subject: [PATCH 2/6] extract waitForLoad --- src/core/drive/page_renderer.ts | 14 +++++--------- src/util.ts | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/core/drive/page_renderer.ts b/src/core/drive/page_renderer.ts index 7b841e149..e8805d1d1 100644 --- a/src/core/drive/page_renderer.ts +++ b/src/core/drive/page_renderer.ts @@ -1,6 +1,7 @@ import { Renderer } from "../renderer" import { PageSnapshot } from "./page_snapshot" import { ReloadReason } from "../native/browser_adapter" +import { waitForLoad } from "../../util" export class PageRenderer extends Renderer { get shouldRender() { @@ -68,21 +69,16 @@ export class PageRenderer extends Renderer { return this.currentHeadSnapshot.trackedElementSignature == this.newHeadSnapshot.trackedElementSignature } - copyNewHeadStylesheetElements() { - const loading = [] + async copyNewHeadStylesheetElements() { + const loadingElements = [] for (const element of this.newHeadStylesheetElements) { - const promise = new Promise((resolve) => { - ;(element as HTMLLinkElement).onload = () => resolve(null) - ;(element as HTMLLinkElement).onerror = () => resolve(null) - }) - - loading.push(promise) + loadingElements.push(waitForLoad(element as HTMLLinkElement)) document.head.appendChild(element) } - return Promise.all(loading) + await Promise.all(loadingElements) } copyNewHeadScriptElements() { diff --git a/src/util.ts b/src/util.ts index 0f5459a79..e09ac32d8 100644 --- a/src/util.ts +++ b/src/util.ts @@ -92,3 +92,19 @@ export function clearBusyState(...elements: Element[]) { element.removeAttribute("aria-busy") } } + +export function waitForLoad(element: HTMLLinkElement): Promise { + return new Promise((resolve, reject) => { + const onLoad = () => { + element.removeEventListener("error", onError) + resolve() + } + const onError = () => { + element.removeEventListener("load", onLoad) + reject() + } + + element.addEventListener("load", onLoad, { once: true }) + element.addEventListener("error", onError, { once: true }) + }) +} From c234cca637ad85857a5ffe61e29cab333d6b2302 Mon Sep 17 00:00:00 2001 From: Manuel Puyol Date: Tue, 28 Jun 2022 17:18:29 -0500 Subject: [PATCH 3/6] don't use reject --- src/util.ts | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/util.ts b/src/util.ts index e09ac32d8..95a368ea4 100644 --- a/src/util.ts +++ b/src/util.ts @@ -94,17 +94,14 @@ export function clearBusyState(...elements: Element[]) { } export function waitForLoad(element: HTMLLinkElement): Promise { - return new Promise((resolve, reject) => { - const onLoad = () => { - element.removeEventListener("error", onError) + return new Promise((resolve) => { + const onComplete = () => { + element.removeEventListener("error", onComplete) + element.removeEventListener("load", onComplete) resolve() } - const onError = () => { - element.removeEventListener("load", onLoad) - reject() - } - element.addEventListener("load", onLoad, { once: true }) - element.addEventListener("error", onError, { once: true }) + element.addEventListener("load", onComplete, { once: true }) + element.addEventListener("error", onComplete, { once: true }) }) } From 324255421e1d632e0d507717707e34759193a625 Mon Sep 17 00:00:00 2001 From: Manuel Puyol Date: Wed, 29 Jun 2022 09:55:24 -0500 Subject: [PATCH 4/6] don't wait more than 2s --- src/util.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/util.ts b/src/util.ts index 95a368ea4..c4aa0d154 100644 --- a/src/util.ts +++ b/src/util.ts @@ -93,7 +93,7 @@ export function clearBusyState(...elements: Element[]) { } } -export function waitForLoad(element: HTMLLinkElement): Promise { +export function waitForLoad(element: HTMLLinkElement, timeoutInMilliseconds = 2000): Promise { return new Promise((resolve) => { const onComplete = () => { element.removeEventListener("error", onComplete) @@ -103,5 +103,6 @@ export function waitForLoad(element: HTMLLinkElement): Promise { element.addEventListener("load", onComplete, { once: true }) element.addEventListener("error", onComplete, { once: true }) + setTimeout(resolve, timeoutInMilliseconds) }) } From 6cd33c72ddb01e4af8a64d4ca8caef7d0b10684a Mon Sep 17 00:00:00 2001 From: Manuel Puyol Date: Thu, 30 Jun 2022 14:22:40 -0500 Subject: [PATCH 5/6] Load scripts and preload links while CSS is loading too Co-authored-by: Anthony Ricaud --- src/core/drive/page_renderer.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/core/drive/page_renderer.ts b/src/core/drive/page_renderer.ts index e8805d1d1..586d4b3a3 100644 --- a/src/core/drive/page_renderer.ts +++ b/src/core/drive/page_renderer.ts @@ -52,10 +52,11 @@ export class PageRenderer extends Renderer { } async mergeHead() { - await this.copyNewHeadStylesheetElements() + const newStylesheetElements = this.copyNewHeadStylesheetElements() this.copyNewHeadScriptElements() this.removeCurrentHeadProvisionalElements() this.copyNewHeadProvisionalElements() + await newStylesheetElements } replaceBody() { From 3171b0ac1c21e83341eaa355c9c7f47526438827 Mon Sep 17 00:00:00 2001 From: Manuel Puyol Date: Wed, 20 Jul 2022 16:48:55 -0500 Subject: [PATCH 6/6] add tests for slow CSS --- src/tests/functional/rendering_tests.ts | 68 +++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/src/tests/functional/rendering_tests.ts b/src/tests/functional/rendering_tests.ts index 8302da35e..d29b82b88 100644 --- a/src/tests/functional/rendering_tests.ts +++ b/src/tests/functional/rendering_tests.ts @@ -187,6 +187,74 @@ test("test does not evaluate head stylesheet elements inside noscript elements", assert.equal(await isNoscriptStylesheetEvaluated(page), false) }) +test("test waits for CSS to be loaded before rendering", async ({ page }) => { + let finishLoadingCSS = (_value?: unknown) => {} + const promise = new Promise((resolve) => { + finishLoadingCSS = resolve + }) + page.route("**/*.css", async (route) => { + await promise + route.continue() + }) + + await page.click("#additional-assets-link") + + assert.equal(await isStylesheetEvaluated(page), false) + assert.notEqual(await page.textContent("h1"), "Additional assets") + + finishLoadingCSS() + + await nextEventNamed(page, "turbo:render") + + assert.equal(await page.textContent("h1"), "Additional assets") + assert.equal(await isStylesheetEvaluated(page), true) +}) + +test("test waits for CSS to fail before rendering", async ({ page }) => { + let finishLoadingCSS = (_value?: unknown) => {} + const promise = new Promise((resolve) => { + finishLoadingCSS = resolve + }) + page.route("**/*.css", async (route) => { + await promise + route.abort() + }) + + await page.click("#additional-assets-link") + + assert.equal(await isStylesheetEvaluated(page), false) + assert.notEqual(await page.textContent("h1"), "Additional assets") + + finishLoadingCSS() + + await nextEventNamed(page, "turbo:render") + + assert.equal(await page.textContent("h1"), "Additional assets") + assert.equal(await isStylesheetEvaluated(page), false) +}) + +test("test waits for some time, but renders if CSS takes too much to load", async ({ page }) => { + let finishLoadingCSS = (_value?: unknown) => {} + const promise = new Promise((resolve) => { + finishLoadingCSS = resolve + }) + page.route("**/*.css", async (route) => { + await promise + route.continue() + }) + + await page.click("#additional-assets-link") + await nextEventNamed(page, "turbo:render") + + assert.equal(await page.textContent("h1"), "Additional assets") + assert.equal(await isStylesheetEvaluated(page), false) + + finishLoadingCSS() + await nextBeat() + + assert.equal(await isStylesheetEvaluated(page), true) +}) + test("skip evaluates head script elements once", async ({ page }) => { assert.equal(await headScriptEvaluationCount(page), undefined)