api: make request.frame() non-null (#1319)

Author: dgozman

docs/api.md

@@ -3405,7 +3405,7 @@ page.on('requestfailed', request => {
 ```
 
 #### request.frame()
-- returns: <?[Frame]> A [Frame] that initiated this request, or `null` if navigating to error pages.
+- returns: <[Frame]> A [Frame] that initiated this request.
 
 #### request.fulfill(response)
 - `response` <[Object]> Response that will fulfill this request
@@ -3516,7 +3516,7 @@ ResourceType will be one of the following: `document`, `stylesheet`, `image`, `m
 - returns: <Promise[?string]> Waits for this response to finish, throws when corresponding request failed.
 
 #### response.frame()
-- returns: <?[Frame]> A [Frame] that initiated this response, or `null` if navigating to error pages.
+- returns: <[Frame]> A [Frame] that initiated this response.
 
 #### response.headers()
 - returns: <[Object]> An object with HTTP headers associated with the response. All header names are lower-case.

src/chromium/crNetworkManager.ts

@@ -42,11 +42,11 @@ export class CRNetworkManager {
     this._eventListeners = this.instrumentNetworkEvents(client);
   }
 
-  instrumentNetworkEvents(session: CRSession): RegisteredListener[] {
+  instrumentNetworkEvents(session: CRSession, workerFrame?: frames.Frame): RegisteredListener[] {
     return [
-      helper.addEventListener(session, 'Fetch.requestPaused', this._onRequestPaused.bind(this)),
+      helper.addEventListener(session, 'Fetch.requestPaused', this._onRequestPaused.bind(this, workerFrame)),
       helper.addEventListener(session, 'Fetch.authRequired', this._onAuthRequired.bind(this)),
-      helper.addEventListener(session, 'Network.requestWillBeSent', this._onRequestWillBeSent.bind(this)),
+      helper.addEventListener(session, 'Network.requestWillBeSent', this._onRequestWillBeSent.bind(this, workerFrame)),
       helper.addEventListener(session, 'Network.responseReceived', this._onResponseReceived.bind(this)),
       helper.addEventListener(session, 'Network.loadingFinished', this._onLoadingFinished.bind(this)),
       helper.addEventListener(session, 'Network.loadingFailed', this._onLoadingFailed.bind(this)),
@@ -102,20 +102,20 @@ export class CRNetworkManager {
     }
   }
 
-  _onRequestWillBeSent(event: Protocol.Network.requestWillBeSentPayload) {
+  _onRequestWillBeSent(workerFrame: frames.Frame | undefined, event: Protocol.Network.requestWillBeSentPayload) {
     // Request interception doesn't happen for data URLs with Network Service.
     if (this._protocolRequestInterceptionEnabled && !event.request.url.startsWith('data:')) {
       const requestId = event.requestId;
       const interceptionId = this._requestIdToInterceptionId.get(requestId);
       if (interceptionId) {
-        this._onRequest(event, interceptionId);
+        this._onRequest(workerFrame, event, interceptionId);
         this._requestIdToInterceptionId.delete(requestId);
       } else {
         this._requestIdToRequestWillBeSentEvent.set(event.requestId, event);
       }
       return;
     }
-    this._onRequest(event, null);
+    this._onRequest(workerFrame, event, null);
   }
 
   _onAuthRequired(event: Protocol.Fetch.authRequiredPayload) {
@@ -133,7 +133,7 @@ export class CRNetworkManager {
     }).catch(debugError);
   }
 
-  _onRequestPaused(event: Protocol.Fetch.requestPausedPayload) {
+  _onRequestPaused(workerFrame: frames.Frame | undefined, event: Protocol.Fetch.requestPausedPayload) {
     if (!this._userRequestInterceptionEnabled && this._protocolRequestInterceptionEnabled) {
       this._client.send('Fetch.continueRequest', {
         requestId: event.requestId
@@ -146,14 +146,14 @@ export class CRNetworkManager {
     const interceptionId = event.requestId;
     const requestWillBeSentEvent = this._requestIdToRequestWillBeSentEvent.get(requestId);
     if (requestWillBeSentEvent) {
-      this._onRequest(requestWillBeSentEvent, interceptionId);
+      this._onRequest(workerFrame, requestWillBeSentEvent, interceptionId);
       this._requestIdToRequestWillBeSentEvent.delete(requestId);
     } else {
       this._requestIdToInterceptionId.set(requestId, interceptionId);
     }
   }
 
-  _onRequest(event: Protocol.Network.requestWillBeSentPayload, interceptionId: string | null) {
+  _onRequest(workerFrame: frames.Frame | undefined, event: Protocol.Network.requestWillBeSentPayload, interceptionId: string | null) {
     if (event.request.url.startsWith('data:'))
       return;
     let redirectChain: network.Request[] = [];
@@ -165,8 +165,12 @@ export class CRNetworkManager {
         redirectChain = request.request._redirectChain;
       }
     }
-    // TODO: how can frame be null here?
-    const frame = event.frameId ? this._page._frameManager.frame(event.frameId) : null;
+    const frame = event.frameId ? this._page._frameManager.frame(event.frameId) : workerFrame;
+    if (!frame) {
+      if (interceptionId)
+        this._client.send('Fetch.continueRequest', { requestId: interceptionId }).catch(debugError);
+      return;
+    }
     const isNavigationRequest = event.requestId === event.loaderId && event.type === 'Document';
     const documentId = isNavigationRequest ? event.loaderId : undefined;
     const request = new InterceptableRequest(this._client, frame, interceptionId, documentId, this._userRequestInterceptionEnabled, event, redirectChain);
@@ -244,7 +248,7 @@ class InterceptableRequest implements network.RequestDelegate {
   _documentId: string | undefined;
   private _client: CRSession;
 
-  constructor(client: CRSession, frame: frames.Frame | null, interceptionId: string | null, documentId: string | undefined, allowInterception: boolean, event: Protocol.Network.requestWillBeSentPayload, redirectChain: network.Request[]) {
+  constructor(client: CRSession, frame: frames.Frame, interceptionId: string | null, documentId: string | undefined, allowInterception: boolean, event: Protocol.Network.requestWillBeSentPayload, redirectChain: network.Request[]) {
     this._client = client;
     this._requestId = event.requestId;
     this._interceptionId = interceptionId;

src/chromium/crPage.ts

@@ -249,7 +249,8 @@ export class CRPage implements PageDelegate {
       this._page._addConsoleMessage(event.type, args, toConsoleMessageLocation(event.stackTrace));
     });
     session.on('Runtime.exceptionThrown', exception => this._page.emit(Events.Page.PageError, exceptionToError(exception.exceptionDetails)));
-    this._networkManager.instrumentNetworkEvents(session);
+    // TODO: attribute workers to the right frame.
+    this._networkManager.instrumentNetworkEvents(session, this._page.mainFrame());
   }
 
   _onDetachedFromTarget(event: Protocol.Target.detachedFromTargetPayload) {

src/frames.ts

@@ -211,11 +211,8 @@ export class FrameManager {
 
   requestStarted(request: network.Request) {
     this._inflightRequestStarted(request);
-    const frame = request.frame();
-    if (frame) {
-      for (const watcher of frame._requestWatchers)
-        watcher(request);
-    }
+    for (const watcher of request.frame()._requestWatchers)
+      watcher(request);
     if (!request._isFavicon)
       this._page._requestStarted(request);
   }
@@ -233,14 +230,13 @@ export class FrameManager {
 
   requestFailed(request: network.Request, canceled: boolean) {
     this._inflightRequestFinished(request);
-    const frame = request.frame();
-    if (request._documentId && frame) {
-      const isCurrentDocument = frame._lastDocumentId === request._documentId;
+    if (request._documentId) {
+      const isCurrentDocument = request.frame()._lastDocumentId === request._documentId;
       if (!isCurrentDocument) {
         let errorText = request.failure()!.errorText;
         if (canceled)
           errorText += '; maybe frame was detached?';
-        for (const watcher of frame._documentWatchers)
+        for (const watcher of request.frame()._documentWatchers)
           watcher(request._documentId, new Error(errorText));
       }
     }
@@ -263,7 +259,7 @@ export class FrameManager {
 
   private _inflightRequestFinished(request: network.Request) {
     const frame = request.frame();
-    if (!frame || request._isFavicon)
+    if (request._isFavicon)
       return;
     if (!frame._inflightRequests.has(request))
       return;
@@ -276,7 +272,7 @@ export class FrameManager {
 
   private _inflightRequestStarted(request: network.Request) {
     const frame = request.frame();
-    if (!frame || request._isFavicon)
+    if (request._isFavicon)
       return;
     frame._inflightRequests.add(request);
     if (frame._inflightRequests.size === 1)

src/network.ts

@@ -107,14 +107,14 @@ export class Request {
   private _method: string;
   private _postData: string | undefined;
   private _headers: Headers;
-  private _frame: frames.Frame | null;
+  private _frame: frames.Frame;
   private _waitForResponsePromise: Promise<Response>;
   private _waitForResponsePromiseCallback: (value: Response) => void = () => {};
   private _waitForFinishedPromise: Promise<Response | null>;
   private _waitForFinishedPromiseCallback: (value: Response | null) => void = () => {};
   private _interceptionHandled = false;
 
-  constructor(delegate: RequestDelegate | null, frame: frames.Frame | null, redirectChain: Request[], documentId: string | undefined,
+  constructor(delegate: RequestDelegate | null, frame: frames.Frame, redirectChain: Request[], documentId: string | undefined,
     url: string, resourceType: string, method: string, postData: string | undefined, headers: Headers) {
     assert(!url.startsWith('data:'), 'Data urls should not fire requests');
     this._delegate = delegate;
@@ -177,7 +177,7 @@ export class Request {
     response._finishedPromise.then(() => this._waitForFinishedPromiseCallback(response));
   }
 
-  frame(): frames.Frame | null {
+  frame(): frames.Frame {
     return this._frame;
   }
 
@@ -309,7 +309,7 @@ export class Response {
     return this._request;
   }
 
-  frame(): frames.Frame | null {
+  frame(): frames.Frame {
     return this._request.frame();
   }
 }

src/webkit/wkInterceptableRequest.ts

@@ -46,7 +46,7 @@ export class WKInterceptableRequest implements network.RequestDelegate {
   _interceptedCallback: () => void = () => {};
   private _interceptedPromise: Promise<unknown>;
 
-  constructor(session: WKSession, allowInterception: boolean, frame: frames.Frame | null, event: Protocol.Network.requestWillBeSentPayload, redirectChain: network.Request[], documentId: string | undefined) {
+  constructor(session: WKSession, allowInterception: boolean, frame: frames.Frame, event: Protocol.Network.requestWillBeSentPayload, redirectChain: network.Request[], documentId: string | undefined) {
     this._session = session;
     this._requestId = event.requestId;
     this.request = new network.Request(allowInterception ? this : null, frame, redirectChain, documentId, event.request.url,

src/webkit/wkPage.ts

@@ -728,7 +728,7 @@ export class WKPage implements PageDelegate {
         redirectChain = request.request._redirectChain;
       }
     }
-    const frame = this._page._frameManager.frame(event.frameId);
+    const frame = this._page._frameManager.frame(event.frameId)!;
     // TODO(einbinder) this will fail if we are an XHR document request
     const isNavigationRequest = event.type === 'Document';
     const documentId = isNavigationRequest ? event.loaderId : undefined;