From 4bce51ec74db329174249d9b484f8b6ecb8607b1 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Fri, 19 Jul 2024 15:38:02 -0700 Subject: [PATCH] Address comments --- .../server/webkit/wkInterceptableRequest.ts | 7 +++--- .../src/server/webkit/wkPage.ts | 25 ++++++------------- .../src/server/webkit/wkProvisionalPage.ts | 20 ++++++++------- 3 files changed, 22 insertions(+), 30 deletions(-) diff --git a/packages/playwright-core/src/server/webkit/wkInterceptableRequest.ts b/packages/playwright-core/src/server/webkit/wkInterceptableRequest.ts index eed433886d..a450127789 100644 --- a/packages/playwright-core/src/server/webkit/wkInterceptableRequest.ts +++ b/packages/playwright-core/src/server/webkit/wkInterceptableRequest.ts @@ -40,9 +40,9 @@ const errorReasons: { [reason: string]: Protocol.Network.ResourceErrorType } = { }; export class WKInterceptableRequest { - private readonly _session: WKSession; - readonly request: network.Request; + private _session: WKSession; private _requestId: string; + readonly request: network.Request; _timestamp: number; _wallTime: number; @@ -59,7 +59,8 @@ export class WKInterceptableRequest { resourceType, event.request.method, postDataBuffer, headersObjectToArray(event.request.headers)); } - changeRequestId(requestId: string) { + adoptRequestFromNewProcess(newSession: WKSession, requestId: string) { + this._session = newSession; this._requestId = requestId; } diff --git a/packages/playwright-core/src/server/webkit/wkPage.ts b/packages/playwright-core/src/server/webkit/wkPage.ts index 171d3d9bdd..d16d013973 100644 --- a/packages/playwright-core/src/server/webkit/wkPage.ts +++ b/packages/playwright-core/src/server/webkit/wkPage.ts @@ -250,11 +250,10 @@ export class WKPage implements PageDelegate { private _onTargetDestroyed(event: Protocol.Target.targetDestroyedPayload) { const { targetId, crashed } = event; if (this._provisionalPage && this._provisionalPage._session.sessionId === targetId) { + this._maybeCancelCoopNavigationRequest(this._provisionalPage); this._provisionalPage._session.dispose(); this._provisionalPage.dispose(); - const provisionalPage = this._provisionalPage; this._provisionalPage = null; - this._maybeCancelNavigationRequest(provisionalPage); } else if (this._session.sessionId === targetId) { this._session.dispose(); eventsHelper.removeEventListeners(this._sessionListeners); @@ -1017,31 +1016,28 @@ export class WKPage implements PageDelegate { return context.createHandle(result.object) as dom.ElementHandle; } - private _maybeCancelNavigationRequest(provisionalPage: WKProvisionalPage) { - const navigationRequest = provisionalPage.pendingNavigationRequest(); + private _maybeCancelCoopNavigationRequest(provisionalPage: WKProvisionalPage) { + const navigationRequest = provisionalPage.coopNavigationRequest(); for (const [requestId, request] of this._requestIdToRequest) { if (request.request === navigationRequest) { // Make sure the request completes if the provisional navigation is canceled. this._onLoadingFailed(provisionalPage._session, { requestId: requestId, errorText: 'Provisiolal navigation canceled.', - timestamp: 0, - canceled: true + timestamp: request._timestamp, + canceled: true, }); return; } } } - _changeRequestId(navigationRequest: network.Request, newRequestId: string) { + _adoptRequestFromNewProcess(navigationRequest: network.Request, newSession: WKSession, newRequestId: string) { for (const [requestId, request] of this._requestIdToRequest) { if (request.request === navigationRequest) { this._requestIdToRequest.delete(requestId); - request.changeRequestId(newRequestId); + request.adoptRequestFromNewProcess(newSession, newRequestId); this._requestIdToRequest.set(newRequestId, request); - // Simply ignore this event as it has already been dispatched from the original process - // and there will ne no requestIntercepted event from the provisional process as it resumes - // existing network load (that has already received reponse headers). return; } } @@ -1051,7 +1047,6 @@ export class WKPage implements PageDelegate { if (event.request.url.startsWith('data:')) return; - // navigation too? // We do not support intercepting redirects. if (this._page.needsRequestInterception() && !event.redirectResponse) this._requestIdToRequestWillBeSentEvent.set(event.requestId, event); @@ -1190,12 +1185,6 @@ export class WKPage implements PageDelegate { if (!request) return; - // If a provisional page was created after Cross-Origin-Opener-Policy headers were received, it - // will take over the request (with a new id). We'll only fail it if the provisional navigation - // fails/get canceled. - if (this._provisionalPage?.pendingNavigationRequest() === request.request) - return; - const response = request.request._existingResponse(); if (response) { response._serverAddrFinished(); diff --git a/packages/playwright-core/src/server/webkit/wkProvisionalPage.ts b/packages/playwright-core/src/server/webkit/wkProvisionalPage.ts index 117ba28417..b8af1b9ca3 100644 --- a/packages/playwright-core/src/server/webkit/wkProvisionalPage.ts +++ b/packages/playwright-core/src/server/webkit/wkProvisionalPage.ts @@ -22,11 +22,10 @@ import type { Protocol } from './protocol'; import { assert } from '../../utils'; import type * as network from '../network'; - export class WKProvisionalPage { readonly _session: WKSession; private readonly _wkPage: WKPage; - private _existingNavigationRequest: network.Request | undefined; + private _coopNavigationRequest: network.Request | undefined; private _sessionListeners: RegisteredListener[] = []; private _mainFrameId: string | null = null; readonly initializationPromise: Promise; @@ -34,13 +33,16 @@ export class WKProvisionalPage { constructor(session: WKSession, page: WKPage) { this._session = session; this._wkPage = page; + // Cross-Origin-Opener-Policy (COOP) request starts in one process and once response headers + // have been received, continues in another. + // // Network.requestWillBeSent and requestIntercepted (if intercepting) from the original web process // will always come before a provisional page is created based on the response COOP headers. // Thereafter we'll receive targetCreated (provisional) and later on in some order loadingFailed from the // original process and requestWillBeSent from the provisional one. We should ignore loadingFailed // as the original request continues in the provisional process. But if the provisional load is later // canceled we should dispatch loadingFailed to the client. - this._existingNavigationRequest = page._page.mainFrame().pendingDocument()?.request; + this._coopNavigationRequest = page._page.mainFrame().pendingDocument()?.request; const overrideFrameId = (handler: (p: any) => void) => { return (payload: any) => { @@ -63,8 +65,8 @@ export class WKProvisionalPage { this.initializationPromise = this._wkPage._initializeSession(session, true, ({ frameTree }) => this._handleFrameTree(frameTree)); } - pendingNavigationRequest(): network.Request | undefined { - return this._existingNavigationRequest; + coopNavigationRequest(): network.Request | undefined { + return this._coopNavigationRequest; } dispose() { @@ -77,10 +79,10 @@ export class WKProvisionalPage { } private _onRequestWillBeSent(event: Protocol.Network.requestWillBeSentPayload) { - if (this._existingNavigationRequest && this._existingNavigationRequest.url() === event.request.url) { + if (this._coopNavigationRequest && this._coopNavigationRequest.url() === event.request.url) { // If it's a continuation of the main frame navigation request after COOP headers were received, // take over original request, and replace its request id with the new one. - this._wkPage._changeRequestId(this._existingNavigationRequest, event.requestId); + this._wkPage._adoptRequestFromNewProcess(this._coopNavigationRequest, this._session, event.requestId); // Simply ignore this event as it has already been dispatched from the original process // and there will ne no requestIntercepted event from the provisional process as it resumes // existing network load (that has already received reponse headers). @@ -90,12 +92,12 @@ export class WKProvisionalPage { } private _onLoadingFinished(event: Protocol.Network.loadingFinishedPayload): void { - this._existingNavigationRequest = undefined; + this._coopNavigationRequest = undefined; this._wkPage._onLoadingFinished(event); } private _onLoadingFailed(event: Protocol.Network.loadingFailedPayload) { - this._existingNavigationRequest = undefined; + this._coopNavigationRequest = undefined; this._wkPage._onLoadingFailed(this._session, event); }