From 20c6b851782943dc7442cbf3c653983cebf2509c Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Tue, 18 Aug 2020 17:34:04 -0700 Subject: [PATCH] chore: remove route/unroute from the server side (#3518) We only use a global "intercept all requests" handler on page and browser context, instead of granular ones. --- src/browserContext.ts | 13 ++++--- src/chromium/crBrowser.ts | 9 +---- src/chromium/crNetworkManager.ts | 2 +- src/firefox/ffBrowser.ts | 12 ++----- src/page.ts | 41 ++++++---------------- src/rpc/server/browserContextDispatcher.ts | 4 +-- src/rpc/server/pageDispatcher.ts | 4 +-- src/webkit/wkBrowser.ts | 9 +---- 8 files changed, 27 insertions(+), 67 deletions(-) diff --git a/src/browserContext.ts b/src/browserContext.ts index ec04265e3f..57f9b9377f 100644 --- a/src/browserContext.ts +++ b/src/browserContext.ts @@ -44,8 +44,7 @@ export interface BrowserContext extends EventEmitter { setHTTPCredentials(httpCredentials?: types.Credentials): Promise; addInitScript(script: Function | string | { path?: string, content?: string }, arg?: any): Promise; exposeBinding(name: string, playwrightBinding: frames.FunctionWithSource): Promise; - route(url: types.URLMatch, handler: network.RouteHandler): Promise; - unroute(url: types.URLMatch, handler?: network.RouteHandler): Promise; + _setRequestInterceptor(handler: network.RouteHandler | undefined): Promise; close(): Promise; } @@ -53,7 +52,7 @@ export abstract class BrowserContextBase extends EventEmitter implements Browser readonly _timeoutSettings = new TimeoutSettings(); readonly _pageBindings = new Map(); readonly _options: types.BrowserContextOptions; - _routes: { url: types.URLMatch, handler: network.RouteHandler }[] = []; + _requestInterceptor?: network.RouteHandler; private _isPersistentContext: boolean; private _closedStatus: 'open' | 'closing' | 'closed' = 'open'; readonly _closePromise: Promise; @@ -107,8 +106,7 @@ export abstract class BrowserContextBase extends EventEmitter implements Browser abstract setOffline(offline: boolean): Promise; abstract _doAddInitScript(expression: string): Promise; abstract _doExposeBinding(binding: PageBinding): Promise; - abstract route(url: types.URLMatch, handler: network.RouteHandler): Promise; - abstract unroute(url: types.URLMatch, handler?: network.RouteHandler): Promise; + abstract _doUpdateRequestInterception(): Promise; abstract _doClose(): Promise; async cookies(urls: string | string[] | undefined = []): Promise { @@ -206,6 +204,11 @@ export abstract class BrowserContextBase extends EventEmitter implements Browser this._options.httpCredentials = { username, password }; } + async _setRequestInterceptor(handler: network.RouteHandler | undefined): Promise { + this._requestInterceptor = handler; + await this._doUpdateRequestInterception(); + } + async close() { if (this._isPersistentContext) { // Default context is only created in 'persistent' mode and closing it should close diff --git a/src/chromium/crBrowser.ts b/src/chromium/crBrowser.ts index f59934674d..e9845dc392 100644 --- a/src/chromium/crBrowser.ts +++ b/src/chromium/crBrowser.ts @@ -416,14 +416,7 @@ export class CRBrowserContext extends BrowserContextBase { await (page._delegate as CRPage).exposeBinding(binding); } - async route(url: types.URLMatch, handler: network.RouteHandler): Promise { - this._routes.push({ url, handler }); - for (const page of this.pages()) - await (page._delegate as CRPage).updateRequestInterception(); - } - - async unroute(url: types.URLMatch, handler?: network.RouteHandler): Promise { - this._routes = this._routes.filter(route => route.url !== url || (handler && route.handler !== handler)); + async _doUpdateRequestInterception(): Promise { for (const page of this.pages()) await (page._delegate as CRPage).updateRequestInterception(); } diff --git a/src/chromium/crNetworkManager.ts b/src/chromium/crNetworkManager.ts index eb91c67da9..6bed18c836 100644 --- a/src/chromium/crNetworkManager.ts +++ b/src/chromium/crNetworkManager.ts @@ -191,7 +191,7 @@ export class CRNetworkManager { if (requestPausedEvent) { // CORS options request is generated by the network stack, it is not associated with the frame id. // If URL matches interception pattern, accept it, assuming that this was intended when setting route. - if (requestPausedEvent.request.method === 'OPTIONS' && this._page._isRouted(requestPausedEvent.request.url)) { + if (requestPausedEvent.request.method === 'OPTIONS' && this._page._needsRequestInterception()) { const requestHeaders = requestPausedEvent.request.headers; const responseHeaders: Protocol.Fetch.HeaderEntry[] = [ { name: 'Access-Control-Allow-Origin', value: requestHeaders['Origin'] || '*' }, diff --git a/src/firefox/ffBrowser.ts b/src/firefox/ffBrowser.ts index 465e579544..20998a9a2f 100644 --- a/src/firefox/ffBrowser.ts +++ b/src/firefox/ffBrowser.ts @@ -319,16 +319,8 @@ export class FFBrowserContext extends BrowserContextBase { await this._browser._connection.send('Browser.addBinding', { browserContextId: this._browserContextId || undefined, name: binding.name, script: binding.source }); } - async route(url: types.URLMatch, handler: network.RouteHandler): Promise { - this._routes.push({ url, handler }); - if (this._routes.length === 1) - await this._browser._connection.send('Browser.setRequestInterception', { browserContextId: this._browserContextId || undefined, enabled: true }); - } - - async unroute(url: types.URLMatch, handler?: network.RouteHandler): Promise { - this._routes = this._routes.filter(route => route.url !== url || (handler && route.handler !== handler)); - if (this._routes.length === 0) - await this._browser._connection.send('Browser.setRequestInterception', { browserContextId: this._browserContextId || undefined, enabled: false }); + async _doUpdateRequestInterception(): Promise { + await this._browser._connection.send('Browser.setRequestInterception', { browserContextId: this._browserContextId || undefined, enabled: !!this._requestInterceptor }); } async _doClose() { diff --git a/src/page.ts b/src/page.ts index 77119b43c5..a54986b829 100644 --- a/src/page.ts +++ b/src/page.ts @@ -113,7 +113,7 @@ export class Page extends EventEmitter { private _workers = new Map(); readonly pdf: ((options?: types.PDFOptions) => Promise) | undefined; readonly coverage: any; - _routes: { url: types.URLMatch, handler: network.RouteHandler }[] = []; + private _requestInterceptor?: network.RouteHandler; _ownedContext: BrowserContext | undefined; constructor(delegate: PageDelegate, browserContext: BrowserContextBase) { @@ -291,16 +291,11 @@ export class Page extends EventEmitter { } _needsRequestInterception(): boolean { - return this._routes.length > 0 || this._browserContext._routes.length > 0; + return !!this._requestInterceptor || !!this._browserContext._requestInterceptor; } - async route(url: types.URLMatch, handler: network.RouteHandler): Promise { - this._routes.push({ url, handler }); - await this._delegate.updateRequestInterception(); - } - - async unroute(url: types.URLMatch, handler?: network.RouteHandler): Promise { - this._routes = this._routes.filter(route => route.url !== url || (handler && route.handler !== handler)); + async _setRequestInterceptor(handler: network.RouteHandler | undefined): Promise { + this._requestInterceptor = handler; await this._delegate.updateRequestInterception(); } @@ -309,33 +304,17 @@ export class Page extends EventEmitter { const route = request._route(); if (!route) return; - for (const { url, handler } of this._routes) { - if (helper.urlMatches(request.url(), url)) { - handler(route, request); - return; - } + if (this._requestInterceptor) { + this._requestInterceptor(route, request); + return; } - for (const { url, handler } of this._browserContext._routes) { - if (helper.urlMatches(request.url(), url)) { - handler(route, request); - return; - } + if (this._browserContext._requestInterceptor) { + this._browserContext._requestInterceptor(route, request); + return; } route.continue(); } - _isRouted(requestURL: string): boolean { - for (const { url } of this._routes) { - if (helper.urlMatches(requestURL, url)) - return true; - } - for (const { url } of this._browserContext._routes) { - if (helper.urlMatches(requestURL, url)) - return true; - } - return false; - } - async screenshot(options: types.ScreenshotOptions = {}): Promise { return this._runAbortableTask( progress => this._screenshotter.screenshotPage(progress, options), diff --git a/src/rpc/server/browserContextDispatcher.ts b/src/rpc/server/browserContextDispatcher.ts index 5110712943..64845cac5b 100644 --- a/src/rpc/server/browserContextDispatcher.ts +++ b/src/rpc/server/browserContextDispatcher.ts @@ -112,10 +112,10 @@ export class BrowserContextDispatcher extends Dispatcher { if (!params.enabled) { - await this._context.unroute('**/*'); + await this._context._setRequestInterceptor(undefined); return; } - this._context.route('**/*', (route, request) => { + this._context._setRequestInterceptor((route, request) => { this._dispatchEvent('route', { route: new RouteDispatcher(this._scope, route), request: RequestDispatcher.from(this._scope, request) }); }); } diff --git a/src/rpc/server/pageDispatcher.ts b/src/rpc/server/pageDispatcher.ts index 56af531903..cf0cc28d32 100644 --- a/src/rpc/server/pageDispatcher.ts +++ b/src/rpc/server/pageDispatcher.ts @@ -123,10 +123,10 @@ export class PageDispatcher extends Dispatcher implements async setNetworkInterceptionEnabled(params: { enabled: boolean }): Promise { if (!params.enabled) { - await this._page.unroute('**/*'); + await this._page._setRequestInterceptor(undefined); return; } - this._page.route('**/*', (route, request) => { + this._page._setRequestInterceptor((route, request) => { this._dispatchEvent('route', { route: new RouteDispatcher(this._scope, route), request: RequestDispatcher.from(this._scope, request) }); }); } diff --git a/src/webkit/wkBrowser.ts b/src/webkit/wkBrowser.ts index 46d1140285..ede3ffc647 100644 --- a/src/webkit/wkBrowser.ts +++ b/src/webkit/wkBrowser.ts @@ -322,14 +322,7 @@ export class WKBrowserContext extends BrowserContextBase { await (page._delegate as WKPage).exposeBinding(binding); } - async route(url: types.URLMatch, handler: network.RouteHandler): Promise { - this._routes.push({ url, handler }); - for (const page of this.pages()) - await (page._delegate as WKPage).updateRequestInterception(); - } - - async unroute(url: types.URLMatch, handler?: network.RouteHandler): Promise { - this._routes = this._routes.filter(route => route.url !== url || (handler && route.handler !== handler)); + async _doUpdateRequestInterception(): Promise { for (const page of this.pages()) await (page._delegate as WKPage).updateRequestInterception(); }