api(request): make request.response a promise (#1377)

Author: pavelfeldman

docs/api.md

@@ -3290,7 +3290,7 @@ Contains the request's resource type as it was perceived by the rendering engine
 ResourceType will be one of the following: `document`, `stylesheet`, `image`, `media`, `font`, `script`, `texttrack`, `xhr`, `fetch`, `eventsource`, `websocket`, `manifest`, `other`.
 
 #### request.response()
-- returns: <?[Response]> A matching [Response] object, or `null` if the response has not been received yet.
+- returns: <[Promise]<?[Response]> A matching [Response] object, or `null` if the response was not received due to error.
 
 #### request.url()
 - returns: <[string]> URL of the request.

src/chromium/crNetworkManager.ts

@@ -215,7 +215,7 @@ export class CRNetworkManager {
 
     // Under certain conditions we never get the Network.responseReceived
     // event from protocol. @see https://crbug.com/883475
-    const response = request.request.response();
+    const response = request.request._existingResponse();
     if (response)
       response._requestFinished();
     this._requestIdToRequest.delete(request._requestId);
@@ -230,7 +230,7 @@ export class CRNetworkManager {
     // @see https://crbug.com/750469
     if (!request)
       return;
-    const response = request.request.response();
+    const response = request.request._existingResponse();
     if (response)
       response._requestFinished();
     this._requestIdToRequest.delete(request._requestId);

src/firefox/ffNetworkManager.ts

@@ -90,7 +90,7 @@ export class FFNetworkManager {
     const request = this._requests.get(event.requestId);
     if (!request)
       return;
-    const response = request.request.response()!;
+    const response = request.request._existingResponse()!;
     // Keep redirected requests in the map for future reference in redirectChain.
     const isRedirected = response.status() >= 300 && response.status() <= 399;
     if (isRedirected) {
@@ -107,7 +107,7 @@ export class FFNetworkManager {
     if (!request)
       return;
     this._requests.delete(request._id);
-    const response = request.request.response();
+    const response = request.request._existingResponse();
     if (response)
       response._requestFinished();
     request.request._setFailureText(event.errorCode);

src/frames.ts

@@ -387,7 +387,7 @@ export class Frame {
 
     disposer.dispose();
 
-    return request ? request._finalRequest._waitForResponse() : null;
+    return request ? request._finalRequest.response() : null;
 
     function throwIfError(error: Error|void): asserts error is void {
       if (!error)
@@ -430,7 +430,7 @@ export class Frame {
     if (error)
       throw error;
 
-    return request ? request._finalRequest._waitForResponse() : null;
+    return request ? request._finalRequest.response() : null;
   }
 
   async _waitForLoadState(options: types.NavigateOptions = {}): Promise<void> {

src/network.ts

@@ -108,10 +108,8 @@ export class Request {
   private _postData: string | null;
   private _headers: Headers;
   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 _waitForResponsePromise: Promise<Response | null>;
+  private _waitForResponsePromiseCallback: (value: Response | null) => void = () => {};
   private _interceptionHandled = false;
 
   constructor(delegate: RequestDelegate | null, frame: frames.Frame, redirectChain: Request[], documentId: string | undefined,
@@ -130,13 +128,12 @@ export class Request {
     this._postData = postData;
     this._headers = headers;
     this._waitForResponsePromise = new Promise(f => this._waitForResponsePromiseCallback = f);
-    this._waitForFinishedPromise = new Promise(f => this._waitForFinishedPromiseCallback = f);
     this._isFavicon = url.endsWith('/favicon.ico');
   }
 
   _setFailureText(failureText: string) {
     this._failureText = failureText;
-    this._waitForFinishedPromiseCallback(null);
+    this._waitForResponsePromiseCallback(null);
   }
 
   url(): string {
@@ -159,22 +156,17 @@ export class Request {
     return this._headers;
   }
 
-  response(): Response | null {
-    return this._response;
-  }
-
-  async _waitForFinished(): Promise<Response | null> {
-    return this._waitForFinishedPromise;
+  response(): Promise<Response | null> {
+    return this._waitForResponsePromise;
   }
 
-  async _waitForResponse(): Promise<Response> {
-    return await this._waitForResponsePromise;
+  _existingResponse(): Response | null {
+    return this._response;
   }
 
   _setResponse(response: Response) {
     this._response = response;
     this._waitForResponsePromiseCallback(response);
-    response._finishedPromise.then(() => this._waitForFinishedPromiseCallback(response));
   }
 
   frame(): frames.Frame {

src/webkit/wkPage.ts

@@ -789,7 +789,7 @@ export class WKPage implements PageDelegate {
 
     // Under certain conditions we never get the Network.responseReceived
     // event from protocol. @see https://crbug.com/883475
-    const response = request.request.response();
+    const response = request.request._existingResponse();
     if (response)
       response._requestFinished();
     this._requestIdToRequest.delete(request._requestId);
@@ -802,7 +802,7 @@ export class WKPage implements PageDelegate {
     // @see https://crbug.com/750469
     if (!request)
       return;
-    const response = request.request.response();
+    const response = request.request._existingResponse();
     if (response)
       response._requestFinished();
     this._requestIdToRequest.delete(request._requestId);

test/interception.spec.js

@@ -314,7 +314,7 @@ module.exports.describe = function({testRunner, expect, defaultBrowserOptions, p
       const response = await page.goto(`data:text/html,<link rel="stylesheet" href="${server.PREFIX}/fonts?helvetica|arial"/>`);
       expect(response).toBe(null);
       expect(requests.length).toBe(1);
-      expect(requests[0].response().status()).toBe(404);
+      expect((await requests[0].response()).status()).toBe(404);
     });
     it('should not throw "Invalid Interception Id" if the request was cancelled', async({page, server}) => {
       await page.setContent('<iframe></iframe>');

test/network.spec.js

@@ -132,7 +132,7 @@ module.exports.describe = function({testRunner, expect, MAC, WIN, FFOX, CHROMIUM
       const response = await page.goto(server.PREFIX + '/foo.html');
       const redirectChain = response.request().redirectChain();
       expect(redirectChain.length).toBe(1);
-      const redirected = redirectChain[0].response();
+      const redirected = await redirectChain[0].response();
       expect(redirected.status()).toBe(302);
       let error = null;
       await redirected.text().catch(e => error = e);
@@ -216,7 +216,7 @@ module.exports.describe = function({testRunner, expect, MAC, WIN, FFOX, CHROMIUM
       expect(requests[0].url()).toBe(server.EMPTY_PAGE);
       expect(requests[0].resourceType()).toBe('document');
       expect(requests[0].method()).toBe('GET');
-      expect(requests[0].response()).toBeTruthy();
+      expect(await requests[0].response()).toBeTruthy();
       expect(requests[0].frame() === page.mainFrame()).toBe(true);
       expect(requests[0].frame().url()).toBe(server.EMPTY_PAGE);
     });
@@ -241,7 +241,7 @@ module.exports.describe = function({testRunner, expect, MAC, WIN, FFOX, CHROMIUM
       await page.goto(server.PREFIX + '/one-style.html');
       expect(failedRequests.length).toBe(1);
       expect(failedRequests[0].url()).toContain('one-style.css');
-      expect(failedRequests[0].response()).toBe(null);
+      expect(await failedRequests[0].response()).toBe(null);
       expect(failedRequests[0].resourceType()).toBe('stylesheet');
       if (CHROMIUM) {
         expect(failedRequests[0].failure().errorText).toBe('net::ERR_INVALID_HTTP_RESPONSE');
@@ -264,7 +264,7 @@ module.exports.describe = function({testRunner, expect, MAC, WIN, FFOX, CHROMIUM
       ]);
       const request = response.request();
       expect(request.url()).toBe(server.EMPTY_PAGE);
-      expect(request.response()).toBeTruthy();
+      expect(await request.response()).toBeTruthy();
       expect(request.frame() === page.mainFrame()).toBe(true);
       expect(request.frame().url()).toBe(server.EMPTY_PAGE);
     });