Filip/playwright
1
0
Fork 0
Code Issues Pull requests Actions Packages Projects Releases Wiki Activity

fix(trace viewer): do not serve 304 responses (#24435)

Browse source 
These do not have any content, and we should server the original
response that was cached by the browser.

Fixes #24255.
This commit is contained in:
Dmitry Gozman 2023-07-27 08:06:00 -07:00 committed by GitHub
parent 9b364f072d
commit 1b233a5ae2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 54 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

36
packages/trace-viewer/src/snapshotRenderer.ts
View file

@ -114,38 +114,36 @@ export class SnapshotRenderer {
resourceByUrl(url: string, method: string): ResourceSnapshot | undefined { resourceByUrl(url: string, method: string): ResourceSnapshot | undefined {
const snapshot = this._snapshot; 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) { for (const resource of this._resources) {
// Only use resources that received response before the snapshot. // Only use resources that received response before the snapshot.
// Note that both snapshot time and request time are taken in the same Node process. // Note that both snapshot time and request time are taken in the same Node process.
if (typeof resource._monotonicTime === 'number' && resource._monotonicTime >= snapshot.timestamp) if (typeof resource._monotonicTime === 'number' && resource._monotonicTime >= snapshot.timestamp)
break; 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; continue;
}
if (resource.request.url === url && resource.request.method === method) { if (resource.request.url === url && resource.request.method === method) {
// Pick the last resource with matching url - most likely it was used // 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. // 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;
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;
}
} }
} }
// 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') { if (result && method.toUpperCase() === 'GET') {
// Patch override if necessary. // Patch override if necessary.
for (const o of snapshot.resourceOverrides) { for (const o of snapshot.resourceOverrides) {

37
tests/library/trace-viewer.spec.ts
View file

@ -919,3 +919,40 @@ test('should prefer later resource request with the same method', async ({ page,
const frame2 = await traceViewer.snapshotFrame('locator.click'); const frame2 = await traceViewer.snapshotFrame('locator.click');
await expect(frame2.locator('body')).toHaveCSS('background-color', 'rgb(123, 123, 123)'); await expect(frame2.locator('body')).toHaveCSS('background-color', 'rgb(123, 123, 123)');
}); });
test('should ignore 304 responses', async ({ page, server, runAndTrace }) => {
const html = `
<head>
<link rel=stylesheet href="style.css" />
</head>
<body>
<div>Hello</div>
</body>
`;
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)');
});