api(network): replace redirectChain with redirectedFrom/redirectedTo (#…

…1401)

docs/api.md

@@ -3138,7 +3138,8 @@ If request gets a 'redirect' response, the request is successfully finished with
 - [request.isNavigationRequest()](#requestisnavigationrequest)
 - [request.method()](#requestmethod)
 - [request.postData()](#requestpostdata)
-- [request.redirectChain()](#requestredirectchain)
+- [request.redirectedFrom()](#requestredirectedfrom)
+- [request.redirectedTo()](#requestredirectedto)
 - [request.resourceType()](#requestresourcetype)
 - [request.response()](#requestresponse)
 - [request.url()](#requesturl)
@@ -3176,31 +3177,29 @@ Whether this request is driving frame's navigation.
 #### request.postData()
 - returns: <?[string]> Request's post body, if any.
 
-#### request.redirectChain()
-- returns: <[Array]<[Request]>>
+#### request.redirectedFrom()
+- returns: <?[Request]> Request that was redirected by the server to this one, if any.
 
-A `redirectChain` is a chain of requests initiated to fetch a resource.
-- If there are no redirects and the request was successful, the chain will be empty.
-- If a server responds with at least a single redirect, then the chain will
-contain all the requests that were redirected.
-
-`redirectChain` is shared between all the requests of the same chain.
-
-For example, if the website `http://example.com` has a single redirect to
-`https://example.com`, then the chain will contain one request:
+When the server responds with a redirect, Playwright creates a new [Request] object. The two requests are connected by `redirectedFrom()` and `redirectedTo()` methods. When multiple server redirects has happened, it is possible to construct the whole redirect chain by repeatedly calling `redirectedFrom()`.
 
+For example, if the website `http://example.com` redirects to `https://example.com`:
 ```js
 const response = await page.goto('http://example.com');
-const chain = response.request().redirectChain();
-console.log(chain.length); // 1
-console.log(chain[0].url()); // 'http://example.com'
+console.log(response.request().redirectedFrom().url()); // 'http://example.com'
 ```
 
-If the website `https://google.com` has no redirects, then the chain will be empty:
+If the website `https://google.com` has no redirects:
 ```js
 const response = await page.goto('https://google.com');
-const chain = response.request().redirectChain();
-console.log(chain.length); // 0
+console.log(response.request().redirectedFrom()); // null
+```
+
+#### request.redirectedTo()
+- returns: <?[Request]> New request issued by the browser iff the server responded with redirect.
+
+This method is the opposite of [request.redirectedFrom()](#requestredirectedfrom):
+```js
+console.log(request.redirectedFrom().redirectedTo() === request); // true
 ```
 
 #### request.resourceType()

src/chromium/crNetworkManager.ts

@@ -156,13 +156,13 @@ export class CRNetworkManager {
   _onRequest(workerFrame: frames.Frame | undefined, event: Protocol.Network.requestWillBeSentPayload, interceptionId: string | null) {
     if (event.request.url.startsWith('data:'))
       return;
-    let redirectChain: network.Request[] = [];
+    let redirectedFrom: network.Request | null = null;
     if (event.redirectResponse) {
       const request = this._requestIdToRequest.get(event.requestId);
       // If we connect late to the target, we could have missed the requestWillBeSent event.
       if (request) {
         this._handleRequestRedirect(request, event.redirectResponse);
-        redirectChain = request.request._redirectChain;
+        redirectedFrom = request.request;
       }
     }
     const frame = event.frameId ? this._page._frameManager.frame(event.frameId) : workerFrame;
@@ -173,7 +173,7 @@ export class CRNetworkManager {
     }
     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);
+    const request = new InterceptableRequest(this._client, frame, interceptionId, documentId, this._userRequestInterceptionEnabled, event, redirectedFrom);
     this._requestIdToRequest.set(event.requestId, request);
     this._page._frameManager.requestStarted(request.request);
   }
@@ -188,7 +188,6 @@ export class CRNetworkManager {
 
   _handleRequestRedirect(request: InterceptableRequest, responsePayload: Protocol.Network.Response) {
     const response = this._createResponse(request, responsePayload);
-    request.request._redirectChain.push(request.request);
     response._requestFinished(new Error('Response body is unavailable for redirect responses'));
     this._requestIdToRequest.delete(request._requestId);
     if (request._interceptionId)
@@ -248,13 +247,13 @@ class InterceptableRequest implements network.RouteDelegate {
   _documentId: string | undefined;
   private _client: CRSession;
 
-  constructor(client: CRSession, frame: frames.Frame, 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, redirectedFrom: network.Request | null) {
     this._client = client;
     this._requestId = event.requestId;
     this._interceptionId = interceptionId;
     this._documentId = documentId;
 
-    this.request = new network.Request(allowInterception ? this : null, frame, redirectChain, documentId,
+    this.request = new network.Request(allowInterception ? this : null, frame, redirectedFrom, documentId,
         event.request.url, (event.type || '').toLowerCase(), event.request.method, event.request.postData || null, headersObject(event.request.headers));
   }
 

src/firefox/ffNetworkManager.ts

@@ -52,17 +52,13 @@ export class FFNetworkManager {
   }
 
   _onRequestWillBeSent(event: Protocol.Network.requestWillBeSentPayload) {
-    const redirected = event.redirectedFrom ? this._requests.get(event.redirectedFrom) : null;
-    const frame = redirected ? redirected.request.frame() : (event.frameId ? this._page._frameManager.frame(event.frameId) : null);
+    const redirectedFrom = event.redirectedFrom ? (this._requests.get(event.redirectedFrom) || null) : null;
+    const frame = redirectedFrom ? redirectedFrom.request.frame() : (event.frameId ? this._page._frameManager.frame(event.frameId) : null);
     if (!frame)
       return;
-    let redirectChain: network.Request[] = [];
-    if (redirected) {
-      redirectChain = redirected.request._redirectChain;
-      redirectChain.push(redirected.request);
-      this._requests.delete(redirected._id);
-    }
-    const request = new InterceptableRequest(this._session, frame, redirectChain, event);
+    if (redirectedFrom)
+      this._requests.delete(redirectedFrom._id);
+    const request = new InterceptableRequest(this._session, frame, redirectedFrom, event);
     this._requests.set(request._id, request);
     this._page._frameManager.requestStarted(request.request);
   }
@@ -91,7 +87,7 @@ export class FFNetworkManager {
     if (!request)
       return;
     const response = request.request._existingResponse()!;
-    // Keep redirected requests in the map for future reference in redirectChain.
+    // Keep redirected requests in the map for future reference as redirectedFrom.
     const isRedirected = response.status() >= 300 && response.status() <= 399;
     if (isRedirected) {
       response._requestFinished(new Error('Response body is unavailable for redirect responses'));
@@ -146,15 +142,15 @@ class InterceptableRequest implements network.RouteDelegate {
   _id: string;
   private _session: FFSession;
 
-  constructor(session: FFSession, frame: frames.Frame, redirectChain: network.Request[], payload: Protocol.Network.requestWillBeSentPayload) {
+  constructor(session: FFSession, frame: frames.Frame, redirectedFrom: InterceptableRequest | null, payload: Protocol.Network.requestWillBeSentPayload) {
     this._id = payload.requestId;
     this._session = session;
 
     const headers: network.Headers = {};
     for (const {name, value} of payload.headers)
       headers[name.toLowerCase()] = value;
 
-    this.request = new network.Request(payload.isIntercepted ? this : null, frame, redirectChain, payload.navigationId,
+    this.request = new network.Request(payload.isIntercepted ? this : null, frame, redirectedFrom ? redirectedFrom.request : null, payload.navigationId,
         payload.url, causeToResourceType[payload.cause] || 'other', payload.method, payload.postData || null, headers);
   }
 

src/frames.ts

@@ -387,7 +387,7 @@ export class Frame {
 
     disposer.dispose();
 
-    return request ? request._finalRequest.response() : 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.response() : null;
+    return request ? request._finalRequest().response() : null;
   }
 
   async _waitForLoadState(options: types.NavigateOptions = {}): Promise<void> {
@@ -519,7 +519,7 @@ export class Frame {
       this._requestWatchers.delete(onRequest);
     };
     const onRequest = (request: network.Request) => {
-      if (!request._documentId || request.redirectChain().length)
+      if (!request._documentId || request.redirectedFrom())
         return;
       requestMap.set(request._documentId, request);
     };

src/network.ts

@@ -95,8 +95,8 @@ export type Headers = { [key: string]: string };
 export class Request {
   readonly _routeDelegate: RouteDelegate | null;
   private _response: Response | null = null;
-  _redirectChain: Request[];
-  _finalRequest: Request;
+  private _redirectedFrom: Request | null;
+  private _redirectedTo: Request | null = null;
   readonly _documentId?: string;
   readonly _isFavicon: boolean;
   private _failureText: string | null = null;
@@ -109,15 +109,14 @@ export class Request {
   private _waitForResponsePromise: Promise<Response | null>;
   private _waitForResponsePromiseCallback: (value: Response | null) => void = () => {};
 
-  constructor(routeDelegate: RouteDelegate | null, frame: frames.Frame, redirectChain: Request[], documentId: string | undefined,
+  constructor(routeDelegate: RouteDelegate | null, frame: frames.Frame, redirectedFrom: Request | null, documentId: string | undefined,
     url: string, resourceType: string, method: string, postData: string | null, headers: Headers) {
     assert(!url.startsWith('data:'), 'Data urls should not fire requests');
     this._routeDelegate = routeDelegate;
     this._frame = frame;
-    this._redirectChain = redirectChain;
-    this._finalRequest = this;
-    for (const request of redirectChain)
-      request._finalRequest = this;
+    this._redirectedFrom = redirectedFrom;
+    if (redirectedFrom)
+      redirectedFrom._redirectedTo = this;
     this._documentId = documentId;
     this._url = stripFragmentFromUrl(url);
     this._resourceType = resourceType;
@@ -166,6 +165,10 @@ export class Request {
     this._waitForResponsePromiseCallback(response);
   }
 
+  _finalRequest(): Request {
+    return this._redirectedTo ? this._redirectedTo._finalRequest() : this;
+  }
+
   frame(): frames.Frame {
     return this._frame;
   }
@@ -174,8 +177,12 @@ export class Request {
     return !!this._documentId;
   }
 
-  redirectChain(): Request[] {
-    return this._redirectChain.slice();
+  redirectedFrom(): Request | null {
+    return this._redirectedFrom;
+  }
+
+  redirectedTo(): Request | null {
+    return this._redirectedTo;
   }
 
   failure(): { errorText: string; } | null {

src/webkit/wkInterceptableRequest.ts

@@ -46,11 +46,12 @@ export class WKInterceptableRequest implements network.RouteDelegate {
   _interceptedCallback: () => void = () => {};
   private _interceptedPromise: Promise<unknown>;
 
-  constructor(session: WKSession, allowInterception: boolean, frame: frames.Frame, event: Protocol.Network.requestWillBeSentPayload, redirectChain: network.Request[], documentId: string | undefined) {
+  constructor(session: WKSession, allowInterception: boolean, frame: frames.Frame, event: Protocol.Network.requestWillBeSentPayload, redirectedFrom: network.Request | null, documentId: string | undefined) {
     this._session = session;
     this._requestId = event.requestId;
-    this.request = new network.Request(allowInterception ? this : null, frame, redirectChain, documentId, event.request.url,
-        event.type ? event.type.toLowerCase() : 'Unknown', event.request.method, event.request.postData || null, headersObject(event.request.headers));
+    const resourceType = event.type ? event.type.toLowerCase() : (redirectedFrom ? redirectedFrom.resourceType() : 'unknown');
+    this.request = new network.Request(allowInterception ? this : null, frame, redirectedFrom, documentId, event.request.url,
+        resourceType, event.request.method, event.request.postData || null, headersObject(event.request.headers));
     this._interceptedPromise = new Promise(f => this._interceptedCallback = f);
   }
 

src/webkit/wkPage.ts

@@ -730,27 +730,26 @@ export class WKPage implements PageDelegate {
   _onRequestWillBeSent(session: WKSession, event: Protocol.Network.requestWillBeSentPayload) {
     if (event.request.url.startsWith('data:'))
       return;
-    let redirectChain: network.Request[] = [];
+    let redirectedFrom: network.Request | null = null;
     if (event.redirectResponse) {
       const request = this._requestIdToRequest.get(event.requestId);
       // If we connect late to the target, we could have missed the requestWillBeSent event.
       if (request) {
         this._handleRequestRedirect(request, event.redirectResponse);
-        redirectChain = request.request._redirectChain;
+        redirectedFrom = request.request;
       }
     }
     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;
-    const request = new WKInterceptableRequest(session, this._page._needsRequestInterception(), frame, event, redirectChain, documentId);
+    const request = new WKInterceptableRequest(session, this._page._needsRequestInterception(), frame, event, redirectedFrom, documentId);
     this._requestIdToRequest.set(event.requestId, request);
     this._page._frameManager.requestStarted(request.request);
   }
 
   private _handleRequestRedirect(request: WKInterceptableRequest, responsePayload: Protocol.Network.Response) {
     const response = request.createResponse(responsePayload);
-    request.request._redirectChain.push(request.request);
     response._requestFinished(new Error('Response body is unavailable for redirect responses'));
     this._requestIdToRequest.delete(request._requestId);
     this._page._frameManager.requestReceivedResponse(response);

test/browsercontext.spec.js

@@ -488,6 +488,7 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FF
         page.evaluate(url => window.open(url), server.EMPTY_PAGE)
       ]);
       expect(otherPage.url()).toBe(server.EMPTY_PAGE);
+      await context.close();
     });
     it.fail(CHROMIUM)('should have url with domcontentloaded', async({browser, server}) => {
       const context = await browser.newContext();
@@ -497,6 +498,7 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FF
         page.evaluate(url => window.open(url), server.EMPTY_PAGE)
       ]);
       expect(otherPage.url()).toBe(server.EMPTY_PAGE);
+      await context.close();
     });
     it('should report when a new page is created and closed', async({browser, server}) => {
       const context = await browser.newContext();

test/interception.spec.js

@@ -204,16 +204,19 @@ module.exports.describe = function({testRunner, expect, defaultBrowserOptions, p
       expect(response.url()).toContain('empty.html');
       expect(requests.length).toBe(5);
       expect(requests[2].resourceType()).toBe('document');
-      // Check redirect chain
-      const redirectChain = response.request().redirectChain();
-      expect(redirectChain.length).toBe(4);
-      expect(redirectChain[0].url()).toContain('/non-existing-page.html');
-      expect(redirectChain[2].url()).toContain('/non-existing-page-3.html');
-      for (let i = 0; i < redirectChain.length; ++i) {
-        const request = redirectChain[i];
-        expect(request.isNavigationRequest()).toBe(true);
-        expect(request.redirectChain().indexOf(request)).toBe(i);
+      const chain = [];
+      for (let r = response.request(); r; r = r.redirectedFrom()) {
+        chain.push(r);
+        expect(r.isNavigationRequest()).toBe(true);
       }
+      expect(chain.length).toBe(5);
+      expect(chain[0].url()).toContain('/empty.html');
+      expect(chain[1].url()).toContain('/non-existing-page-4.html');
+      expect(chain[2].url()).toContain('/non-existing-page-3.html');
+      expect(chain[3].url()).toContain('/non-existing-page-2.html');
+      expect(chain[4].url()).toContain('/non-existing-page.html');
+      for (let i = 0; i < chain.length; i++)
+        expect(chain[i].redirectedTo()).toBe(i ? chain[i - 1] : null);
     });
     it('should work with redirects for subresources', async({page, server}) => {
       const requests = [];
@@ -231,12 +234,14 @@ module.exports.describe = function({testRunner, expect, defaultBrowserOptions, p
       expect(response.url()).toContain('one-style.html');
       expect(requests.length).toBe(5);
       expect(requests[0].resourceType()).toBe('document');
-      expect(requests[1].resourceType()).toBe('stylesheet');
-      // Check redirect chain
-      const redirectChain = requests[1].redirectChain();
-      expect(redirectChain.length).toBe(3);
-      expect(redirectChain[0].url()).toContain('/one-style.css');
-      expect(redirectChain[2].url()).toContain('/three-style.css');
+
+      let r = requests.find(r => r.url().includes('/four-style.css'));
+      for (const url of ['/four-style.css', '/three-style.css', '/two-style.css', '/one-style.css']) {
+        expect(r.resourceType()).toBe('stylesheet');
+        expect(r.url()).toContain(url);
+        r = r.redirectedFrom();
+      }
+      expect(r).toBe(null);
     });
     it('should work with equal requests', async({page, server}) => {
       await page.goto(server.EMPTY_PAGE);

test/network.spec.js

@@ -130,9 +130,9 @@ module.exports.describe = function({testRunner, expect, MAC, WIN, FFOX, CHROMIUM
     it('should throw when requesting body of redirected response', async({page, server}) => {
       server.setRedirect('/foo.html', '/empty.html');
       const response = await page.goto(server.PREFIX + '/foo.html');
-      const redirectChain = response.request().redirectChain();
-      expect(redirectChain.length).toBe(1);
-      const redirected = await redirectChain[0].response();
+      const redirectedFrom = response.request().redirectedFrom();
+      expect(redirectedFrom).toBeTruthy();
+      const redirected = await redirectedFrom.response();
       expect(redirected.status()).toBe(302);
       let error = null;
       await redirected.text().catch(e => error = e);
@@ -295,11 +295,10 @@ module.exports.describe = function({testRunner, expect, MAC, WIN, FFOX, CHROMIUM
         `200 ${server.EMPTY_PAGE}`,
         `DONE ${server.EMPTY_PAGE}`
       ]);
-
-      // Check redirect chain
-      const redirectChain = response.request().redirectChain();
-      expect(redirectChain.length).toBe(1);
-      expect(redirectChain[0].url()).toContain('/foo.html');
+      const redirectedFrom = response.request().redirectedFrom();
+      expect(redirectedFrom.url()).toContain('/foo.html');
+      expect(redirectedFrom.redirectedFrom()).toBe(null);
+      expect(redirectedFrom.redirectedTo()).toBe(response.request());
     });
   });