feat(network): ignore favicon requests - these are too unpredictable (#533)

docs/api.md

@@ -3565,6 +3565,8 @@ Browser websocket endpoint which can be used as an argument to [firefoxPlaywrigh
 
 #### webkitPlaywright.defaultArgs([options])
 - `options` <[Object]>  Set of configurable options to set on the browser. Can have the following fields:
+  - `headless` <[boolean]> Whether to run WebKit in headless mode. Defaults to `true`.
+  - `userDataDir` <[string]> Path to a User Data Directory, which stores browser session data like cookies and local storage.
   - `args` <[Array]<[string]>> Additional arguments to pass to the browser instance.
 - returns: <[Array]<[string]>>
 
@@ -3575,6 +3577,7 @@ The default flags that WebKit will be launched with.
   - `headless` <[boolean]> Whether to run WebKit in headless mode. Defaults to `true`.
   - `executablePath` <[string]> Path to a WebKit executable to run instead of the bundled WebKit. If `executablePath` is a relative path, then it is resolved relative to [current working directory](https://nodejs.org/api/process.html#process_process_cwd). **BEWARE**: Playwright is only guaranteed to work with the bundled WebKit, use at your own risk.
   - `slowMo` <[number]> Slows down Playwright operations by the specified amount of milliseconds. Useful so that you can see what is going on.
+  - `userDataDir` <[string]> Path to a User Data Directory, which stores browser session data like cookies and local storage.
   - `args` <[Array]<[string]>> Additional arguments to pass to the browser instance.
   - `ignoreDefaultArgs` <[boolean]|[Array]<[string]>> If `true`, then do not use [`webKitPlaywright.defaultArgs()`](#webkitplaywrightdefaultargsoptions). If an array is given, then filter out the given default arguments. Dangerous option; use with care. Defaults to `false`.
   - `handleSIGINT` <[boolean]> Close the browser process on Ctrl-C. Defaults to `true`.
@@ -3598,6 +3601,7 @@ const browser = await playwright.launch({
   - `headless` <[boolean]> Whether to run WebKit in headless mode. Defaults to `true`.
   - `executablePath` <[string]> Path to a WebKit executable to run instead of the bundled WebKit. If `executablePath` is a relative path, then it is resolved relative to [current working directory](https://nodejs.org/api/process.html#process_process_cwd). **BEWARE**: Playwright is only guaranteed to work with the bundled WebKit, use at your own risk.
   - `slowMo` <[number]> Slows down Playwright operations by the specified amount of milliseconds. Useful so that you can see what is going on.
+  - `userDataDir` <[string]> Path to a User Data Directory, which stores browser session data like cookies and local storage.
   - `args` <[Array]<[string]>> Additional arguments to pass to the browser instance.
   - `ignoreDefaultArgs` <[boolean]|[Array]<[string]>> If `true`, then do not use [`webKitPlaywright.defaultArgs()`](#webkitplaywrightdefaultargsoptions). If an array is given, then filter out the given default arguments. Dangerous option; use with care. Defaults to `false`.
   - `handleSIGINT` <[boolean]> Close the browser process on Ctrl-C. Defaults to `true`.

src/frames.ts

@@ -191,16 +191,19 @@ export class FrameManager {
       for (const watcher of this._lifecycleWatchers)
         watcher._onNavigationRequest(frame, request);
     }
-    this._page.emit(Events.Page.Request, request);
+    if (!request._isFavicon)
+      this._page.emit(Events.Page.Request, request);
   }
 
   requestReceivedResponse(response: network.Response) {
-    this._page.emit(Events.Page.Response, response);
+    if (!response.request()._isFavicon)
+      this._page.emit(Events.Page.Response, response);
   }
 
   requestFinished(request: network.Request) {
     this._inflightRequestFinished(request);
-    this._page.emit(Events.Page.RequestFinished, request);
+    if (!request._isFavicon)
+      this._page.emit(Events.Page.RequestFinished, request);
   }
 
   requestFailed(request: network.Request, canceled: boolean) {
@@ -216,7 +219,8 @@ export class FrameManager {
           watcher._onAbortedNewDocumentNavigation(frame, request._documentId, errorText);
       }
     }
-    this._page.emit(Events.Page.RequestFailed, request);
+    if (!request._isFavicon)
+      this._page.emit(Events.Page.RequestFailed, request);
   }
 
   provisionalLoadFailed(documentId: string, error: string) {
@@ -236,7 +240,7 @@ export class FrameManager {
 
   private _inflightRequestFinished(request: network.Request) {
     const frame = request.frame();
-    if (!frame || request.url().endsWith('favicon.ico'))
+    if (!frame || request._isFavicon)
       return;
     if (!frame._inflightRequests.has(request))
       return;
@@ -249,7 +253,7 @@ export class FrameManager {
 
   private _inflightRequestStarted(request: network.Request) {
     const frame = request.frame();
-    if (!frame || request.url().endsWith('favicon.ico'))
+    if (!frame || request._isFavicon)
       return;
     frame._inflightRequests.add(request);
     if (frame._inflightRequests.size === 1)

src/network.ts

@@ -97,6 +97,7 @@ export class Request {
   _redirectChain: Request[];
   _finalRequest: Request;
   readonly _documentId?: string;
+  readonly _isFavicon: boolean;
   private _failureText: string | null = null;
   private _url: string;
   private _resourceType: string;
@@ -127,6 +128,7 @@ export class Request {
     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) {

test/interception.spec.js

@@ -28,10 +28,6 @@ module.exports.describe = function({testRunner, expect, defaultBrowserOptions, p
     it('should intercept', async({page, server}) => {
       await page.setRequestInterception(true);
       page.on('request', request => {
-        if (utils.isFavicon(request)) {
-          request.continue();
-          return;
-        }
         expect(request.url()).toContain('empty.html');
         expect(request.headers()['user-agent']).toBeTruthy();
         expect(request.method()).toBe('GET');
@@ -94,8 +90,7 @@ module.exports.describe = function({testRunner, expect, defaultBrowserOptions, p
       await page.setRequestInterception(true);
       const requests = [];
       page.on('request', request => {
-        if (!utils.isFavicon(request))
+        requests.push(request);
-          requests.push(request);
         request.continue();
       });
       await page.goto(server.PREFIX + '/one-style.html');
@@ -217,8 +212,7 @@ module.exports.describe = function({testRunner, expect, defaultBrowserOptions, p
       const requests = [];
       page.on('request', request => {
         request.continue();
-        if (!utils.isFavicon(request))
+        requests.push(request);
-          requests.push(request);
       });
       server.setRedirect('/non-existing-page.html', '/non-existing-page-2.html');
       server.setRedirect('/non-existing-page-2.html', '/non-existing-page-3.html');
@@ -245,8 +239,7 @@ module.exports.describe = function({testRunner, expect, defaultBrowserOptions, p
       const requests = [];
       page.on('request', request => {
         request.continue();
-        if (!utils.isFavicon(request))
+        requests.push(request);
-          requests.push(request);
       });
       server.setRedirect('/one-style.css', '/two-style.css');
       server.setRedirect('/two-style.css', '/three-style.css');
@@ -274,10 +267,6 @@ module.exports.describe = function({testRunner, expect, defaultBrowserOptions, p
       let spinner = false;
       // Cancel 2nd request.
       page.on('request', request => {
-        if (utils.isFavicon(request)) {
-          request.continue();
-          return;
-        }
         spinner ? request.abort() : request.continue();
         spinner = !spinner;
       });
@@ -292,8 +281,7 @@ module.exports.describe = function({testRunner, expect, defaultBrowserOptions, p
       await page.setRequestInterception(true);
       const requests = [];
       page.on('request', request => {
-        if (!utils.isFavicon(request))
+        requests.push(request);
-          requests.push(request);
         request.continue();
       });
       const dataURL = 'data:text/html,<div>yo</div>';
@@ -306,8 +294,7 @@ module.exports.describe = function({testRunner, expect, defaultBrowserOptions, p
       await page.setRequestInterception(true);
       const requests = [];
       page.on('request', request => {
-        if (!utils.isFavicon(request))
+        requests.push(request);
-          requests.push(request);
         request.continue();
       });
       const dataURL = 'data:text/html,<div>yo</div>';
@@ -319,8 +306,7 @@ module.exports.describe = function({testRunner, expect, defaultBrowserOptions, p
       await page.setRequestInterception(true);
       const requests = [];
       page.on('request', request => {
-        if (!utils.isFavicon(request))
+        requests.push(request);
-          requests.push(request);
         request.continue();
       });
       const response = await page.goto(server.EMPTY_PAGE + '#hash');
@@ -351,8 +337,7 @@ module.exports.describe = function({testRunner, expect, defaultBrowserOptions, p
       const requests = [];
       page.on('request', request => {
         request.continue();
-        if (!utils.isFavicon(request))
+        requests.push(request);
-          requests.push(request);
       });
       const response = await page.goto(`data:text/html,<link rel="stylesheet" href="${server.PREFIX}/fonts?helvetica|arial"/>`);
       expect(response).toBe(null);

test/navigation.spec.js

@@ -280,7 +280,7 @@ module.exports.describe = function({testRunner, expect, playwright, FFOX, CHROMI
     });
     it('should navigate to dataURL and not fire dataURL requests', async({page, server}) => {
       const requests = [];
-      page.on('request', request => !utils.isFavicon(request) && requests.push(request));
+      page.on('request', request => requests.push(request));
       const dataURL = 'data:text/html,<div>yo</div>';
       const response = await page.goto(dataURL);
       expect(response).toBe(null);
@@ -288,7 +288,7 @@ module.exports.describe = function({testRunner, expect, playwright, FFOX, CHROMI
     });
     it('should navigate to URL with hash and fire requests without hash', async({page, server}) => {
       const requests = [];
-      page.on('request', request => !utils.isFavicon(request) && requests.push(request));
+      page.on('request', request => requests.push(request));
       const response = await page.goto(server.EMPTY_PAGE + '#hash');
       expect(response.status()).toBe(200);
       expect(response.url()).toBe(server.EMPTY_PAGE);

test/network.spec.js

@@ -27,20 +27,20 @@ module.exports.describe = function({testRunner, expect, FFOX, CHROMIUM, WEBKIT})
   describe('Page.Events.Request', function() {
     it('should fire for navigation requests', async({page, server}) => {
       const requests = [];
-      page.on('request', request => !utils.isFavicon(request) && requests.push(request));
+      page.on('request', request => requests.push(request));
       await page.goto(server.EMPTY_PAGE);
       expect(requests.length).toBe(1);
     });
     it('should fire for iframes', async({page, server}) => {
       const requests = [];
-      page.on('request', request => !utils.isFavicon(request) && requests.push(request));
+      page.on('request', request => requests.push(request));
       await page.goto(server.EMPTY_PAGE);
       await utils.attachFrame(page, 'frame1', server.EMPTY_PAGE);
       expect(requests.length).toBe(2);
     });
     it('should fire for fetches', async({page, server}) => {
       const requests = [];
-      page.on('request', request => !utils.isFavicon(request) && requests.push(request));
+      page.on('request', request => requests.push(request));
       await page.goto(server.EMPTY_PAGE);
       await page.evaluate(() => fetch('/empty.html'));
       expect(requests.length).toBe(2);
@@ -50,7 +50,7 @@ module.exports.describe = function({testRunner, expect, FFOX, CHROMIUM, WEBKIT})
   describe('Request.frame', function() {
     it('should work for main frame navigation request', async({page, server}) => {
       const requests = [];
-      page.on('request', request => !utils.isFavicon(request) && requests.push(request));
+      page.on('request', request => requests.push(request));
       await page.goto(server.EMPTY_PAGE);
       expect(requests.length).toBe(1);
       expect(requests[0].frame()).toBe(page.mainFrame());
@@ -58,7 +58,7 @@ module.exports.describe = function({testRunner, expect, FFOX, CHROMIUM, WEBKIT})
     it('should work for subframe navigation request', async({page, server}) => {
       await page.goto(server.EMPTY_PAGE);
       const requests = [];
-      page.on('request', request => !utils.isFavicon(request) && requests.push(request));
+      page.on('request', request => requests.push(request));
       await utils.attachFrame(page, 'frame1', server.EMPTY_PAGE);
       expect(requests.length).toBe(1);
       expect(requests[0].frame()).toBe(page.frames()[1]);
@@ -66,7 +66,7 @@ module.exports.describe = function({testRunner, expect, FFOX, CHROMIUM, WEBKIT})
     it('should work for fetch requests', async({page, server}) => {
       await page.goto(server.EMPTY_PAGE);
       let requests = [];
-      page.on('request', request => !utils.isFavicon(request) && requests.push(request));
+      page.on('request', request => requests.push(request));
       await page.evaluate(() => fetch('/digits/1.png'));
       requests = requests.filter(request => !request.url().includes('favicon'));
       expect(requests.length).toBe(1);
@@ -151,7 +151,7 @@ module.exports.describe = function({testRunner, expect, FFOX, CHROMIUM, WEBKIT})
       page.on('requestfinished', r => requestFinished = requestFinished || r.url().includes('/get'));
       // send request and wait for server response
       const [pageResponse] = await Promise.all([
-        page.waitForEvent('response', { predicate: r => !utils.isFavicon(r.request()) }),
+        page.waitForEvent('response'),
         page.evaluate(() => fetch('./get', { method: 'GET'})),
         server.waitForRequest('/get'),
       ]);
@@ -207,7 +207,7 @@ module.exports.describe = function({testRunner, expect, FFOX, CHROMIUM, WEBKIT})
   describe('Network Events', function() {
     it('Page.Events.Request', async({page, server}) => {
       const requests = [];
-      page.on('request', request => !utils.isFavicon(request) && requests.push(request));
+      page.on('request', request => requests.push(request));
       await page.goto(server.EMPTY_PAGE);
       expect(requests.length).toBe(1);
       expect(requests[0].url()).toBe(server.EMPTY_PAGE);
@@ -220,7 +220,7 @@ module.exports.describe = function({testRunner, expect, FFOX, CHROMIUM, WEBKIT})
     // FIXME: WebKit doesn't provide remoteIPAddress in the response.
     it.skip(WEBKIT)('Page.Events.Response', async({page, server}) => {
       const responses = [];
-      page.on('response', response => !utils.isFavicon(response.request()) && responses.push(response));
+      page.on('response', response => responses.push(response));
       await page.goto(server.EMPTY_PAGE);
       expect(responses.length).toBe(1);
       expect(responses[0].url()).toBe(server.EMPTY_PAGE);
@@ -261,7 +261,7 @@ module.exports.describe = function({testRunner, expect, FFOX, CHROMIUM, WEBKIT})
     });
     it('Page.Events.RequestFinished', async({page, server}) => {
       const requests = [];
-      page.on('requestfinished', request => !utils.isFavicon(request) && requests.push(request));
+      page.on('requestfinished', request => requests.push(request));
       await page.goto(server.EMPTY_PAGE);
       expect(requests.length).toBe(1);
       expect(requests[0].url()).toBe(server.EMPTY_PAGE);
@@ -271,18 +271,18 @@ module.exports.describe = function({testRunner, expect, FFOX, CHROMIUM, WEBKIT})
     });
     it('should fire events in proper order', async({page, server}) => {
       const events = [];
-      page.on('request', request => !utils.isFavicon(request) && events.push('request'));
+      page.on('request', request => events.push('request'));
-      page.on('response', response => !utils.isFavicon(response.request()) && events.push('response'));
+      page.on('response', response => events.push('response'));
-      page.on('requestfinished', request => !utils.isFavicon(request) && events.push('requestfinished'));
+      page.on('requestfinished', request => events.push('requestfinished'));
       await page.goto(server.EMPTY_PAGE);
       expect(events).toEqual(['request', 'response', 'requestfinished']);
     });
     it('should support redirects', async({page, server}) => {
       const events = [];
-      page.on('request', request => !utils.isFavicon(request) && events.push(`${request.method()} ${request.url()}`));
+      page.on('request', request => events.push(`${request.method()} ${request.url()}`));
-      page.on('response', response => !utils.isFavicon(response.request()) && events.push(`${response.status()} ${response.url()}`));
+      page.on('response', response => events.push(`${response.status()} ${response.url()}`));
-      page.on('requestfinished', request => !utils.isFavicon(request) && events.push(`DONE ${request.url()}`));
+      page.on('requestfinished', request => events.push(`DONE ${request.url()}`));
-      page.on('requestfailed', request => !utils.isFavicon(request) && events.push(`FAIL ${request.url()}`));
+      page.on('requestfailed', request => events.push(`FAIL ${request.url()}`));
       server.setRedirect('/foo.html', '/empty.html');
       const FOO_URL = server.PREFIX + '/foo.html';
       const response = await page.goto(FOO_URL);

test/utils.js

@@ -122,19 +122,11 @@ const utils = module.exports = {
       frame.src = url;
       frame.id = frameId;
       document.body.appendChild(frame);
-      // TODO(einbinder) do this right
-      // Access a scriptable global object to ensure JS context is
-      // initialized. WebKit will create it lazily only as need be.
-      frame.contentWindow;
       await new Promise(x => frame.onload = x);
       return frame;
     }
   },
 
-  isFavicon: function(request) {
-    return request.url().includes('favicon.ico');
-  },
-
   /**
    * @param {!Page} page
    * @param {string} frameId