test: implement workarounds for current state of WebDriver BiDi (#11911)

Co-authored-by: jrandolf <101637635+jrandolf@users.noreply.github.com>

packages/puppeteer-core/src/bidi/Frame.ts

@@ -4,7 +4,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import type * as Bidi from 'chromium-bidi/lib/cjs/protocol/protocol.js';
+import * as Bidi from 'chromium-bidi/lib/cjs/protocol/protocol.js';
 
 import type {Observable} from '../../third_party/rxjs/rxjs.js';
 import {
@@ -265,7 +265,14 @@ export class BidiFrame extends Frame {
   ): Promise<BidiHTTPResponse | null> {
     const [response] = await Promise.all([
       this.waitForNavigation(options),
-      this.browsingContext.navigate(url),
+      this.browsingContext.navigate(
+        url,
+        // Some implementations currently only report errors when the
+        // readiness=interactive. This also ensures that old frames have been
+        // removed.
+        // Related: https://bugzilla.mozilla.org/show_bug.cgi?id=1846601
+        Bidi.BrowsingContext.ReadinessState.Interactive
+      ),
     ]).catch(
       rewriteNavigationError(
         url,

packages/puppeteer-core/src/bidi/core/Session.ts

@@ -105,6 +105,19 @@ export class Session
     browserEmitter.once('closed', ({reason}) => {
       this.dispose(reason);
     });
+
+    // TODO: Currently, some implementations do not emit navigationStarted event
+    // for fragment navigations (as per spec) and some do. This could emits a
+    // synthetic navigationStarted to work around this inconsistency.
+    const seen = new WeakSet();
+    this.on('browsingContext.fragmentNavigated', info => {
+      if (seen.has(info)) {
+        return;
+      }
+      seen.add(info);
+      this.emit('browsingContext.navigationStarted', info);
+      this.emit('browsingContext.fragmentNavigated', info);
+    });
   }
 
   // keep-sorted start block=yes

test/TestExpectations.json

@@ -822,13 +822,6 @@
     "parameters": ["webDriverBiDi"],
     "expectations": ["FAIL"]
   },
-  {
-    "testIdPattern": "[navigation.spec] navigation Page.goto should fail when navigating to bad SSL",
-    "platforms": ["darwin", "linux", "win32"],
-    "parameters": ["firefox"],
-    "expectations": ["SKIP"],
-    "comment": "https://github.com/w3c/webdriver-bidi/issues/657"
-  },
   {
     "testIdPattern": "[navigation.spec] navigation Page.goto should send referer",
     "platforms": ["darwin", "linux", "win32"],
@@ -2226,8 +2219,7 @@
     "testIdPattern": "[frame.spec] Frame specs Frame Management should detach child frames on navigation",
     "platforms": ["darwin", "linux", "win32"],
     "parameters": ["firefox", "webDriverBiDi"],
-    "expectations": ["FAIL"],
+    "expectations": ["PASS"]
-    "comment": "https://github.com/w3c/webdriver-bidi/issues/659"
   },
   {
     "testIdPattern": "[frame.spec] Frame specs Frame Management should report different frame instance when frame re-attaches",
@@ -2269,8 +2261,7 @@
     "testIdPattern": "[frame.spec] Frame specs Frame Management should send \"framenavigated\" when navigating on anchor URLs",
     "platforms": ["darwin", "linux", "win32"],
     "parameters": ["firefox", "webDriverBiDi"],
-    "expectations": ["SKIP"],
+    "expectations": ["PASS"]
-    "comment": "https://github.com/w3c/webdriver-bidi/issues/657"
   },
   {
     "testIdPattern": "[frame.spec] Frame specs Frame Management should send events when frames are manipulated dynamically",
@@ -2288,8 +2279,7 @@
     "testIdPattern": "[frame.spec] Frame specs Frame Management should support framesets",
     "platforms": ["darwin", "linux", "win32"],
     "parameters": ["firefox", "webDriverBiDi"],
-    "expectations": ["FAIL"],
+    "expectations": ["PASS"]
-    "comment": "https://github.com/w3c/webdriver-bidi/issues/659"
   },
   {
     "testIdPattern": "[frame.spec] Frame specs Frame Management should support lazy frames",
@@ -3100,29 +3090,37 @@
     "testIdPattern": "[navigation.spec] navigation Page.goBack should work with HistoryAPI",
     "platforms": ["darwin", "linux", "win32"],
     "parameters": ["firefox", "webDriverBiDi"],
-    "expectations": ["SKIP"],
+    "expectations": ["PASS"]
-    "comment": "browsingContext.navigationStarted event not emitted for fragment navigation"
   },
   {
     "testIdPattern": "[navigation.spec] navigation Page.goto should fail when main resources failed to load",
     "platforms": ["darwin", "linux", "win32"],
     "parameters": ["firefox", "webDriverBiDi"],
-    "expectations": ["SKIP"],
+    "expectations": ["PASS"]
-    "comment": "https://github.com/w3c/webdriver-bidi/issues/657"
   },
   {
     "testIdPattern": "[navigation.spec] navigation Page.goto should fail when navigating and show the url at the error message",
     "platforms": ["darwin", "linux", "win32"],
     "parameters": ["firefox", "webDriverBiDi"],
-    "expectations": ["SKIP"],
+    "expectations": ["PASS"]
-    "comment": "https://github.com/w3c/webdriver-bidi/issues/657"
+  },
+  {
+    "testIdPattern": "[navigation.spec] navigation Page.goto should fail when navigating to bad SSL",
+    "platforms": ["darwin", "linux", "win32"],
+    "parameters": ["firefox", "webDriverBiDi"],
+    "expectations": ["PASS"]
+  },
+  {
+    "testIdPattern": "[navigation.spec] navigation Page.goto should fail when navigating to bad SSL",
+    "platforms": ["darwin", "linux", "win32"],
+    "parameters": ["cdp", "firefox"],
+    "expectations": ["FAIL"]
   },
   {
     "testIdPattern": "[navigation.spec] navigation Page.goto should fail when navigating to bad SSL after redirects",
     "platforms": ["darwin", "linux", "win32"],
     "parameters": ["firefox", "webDriverBiDi"],
-    "expectations": ["SKIP"],
+    "expectations": ["PASS"]
-    "comment": "https://github.com/w3c/webdriver-bidi/issues/657"
   },
   {
     "testIdPattern": "[navigation.spec] navigation Page.goto should fail when server returns 204",
@@ -3134,8 +3132,7 @@
     "testIdPattern": "[navigation.spec] navigation Page.goto should fail when server returns 204",
     "platforms": ["darwin", "linux", "win32"],
     "parameters": ["firefox", "webDriverBiDi"],
-    "expectations": ["SKIP"],
+    "expectations": ["PASS"]
-    "comment": "https://github.com/w3c/webdriver-bidi/issues/657"
   },
   {
     "testIdPattern": "[navigation.spec] navigation Page.goto should navigate to dataURL and fire dataURL requests",
@@ -3233,6 +3230,13 @@
     "parameters": ["cdp", "firefox"],
     "expectations": ["FAIL"]
   },
+  {
+    "testIdPattern": "[navigation.spec] navigation Page.goto should work when page calls history API in beforeunload",
+    "platforms": ["darwin", "linux", "win32"],
+    "parameters": ["firefox", "webDriverBiDi"],
+    "expectations": ["FAIL"],
+    "comment": "History navigation is breaking the Puppeteer expecation about navigation."
+  },
   {
     "testIdPattern": "[navigation.spec] navigation Page.goto should work with anchor navigation",
     "platforms": ["darwin", "linux", "win32"],
@@ -3243,8 +3247,7 @@
     "testIdPattern": "[navigation.spec] navigation Page.goto should work with anchor navigation",
     "platforms": ["darwin", "linux", "win32"],
     "parameters": ["firefox", "webDriverBiDi"],
-    "expectations": ["SKIP"],
+    "expectations": ["PASS"]
-    "comment": "browsingContext.navigationStarted event not emitted for fragment navigation"
   },
   {
     "testIdPattern": "[navigation.spec] navigation Page.goto should work with subframes return 204",
@@ -3286,8 +3289,7 @@
     "testIdPattern": "[navigation.spec] navigation Page.waitForNavigation should work with DOM history.back()/history.forward()",
     "platforms": ["darwin", "linux", "win32"],
     "parameters": ["firefox", "webDriverBiDi"],
-    "expectations": ["SKIP"],
+    "expectations": ["PASS"]
-    "comment": "browsingContext.navigationStarted event not emitted for fragment navigation"
   },
   {
     "testIdPattern": "[navigation.spec] navigation Page.waitForNavigation should work with history.pushState()",
@@ -3299,8 +3301,7 @@
     "testIdPattern": "[navigation.spec] navigation Page.waitForNavigation should work with history.pushState()",
     "platforms": ["darwin", "linux", "win32"],
     "parameters": ["firefox", "webDriverBiDi"],
-    "expectations": ["SKIP"],
+    "expectations": ["PASS"]
-    "comment": "browsingContext.navigationStarted event not emitted for fragment navigation"
   },
   {
     "testIdPattern": "[navigation.spec] navigation Page.waitForNavigation should work with history.replaceState()",
@@ -3542,7 +3543,8 @@
     "testIdPattern": "[network.spec] network Response.fromCache should work",
     "platforms": ["darwin", "linux", "win32"],
     "parameters": ["firefox", "webDriverBiDi"],
-    "expectations": ["FAIL"]
+    "expectations": ["FAIL"],
+    "comment": "Needs investigation, it looks like a bug in Puppeteer"
   },
   {
     "testIdPattern": "[network.spec] network Response.fromCache should work",
@@ -3681,8 +3683,7 @@
     "testIdPattern": "[oopif.spec] OOPIF should support frames within OOP iframes",
     "platforms": ["darwin", "linux", "win32"],
     "parameters": ["firefox", "webDriverBiDi"],
-    "expectations": ["SKIP"],
+    "expectations": ["PASS"]
-    "comment": "Fetch error"
   },
   {
     "testIdPattern": "[oopif.spec] OOPIF should support lazy OOP frames",
@@ -3750,7 +3751,7 @@
     "platforms": ["darwin", "linux", "win32"],
     "parameters": ["firefox", "webDriverBiDi"],
     "expectations": ["SKIP"],
-    "comment": "Fetch error"
+    "comment": "Chrome-specific test (uses DNS mapping); does not work with Firefox."
   },
   {
     "testIdPattern": "[page.spec] Page Page.addScriptTag should throw when added with content to the CSP page",

test/src/navigation.spec.ts

@@ -154,10 +154,10 @@ describe('navigation', function () {
     });
 
     const EXPECTED_SSL_CERT_MESSAGE_REGEX =
-      /net::ERR_CERT_INVALID|net::ERR_CERT_AUTHORITY_INVALID/;
+      /net::ERR_CERT_INVALID|net::ERR_CERT_AUTHORITY_INVALID|MOZILLA_PKIX_ERROR_SELF_SIGNED_CERT|SSL_ERROR_UNKNOWN/;
 
     it('should fail when navigating to bad SSL', async () => {
-      const {page, httpsServer, isChrome} = await getTestState();
+      const {page, httpsServer} = await getTestState();
 
       // Make sure that network events do not emit 'undefined'.
       // @see https://crbug.com/750469
@@ -176,18 +176,14 @@ describe('navigation', function () {
       await page.goto(httpsServer.EMPTY_PAGE).catch(error_ => {
         return (error = error_);
       });
-      if (isChrome) {
       expect(error.message).toMatch(EXPECTED_SSL_CERT_MESSAGE_REGEX);
-      } else {
-        expect(error.message).toContain('SSL_ERROR_UNKNOWN');
-      }
 
       expect(requests).toHaveLength(2);
       expect(requests[0]).toBe('request');
       expect(requests[1]).toBe('requestfailed');
     });
     it('should fail when navigating to bad SSL after redirects', async () => {
-      const {page, server, httpsServer, isChrome} = await getTestState();
+      const {page, server, httpsServer} = await getTestState();
 
       server.setRedirect('/redirect/1.html', '/redirect/2.html');
       server.setRedirect('/redirect/2.html', '/empty.html');
@@ -195,17 +191,10 @@ describe('navigation', function () {
       await page.goto(httpsServer.PREFIX + '/redirect/1.html').catch(error_ => {
         return (error = error_);
       });
-      if (isChrome) {
       expect(error.message).toMatch(EXPECTED_SSL_CERT_MESSAGE_REGEX);
-      } else {
-        expect(error.message).atLeastOneToContain([
-          'MOZILLA_PKIX_ERROR_SELF_SIGNED_CERT', // Firefox WebDriver BiDi.
-          'SSL_ERROR_UNKNOWN ', // Others.
-        ]);
-      }
     });
     it('should fail when main resources failed to load', async () => {
-      const {page, isChrome} = await getTestState();
+      const {page} = await getTestState();
 
       let error!: Error;
       await page
@@ -213,11 +202,9 @@ describe('navigation', function () {
         .catch(error_ => {
           return (error = error_);
         });
-      if (isChrome) {
+      expect(error.message).toMatch(
-        expect(error.message).toContain('net::ERR_CONNECTION_REFUSED');
+        /net::ERR_CONNECTION_REFUSED|NS_ERROR_CONNECTION_REFUSED/
-      } else {
+      );
-        expect(error.message).toContain('NS_ERROR_CONNECTION_REFUSED');
-      }
     });
     it('should fail when exceeding maximum navigation timeout', async () => {
       const {page, server} = await getTestState();

tools/sort-test-expectations.mjs

@@ -43,6 +43,10 @@ testExpectations.forEach(item => {
   item.parameters.sort();
   item.expectations.sort();
   item.platforms.sort();
+  // Delete comments for PASS expectations. They are likely outdated.
+  if (item.expectations.length === 1 && item.expectations[0] === 'PASS') {
+    delete item.comment;
+  }
 });
 
 if (process.argv.includes('--lint')) {