Address comments

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;
   }
 

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();

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<void>;
@@ -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);
   }