diff --git a/packages/trace-viewer/src/snapshotRenderer.ts b/packages/trace-viewer/src/snapshotRenderer.ts index da725fe8f1..0dc67da521 100644 --- a/packages/trace-viewer/src/snapshotRenderer.ts +++ b/packages/trace-viewer/src/snapshotRenderer.ts @@ -114,38 +114,36 @@ export class SnapshotRenderer { resourceByUrl(url: string, method: string): ResourceSnapshot | undefined { const snapshot = this._snapshot; - let result: ResourceSnapshot | undefined; + let sameFrameResource: ResourceSnapshot | undefined; + let otherFrameResource: ResourceSnapshot | undefined; - // First try locating exact resource belonging to this frame. for (const resource of this._resources) { // Only use resources that received response before the snapshot. // Note that both snapshot time and request time are taken in the same Node process. if (typeof resource._monotonicTime === 'number' && resource._monotonicTime >= snapshot.timestamp) break; - if (resource._frameref !== snapshot.frameId) + if (resource.response.status === 304) { + // "Not Modified" responses are issued when browser requests the same resource + // multiple times, meanwhile indicating that it has the response cached. + // + // When rendering the snapshot, browser most likely will not have the resource cached, + // so we should respond with the real content instead, picking the last response that + // is not 304. continue; + } if (resource.request.url === url && resource.request.method === method) { // 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; - } - } - - if (!result) { - // Then fall back to resource with this URL to account for memory cache. - for (const resource of this._resources) { - // Only use resources that received response before the snapshot. - // Note that both snapshot time and request time are taken in the same Node process. - if (typeof resource._monotonicTime === 'number' && resource._monotonicTime >= snapshot.timestamp) - break; - if (resource.request.url === url && resource.request.method === method) { - // 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; - } + if (resource._frameref === snapshot.frameId) + sameFrameResource = resource; + else + otherFrameResource = resource; } } + // First try locating exact resource belonging to this frame, + // then fall back to resource with this URL to account for memory cache. + let result = sameFrameResource ?? otherFrameResource; if (result && method.toUpperCase() === 'GET') { // Patch override if necessary. for (const o of snapshot.resourceOverrides) { diff --git a/tests/library/trace-viewer.spec.ts b/tests/library/trace-viewer.spec.ts index e76978514b..ba86b7a0b0 100644 --- a/tests/library/trace-viewer.spec.ts +++ b/tests/library/trace-viewer.spec.ts @@ -919,3 +919,40 @@ test('should prefer later resource request with the same method', async ({ page, const frame2 = await traceViewer.snapshotFrame('locator.click'); await expect(frame2.locator('body')).toHaveCSS('background-color', 'rgb(123, 123, 123)'); }); + +test('should ignore 304 responses', async ({ page, server, runAndTrace }) => { + const html = ` + + + + +
Hello
+ + `; + + server.setRoute('/style.css', async (req, res) => { + if (req.headers['if-modified-since']) { + res.statusCode = 304; // not modified + res.end(); + return; + } + res.setHeader('Cache-Control', 'public, max-age=31536000, no-cache'); + res.setHeader('Last-Modified', (new Date()).toISOString()); + res.end('body { background-color: rgb(123, 123, 123) }'); + }); + server.setRoute('/index.html', (req, res) => res.end(html)); + + const traceViewer = await runAndTrace(async () => { + const request1 = page.waitForEvent('requestfinished', req => req.url() === server.PREFIX + '/style.css'); + await page.goto(server.PREFIX + '/index.html'); + await request1; + await page.waitForTimeout(1000); + const request2 = page.waitForEvent('requestfinished', req => req.url() === server.PREFIX + '/style.css'); + await page.goto(server.PREFIX + '/index.html'); + await request2; + await page.waitForTimeout(1000); + await page.locator('div').click(); + }); + const frame = await traceViewer.snapshotFrame('locator.click'); + await expect(frame.locator('body')).toHaveCSS('background-color', 'rgb(123, 123, 123)'); +});