From 72a24973f3246c952de918f607ca370443f8d4c3 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Mon, 26 Sep 2022 17:12:47 -0700 Subject: [PATCH] fix: ignore timing data when request served from memory cache (#17595) `Response.timing` contains stale data when the request is served from memory cache, in that we should ignore it and return -1 where we don't know the value. Fixes https://github.com/microsoft/playwright-java/issues/1080 --- .../src/client/browserContext.ts | 6 ++-- .../playwright-core/src/client/network.ts | 6 ++++ .../src/server/chromium/crNetworkManager.ts | 7 ++++- .../playwright-core/src/server/network.ts | 3 ++ tests/library/resource-timing.spec.ts | 28 +++++++++++++++++++ 5 files changed, 45 insertions(+), 5 deletions(-) diff --git a/packages/playwright-core/src/client/browserContext.ts b/packages/playwright-core/src/client/browserContext.ts index 1edb223957..8e0693aa2a 100644 --- a/packages/playwright-core/src/client/browserContext.ts +++ b/packages/playwright-core/src/client/browserContext.ts @@ -126,8 +126,7 @@ export class BrowserContext extends ChannelOwner private _onRequestFailed(request: network.Request, responseEndTiming: number, failureText: string | undefined, page: Page | null) { request._failureText = failureText || null; - if (request._timing) - request._timing.responseEnd = responseEndTiming; + request._setResponseEndTiming(responseEndTiming); this.emit(Events.BrowserContext.RequestFailed, request); if (page) page.emit(Events.Page.RequestFailed, request); @@ -138,8 +137,7 @@ export class BrowserContext extends ChannelOwner const request = network.Request.from(params.request); const response = network.Response.fromNullable(params.response); const page = Page.fromNullable(params.page); - if (request._timing) - request._timing.responseEnd = responseEndTiming; + request._setResponseEndTiming(responseEndTiming); this.emit(Events.BrowserContext.RequestFinished, request); if (page) page.emit(Events.Page.RequestFinished, request); diff --git a/packages/playwright-core/src/client/network.ts b/packages/playwright-core/src/client/network.ts index 3d69720a56..b7f889bd9b 100644 --- a/packages/playwright-core/src/client/network.ts +++ b/packages/playwright-core/src/client/network.ts @@ -240,6 +240,12 @@ export class Request extends ChannelOwner implements ap return (await response._channel.sizes()).sizes; } + _setResponseEndTiming(responseEndTiming: number) { + this._timing.responseEnd = responseEndTiming; + if (this._timing.responseStart === -1) + this._timing.responseStart = responseEndTiming; + } + _finalRequest(): Request { return this._redirectedTo ? this._redirectedTo._finalRequest() : this; } diff --git a/packages/playwright-core/src/server/chromium/crNetworkManager.ts b/packages/playwright-core/src/server/chromium/crNetworkManager.ts index 00bd6c8a6d..b2a67da39d 100644 --- a/packages/playwright-core/src/server/chromium/crNetworkManager.ts +++ b/packages/playwright-core/src/server/chromium/crNetworkManager.ts @@ -303,7 +303,7 @@ export class CRNetworkManager { }; const timingPayload = responsePayload.timing!; let timing: network.ResourceTiming; - if (timingPayload) { + if (timingPayload && !this._responseExtraInfoTracker.servedFromCache(request._requestId)) { timing = { startTime: (timingPayload.requestTime - request._timestamp + request._wallTime) * 1000, domainLookupStart: timingPayload.dnsStart, @@ -638,6 +638,11 @@ class ResponseExtraInfoTracker { info.servedFromCache = true; } + servedFromCache(requestId: string): boolean { + const info = this._requests.get(requestId); + return !!info?.servedFromCache; + } + responseReceivedExtraInfo(event: Protocol.Network.responseReceivedExtraInfoPayload) { const info = this._getOrCreateEntry(event.requestId); info.responseReceivedExtraInfo.push(event); diff --git a/packages/playwright-core/src/server/network.ts b/packages/playwright-core/src/server/network.ts index 494ebc15c7..ed17553451 100644 --- a/packages/playwright-core/src/server/network.ts +++ b/packages/playwright-core/src/server/network.ts @@ -415,6 +415,9 @@ export class Response extends SdkObject { _requestFinished(responseEndTiming: number) { this._request._responseEndTiming = Math.max(responseEndTiming, this._timing.responseStart); + // Set start time equal to end when request is served from memory cache. + if (this._timing.requestStart === -1) + this._timing.requestStart = this._request._responseEndTiming; this._finishedPromise.resolve(); } diff --git a/tests/library/resource-timing.spec.ts b/tests/library/resource-timing.spec.ts index 383c6c5f44..5a5a03c9d2 100644 --- a/tests/library/resource-timing.spec.ts +++ b/tests/library/resource-timing.spec.ts @@ -96,6 +96,34 @@ it('should work for redirect', async ({ contextFactory, browserName, server }) = await context.close(); }); +it('should work when serving from memory cache', async ({ contextFactory, server, browserName }) => { + it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright-java/issues/1080' }); + it.fixme(browserName === 'firefox', 'Response event is not fired in Firefox'); + server.setRoute('/one-style.css', (req, res) => { + res.writeHead(200, { + 'Content-Type': 'text/css', + 'Cache-Control': 'public, max-age=10031518' + }); + res.end(`body { background: red }`); + }); + + const context = await contextFactory(); + const page = await context.newPage(); + await page.goto(server.PREFIX + '/one-style.html'); + const [response] = await Promise.all([ + page.waitForResponse('**/one-style.css'), + page.reload() + ]); + await response.finished(); + + const timing = response.request().timing(); + verifyConnectionTimingConsistency(timing); + + expect(timing.responseStart).toBe(timing.responseEnd); + expect(timing.responseEnd).toBeLessThan(1000); + await context.close(); +}); + function verifyTimingValue(value: number, previous: number) { expect(value === -1 || value > 0 && value >= previous); }