From 49a150cd87a2d2aa5a3863816f419eeaa51e8ef5 Mon Sep 17 00:00:00 2001 From: Robin Richtsfeld Date: Thu, 8 Apr 2021 03:01:38 +0200 Subject: [PATCH 1/5] test(requestinterception): add failing test for fonts issue --- test/assets/cached/one-style-font.css | 9 +++++++++ test/assets/cached/one-style-font.html | 2 ++ test/requestinterception.spec.ts | 9 +++++++++ 3 files changed, 20 insertions(+) create mode 100644 test/assets/cached/one-style-font.css create mode 100644 test/assets/cached/one-style-font.html diff --git a/test/assets/cached/one-style-font.css b/test/assets/cached/one-style-font.css new file mode 100644 index 0000000000000..6178de0350e98 --- /dev/null +++ b/test/assets/cached/one-style-font.css @@ -0,0 +1,9 @@ +@font-face { + font-family: 'one-style'; + src: url('./one-style.woff') format('woff'); +} + +body { + background-color: pink; + font-family: 'one-style', sans-serif; +} diff --git a/test/assets/cached/one-style-font.html b/test/assets/cached/one-style-font.html new file mode 100644 index 0000000000000..8e7236dfb35e3 --- /dev/null +++ b/test/assets/cached/one-style-font.html @@ -0,0 +1,2 @@ + +
hello, world!
diff --git a/test/requestinterception.spec.ts b/test/requestinterception.spec.ts index 35e225bb41d8f..57a06ac251e6a 100644 --- a/test/requestinterception.spec.ts +++ b/test/requestinterception.spec.ts @@ -525,6 +525,15 @@ describe('request interception', function () { await page.reload(); expect(cached.length).toBe(1); }); + it('should load fonts if cache-safe', async () => { + const { page, server } = getTestState(); + + await page.setRequestInterception(true, true); + page.on('request', (request) => request.continue()); + + await page.goto(server.PREFIX + '/cached/one-style-font.html'); + await page.waitForResponse((r) => r.url().endsWith('/one-style.woff')); + }); }); describeFailsFirefox('Request.continue', function () { From 71d4c4af0e0f24ff02d3df1185fba2072d4a1be4 Mon Sep 17 00:00:00 2001 From: Robin Richtsfeld Date: Thu, 8 Apr 2021 03:07:38 +0200 Subject: [PATCH 2/5] refactor(requestinterception): refactor NetworkManager.ts --- src/common/NetworkManager.ts | 68 +++++++++++++++++++++++++----------- 1 file changed, 47 insertions(+), 21 deletions(-) diff --git a/src/common/NetworkManager.ts b/src/common/NetworkManager.ts index e29a06f94e2a8..7827c3f9c8827 100644 --- a/src/common/NetworkManager.ts +++ b/src/common/NetworkManager.ts @@ -69,11 +69,17 @@ export class NetworkManager extends EventEmitter { _client: CDPSession; _ignoreHTTPSErrors: boolean; _frameManager: FrameManager; - _requestIdToRequest = new Map(); + _requestIdToRequestWillBeSentEvent = new Map< string, Protocol.Network.RequestWillBeSentEvent >(); + _requestIdToRequestPausedEvent = new Map< + string, + Protocol.Fetch.RequestPausedEvent + >(); + _requestIdToRequest = new Map(); + _extraHTTPHeaders: Record = {}; _credentials?: Credentials = null; _attemptedAuthentications = new Set(); @@ -81,7 +87,6 @@ export class NetworkManager extends EventEmitter { _userRequestInterceptionCacheSafe = false; _protocolRequestInterceptionEnabled = false; _userCacheDisabled = false; - _requestIdToInterceptionId = new Map(); _emulatedNetworkConditions: InternalNetworkConditions = { offline: false, upload: -1, @@ -222,12 +227,17 @@ export class NetworkManager extends EventEmitter { } } + _cacheDisabled(): boolean { + return ( + this._userCacheDisabled || + (this._userRequestInterceptionEnabled && + !this._userRequestInterceptionCacheSafe) + ); + } + async _updateProtocolCacheDisabled(): Promise { await this._client.send('Network.setCacheDisabled', { - cacheDisabled: - this._userCacheDisabled || - (this._userRequestInterceptionEnabled && - !this._userRequestInterceptionCacheSafe), + cacheDisabled: this._cacheDisabled(), }); } @@ -238,13 +248,18 @@ export class NetworkManager extends EventEmitter { !event.request.url.startsWith('data:') ) { const requestId = event.requestId; - const interceptionId = this._requestIdToInterceptionId.get(requestId); - if (interceptionId) { + const requestPausedEvent = this._requestIdToRequestPausedEvent.get( + requestId + ); + + if (requestPausedEvent) { + const interceptionId = requestPausedEvent.requestId; this._onRequest(event, interceptionId); - this._requestIdToInterceptionId.delete(requestId); + this._requestIdToRequestPausedEvent.delete(requestId); } else { - this._requestIdToRequestWillBeSentEvent.set(event.requestId, event); + this._requestIdToRequestWillBeSentEvent.set(requestId, event); } + return; } this._onRequest(event, null); @@ -288,14 +303,20 @@ export class NetworkManager extends EventEmitter { const requestId = event.networkId; const interceptionId = event.requestId; - if (requestId && this._requestIdToRequestWillBeSentEvent.has(requestId)) { - const requestWillBeSentEvent = this._requestIdToRequestWillBeSentEvent.get( - requestId - ); + + if (!requestId) { + return; + } + + const requestWillBeSentEvent = this._requestIdToRequestWillBeSentEvent.get( + requestId + ); + + if (requestWillBeSentEvent) { this._onRequest(requestWillBeSentEvent, interceptionId); this._requestIdToRequestWillBeSentEvent.delete(requestId); } else { - this._requestIdToInterceptionId.set(requestId, interceptionId); + this._requestIdToRequestPausedEvent.set(requestId, event); } } @@ -346,8 +367,7 @@ export class NetworkManager extends EventEmitter { response._resolveBody( new Error('Response body is unavailable for redirect responses') ); - this._requestIdToRequest.delete(request._requestId); - this._attemptedAuthentications.delete(request._interceptionId); + this._forgetRequest(request); this.emit(NetworkManagerEmittedEvents.Response, response); this.emit(NetworkManagerEmittedEvents.RequestFinished, request); } @@ -361,6 +381,14 @@ export class NetworkManager extends EventEmitter { this.emit(NetworkManagerEmittedEvents.Response, response); } + _forgetRequest(request: HTTPRequest): void { + const requestId = request._requestId; + const interceptionId = request._interceptionId; + + this._requestIdToRequest.delete(requestId); + this._attemptedAuthentications.delete(interceptionId); + } + _onLoadingFinished(event: Protocol.Network.LoadingFinishedEvent): void { const request = this._requestIdToRequest.get(event.requestId); // For certain requestIds we never receive requestWillBeSent event. @@ -370,8 +398,7 @@ export class NetworkManager extends EventEmitter { // Under certain conditions we never get the Network.responseReceived // event from protocol. @see https://crbug.com/883475 if (request.response()) request.response()._resolveBody(null); - this._requestIdToRequest.delete(request._requestId); - this._attemptedAuthentications.delete(request._interceptionId); + this._forgetRequest(request); this.emit(NetworkManagerEmittedEvents.RequestFinished, request); } @@ -383,8 +410,7 @@ export class NetworkManager extends EventEmitter { request._failureText = event.errorText; const response = request.response(); if (response) response._resolveBody(null); - this._requestIdToRequest.delete(request._requestId); - this._attemptedAuthentications.delete(request._interceptionId); + this._forgetRequest(request); this.emit(NetworkManagerEmittedEvents.RequestFailed, request); } } From 5f183e68e691acc7b8115f7c09345921214e4a72 Mon Sep 17 00:00:00 2001 From: Robin Richtsfeld Date: Thu, 8 Apr 2021 03:08:57 +0200 Subject: [PATCH 3/5] fix(requestinterception): fix fonts loading issue --- src/common/NetworkManager.ts | 48 ++++++++++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 7 deletions(-) diff --git a/src/common/NetworkManager.ts b/src/common/NetworkManager.ts index 7827c3f9c8827..2ecafb25076e2 100644 --- a/src/common/NetworkManager.ts +++ b/src/common/NetworkManager.ts @@ -70,6 +70,25 @@ export class NetworkManager extends EventEmitter { _ignoreHTTPSErrors: boolean; _frameManager: FrameManager; + /* + * There are four possible orders of events: + * A. `_onRequestWillBeSent` + * B. `_onRequestWillBeSent`, `_onRequestPaused` + * C. `_onRequestPaused`, `_onRequestWillBeSent` + * D. `_onRequestPaused`, `_onRequestWillBeSent`, `_onRequestPaused` + * (see crbug.com/1196004) + * + * For `_onRequest` we need the event from `_onRequestWillBeSent` and + * optionally the `interceptionId` from `_onRequestPaused`. + * + * If request interception is disabled, call `_onRequest` once per call to + * `_onRequestWillBeSent`. + * If request interception is enabled, call `_onRequest` once per call to + * `_onRequestPaused` (once per `interceptionId`). + * + * Events are stored to allow for subsequent events to call `_onRequest`. + * Note that redirect requests have the same `requestId` (!). + */ _requestIdToRequestWillBeSentEvent = new Map< string, Protocol.Network.RequestWillBeSentEvent @@ -252,12 +271,12 @@ export class NetworkManager extends EventEmitter { requestId ); + this._requestIdToRequestWillBeSentEvent.set(requestId, event); + if (requestPausedEvent) { const interceptionId = requestPausedEvent.requestId; this._onRequest(event, interceptionId); this._requestIdToRequestPausedEvent.delete(requestId); - } else { - this._requestIdToRequestWillBeSentEvent.set(requestId, event); } return; @@ -308,10 +327,20 @@ export class NetworkManager extends EventEmitter { return; } - const requestWillBeSentEvent = this._requestIdToRequestWillBeSentEvent.get( + let requestWillBeSentEvent = this._requestIdToRequestWillBeSentEvent.get( requestId ); + // redirect requests have the same `requestId`, + if ( + requestWillBeSentEvent && + (requestWillBeSentEvent.request.url !== event.request.url || + requestWillBeSentEvent.request.method !== event.request.method) + ) { + this._requestIdToRequestWillBeSentEvent.delete(requestId); + requestWillBeSentEvent = null; + } + if (requestWillBeSentEvent) { this._onRequest(requestWillBeSentEvent, interceptionId); this._requestIdToRequestWillBeSentEvent.delete(requestId); @@ -367,7 +396,7 @@ export class NetworkManager extends EventEmitter { response._resolveBody( new Error('Response body is unavailable for redirect responses') ); - this._forgetRequest(request); + this._forgetRequest(request, false); this.emit(NetworkManagerEmittedEvents.Response, response); this.emit(NetworkManagerEmittedEvents.RequestFinished, request); } @@ -381,12 +410,17 @@ export class NetworkManager extends EventEmitter { this.emit(NetworkManagerEmittedEvents.Response, response); } - _forgetRequest(request: HTTPRequest): void { + _forgetRequest(request: HTTPRequest, events: boolean): void { const requestId = request._requestId; const interceptionId = request._interceptionId; this._requestIdToRequest.delete(requestId); this._attemptedAuthentications.delete(interceptionId); + + if (events) { + this._requestIdToRequestWillBeSentEvent.delete(requestId); + this._requestIdToRequestPausedEvent.delete(requestId); + } } _onLoadingFinished(event: Protocol.Network.LoadingFinishedEvent): void { @@ -398,7 +432,7 @@ export class NetworkManager extends EventEmitter { // Under certain conditions we never get the Network.responseReceived // event from protocol. @see https://crbug.com/883475 if (request.response()) request.response()._resolveBody(null); - this._forgetRequest(request); + this._forgetRequest(request, true); this.emit(NetworkManagerEmittedEvents.RequestFinished, request); } @@ -410,7 +444,7 @@ export class NetworkManager extends EventEmitter { request._failureText = event.errorText; const response = request.response(); if (response) response._resolveBody(null); - this._forgetRequest(request); + this._forgetRequest(request, true); this.emit(NetworkManagerEmittedEvents.RequestFailed, request); } } From 4ada9b481083f3f43bb720603e3bdda2d8547bf1 Mon Sep 17 00:00:00 2001 From: Robin Richtsfeld Date: Tue, 27 Apr 2021 20:58:07 +0200 Subject: [PATCH 4/5] docs(requestinterception): document the expected order of events for redirects --- src/common/NetworkManager.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/common/NetworkManager.ts b/src/common/NetworkManager.ts index 2ecafb25076e2..b07868e97f294 100644 --- a/src/common/NetworkManager.ts +++ b/src/common/NetworkManager.ts @@ -87,7 +87,18 @@ export class NetworkManager extends EventEmitter { * `_onRequestPaused` (once per `interceptionId`). * * Events are stored to allow for subsequent events to call `_onRequest`. - * Note that redirect requests have the same `requestId` (!). + * + * Note that (chains of) redirect requests have the same `requestId` (!) as + * the original request. We have to anticipate series of events like these: + * A. `_onRequestWillBeSent`, + * `_onRequestWillBeSent`, ... + * B. `_onRequestWillBeSent`, `_onRequestPaused`, + * `_onRequestWillBeSent`, `_onRequestPaused`, ... + * C. `_onRequestWillBeSent`, `_onRequestPaused`, + * `_onRequestPaused`, `_onRequestWillBeSent`, ... + * D. `_onRequestPaused`, `_onRequestWillBeSent`, + * `_onRequestPaused`, `_onRequestWillBeSent`, `_onRequestPaused`, ... + * (see crbug.com/1196004) */ _requestIdToRequestWillBeSentEvent = new Map< string, From 129c3bba403e41171e42060512f00e8d0ec4a5fe Mon Sep 17 00:00:00 2001 From: Jan Scheffler Date: Mon, 3 May 2021 10:17:15 +0200 Subject: [PATCH 5/5] Disable flaky test on firefox (#7182) --- test/click.spec.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/click.spec.ts b/test/click.spec.ts index fa6f0309c1f38..75779bab0e1d1 100644 --- a/test/click.spec.ts +++ b/test/click.spec.ts @@ -239,7 +239,8 @@ describe('Page.click', function () { ) ).toBe('clicked'); }); - it('should double click the button', async () => { + // See https://github.com/puppeteer/puppeteer/issues/7175 + itFailsFirefox('should double click the button', async () => { const { page, server } = getTestState(); await page.goto(server.PREFIX + '/input/button.html');