From aabdac83804bfb28fba0a93a5d4f61ce17920f41 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Thu, 5 Mar 2020 10:09:04 -0800 Subject: [PATCH] api: remove Page.setCacheEnabled (#1231) --- docs/api.md | 11 ++--------- src/chromium/crNetworkManager.ts | 16 ++-------------- src/chromium/crPage.ts | 4 ---- src/firefox/ffPage.ts | 4 ---- src/page.ts | 10 ---------- src/webkit/wkPage.ts | 7 ------- test/navigation.spec.js | 11 +++++------ test/page.spec.js | 23 ----------------------- 8 files changed, 9 insertions(+), 77 deletions(-) diff --git a/docs/api.md b/docs/api.md index f1e832b494..e307a44298 100644 --- a/docs/api.md +++ b/docs/api.md @@ -209,7 +209,7 @@ Indicates that the browser is connected. - `locale` Specify user locale, for example `en-GB`, `de-DE`, etc. Locale will affect `navigator.language` value, `Accept-Language` request header value as well as number and date formatting rules. - `permissions` <[Object]> A map from origin keys to permissions values. See [browserContext.setPermissions](#browsercontextsetpermissionsorigin-permissions) for more details. - `extraHTTPHeaders` <[Object]> An object containing additional HTTP headers to be sent with every request. All header values must be strings. - - `offline` <[boolean]> Whether to emulate network being offline for the browser context. + - `offline` <[boolean]> Whether to emulate network being offline. Defaults to `false`. - returns: <[Promise]<[BrowserContext]>> Creates a new browser context. It won't share cookies/cache with other browser contexts. @@ -244,7 +244,7 @@ Creates a new browser context. It won't share cookies/cache with other browser c - `locale` Specify user locale, for example `en-GB`, `de-DE`, etc. Locale will affect `navigator.language` value, `Accept-Language` request header value as well as number and date formatting rules. - `permissions` <[Object]> A map from origin keys to permissions values. See [browserContext.setPermissions](#browsercontextsetpermissionsorigin-permissions) for more details. - `extraHTTPHeaders` <[Object]> An object containing additional HTTP headers to be sent with every request. All header values must be strings. - - `offline` <[boolean]> Whether to emulate network being offline for the browser context. + - `offline` <[boolean]> Whether to emulate network being offline. Defaults to `false`. - returns: <[Promise]<[Page]>> Creates a new page in a new browser context. Closing this page will close the context as well. @@ -637,7 +637,6 @@ page.removeListener('request', logRequest); - [page.route(url, handler)](#pagerouteurl-handler) - [page.screenshot([options])](#pagescreenshotoptions) - [page.select(selector, value[, options])](#pageselectselector-value-options) -- [page.setCacheEnabled([enabled])](#pagesetcacheenabledenabled) - [page.setContent(html[, options])](#pagesetcontenthtml-options) - [page.setDefaultNavigationTimeout(timeout)](#pagesetdefaultnavigationtimeouttimeout) - [page.setDefaultTimeout(timeout)](#pagesetdefaulttimeouttimeout) @@ -1434,12 +1433,6 @@ page.select('select#colors', { value: 'blue' }, { index: 2 }, 'red'); Shortcut for [page.mainFrame().select()](#frameselectselector-values) -#### page.setCacheEnabled([enabled]) -- `enabled` <[boolean]> sets the `enabled` state of the cache. Defaults to `true`. -- returns: <[Promise]> - -Toggles ignoring cache for each request based on the enabled state. By default, caching is enabled. - #### page.setContent(html[, options]) - `html` <[string]> HTML markup to assign to the page. - `options` <[Object]> Parameters which might have the following properties: diff --git a/src/chromium/crNetworkManager.ts b/src/chromium/crNetworkManager.ts index 4ce018e2c9..dbb56bf372 100644 --- a/src/chromium/crNetworkManager.ts +++ b/src/chromium/crNetworkManager.ts @@ -33,7 +33,6 @@ export class CRNetworkManager { private _attemptedAuthentications = new Set(); private _userRequestInterceptionEnabled = false; private _protocolRequestInterceptionEnabled = false; - private _userCacheDisabled = false; private _requestIdToInterceptionId = new Map(); private _eventListeners: RegisteredListener[]; @@ -77,11 +76,6 @@ export class CRNetworkManager { }); } - async setCacheEnabled(enabled: boolean) { - this._userCacheDisabled = !enabled; - await this._updateProtocolCacheDisabled(); - } - async setRequestInterception(value: boolean) { this._userRequestInterceptionEnabled = value; await this._updateProtocolRequestInterception(); @@ -94,7 +88,7 @@ export class CRNetworkManager { this._protocolRequestInterceptionEnabled = enabled; if (enabled) { await Promise.all([ - this._updateProtocolCacheDisabled(), + this._client.send('Network.setCacheDisabled', { cacheDisabled: true }), this._client.send('Fetch.enable', { handleAuthRequests: true, patterns: [{urlPattern: '*'}], @@ -102,18 +96,12 @@ export class CRNetworkManager { ]); } else { await Promise.all([ - this._updateProtocolCacheDisabled(), + this._client.send('Network.setCacheDisabled', { cacheDisabled: false }), this._client.send('Fetch.disable') ]); } } - async _updateProtocolCacheDisabled() { - await this._client.send('Network.setCacheDisabled', { - cacheDisabled: this._userCacheDisabled || this._protocolRequestInterceptionEnabled - }); - } - _onRequestWillBeSent(event: Protocol.Network.requestWillBeSentPayload) { // Request interception doesn't happen for data URLs with Network Service. if (this._protocolRequestInterceptionEnabled && !event.request.url.startsWith('data:')) { diff --git a/src/chromium/crPage.ts b/src/chromium/crPage.ts index 32fd594cdf..57d9f64825 100644 --- a/src/chromium/crPage.ts +++ b/src/chromium/crPage.ts @@ -372,10 +372,6 @@ export class CRPage implements PageDelegate { await this._client.send('Emulation.setEmulatedMedia', { media: mediaType || '', features }); } - setCacheEnabled(enabled: boolean): Promise { - return this._networkManager.setCacheEnabled(enabled); - } - async setRequestInterception(enabled: boolean): Promise { await this._networkManager.setRequestInterception(enabled); } diff --git a/src/firefox/ffPage.ts b/src/firefox/ffPage.ts index 0965160346..f76686d68e 100644 --- a/src/firefox/ffPage.ts +++ b/src/firefox/ffPage.ts @@ -271,10 +271,6 @@ export class FFPage implements PageDelegate { }); } - async setCacheEnabled(enabled: boolean): Promise { - await this._session.send('Page.setCacheDisabled', {cacheDisabled: !enabled}); - } - async setRequestInterception(enabled: boolean): Promise { await this._networkManager.setRequestInterception(enabled); } diff --git a/src/page.ts b/src/page.ts index 3c8425e927..856f0dffa2 100644 --- a/src/page.ts +++ b/src/page.ts @@ -48,7 +48,6 @@ export interface PageDelegate { updateExtraHTTPHeaders(): Promise; setViewportSize(viewportSize: types.Size): Promise; setEmulateMedia(mediaType: types.MediaType | null, colorScheme: types.ColorScheme | null): Promise; - setCacheEnabled(enabled: boolean): Promise; setRequestInterception(enabled: boolean): Promise; authenticate(credentials: types.Credentials | null): Promise; setFileChooserIntercepted(enabled: boolean): Promise; @@ -79,7 +78,6 @@ type PageState = { mediaType: types.MediaType | null; colorScheme: types.ColorScheme | null; extraHTTPHeaders: network.Headers | null; - cacheEnabled: boolean | null; interceptNetwork: boolean | null; credentials: types.Credentials | null; hasTouch: boolean | null; @@ -145,7 +143,6 @@ export class Page extends platform.EventEmitter { mediaType: null, colorScheme: null, extraHTTPHeaders: null, - cacheEnabled: null, interceptNetwork: null, credentials: null, hasTouch: null, @@ -390,13 +387,6 @@ export class Page extends platform.EventEmitter { await this._delegate.evaluateOnNewDocument(await helper.evaluationScript(script, args)); } - async setCacheEnabled(enabled: boolean = true) { - if (this._state.cacheEnabled === enabled) - return; - this._state.cacheEnabled = enabled; - await this._delegate.setCacheEnabled(enabled); - } - async route(url: types.URLMatch, handler: (request: network.Request) => void) { if (!this._state.interceptNetwork) { this._state.interceptNetwork = true; diff --git a/src/webkit/wkPage.ts b/src/webkit/wkPage.ts index 031525ca39..37c01354f0 100644 --- a/src/webkit/wkPage.ts +++ b/src/webkit/wkPage.ts @@ -129,8 +129,6 @@ export class WKPage implements PageDelegate { if (this._page._state.interceptNetwork) promises.push(session.send('Network.setInterceptionEnabled', { enabled: true, interceptRequests: true })); - if (this._page._state.cacheEnabled === false) - promises.push(session.send('Network.setResourceCachingDisabled', { disabled: true })); const contextOptions = this._page.context()._options; if (contextOptions.userAgent) @@ -422,11 +420,6 @@ export class WKPage implements PageDelegate { await Promise.all(promises); } - async setCacheEnabled(enabled: boolean): Promise { - const disabled = !enabled; - await this._updateState('Network.setResourceCachingDisabled', { disabled }); - } - async setRequestInterception(enabled: boolean): Promise { await this._updateState('Network.setInterceptionEnabled', { enabled, interceptRequests: enabled }); } diff --git a/test/navigation.spec.js b/test/navigation.spec.js index cd99d8ff5a..e0317d52d7 100644 --- a/test/navigation.spec.js +++ b/test/navigation.spec.js @@ -1027,8 +1027,6 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF expect(error.stack).toContain('Frame.goto') }); it('should return matching responses', async({page, server}) => { - // Disable cache: otherwise, chromium will cache similar requests. - await page.setCacheEnabled(false); await page.goto(server.EMPTY_PAGE); // Attach three frames. const frames = [ @@ -1036,13 +1034,14 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF await utils.attachFrame(page, 'frame2', server.EMPTY_PAGE), await utils.attachFrame(page, 'frame3', server.EMPTY_PAGE), ]; - // Navigate all frames to the same URL. const serverResponses = []; - server.setRoute('/one-style.html', (req, res) => serverResponses.push(res)); + server.setRoute('/0.html', (req, res) => serverResponses.push(res)); + server.setRoute('/1.html', (req, res) => serverResponses.push(res)); + server.setRoute('/2.html', (req, res) => serverResponses.push(res)); const navigations = []; for (let i = 0; i < 3; ++i) { - navigations.push(frames[i].goto(server.PREFIX + '/one-style.html')); - await server.waitForRequest('/one-style.html'); + navigations.push(frames[i].goto(server.PREFIX + '/' + i + '.html')); + await server.waitForRequest('/' + i + '.html'); } // Respond from server out-of-order. const serverResponseTexts = ['AAA', 'BBB', 'CCC']; diff --git a/test/page.spec.js b/test/page.spec.js index 53b13b746d..3b2db2e4b1 100644 --- a/test/page.spec.js +++ b/test/page.spec.js @@ -756,29 +756,6 @@ module.exports.describe = function({testRunner, expect, headless, playwright, FF }); }); - describe('Page.setCacheEnabled', function() { - it('should enable or disable the cache based on the state passed', async({page, server}) => { - await page.goto(server.PREFIX + '/cached/one-style.html'); - // WebKit does r.setCachePolicy(ResourceRequestCachePolicy::ReloadIgnoringCacheData); - // when navigating to the same url, load empty.html to avoid that. - await page.goto(server.EMPTY_PAGE); - const [cachedRequest] = await Promise.all([ - server.waitForRequest('/cached/one-style.html'), - page.goto(server.PREFIX + '/cached/one-style.html'), - ]); - // Rely on "if-modified-since" caching in our test server. - expect(cachedRequest.headers['if-modified-since']).not.toBe(undefined); - - await page.setCacheEnabled(false); - await page.goto(server.EMPTY_PAGE); - const [nonCachedRequest] = await Promise.all([ - server.waitForRequest('/cached/one-style.html'), - page.goto(server.PREFIX + '/cached/one-style.html'), - ]); - expect(nonCachedRequest.headers['if-modified-since']).toBe(undefined); - }); - }); - describe('Page.title', function() { it('should return the page title', async({page, server}) => { await page.goto(server.PREFIX + '/title.html');