From ae05808f4b5eca5c35e15f6f51856c163d95838f Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Sat, 18 Feb 2023 11:41:24 -0800 Subject: [PATCH] chore: align js routing logic with other langs (#21010) Mostly reverts #20353. This makes porting easier. --- .../src/client/browserContext.ts | 32 ++++++-- .../playwright-core/src/client/harRouter.ts | 24 +++++- .../playwright-core/src/client/network.ts | 77 ++++--------------- packages/playwright-core/src/client/page.ts | 36 ++++++--- 4 files changed, 88 insertions(+), 81 deletions(-) diff --git a/packages/playwright-core/src/client/browserContext.ts b/packages/playwright-core/src/client/browserContext.ts index a04bdcfe16..4fad02d749 100644 --- a/packages/playwright-core/src/client/browserContext.ts +++ b/packages/playwright-core/src/client/browserContext.ts @@ -40,10 +40,11 @@ import { Artifact } from './artifact'; import { APIRequestContext } from './fetch'; import { createInstrumentation } from './clientInstrumentation'; import { rewriteErrorMessage } from '../utils/stackTrace'; +import { HarRouter } from './harRouter'; export class BrowserContext extends ChannelOwner implements api.BrowserContext { _pages = new Set(); - private _router: network.NetworkRouter; + private _routes: network.RouteHandler[] = []; readonly _browser: Browser | null = null; private _browserType: BrowserType | undefined; readonly _bindings = new Map any>(); @@ -72,7 +73,6 @@ export class BrowserContext extends ChannelOwner if (parent instanceof Browser) this._browser = parent; this._isChromium = this._browser?._name === 'chromium'; - this._router = new network.NetworkRouter(this, this._options.baseURL); this.tracing = Tracing.from(initializer.tracing); this.request = APIRequestContext.from(initializer.requestContext); @@ -153,8 +153,18 @@ export class BrowserContext extends ChannelOwner } async _onRoute(route: network.Route) { - if (await this._router.handleRoute(route)) - return; + const routeHandlers = this._routes.slice(); + for (const routeHandler of routeHandlers) { + if (!routeHandler.matches(route.request().url())) + continue; + if (routeHandler.willExpire()) + this._routes.splice(this._routes.indexOf(routeHandler), 1); + const handled = await routeHandler.handle(route); + if (!this._routes.length) + this._wrapApiCall(() => this._updateInterceptionPatterns(), true).catch(() => {}); + if (handled) + return; + } await route._innerContinue(true); } @@ -251,7 +261,8 @@ export class BrowserContext extends ChannelOwner } async route(url: URLMatch, handler: network.RouteHandlerCallback, options: { times?: number } = {}): Promise { - await this._router.route(url, handler, options); + this._routes.unshift(new network.RouteHandler(this._options.baseURL, url, handler, options.times)); + await this._updateInterceptionPatterns(); } async _recordIntoHAR(har: string, page: Page | null, options: { url?: string | RegExp, notFound?: 'abort' | 'fallback', update?: boolean } = {}): Promise { @@ -272,11 +283,18 @@ export class BrowserContext extends ChannelOwner await this._recordIntoHAR(har, null, options); return; } - await this._router.routeFromHAR(har, options); + const harRouter = await HarRouter.create(this._connection.localUtils(), har, options.notFound || 'abort', { urlMatch: options.url }); + harRouter.addContextRoute(this); } async unroute(url: URLMatch, handler?: network.RouteHandlerCallback): Promise { - await this._router.unroute(url, handler); + this._routes = this._routes.filter(route => route.url !== url || (handler && route.handler !== handler)); + await this._updateInterceptionPatterns(); + } + + private async _updateInterceptionPatterns() { + const patterns = network.RouteHandler.prepareInterceptionPatterns(this._routes); + await this._channel.setNetworkInterceptionPatterns({ patterns }); } async waitForEvent(event: string, optionsOrPredicate: WaitForEventOptions = {}): Promise { diff --git a/packages/playwright-core/src/client/harRouter.ts b/packages/playwright-core/src/client/harRouter.ts index 96e4e5896b..92cac024f9 100644 --- a/packages/playwright-core/src/client/harRouter.ts +++ b/packages/playwright-core/src/client/harRouter.ts @@ -15,8 +15,12 @@ */ import { debugLogger } from '../common/debugLogger'; +import type { BrowserContext } from './browserContext'; +import { Events } from './events'; import type { LocalUtils } from './localUtils'; import type { Route } from './network'; +import type { URLMatch } from './types'; +import type { Page } from './page'; type HarNotFoundAction = 'abort' | 'fallback'; @@ -24,21 +28,23 @@ export class HarRouter { private _localUtils: LocalUtils; private _harId: string; private _notFoundAction: HarNotFoundAction; + private _options: { urlMatch?: URLMatch; baseURL?: string; }; - static async create(localUtils: LocalUtils, file: string, notFoundAction: HarNotFoundAction): Promise { + static async create(localUtils: LocalUtils, file: string, notFoundAction: HarNotFoundAction, options: { urlMatch?: URLMatch }): Promise { const { harId, error } = await localUtils._channel.harOpen({ file }); if (error) throw new Error(error); - return new HarRouter(localUtils, harId!, notFoundAction); + return new HarRouter(localUtils, harId!, notFoundAction, options); } - private constructor(localUtils: LocalUtils, harId: string, notFoundAction: HarNotFoundAction) { + private constructor(localUtils: LocalUtils, harId: string, notFoundAction: HarNotFoundAction, options: { urlMatch?: URLMatch }) { this._localUtils = localUtils; this._harId = harId; + this._options = options; this._notFoundAction = notFoundAction; } - async handleRoute(route: Route) { + private async _handle(route: Route) { const request = route.request(); const response = await this._localUtils._channel.harLookup({ @@ -77,6 +83,16 @@ export class HarRouter { await route.fallback(); } + async addContextRoute(context: BrowserContext) { + await context.route(this._options.urlMatch || '**/*', route => this._handle(route)); + context.once(Events.BrowserContext.Close, () => this.dispose()); + } + + async addPageRoute(page: Page) { + await page.route(this._options.urlMatch || '**/*', route => this._handle(route)); + page.once(Events.Page.Close, () => this.dispose()); + } + dispose() { this._localUtils._channel.harClose({ harId: this._harId }).catch(() => {}); } diff --git a/packages/playwright-core/src/client/network.ts b/packages/playwright-core/src/client/network.ts index 4c09a9989c..6290492466 100644 --- a/packages/playwright-core/src/client/network.ts +++ b/packages/playwright-core/src/client/network.ts @@ -33,8 +33,6 @@ import { urlMatches } from '../utils/network'; import { MultiMap } from '../utils/multimap'; import { APIResponse } from './fetch'; import type { Serializable } from '../../types/structs'; -import type { BrowserContext } from './browserContext'; -import { HarRouter } from './harRouter'; import { kBrowserOrContextClosedError } from '../common/errors'; export type NetworkCookie = { @@ -629,64 +627,7 @@ export function validateHeaders(headers: Headers) { } } -export class NetworkRouter { - private _owner: Page | BrowserContext; - private _baseURL: string | undefined; - private _routes: RouteHandler[] = []; - - constructor(owner: Page | BrowserContext, baseURL: string | undefined) { - this._owner = owner; - this._baseURL = baseURL; - } - - async route(url: URLMatch, handler: RouteHandlerCallback, options: { times?: number } = {}): Promise { - this._routes.unshift(new RouteHandler(this._baseURL, url, handler, options.times)); - await this._updateInterception(); - } - - async routeFromHAR(har: string, options: { url?: string | RegExp, notFound?: 'abort' | 'fallback' } = {}): Promise { - const harRouter = await HarRouter.create(this._owner._connection.localUtils(), har, options.notFound || 'abort'); - await this.route(options.url || '**/*', route => harRouter.handleRoute(route)); - this._owner.once('close', () => harRouter.dispose()); - } - - async unroute(url: URLMatch, handler?: RouteHandlerCallback): Promise { - this._routes = this._routes.filter(route => route.url !== url || (handler && route.handler !== handler)); - await this._updateInterception(); - } - - async handleRoute(route: Route) { - const routeHandlers = this._routes.slice(); - for (const routeHandler of routeHandlers) { - if (!routeHandler.matches(route.request().url())) - continue; - if (routeHandler.willExpire()) - this._routes.splice(this._routes.indexOf(routeHandler), 1); - const handled = await routeHandler.handle(route); - if (!this._routes.length) - this._owner._wrapApiCall(() => this._updateInterception(), true).catch(() => {}); - if (handled) - return true; - } - return false; - } - - private async _updateInterception() { - const patterns: channels.BrowserContextSetNetworkInterceptionPatternsParams['patterns'] = []; - let all = false; - for (const handler of this._routes) { - if (isString(handler.url)) - patterns.push({ glob: handler.url }); - else if (isRegExp(handler.url)) - patterns.push({ regexSource: handler.url.source, regexFlags: handler.url.flags }); - else - all = true; - } - await this._owner._channel.setNetworkInterceptionPatterns(all ? { patterns: [{ glob: '**/*' }] } : { patterns }); - } -} - -class RouteHandler { +export class RouteHandler { private handledCount = 0; private readonly _baseURL: string | undefined; private readonly _times: number; @@ -700,6 +641,22 @@ class RouteHandler { this.handler = handler; } + static prepareInterceptionPatterns(handlers: RouteHandler[]) { + const patterns: channels.BrowserContextSetNetworkInterceptionPatternsParams['patterns'] = []; + let all = false; + for (const handler of handlers) { + if (isString(handler.url)) + patterns.push({ glob: handler.url }); + else if (isRegExp(handler.url)) + patterns.push({ regexSource: handler.url.source, regexFlags: handler.url.flags }); + else + all = true; + } + if (all) + return [{ glob: '**/*' }]; + return patterns; + } + public matches(requestURL: string): boolean { return urlMatches(this._baseURL, requestURL, this.url); } diff --git a/packages/playwright-core/src/client/page.ts b/packages/playwright-core/src/client/page.ts index ca89d7ae1e..733c5a6b70 100644 --- a/packages/playwright-core/src/client/page.ts +++ b/packages/playwright-core/src/client/page.ts @@ -46,13 +46,12 @@ import { Keyboard, Mouse, Touchscreen } from './input'; import { assertMaxArguments, JSHandle, parseResult, serializeArgument } from './jsHandle'; import type { FrameLocator, Locator, LocatorOptions } from './locator'; import type { ByRoleOptions } from '../utils/isomorphic/locatorUtils'; -import { NetworkRouter, type RouteHandlerCallback } from './network'; -import { Response, Route, validateHeaders, WebSocket } from './network'; -import type { Request } from './network'; +import { type RouteHandlerCallback, type Request, Response, Route, RouteHandler, validateHeaders, WebSocket } from './network'; import type { FilePayload, Headers, LifecycleEvent, SelectOption, SelectOptionOptions, Size, URLMatch, WaitForEventOptions, WaitForFunctionOptions } from './types'; import { Video } from './video'; import { Waiter } from './waiter'; import { Worker } from './worker'; +import { HarRouter } from './harRouter'; type PDFOptions = Omit & { width?: string | number, @@ -84,7 +83,7 @@ export class Page extends ChannelOwner implements api.Page private _closed = false; _closedOrCrashedPromise: Promise; private _viewportSize: Size | null; - private _router: NetworkRouter; + private _routes: RouteHandler[] = []; readonly accessibility: Accessibility; readonly coverage: Coverage; @@ -110,7 +109,6 @@ export class Page extends ChannelOwner implements api.Page super(parent, type, guid, initializer); this._browserContext = parent as unknown as BrowserContext; this._timeoutSettings = new TimeoutSettings(this._browserContext._timeoutSettings); - this._router = new NetworkRouter(this, this._browserContext._options.baseURL); this.accessibility = new Accessibility(this._channel); this.keyboard = new Keyboard(this); @@ -187,8 +185,18 @@ export class Page extends ChannelOwner implements api.Page } private async _onRoute(route: Route) { - if (await this._router.handleRoute(route)) - return; + const routeHandlers = this._routes.slice(); + for (const routeHandler of routeHandlers) { + if (!routeHandler.matches(route.request().url())) + continue; + if (routeHandler.willExpire()) + this._routes.splice(this._routes.indexOf(routeHandler), 1); + const handled = await routeHandler.handle(route); + if (!this._routes.length) + this._wrapApiCall(() => this._updateInterceptionPatterns(), true).catch(() => {}); + if (handled) + return; + } await this._browserContext._onRoute(route); } @@ -449,7 +457,8 @@ export class Page extends ChannelOwner implements api.Page } async route(url: URLMatch, handler: RouteHandlerCallback, options: { times?: number } = {}): Promise { - await this._router.route(url, handler, options); + this._routes.unshift(new RouteHandler(this._browserContext._options.baseURL, url, handler, options.times)); + await this._updateInterceptionPatterns(); } async routeFromHAR(har: string, options: { url?: string | RegExp, notFound?: 'abort' | 'fallback', update?: boolean } = {}): Promise { @@ -457,11 +466,18 @@ export class Page extends ChannelOwner implements api.Page await this._browserContext._recordIntoHAR(har, this, options); return; } - await this._router.routeFromHAR(har, options); + const harRouter = await HarRouter.create(this._connection.localUtils(), har, options.notFound || 'abort', { urlMatch: options.url }); + harRouter.addPageRoute(this); } async unroute(url: URLMatch, handler?: RouteHandlerCallback): Promise { - await this._router.unroute(url, handler); + this._routes = this._routes.filter(route => route.url !== url || (handler && route.handler !== handler)); + await this._updateInterceptionPatterns(); + } + + private async _updateInterceptionPatterns() { + const patterns = RouteHandler.prepareInterceptionPatterns(this._routes); + await this._channel.setNetworkInterceptionPatterns({ patterns }); } async screenshot(options: Omit & { path?: string, mask?: Locator[] } = {}): Promise {