From e8d6bf03a28913c56898432ec59d48e39137168a Mon Sep 17 00:00:00 2001 From: Robin Richtsfeld Date: Tue, 16 Mar 2021 03:52:13 +0100 Subject: [PATCH 1/7] Request interception and caching compatibility --- src/common/NetworkManager.ts | 6 ++++-- src/common/Page.ts | 10 +++++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/common/NetworkManager.ts b/src/common/NetworkManager.ts index 6c82d60fa491b..c5805fa4404a9 100644 --- a/src/common/NetworkManager.ts +++ b/src/common/NetworkManager.ts @@ -75,6 +75,7 @@ export class NetworkManager extends EventEmitter { _credentials?: Credentials = null; _attemptedAuthentications = new Set(); _userRequestInterceptionEnabled = false; + _userRequestInterceptionCacheSafe = false; _protocolRequestInterceptionEnabled = false; _userCacheDisabled = false; _requestIdToInterceptionId = new Map(); @@ -189,8 +190,9 @@ export class NetworkManager extends EventEmitter { await this._updateProtocolCacheDisabled(); } - async setRequestInterception(value: boolean): Promise { + async setRequestInterception(value: boolean, cacheSafe: boolean = false): Promise { this._userRequestInterceptionEnabled = value; + this._userRequestInterceptionCacheSafe = cacheSafe; await this._updateProtocolRequestInterception(); } @@ -217,7 +219,7 @@ export class NetworkManager extends EventEmitter { async _updateProtocolCacheDisabled(): Promise { await this._client.send('Network.setCacheDisabled', { cacheDisabled: - this._userCacheDisabled || this._userRequestInterceptionEnabled, + this._userCacheDisabled || (this._userRequestInterceptionEnabled && !this._userRequestInterceptionCacheSafe), }); } diff --git a/src/common/Page.ts b/src/common/Page.ts index 92c39fa660bea..6f09b4e493091 100644 --- a/src/common/Page.ts +++ b/src/common/Page.ts @@ -688,6 +688,8 @@ export class Page extends EventEmitter { /** * @param value - Whether to enable request interception. + * @param cacheSafe - Whether to trust browser caching. If set to false, + * enabling request interception disables page caching. Defaults to false. * * @remarks * Activating request interception enables {@link HTTPRequest.abort}, @@ -695,9 +697,7 @@ export class Page extends EventEmitter { * provides the capability to modify network requests that are made by a page. * * Once request interception is enabled, every request will stall unless it's - * continued, responded or aborted. - * - * **NOTE** Enabling request interception disables page caching. + * continued, responded or aborted; or completed using the browser cache. * * @example * An example of a naïve request interceptor that aborts all image requests: @@ -719,8 +719,8 @@ export class Page extends EventEmitter { * })(); * ``` */ - async setRequestInterception(value: boolean): Promise { - return this._frameManager.networkManager().setRequestInterception(value); + async setRequestInterception(value: boolean, cacheSafe: boolean = false): Promise { + return this._frameManager.networkManager().setRequestInterception(value, cacheSafe); } /** From fc5f3759bbb15ffc51bcae8c17adf81f661a2e18 Mon Sep 17 00:00:00 2001 From: Robin Richtsfeld Date: Tue, 16 Mar 2021 18:26:46 +0100 Subject: [PATCH 2/7] style: satisfy linter --- src/common/NetworkManager.ts | 9 +++++++-- src/common/Page.ts | 9 +++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/common/NetworkManager.ts b/src/common/NetworkManager.ts index c5805fa4404a9..4b0ffc0dfa751 100644 --- a/src/common/NetworkManager.ts +++ b/src/common/NetworkManager.ts @@ -190,7 +190,10 @@ export class NetworkManager extends EventEmitter { await this._updateProtocolCacheDisabled(); } - async setRequestInterception(value: boolean, cacheSafe: boolean = false): Promise { + async setRequestInterception( + value: boolean, + cacheSafe = false + ): Promise { this._userRequestInterceptionEnabled = value; this._userRequestInterceptionCacheSafe = cacheSafe; await this._updateProtocolRequestInterception(); @@ -219,7 +222,9 @@ export class NetworkManager extends EventEmitter { async _updateProtocolCacheDisabled(): Promise { await this._client.send('Network.setCacheDisabled', { cacheDisabled: - this._userCacheDisabled || (this._userRequestInterceptionEnabled && !this._userRequestInterceptionCacheSafe), + this._userCacheDisabled || + (this._userRequestInterceptionEnabled && + !this._userRequestInterceptionCacheSafe), }); } diff --git a/src/common/Page.ts b/src/common/Page.ts index 6f09b4e493091..9a96c805c114d 100644 --- a/src/common/Page.ts +++ b/src/common/Page.ts @@ -719,8 +719,13 @@ export class Page extends EventEmitter { * })(); * ``` */ - async setRequestInterception(value: boolean, cacheSafe: boolean = false): Promise { - return this._frameManager.networkManager().setRequestInterception(value, cacheSafe); + async setRequestInterception( + value: boolean, + cacheSafe = false + ): Promise { + return this._frameManager + .networkManager() + .setRequestInterception(value, cacheSafe); } /** From 142c96c736a998b9682d76fe0e3b7d6345f4ed9b Mon Sep 17 00:00:00 2001 From: Robin Richtsfeld Date: Wed, 17 Mar 2021 01:15:38 +0100 Subject: [PATCH 3/7] docs: update api.md --- docs/api.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/api.md b/docs/api.md index 6c30b9c2961c7..892ea053e42ea 100644 --- a/docs/api.md +++ b/docs/api.md @@ -166,7 +166,7 @@ * [page.setGeolocation(options)](#pagesetgeolocationoptions) * [page.setJavaScriptEnabled(enabled)](#pagesetjavascriptenabledenabled) * [page.setOfflineMode(enabled)](#pagesetofflinemodeenabled) - * [page.setRequestInterception(value)](#pagesetrequestinterceptionvalue) + * [page.setRequestInterception(value[, cacheSafe])](#pagesetrequestinterceptionvalue-cachesafe) * [page.setUserAgent(userAgent)](#pagesetuseragentuseragent) * [page.setViewport(viewport)](#pagesetviewportviewport) * [page.tap(selector)](#pagetapselector) @@ -2039,8 +2039,9 @@ await page.setGeolocation({latitude: 59.95, longitude: 30.31667}); - `enabled` <[boolean]> When `true`, enables offline mode for the page. - returns: <[Promise]> -#### page.setRequestInterception(value) +#### page.setRequestInterception(value[, cacheSafe]) - `value` <[boolean]> Whether to enable request interception. +- `cacheSafe` <[boolean]> Whether to trust browser caching. If set to false, enabling request interception disables page caching. Defaults to false. - returns: <[Promise]> Activating request interception enables `request.abort`, `request.continue` and From cc94ecdadde349a6403580e5345b585e0442b879 Mon Sep 17 00:00:00 2001 From: Robin Richtsfeld Date: Wed, 17 Mar 2021 14:43:17 +0100 Subject: [PATCH 4/7] feat(network): add Page.Events.RequestServedFromCache --- src/common/NetworkManager.ts | 2 ++ src/common/Page.ts | 11 +++++++++++ 2 files changed, 13 insertions(+) diff --git a/src/common/NetworkManager.ts b/src/common/NetworkManager.ts index 4b0ffc0dfa751..f0eb65f0ddc20 100644 --- a/src/common/NetworkManager.ts +++ b/src/common/NetworkManager.ts @@ -54,6 +54,7 @@ export interface InternalNetworkConditions extends NetworkConditions { */ export const NetworkManagerEmittedEvents = { Request: Symbol('NetworkManager.Request'), + RequestServedFromCache: Symbol('NetworkManager.RequestServedFromCache'), Response: Symbol('NetworkManager.Response'), RequestFailed: Symbol('NetworkManager.RequestFailed'), RequestFinished: Symbol('NetworkManager.RequestFinished'), @@ -330,6 +331,7 @@ export class NetworkManager extends EventEmitter { ): void { const request = this._requestIdToRequest.get(event.requestId); if (request) request._fromMemoryCache = true; + this.emit(NetworkManagerEmittedEvents.RequestServedFromCache, request); } _handleRequestRedirect( diff --git a/src/common/Page.ts b/src/common/Page.ts index 9a96c805c114d..6eb5440d05faf 100644 --- a/src/common/Page.ts +++ b/src/common/Page.ts @@ -287,6 +287,14 @@ export const enum PageEmittedEvents { * and mutating requests. */ Request = 'request', + /** + * Emitted when a request ended up loading from cache. Contains a {@link HTTPRequest}. + * + * @remarks + * For certain requests, might contain undefined. + * @see https://crbug.com/750469 + */ + RequestServedFromCache = 'requestservedfromcache', /** * Emitted when a request fails, for example by timing out. * @@ -483,6 +491,9 @@ export class Page extends EventEmitter { networkManager.on(NetworkManagerEmittedEvents.Request, (event) => this.emit(PageEmittedEvents.Request, event) ); + networkManager.on(NetworkManagerEmittedEvents.RequestServedFromCache, (event) => + this.emit(PageEmittedEvents.RequestServedFromCache, event) + ); networkManager.on(NetworkManagerEmittedEvents.Response, (event) => this.emit(PageEmittedEvents.Response, event) ); From 0ca7e9ed1ff9dd5cf4e34634dd4c280b11347041 Mon Sep 17 00:00:00 2001 From: Robin Richtsfeld Date: Wed, 17 Mar 2021 14:44:42 +0100 Subject: [PATCH 5/7] test(network): add test for Page.Events.RequestServedFromCache --- test/network.spec.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/network.spec.ts b/test/network.spec.ts index 630771d1d90aa..52bc831c1e817 100644 --- a/test/network.spec.ts +++ b/test/network.spec.ts @@ -355,6 +355,18 @@ describe('network', function () { expect(requests[0].frame() === page.mainFrame()).toBe(true); expect(requests[0].frame().url()).toBe(server.EMPTY_PAGE); }); + it('Page.Events.RequestServedFromCache', async () => { + const { page, server } = getTestState(); + + let cached = []; + page.on('requestservedfromcache', (r) => cached.push(r.url().split('/').pop())); + + await page.goto(server.PREFIX + '/cached/one-style.html'); + expect(cached).toEqual([]); + + await page.reload(); + expect(cached).toEqual(['one-style.css']); + }); it('Page.Events.Response', async () => { const { page, server } = getTestState(); From fc9d24ace798cb2326f39c6fc94268c9194150c4 Mon Sep 17 00:00:00 2001 From: Robin Richtsfeld Date: Wed, 17 Mar 2021 14:45:26 +0100 Subject: [PATCH 6/7] test(request-interception): add tests for caching behavior --- test/requestinterception.spec.ts | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/test/requestinterception.spec.ts b/test/requestinterception.spec.ts index de3c9b24ab870..5c5ac120f168a 100644 --- a/test/requestinterception.spec.ts +++ b/test/requestinterception.spec.ts @@ -495,6 +495,36 @@ describe('request interception', function () { expect(urls.has('one-style.html')).toBe(true); expect(urls.has('one-style.css')).toBe(true); }); + it('should not cache if not cache-safe', async () => { + const { page, server } = getTestState(); + + // Load and re-load to make sure it's cached. + await page.goto(server.PREFIX + '/cached/one-style.html'); + + await page.setRequestInterception(true, false); + page.on('request', (request) => request.continue()); + + let cached = []; + page.on('requestservedfromcache', (r) => cached.push(r)); + + await page.reload(); + expect(cached.length).toBe(0); + }); + it('should cache if cache-safe', async () => { + const { page, server } = getTestState(); + + // Load and re-load to make sure it's cached. + await page.goto(server.PREFIX + '/cached/one-style.html'); + + await page.setRequestInterception(true, true); + page.on('request', (request) => request.continue()); + + let cached = []; + page.on('requestservedfromcache', (r) => cached.push(r)); + + await page.reload(); + expect(cached.length).toBe(1); + }); }); describeFailsFirefox('Request.continue', function () { From c64dc2088f4c39c19b1f14c541be1c884b9cb012 Mon Sep 17 00:00:00 2001 From: Robin Richtsfeld Date: Wed, 17 Mar 2021 14:51:02 +0100 Subject: [PATCH 7/7] style: satisfy linter --- src/common/Page.ts | 5 +++-- test/network.spec.ts | 6 ++++-- test/requestinterception.spec.ts | 4 ++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/common/Page.ts b/src/common/Page.ts index 6eb5440d05faf..b708c23ba9b7c 100644 --- a/src/common/Page.ts +++ b/src/common/Page.ts @@ -491,8 +491,9 @@ export class Page extends EventEmitter { networkManager.on(NetworkManagerEmittedEvents.Request, (event) => this.emit(PageEmittedEvents.Request, event) ); - networkManager.on(NetworkManagerEmittedEvents.RequestServedFromCache, (event) => - this.emit(PageEmittedEvents.RequestServedFromCache, event) + networkManager.on( + NetworkManagerEmittedEvents.RequestServedFromCache, + (event) => this.emit(PageEmittedEvents.RequestServedFromCache, event) ); networkManager.on(NetworkManagerEmittedEvents.Response, (event) => this.emit(PageEmittedEvents.Response, event) diff --git a/test/network.spec.ts b/test/network.spec.ts index 52bc831c1e817..787fee18f43ef 100644 --- a/test/network.spec.ts +++ b/test/network.spec.ts @@ -358,8 +358,10 @@ describe('network', function () { it('Page.Events.RequestServedFromCache', async () => { const { page, server } = getTestState(); - let cached = []; - page.on('requestservedfromcache', (r) => cached.push(r.url().split('/').pop())); + const cached = []; + page.on('requestservedfromcache', (r) => + cached.push(r.url().split('/').pop()) + ); await page.goto(server.PREFIX + '/cached/one-style.html'); expect(cached).toEqual([]); diff --git a/test/requestinterception.spec.ts b/test/requestinterception.spec.ts index 5c5ac120f168a..35e225bb41d8f 100644 --- a/test/requestinterception.spec.ts +++ b/test/requestinterception.spec.ts @@ -504,7 +504,7 @@ describe('request interception', function () { await page.setRequestInterception(true, false); page.on('request', (request) => request.continue()); - let cached = []; + const cached = []; page.on('requestservedfromcache', (r) => cached.push(r)); await page.reload(); @@ -519,7 +519,7 @@ describe('request interception', function () { await page.setRequestInterception(true, true); page.on('request', (request) => request.continue()); - let cached = []; + const cached = []; page.on('requestservedfromcache', (r) => cached.push(r)); await page.reload();