Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions packages/playwright-core/src/server/bidi/bidiBrowser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export class BidiBrowser extends Browser {
readonly _contexts = new Map<string, BidiBrowserContext>();
readonly _bidiPages = new Map<bidi.BrowsingContext.BrowsingContext, BidiPage>();
private readonly _eventListeners: RegisteredListener[];
private _cacheBehavior: bidi.Network.SetCacheBehaviorParameters['cacheBehavior'] = 'default';

static async connect(parent: SdkObject, transport: ConnectionTransport, options: BrowserOptions): Promise<BidiBrowser> {
const browser = new BidiBrowser(parent, transport, options);
Expand Down Expand Up @@ -74,6 +75,8 @@ export class BidiBrowser extends Browser {
],
});

await browser._browserSession.send('network.addIntercept', { phases: [bidi.Network.InterceptPhase.AuthRequired] });
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume it doesn't affect performance of non-auth requests.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, it doesn't.
Furthermore, we were previously adding a BeforeRequestSent interception (impacting performance of all requests) even if only the AuthRequired interception was needed, so this PR will improve performance in some cases.


await browser._browserSession.send('network.addDataCollector', {
dataTypes: [bidi.Network.DataType.Response],
maxEncodedDataSize: 20_000_000, // same default as in CDP: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/inspector/inspector_network_agent.cc;l=134;drc=4128411589187a396829a827f59a655bed876aa7
Expand Down Expand Up @@ -132,6 +135,14 @@ export class BidiBrowser extends Browser {
return !this._connection.isClosed();
}

async updateCacheBehavior() {
const cacheBehavior = [...this._contexts.values()].some(context => context.requestInterceptors.length > 0) ? 'bypass' : 'default';
if (this._cacheBehavior !== cacheBehavior) {
await this._browserSession.send('network.setCacheBehavior', { cacheBehavior });
this._cacheBehavior = cacheBehavior;
}
}

private _onBrowsingContextCreated(event: bidi.BrowsingContext.Info) {
if (event.parent) {
const parentFrameId = event.parent;
Expand Down Expand Up @@ -415,18 +426,18 @@ export class BidiBrowserContext extends BrowserContext {
}

async doUpdateRequestInterception(): Promise<void> {
let interceptPromise = Promise.resolve<any>(undefined);
if (this.requestInterceptors.length > 0 && !this._interceptId) {
const { intercept } = await this._browser._browserSession.send('network.addIntercept', {
interceptPromise = this._browser._browserSession.send('network.addIntercept', {
phases: [bidi.Network.InterceptPhase.BeforeRequestSent],
urlPatterns: [{ type: 'pattern' }],
});
this._interceptId = intercept;
}).then(({ intercept }) => this._interceptId = intercept);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: let's unify the way send('network.addIntercept' and send('network.removeIntercept' are waited - either await both inline or assign both to the interceptPromise, so that we don't accidentally end up with unhandled promise rejection.

}
if (this.requestInterceptors.length === 0 && this._interceptId) {
const intercept = this._interceptId;
this._interceptId = undefined;
await this._browser._browserSession.send('network.removeIntercept', { intercept });
interceptPromise = this._browser._browserSession.send('network.removeIntercept', { intercept });
}
await Promise.all([this._browser.updateCacheBehavior(), interceptPromise]);
}

override async doUpdateDefaultViewport() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import type * as frames from '../frames';
import type { Page } from '../page';
import type * as types from '../types';
import type { BidiSession } from './bidiConnection';
import type { BidiPage } from './bidiPage';

const REQUEST_BODY_HEADERS = new Set(['content-encoding', 'content-language', 'content-location', 'content-type']);

Expand Down Expand Up @@ -245,13 +246,13 @@ export class BidiNetworkManager {
this._protocolRequestInterceptionEnabled = enabled;
if (initial && !enabled)
return;
const cachePromise = this._session.send('network.setCacheBehavior', { cacheBehavior: enabled ? 'bypass' : 'default' });
const contexts: [string] = [(this._page.delegate as BidiPage)._session.sessionId];
const cachePromise = this._session.send('network.setCacheBehavior', { cacheBehavior: enabled ? 'bypass' : 'default', contexts });
let interceptPromise = Promise.resolve<any>(undefined);
if (enabled) {
interceptPromise = this._session.send('network.addIntercept', {
phases: [bidi.Network.InterceptPhase.AuthRequired, bidi.Network.InterceptPhase.BeforeRequestSent],
urlPatterns: [{ type: 'pattern' }],
// urlPatterns: [{ type: 'string', pattern: '*' }],
phases: [bidi.Network.InterceptPhase.BeforeRequestSent],
contexts,
}).then(r => {
this._intercepId = r.intercept;
});
Expand Down
8 changes: 5 additions & 3 deletions tests/bidi/expectations/moz-firefox-nightly-library.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ library/browsercontext-cookies-third-party.spec.ts › save/load third party non
library/browsercontext-cookies-third-party.spec.ts › save/load third party 'Partitioned;' cookies [fail]
library/browsercontext-cookies-third-party.spec.ts › should be able to send third party cookies via an iframe [fail]
library/browsercontext-cookies.spec.ts › should support requestStorageAccess [fail]
library/browsercontext-credentials.spec.ts › should fail without credentials [timeout]
library/browsercontext-credentials.spec.ts › should work with setHTTPCredentials [timeout]
library/browsercontext-csp.spec.ts › should bypass after cross-process navigation [fail]
library/browsercontext-csp.spec.ts › should bypass CSP header [fail]
library/browsercontext-csp.spec.ts › should bypass CSP in iframes as well [fail]
Expand Down Expand Up @@ -155,7 +153,10 @@ library/popup.spec.ts › should use viewport size from window features [timeout
library/role-utils.spec.ts › axe-core accessible-text [timeout]
library/role-utils.spec.ts › wpt accname #2 [timeout]
library/route-web-socket.spec.ts › should emit close upon frame detach [timeout]
library/screencast.spec.ts › screencast.start emits screencastframe events [fail]
library/screencast.spec.ts › screencast.start delivers frames via onFrame callback [fail]
library/screencast.spec.ts › start returns a disposable that stops screencast [fail]
library/screencast.spec.ts › start reuses existing screencast when video recording is running [fail]
library/screencast.spec.ts › supports multiple onFrame listeners [fail]
library/screenshot.spec.ts › page screenshot › should work with device scale factor and scale:css [fail]
library/screenshot.spec.ts › page screenshot › should work with device scale factor, clip and scale:css [fail]
library/selector-generator.spec.ts › selector generator › should work in dynamic iframes without navigation [fail]
Expand Down Expand Up @@ -209,6 +210,7 @@ library/video.spec.ts › screencast › should work with old options [timeout]
library/video.spec.ts › screencast › should work with relative path for recordVideo.dir [timeout]
library/video.spec.ts › screencast › should work with video+trace [timeout]
library/video.spec.ts › screencast › should work with weird screen resolution [timeout]
library/video.spec.ts › screencast › video.start dispose stops recording [fail]
library/video.spec.ts › screencast › video.start should fail when recordVideo is set, but stop should work [fail]
library/video.spec.ts › screencast › video.start/stop twice [timeout]
library/video.spec.ts › should saveAs video [timeout]
Expand Down
1 change: 0 additions & 1 deletion tests/bidi/expectations/moz-firefox-nightly-page.txt
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ page/page-network-request.spec.ts › should return multipart/form-data [fail]
page/page-network-request.spec.ts › should return postData [fail]
page/page-network-request.spec.ts › should work with binary post data [fail]
page/page-network-request.spec.ts › should work with binary post data and interception [fail]
page/page-network-response.spec.ts › should bypass disk cache when context interception is enabled [fail]
page/page-network-response.spec.ts › should reject response.finished if context closes [timeout]
page/page-network-response.spec.ts › should report all headers [fail]
page/page-network-response.spec.ts › should report if request was fromServiceWorker [fail]
Expand Down