The marionette navigate module exposes several useful APIs for navigation, but they are coupled to the marionette driver.

The goal of this bug is to extract most of the logic of the waitForNavigationCompleted helper (searchfox) from navigate.js and move it to the shared Navigate.jsm module.

The features of the helper should remain similar:

• it should accept a callbackFn which triggers the navigation
• it should support the various webdriver PageLoad strategies: none, eager and normal (searchfox)

The main task here is to remove any usage of the marionette driver, so that the helper can be used from BiDi (and potentially CDP).

In remote/marionette/navigate.js, we might keep a waitForNavigationCompleted wrapper which will have to translate the usual arguments used for marionette's helper to arguments compatible with the new version in remote/shared/Navigate.jsm. Or we can update existing call sites, depending on how verbose this will be.

In terms of tests, we can probably still rely on the existing coverage from marionette tests.

This bug was initially spawned from Bug 1664165 which aimed both at moving the helper to shared/Navigate.jsm and migrate it to use webprogresslistener. It's not clear if webprogresslistener can be used at the moment for the eager PageLoadStrategy so we are splitting out this refactor to unblock other bugs in the milestone.

(In reply to Julian Descottes [:jdescottes] from comment #0)

The features of the helper should remain similar:

• it should accept a callbackFn which triggers the navigation

If doable I would prefer to return a Promise directly that could be waited for after running any arbitrary code. That way we could get around the usage of a callback.

• it should support the various webdriver PageLoad strategies: none, eager and normal (searchfox)

The main task here is to remove any usage of the marionette driver, so that the helper can be used from BiDi (and potentially CDP).

CDP has actually some more logic baked in around Network events. It's something you might also wanna have a look into. I stumbled over some parts yesterday.

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #1)

(In reply to Julian Descottes [:jdescottes] from comment #0)

The features of the helper should remain similar:

• it should accept a callbackFn which triggers the navigation

If doable I would prefer to return a Promise directly that could be waited for after running any arbitrary code. That way we could get around the usage of a callback.

Thanks, that sounds fine and a bit less outdated in terms of pattern :)

• it should support the various webdriver PageLoad strategies: none, eager and normal (searchfox)

The main task here is to remove any usage of the marionette driver, so that the helper can be used from BiDi (and potentially CDP).

CDP has actually some more logic baked in around Network events. It's something you might also wanna have a look into. I stumbled over some parts yesterday.

I'll take a look but my main goal with this bug is to unblock the rest of the work, so I will avoid any scope creep.

As discussed with :whimboo, we will not attempt to share the current helper for Marionette with the other protocols.
The main reason is that we can't fully implement this without using page load events, and to do that we need content -> parent process communication, which is precisely something which is done very differently between CDP/Marionette/BiDi/DevTools etc...
We could have added yet another communication mechanism, used only by this helper, but in the end we prefer for now to focus on creating a helper for BiDi using both webprogress and page load events.

In the future, we might still be able to share the navigation logic which is purely related to webprogress listener (and can fully reside in the parent process) and only expect each protocol to provide page load events. This would happen over in Bug 1664165.