From 132a5a4bf5b44058710597402bc41a968961c806 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Sat, 17 Jun 2023 06:58:16 -0700 Subject: [PATCH] fix(trace viewer): prefer latest resource with the same url (#23763) When rendering snapshot, disregard earlier resources with the same url, because it's most likely that the latest one was used for rendering. An example would be reloading the page before the stylesheet has finished loading. In this case, the stylesheet will be requested twice, and the second copy that was not aborted should be used for the snapshot. Fixes #23709. --- packages/trace-viewer/src/snapshotRenderer.ts | 10 ++++-- tests/library/trace-viewer.spec.ts | 35 +++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/packages/trace-viewer/src/snapshotRenderer.ts b/packages/trace-viewer/src/snapshotRenderer.ts index 4868515451..1b6c177a68 100644 --- a/packages/trace-viewer/src/snapshotRenderer.ts +++ b/packages/trace-viewer/src/snapshotRenderer.ts @@ -123,8 +123,9 @@ export class SnapshotRenderer { if (resource._frameref !== snapshot.frameId) continue; if (resource.request.url === url) { + // Pick the last resource with matching url - most likely it was used + // at the time of snapshot, not the earlier aborted resource with the same url. result = resource; - break; } } @@ -133,8 +134,11 @@ export class SnapshotRenderer { for (const resource of this._resources) { if (typeof resource._monotonicTime === 'number' && resource._monotonicTime >= snapshot.timestamp) break; - if (resource.request.url === url) - return resource; + if (resource.request.url === url) { + // Pick the last resource with matching url - most likely it was used + // at the time of snapshot, not the earlier aborted resource with the same url. + result = resource; + } } } diff --git a/tests/library/trace-viewer.spec.ts b/tests/library/trace-viewer.spec.ts index ff748852ee..825ae96900 100644 --- a/tests/library/trace-viewer.spec.ts +++ b/tests/library/trace-viewer.spec.ts @@ -850,3 +850,38 @@ test('should open trace-1.31', async ({ showTraceViewer }) => { const snapshot = await traceViewer.snapshotFrame('locator.click'); await expect(snapshot.locator('[__playwright_target__]')).toHaveText(['Submit']); }); + +test('should prefer later resource request', async ({ page, server, runAndTrace }) => { + const html = ` + + + + `; + + let reloadStartedCallback = () => {}; + const reloadStartedPromise = new Promise(f => reloadStartedCallback = f); + server.setRoute('/style.css', async (req, res) => { + // Make sure reload happens before style arrives. + await reloadStartedPromise; + res.end('body { background-color: rgb(123, 123, 123) }'); + }); + server.setRoute('/index.html', (req, res) => res.end(html)); + server.setRoute('/index.html?reloaded', (req, res) => { + reloadStartedCallback(); + res.end(html); + }); + + const traceViewer = await runAndTrace(async () => { + await page.goto(server.PREFIX + '/index.html'); + }); + const frame = await traceViewer.snapshotFrame('page.goto'); + await expect(frame.locator('body')).toHaveCSS('background-color', 'rgb(123, 123, 123)'); +});