test: Wait for a page load in Browser.open()

Commit 8284618 dropped all explicit "wait for page load" calls (fka.
`expect_load()`) in favor of allowing page loads during wait commands.
This is better in almost all cases, but it created a race condition on
the login page in tests which logged out and back in. In particular,
this can happen:

 1. Test calls `Browser.logout()`.

 2. That destroys a lot of execution contexts from the session, and
    creates a new one for the login page. It waits until the `#login`
    field becomes visible.

 3. Test starts a new session by calling `Browser.open()`  usually via
    `Browser.login_and_go()`. This results in a `Page.navigate()` CDP
    call. Sometimes this takes a while.

 4. `Browser.try_login()` fills in the login form. In *most cases*, the
    `wait: ph_is_present("#login")` command runs into the page load from
    3., sees the destroyed/new execution context, and resumes on the new
    frame.

    However, if cockpit/ws or the browser take a while to load the login
    page, it could happen that it gets all the way to e.g. filling in
    the user or even password form before the page load from 3. happens.
    That page load resets the form.

 5. `try_login()` clicks on "Login" button, which fails because no user
    or password is given.

Fix this by always waiting for a page load in `Browser.open()` after a
`Page.navigate()`. But we don't need the unnecessary complex and brittle
polling machinery of the old `expectLoad()` before commit 8284618. We
can use the `loadEventFired` CDP signal, which happens *after* the page
and all of its frames finished loading, and execution contexts are set
up. Introduce a new `waitPageLoad()` helper which re-uses the existing
`pageLoadHandler` promise.

test/common/chromium-cdp-driver.js

@@ -199,8 +199,10 @@ function setupFrameTracking(client) {
     });
 
     client.Page.loadEventFired(() => {
-        if (pageLoadHandler)
+        if (pageLoadHandler) {
+            debug("loadEventFired, resolving pageLoadHandler");
             pageLoadHandler();
+        }
     });
 
     // track execution contexts so that we can map between context and frame IDs
@@ -221,12 +223,22 @@ function setupFrameTracking(client) {
 }
 
 function setupLocalFunctions(client) {
-    client.reloadPageAndWait = (args) => {
-        return new Promise((resolve, reject) => {
-            pageLoadHandler = () => { pageLoadHandler = null; resolve({}) };
-            client.Page.reload(args);
-        });
-    };
+    client.waitPageLoad = (args) => new Promise((resolve, reject) => {
+        const timeout = setTimeout(() => {
+            pageLoadHandler = null;
+            reject("Timeout waiting for page load"); // eslint-disable-line prefer-promise-reject-errors
+        }, 15000);
+        pageLoadHandler = () => {
+            clearTimeout(timeout);
+            pageLoadHandler = null;
+            resolve({});
+        };
+    });
+
+    client.reloadPageAndWait = (args) => new Promise((resolve, reject) => {
+        pageLoadHandler = () => { pageLoadHandler = null; resolve({}) };
+        client.Page.reload(args);
+    });
 
     async function setCSS({ text, frame }) {
         await client.DOM.enable();

test/common/firefox-cdp-driver.js

@@ -223,8 +223,10 @@ function setupFrameTracking(client) {
     });
 
     client.Page.loadEventFired(() => {
-        if (pageLoadHandler)
+        if (pageLoadHandler) {
+            debug("loadEventFired, resolving pageLoadHandler");
             pageLoadHandler();
+        }
     });
 
     // track execution contexts so that we can map between context and frame IDs
@@ -275,12 +277,22 @@ function setupFrameTracking(client) {
 }
 
 function setupLocalFunctions(client) {
-    client.reloadPageAndWait = (args) => {
-        return new Promise((resolve, reject) => {
-            pageLoadHandler = () => { pageLoadHandler = null; resolve({}) };
-            client.Page.reload(args);
-        });
-    };
+    client.waitPageLoad = (args) => new Promise((resolve, reject) => {
+        const timeout = setTimeout(() => {
+            pageLoadHandler = null;
+            reject("Timeout waiting for page load"); // eslint-disable-line prefer-promise-reject-errors
+        }, 15000);
+        pageLoadHandler = () => {
+            clearTimeout(timeout);
+            pageLoadHandler = null;
+            resolve({});
+        };
+    });
+
+    client.reloadPageAndWait = (args) => new Promise((resolve, reject) => {
+        pageLoadHandler = () => { pageLoadHandler = null; resolve({}) };
+        client.Page.reload(args);
+    });
 }
 
 // helper functions for testlib.py which are too unwieldy to be poked in from Python

test/common/testlib.py

@@ -234,7 +234,15 @@ def open(self, href: str, cookie: Optional[Dict[str, str]] = None, tls: bool = F
             self.cdp.invoke("Network.setCookie", **cookie)
 
         self.switch_to_top()
-        self.cdp.invoke("Page.navigate", url=href)
+        opts = {}
+        if self.cdp.browser.name == "firefox":
+            # by default, Firefox optimizes this away if the current and the given href URL
+            # are the same (Like in TestKeys.testAuthorizedKeys).
+            # Force a reload in this case, to make tests and the waitPageLoad below predictable
+            # But that option has the inverse effect with Chromium (argh)
+            opts["transitionType"] = "reload"
+        self.cdp.invoke("Page.navigate", url=href, **opts)
+        self.cdp.invoke("waitPageLoad")
 
     def set_user_agent(self, ua: str):
         """Set the user agent of the browser