From c9978d20d5584c9fd2dc902e4b4ac86ed8ea5d6e Mon Sep 17 00:00:00 2001 From: Robin Richtsfeld Date: Thu, 6 May 2021 08:36:34 +0200 Subject: [PATCH] fix(requestinterception): fix font loading issue (#7060) See https://github.com/puppeteer/puppeteer/pull/6996#issuecomment-811546501 and https://github.com/puppeteer/puppeteer/pull/6996#issuecomment-813797393 for context. Issue: #7038 --- src/common/NetworkManager.ts | 115 ++++++++++++++++++++----- test/assets/cached/one-style-font.css | 9 ++ test/assets/cached/one-style-font.html | 2 + test/requestinterception.spec.ts | 9 ++ 4 files changed, 113 insertions(+), 22 deletions(-) create mode 100644 test/assets/cached/one-style-font.css create mode 100644 test/assets/cached/one-style-font.html diff --git a/src/common/NetworkManager.ts b/src/common/NetworkManager.ts index e29a06f94e2a8..b07868e97f294 100644 --- a/src/common/NetworkManager.ts +++ b/src/common/NetworkManager.ts @@ -69,11 +69,47 @@ export class NetworkManager extends EventEmitter { _client: CDPSession; _ignoreHTTPSErrors: boolean; _frameManager: FrameManager; - _requestIdToRequest = new Map(); + + /* + * 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 (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, Protocol.Network.RequestWillBeSentEvent >(); + _requestIdToRequestPausedEvent = new Map< + string, + Protocol.Fetch.RequestPausedEvent + >(); + _requestIdToRequest = new Map(); + _extraHTTPHeaders: Record = {}; _credentials?: Credentials = null; _attemptedAuthentications = new Set(); @@ -81,7 +117,6 @@ export class NetworkManager extends EventEmitter { _userRequestInterceptionCacheSafe = false; _protocolRequestInterceptionEnabled = false; _userCacheDisabled = false; - _requestIdToInterceptionId = new Map(); _emulatedNetworkConditions: InternalNetworkConditions = { offline: false, upload: -1, @@ -222,12 +257,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 +278,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 + ); + + this._requestIdToRequestWillBeSentEvent.set(requestId, event); + + if (requestPausedEvent) { + const interceptionId = requestPausedEvent.requestId; this._onRequest(event, interceptionId); - this._requestIdToInterceptionId.delete(requestId); - } else { - this._requestIdToRequestWillBeSentEvent.set(event.requestId, event); + this._requestIdToRequestPausedEvent.delete(requestId); } + return; } this._onRequest(event, null); @@ -288,14 +333,30 @@ 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; + } + + 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); } else { - this._requestIdToInterceptionId.set(requestId, interceptionId); + this._requestIdToRequestPausedEvent.set(requestId, event); } } @@ -346,8 +407,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, false); this.emit(NetworkManagerEmittedEvents.Response, response); this.emit(NetworkManagerEmittedEvents.RequestFinished, request); } @@ -361,6 +421,19 @@ export class NetworkManager extends EventEmitter { this.emit(NetworkManagerEmittedEvents.Response, response); } + _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 { const request = this._requestIdToRequest.get(event.requestId); // For certain requestIds we never receive requestWillBeSent event. @@ -370,8 +443,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, true); this.emit(NetworkManagerEmittedEvents.RequestFinished, request); } @@ -383,8 +455,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, true); this.emit(NetworkManagerEmittedEvents.RequestFailed, request); } } 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 () {