1632141 - [meta] Figure out a replacement of "navigate" in a Fission world.

Status: NEW

(DevTools :: Framework, enhancement)

Description

[Alexandre Poirot [:ochameau]]

There is an issue regarding navigate event, which may be sent in between onTargetAvailable and when we call target.on("navigate", ...). When this happen, you miss the event and never get a chance to know when the document is ready.

At the end, this issue is the same that ResourceWatcher and TargetList APIs are trying to mitigate. It is about migrating:

• from EventEmitter API, which only allows you to listen to "future" events
• to "watch" API, which allows you to listen to "already available" and "future" events/resources.

I'm wondering if we should:

1. Tweak this target front code in order to stop listening to tabNavigated (FYI, it is only emitted for the top level document. See this) And instead:
   * emit will-navigate from onTargetAvailable when isTopLevel is true. Or when we receive DocumentEvent's "dom-loading" if that's any better. (But I doubt so)
   * emit navigate when we receive DocumentEvent's "dom-complete".

Note that this may be complex as the ResourceWatcher API isn't designed to be called from more than one callsite about the same resource type. It will not emit the "already existing" resources when you call it the second and following times. This may be something we will have to fix in ResourceWatcher anyway.
And this would probably not address the callsites of target.on("navigate") which will still not receive the event because EventEmitter isn't a "watch-like API".

2. Or, should we try to get rid of all usages of will-navigate and navigate everywhere?
   * will-navigate should be replaced by onTargetAvailable when isTopLevel is true.
   * navigate should be replaced with:
   • DocumentEvents "dom-complete" listening? May be that's too complex to do everywhere?
   • Should we introduce a onTargetLoaded? It may not work well for all target types? Like workers and processes.
   • Something else??

One important thing to note is that someday in future, Targets will be instanciate for each WindowGlobal, instead of once per DocShell. It means that each time we will navigate, we will receive a new Target. So, for each Target, we may receive only one will-navigate (which, in this case is really equivalent to onTargetAvailable), and only one navigate (which has no equivalent yet in fission world).

Comment 1

[Alexandre Poirot [:ochameau]]

FYI, daisuke is trying to address the netmonitor in this patch.

Comment 2

[Julian Descottes [:jdescottes]]

Thanks for filing and adding proposals already! In principle I like option 2 better.

will-navigate should be replaced by onTargetAvailable when isTopLevel is true.

I thought will-navigate would rather be mapped to onTargetDestroyed?

navigate should be replaced with: ...

It would be nice to review the call sites for "navigate" (https://searchfox.org/mozilla-central/search?q=on%28%22navigate&path=devtools might be missing some) and see what they expect. For some of them, it might be fine to just replace them by onTargetAvailable?

Comment 3

[Alexandre Poirot [:ochameau]]

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

will-navigate should be replaced by onTargetAvailable when isTopLevel is true.

I thought will-navigate would rather be mapped to onTargetDestroyed?

1. Depending on how to support BFCache in detail, onTargetDestroyed may not have the behavior you are expecting here. A target may not be destroyed if we navigate and the previous page is frozen and saved into the BFCache. But it may be simplier to always destroy targets to avoid BFCache dedicated codepath? I imagine it would be important to figure this out during the course of this bug as it will have an impact on there two events replacements.
2. I would rather consider onTargetAvailable in favor of onTargetDestroyed, as onTargetAvailable may provide the new url while destroyed probably won't.
3. Another decision criteria would be: what is the first event to be fired? May be we should pick the first one? I wouldn't be surprise if the next target is created before the previous is notified to be destroyed.
4. May be each panel has different expectations. May be some rather care about cleaning up previous target resources, while others care more about the next document?

Comment 4

[Alexandre Poirot [:ochameau]]

https://phabricator.services.mozilla.com/D71881#inline-422613

@Daisuke Let me try to use DocumentEvent's dom-loading instead of will-navitate here!

And, regarding applying to all panes, as it seems that both events of dom-complete and navigate will be fired responding to DOMContentLoaded > event, I think we can replace.

However, we might need to think to extend ResourceWatcher API about such the following usage.
await this.currentTarget.once("navigate");
https://searchfox.org/mozilla-central/rev/41c3ea3ee8eab9ce7b82932257cb80b703cbba67/devtools/client/netmonitor/src/connector/firefox-connector.js#364
If we apply current ResourceWatcher as is to this usage, it seems that it will be hard and complex a bit..

Yes, this is where EventEmitter is a problem. As said in comment 0 last paragraph, if we assume the long term behavior of having a Target per WindowGlobal, we may have a navigated promise, which will be resolved once the target's document is done loading. I believe that it would be the simpliest API to have.
But... in the meantime, we may have to have a "watch like API", which may fire a callback:

• immediately if already loaded,
• or later if it isn't yet loaded,
• or even later, for the next WindowGlobal

I'm trying to use DocumentEvent's dom-loading instead of will-navigate, but it seems we can not replace it simply. Many tests are failed now. As it looks the time gap between dom-loading and dom-interactive is too short compared with using will-navigate event, might need to change something. I'll investigate.

Yes, I was expecting issues about that. I believe we should use onTargetAvailable with isTopLevel=true instead of will-navigate. Or onTargetDestroyed per Julian suggestion in comment 2 and 3.

Comment 5

[Chris Peterson [:cpeterson]]

Tracking Fission DevTools bugs for Fission Nightly (M6) milestone

Comment 6

[Alexandre Poirot [:ochameau]]

Here is another suggestion from Daisuke:
https://phabricator.services.mozilla.com/D72126
Using DocumentEvents "dom-loading" as an equivalent to will-navigate and "dom-complete" as navigate, and, while doing so, replace the onTargetAvailable call, by the first call to onResourceAvailable for the "dom-loading" event.

I'm not convinced with this patch for the following reasons:

• It would be better to avoid caching the target (FirefoxConnector._currentTarget), and instead always refer to targetList.targetFront. This one is always correct and up-to-date.
• I think it is less error prone to keep using onTargetAvailable instead of somewhat hiding the discovery of a new target in a random call to onResourceAvailable. We have no guarantee that it will always be changing the target for the DOCUMENT_EVENTS resource, if we are listening to many resource types.

Comment 7

[Julian Descottes [:jdescottes]]

Testing today with fission enabled, I still receive the "will-navigate" event from the BrowsingContextTargetFront (which received the TabNavigated event from the actor). Might not be something we can rely on but also might be worth checking with platform.

Comment 8

[Alexandre Poirot [:ochameau]]

We should get to a conclusion here sooner than later before introducing too many unexpected usages of DOCUMENT_EVENTS.

Comment 9

[Alexandre Poirot [:ochameau]]

Let me try to reply to myself about my very first comment. A few suggestions were wrong/misleading:

(In reply to Alexandre Poirot [:ochameau] from comment #0)

1. Tweak this target front code in order to stop listening to tabNavigated (FYI, it is only emitted for the top level document. See this) And instead:
   * emit will-navigate from onTargetAvailable when isTopLevel is true. Or when we receive DocumentEvent's "dom-loading" if that's any better. (But I doubt so)
   * emit navigate when we receive DocumentEvent's "dom-complete".

This makes no sense because of target switching. We can't rely on any API put on target, because the target may be destroyed in case of navigation to another origin/process.

And this would probably not address the callsites of target.on("navigate") which will still not receive the event because EventEmitter isn't a "watch-like API".

EventEmitter API can really be an issue in some cases. But I'm not sure we get into this (yet?).

2. Or, should we try to get rid of all usages of will-navigate and navigate everywhere?
   * will-navigate should be replaced by onTargetAvailable when isTopLevel is true.

This suggestion is wrong. onTargetAvailable is only called when we navigate to a new process/origin.
So that it won't work for reload and navigation to the same origin.
BUT we know that's what we will do in future, so we should tend toward something close to that.
We will use this when we will make the top level target follow the WindowGlobal lifecycle, like any additional frame target.

• navigate should be replaced with:
  • DocumentEvents "dom-complete" listening? May be that's too complex to do everywhere?
  • Should we introduce a onTargetLoaded? It may not work well for all target types? Like workers and processes.
  • Something else??

These suggestions still make sense to me, and feedback would be appreciated!

Otherwise, for now there is three usages of DOCUMENT_EVENT for such purpose:
https://searchfox.org/mozilla-central/rev/fac90408bcf52ca88a3dcd2ef30a379b68ab24e2/devtools/client/netmonitor/src/connector/firefox-connector.js#157
https://searchfox.org/mozilla-central/rev/fac90408bcf52ca88a3dcd2ef30a379b68ab24e2/devtools/client/netmonitor/src/connector/firefox-connector.js#343-345

async onResourceAvailable({ resourceType, targetFront, resource }) {
    if (resourceType === this.toolbox.resourceWatcher.TYPES.DOCUMENT_EVENT && resource.name == "dom-complete") {
      this.navigate();
    }
  }
}

=> This one usage cares about knowing when, each top level document is done loading.

https://searchfox.org/mozilla-central/rev/eef39502e08bcd3c40573c65a6827828dce4a032/toolkit/components/extensions/ExtensionParent.jsm#651-657
https://searchfox.org/mozilla-central/rev/eef39502e08bcd3c40573c65a6827828dce4a032/toolkit/components/extensions/ExtensionParent.jsm#727-734

 await this.devToolsToolbox.resourceWatcher.watchResources(
        [this.devToolsToolbox.resourceWatcher.TYPES.DOCUMENT_EVENT],
        {
          onAvailable: this._onResourceAvailable,
          ignoreExistingResources: true,
        }
      );
...
  async _onResourceAvailable({ targetFront, resource }) {
    if (targetFront.isTopLevel && resource.name === "dom-complete") {
      const url = targetFront.localTab.linkedBrowser.currentURI.spec;
      for (const listener of this._onNavigatedListeners) {
        listener(url);
      }
    }
  }

=> This one usage is similar to the previous one, but only cares about the future navigations. So it doesn't care about existing targets loading state. It wants to notify about all following top level document load ends.

https://searchfox.org/mozilla-central/rev/eef39502e08bcd3c40573c65a6827828dce4a032/devtools/client/inspector/changes/ChangesView.js#100-123

  this.resourceWatcher.watchResources(
    [this.resourceWatcher.TYPES.DOCUMENT_EVENT],
    { onAvailable: this.onResourceAvailable }
  );
   ...
  onResourceAvailable({ resource }) {
    if (resource.name === "dom-loading" && resource.targetFront.isTopLevel) {
      // will-navigate doesn't work when we navigate to a new process,
      // and for now, onTargetAvailable/onTargetDestroyed doesn't fire on navigation and
      // only when navigating to another process.
      // So we fallback on DOCUMENT_EVENTS to be notified when we navigates. When we
      // navigate within the same process as well as when we navigate to a new process.
      // (We would probably revisit that in bug 1632141)
      this.onClearChanges();
    }
  }

=> This one usage is different and should ideally be using onTargetAvailable. It wants to know when the top level just started navigating so that we clear the UI.

Comment 10

[Alexandre Poirot [:ochameau]]

Here is a quick recap of what is being done around will-navigate and navigate:

will-navigate

• debugger reset everything: sourceQueue, sourceMaps, wasm states, document, parser
• source maps service "reset all state"
• toolbox starts recording time and wait for "reloaded" panel event to record a telemetry
• toolbox toggles off some highlighters and updates the toolbox buttons accordingly (I'm not sure (this code)[https://searchfox.org/mozilla-central/rev/5e6c7717255ca9638b2856c2b2058919aec1d21d/devtools/client/framework/toolbox.js#2140-2157] belongs to the toolbox)
• thread front removes all its SourceFront children
• inspector resets the selected node (front), destroy the markup view
• highlighter overlay clear all its internal maps
• netmonitor clear everything if persist is off, or just the timing marker if true. It also resume recording and reset "ongoing search"
• styleeditor seems to clear about everything, we do the same clearance when the top level target is destroyed
• webconsole only display a "navigating" message if BrowserConsole or persist is on. Otherwise, when persist is off, it resets autocomplete state, clear messages and network requests.
• ResourceWatcher clear its cache

navigate

• accessibility panel forces a refresh of the panel, which we also do on panel's opening
• application panel also update its view, the same way it is done on opening and target switching
• debugger emits the "reloaded" event (this is wrong as this doesn't try to wait for all sources to be loaded)
• DOM panel uses exact same code as accessibility
• toolbox refreshes title and target specific data when displayed in a page
• rules view clears user properties
• netmonitor uses it to know when the page is reloaded when forcing a reload
• styleeditor update the stylesheets the same way it is done on panel's opening and target switching
• webconsole wait for any pending async update and emit the "reloaded" event. Also notify about console object being replaced by the page.
• highlighters destroy the highlighters if we changed the type xul to non-xul or xul to non-xul
• LegacyServiceWorker fetch the new list of service workers and notify the TargetList accordingly to the new target domain

Accessibility, Application, DOM, StyleEditor are all following the same pattern around "navigate".
But this is going to be simplified when migrating to the ResourceWatcher API.
Instead of pulling resources via front on opening and on navigate,
the ResourceWatcher will provide the resources via onResourceAvailable.
The logic of pulling the resources will be hidden in the legacy resource watcher and the server side equivalent.
Unfortunately, this is only going to be applied to StyleEditor. I'm not sure there is any resource for Accessibility, Application and DOM panels.

Application panel code is actually a good, simple example, which today requires lots of code:

  initialize() {
    // awaiting for watchTargets will return the targets that are currently
    // available, so we can have our first render with all the data ready
    await this.toolbox.targetList.watchTargets(
      [this.toolbox.targetList.TYPES.FRAME],
      this.onTargetAvailable,
      this.onTargetDestroyed
    );
  },

  // This method is the "update"/"refresh"/"reload" method, which will query actors
  // for up-to-date data and update the UI.
  handleOnNavigate() {
    this.updateDomain(); // This is solely based on `targetFront.url`
    this.actions.resetManifest(); // This reset some state in reducer
  },
  
  // This is called everytime there is a new top level target
  setupTarget(targetFront) {
    this.handleOnNavigate(); // update domain and manifest for the new target
    targetFront.on("navigate", this.handleOnNavigate);
  },

  cleanUpTarget(targetFront) {
    targetFront.off("navigate", this.handleOnNavigate);
  },

  onTargetAvailable({ targetFront }) {
    if (!targetFront.isTopLevel) {
      return; // ignore target frames that are not top level for now
    }

    this.setupTarget(targetFront);
  },

  onTargetDestroyed({ targetFront }) {
    if (!targetFront.isTopLevel) {
      return; // ignore target frames that are not top level for now
    }

    this.cleanUpTarget(targetFront);
  },

  destroy() {
    this.toolbox.targetList.unwatchTargets(
      [this.toolbox.targetList.TYPES.FRAME],
      this.onTargetAvailable,
      this.onTargetDestroyed
    );

    this.cleanUpTarget(this.toolbox.target);
  }

This could be refactored to this, if we follow current pattern using DOCUMENT_EVENT resource

  initialize() {
    // awaiting for watchTargets will return the targets that are currently
    // available, so we can have our first render with all the data ready
    await this.toolbox.resourceWatcher.watchResources(
      [this.toolbox.resourceWatcher.TYPES.DOCUMENT_EVENT],
      this.onResourceAvailable
    );
  },

  // This method is the "update"/"refresh"/"reload" method, which will query actors
  // for up-to-date data and update the UI.
  onResourceAvailable({ resourceType, name, targetFront }) {
    if (resourceType == this.toolbox.resourceWatcher.TYPES.DOCUMENT_EVENT && name == "dom-complete" && targetFront.isTopLevel) {
      this.updateDomain(); // This is solely based on `targetFront.url`
      this.actions.resetManifest(); // This reset some state in reducer
    }
  },
  
  destroy() {
    await this.toolbox.resourceWatcher.unwatchResources(
      [this.toolbox.resourceWatcher.TYPES.DOCUMENT_EVENT],
      this.onResourceAvailable
    );
  }

But I still share a mixed feeling about DOCUMENT_EVENT:

• We wait for this, but at the end of the day, we are waiting for data coming via navigate packet. Typically, target's url/title.
• This sounds like data related to Target, but we are using a very distinct holder for this data. I'm wondering if such data should be updated via protocol.js's form() mechanism or a target-updated-form event.

Comment 11

[Chris Peterson [:cpeterson]]

Tracking dt-fission-m2-mvp bugs for Fission Nightly milestone (M6c).

Comment 13

[Alexandre Poirot [:ochameau]]

(In reply to Alexandre Poirot [:ochameau] from comment #10)

But I still share a mixed feeling about DOCUMENT_EVENT:

• We wait for this, but at the end of the day, we are waiting for data coming via navigate packet. Typically, target's url/title.
• This sounds like data related to Target, but we are using a very distinct holder for this data. I'm wondering if such data should be updated via protocol.js's form() mechanism or a target-updated-form event.

I think I realize why I have mixed feeling about DOCUMENT_EVENT:

• DOCUMENT_EVENT only works for BrowsingContext targets (it doesn't exists for Content processes and workers). navigate/will-navigate is also specific to these targets. So it reinforces the connection between the three: BC targets, navigate and DOCUMENT_EVENT.
• With this in mind, it sounds like BC Target could be resources, where DOCUMENT_EVENT wouldn't really be resources, but be updates of this new resource type. TargetList doesn't support onTargetUpdated, but we could also introduce onTargetUpdated in order to match ResourceWatcher, without requiring to make tarket becomes resources today.
• DOCUMENT_EVENT with name == "dom-ready" will be equivalent to onTargetAvailable once we create target for each WindowGlobal.

onTargetAvailable(targetFront) {
  // With this approach we have to handle the case where the target is already loaded
  // This is something we don't have to do with DOCUMENT_EVENT
  if (targetFront.readyState == "dom-complete" && targetFront.isTopLevel) {
     this.update();
  }
}
onTargetUpdated(targetFront) {
  if (targetFront.readyState == "dom-complete" && targetFront.isTopLevel) {
     this.update();
  }
}

Could the legacy frame target listener fire fake target updated based on will-navigate and navigate events and/or DOCUMENT_EVENTS?

Comment 14

[Neha Kochar [:neha]]

Bulk move of all dt-fission-m3-mvp bugs to Fission MVP milestone.

Comment 15

[Chris Peterson [:cpeterson]]

Moving some dt-fission-m3-mvp bugs from Fission MVP to M7 (blocking Beta experiment).

Comment 16

[Chris Peterson [:cpeterson]]

Tracking dt-fission-m3-mvp test and infrastructure bugs for Fission M8 (blocking Release experiment).

Comment 17

[Julian Descottes [:jdescottes]]

I think this bug is more or less a meta for fixing all issues related to navigation + target switching.

The framework level solution is using onTargetAvailable/onTargetDestroyed (mostly onTargetAvailable), or DOCUMENT_EVENTS for specific edge cases.

However this can only be fully verified once all our top level tab targets are following the window lifecycle. Until then we

Could the legacy frame target listener fire fake target updated based on will-navigate and navigate events and/or DOCUMENT_EVENTS?

Was thinking about this as well, but I feel like we might introduce more complexity in the code without highlighting the real issues that will occur when will-navigate/navigate will no longer be available. For instance, this wouldn't help detect the issue with early document requests.

The only action I can see for now is to make sure we have tests which exercise target switching for the use cases listed in comment 10. I'll make a pass and try to file some bugs for that.

Comment 18

[Julian Descottes [:jdescottes]]

Another issue with this bug is that until have one target per window global and can fully rely on onTargetAvailable, we will still have codepaths that rely on will-navigate even in target switching scenarios.

For instance, the toolbox supports target-switching but still performs part of its cleanup on will-navigate. It can do so because will-navigate is still emitted for targets that will trigger a target switch.

The risk is that when we move to onTargetAvailable, the timings will be different, which will trigger regressions.
But the only alternative would to start relying on a completely different event (eg document event).

Maybe in a first step, we can keep relying on will-navigate, and mostly focus on "navigate".

Edit: While "navigate" is not emitted for parent-process -> content-process navigations, we seem to get it consistently for cross-process navigations when Fission is enabled. Again this will make it difficult to make progress on this topic before the target switching work is done.

Comment 19

[Chris Peterson [:cpeterson]]

Tracking DevTools Post-M8 bugs for Fission MVP milestone. They don't block the Fission Release channel experiment, but we would like them to be completed before we roll out Fission by default.

Comment 20

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1632141 - A test to simulate DevTools and highlight order of many platform events listened by DevTools.

Comment 21

[Alexandre Poirot [:ochameau]]

Comment on attachment 9217008 [details]
Bug 1632141 - A test to simulate DevTools and highlight order of many platform events listened by DevTools.

This test allows to assert the behavior and order of the various platform events we interact with.
Let's go into detail of the three typical navigations. I'm highlighting in bold the slight differences.

Simple page reload:

Current tab is loaded in process 65603 with innerWindowId:10737418241
=> Simply reload the page
[PARENT       ] > start http request:https://example.com/browser/devtools/shared/commands/target/tests/simple_document.html BrowsingContextID:50 innerWindowID:0 [http-on-opening-request]
[CONTENT-65603] >>> will-navigate (isDocument+isStart) innerWindowId:10737418241 request:https://example.com/browser/devtools/shared/commands/target/tests/simple_document.html [web-progress-listener]
[CONTENT-65603] > start http request:https://example.com/browser/devtools/shared/commands/target/tests/simple_document.html BrowsingContextID:50 innerWindowID:0 [document-on-opening-request]
[PARENT       ] < end   http request:https://example.com/browser/devtools/shared/commands/target/tests/simple_document.html BrowsingContextID:50 innerWindowID:0 [http-on-stop-request]

[CONTENT-65603] + WindowGlobal -- innerWindowID:10737418242 BrowsingContextID:50 [JSWindowActor.DOMWindowCreated]
[PARENT       ] + WindowGlobal -- innerWindowID:10737418242 BrowsingContextID:50 location:https://example.com/browser/devtools/shared/commands/target/tests/simple_document.html [window-global-created]
[CONTENT-65603] + console-message with innerWindowID:10737418241 errorMessage:Error: late exception [nsIConsoleListener]
[PARENT       ] - WindowGlobal -- innerWindowID:10737418241 BrowsingContextID:50 location:https://example.com/browser/devtools/shared/commands/target/tests/simple_document.html [window-global-destroyed]
[CONTENT-65603] - WindowGlobal -- innerWindowID:10737418241 BrowsingContextID:50 [JSWindowActor.didDestroy]
[CONTENT-65603] > start http request:https://example.com/browser/devtools/shared/commands/target/tests/simple_script.js BrowsingContextID:50 innerWindowID:10737418242 [http-on-opening-request]
[PARENT       ] > start http request:https://example.com/browser/devtools/shared/commands/target/tests/simple_script.js BrowsingContextID:50 innerWindowID:10737418242 [http-on-opening-request]
[PARENT       ] < end   http request:https://example.com/browser/devtools/shared/commands/target/tests/simple_script.js BrowsingContextID:50 innerWindowID:10737418242 [http-on-stop-request]
[CONTENT-65603] + console-message with innerWindowID:10737418242 errorMessage:Error: early error [nsIConsoleListener]
[CONTENT-65603] + console-message with innerWindowID:10737418242 errorMessage:An unsupported character encoding was declared for the HTML document using a meta tag. The declaration was ignored. [nsIConsoleListener]
[CONTENT-65603] <<< navigate (isWindow+isStop) innerWindowId:10737418242 [web-progress-listener]
<= Page reloaded

1. In the parent, the HTTP request is notified (via http-on-opening-request)
2. In the content, the WebProgressListener notifies about the load of a new document ("will-navigate"). And the HTTP request is also notified right after (via document-on-opening-request)
3. In the parent, and because the request is fast to respond, the HTTP request is notified as completed (via http-on-stop-request)
4. In the content and right after in the parent, the new WindowGlobal is created (via JSWindowActor DOMWindowCreated/window-global-created)
5. In the content, the previous document's unload listeners are fired. And its WindowGlobal is destroyed in both processes (via JSWindowActor.didDestroy/window-global-destroyed)
6. In both processes, we get notifications about the JavaScript file requests (<script src="...">)
7. In the content, we get notified about early messages (via nsIConsoleListener)
8. In the content, WebProgressListener notifies about the end of the load ("navigate")
   => The very first notification is the http-on-opening-request in the parent, followed by WebProgressListener's "will-navigate" in the content process.

Navigation the a same origin page (same BrowsingContext, same process):

Current tab is loaded in process 66597 with innerWindowId:10737418242
=> Load a second page with same origin: https://example.com/browser/devtools/shared/commands/target/tests/simple_document.html?foo
TEST-PASS | devtools/shared/commands/target/tests/browser_platform_events.js | Fetch the tab's browsing context id before navigation - 
[PARENT       ] > start http request  URI:https://example.com/browser/devtools/shared/commands/target/tests/simple_document.html?foo BrowsingContextID:50 innerWindowID:0 [http-on-opening-request]
[CONTENT-66597] >>> will-navigate (isDocument+isStart) innerWindowId:10737418242 request:https://example.com/browser/devtools/shared/commands/target/tests/simple_document.html?foo [web-progress-listener]
[CONTENT-66597] > start http request  URI:https://example.com/browser/devtools/shared/commands/target/tests/simple_document.html?foo BrowsingContextID:50 innerWindowID:0 [document-on-opening-request]
[PARENT       ] < end   http request:https://example.com/browser/devtools/shared/commands/target/tests/simple_document.html?foo BrowsingContextID:50 innerWindowID:0 [http-on-stop-request]
[PARENT       ] + WindowGlobal -- innerWindowID:10737418243 BrowsingContextID:50 location:https://example.com/browser/devtools/shared/commands/target/tests/simple_document.html?foo [window-global-created]
[CONTENT-66597] + WindowGlobal -- innerWindowID:10737418243 BrowsingContextID:50 [JSWindowActor.DOMWindowCreated]
[PARENT       ] - WindowGlobal -- innerWindowID:10737418242 BrowsingContextID:50 location:https://example.com/browser/devtools/shared/commands/target/tests/simple_document.html [window-global-destroyed]
[CONTENT-66597] + console-message with innerWindowID:10737418242 errorMessage:Error: late exception [nsIConsoleListener]
[CONTENT-66597] - WindowGlobal -- innerWindowID:10737418242 BrowsingContextID:50 [JSWindowActor.didDestroy]
[CONTENT-66597] > start http request  URI:https://example.com/browser/devtools/shared/commands/target/tests/simple_script.js BrowsingContextID:50 innerWindowID:10737418243 [http-on-opening-request]
[PARENT       ] > start http request  URI:https://example.com/browser/devtools/shared/commands/target/tests/simple_script.js BrowsingContextID:50 innerWindowID:10737418243 [http-on-opening-request]
[PARENT       ] < end   http request:https://example.com/browser/devtools/shared/commands/target/tests/simple_script.js BrowsingContextID:50 innerWindowID:10737418243 [http-on-stop-request]
[CONTENT-66597] + console-message with innerWindowID:10737418243 errorMessage:Error: early error [nsIConsoleListener]
[CONTENT-66597] + console-message with innerWindowID:10737418243 errorMessage:An unsupported character encoding was declared for the HTML document using a meta tag. The declaration was ignored. [nsIConsoleListener]
<= Second page loaded
[CONTENT-66597] <<< navigate (isWindow+isStop) innerWindowId:10737418243 [web-progress-listener]

It is actually the exact same steps as page reload.
There is only a different in step 4. The parent is notified first about the new WindowGlobal, before the content.
I could easily imagine the order is racy and may occur in both order in both STRs.

Navigation to another origin, another process, but same BrowsingContext:

Current tab is loaded in process 65603 with innerWindowId:10737418242
=> Load a second page with distinct origin: https://example.org/browser/devtools/shared/commands/target/tests/simple_document.html
[PARENT       ] > start http request:https://example.org/browser/devtools/shared/commands/target/tests/simple_document.html BrowsingContextID:50 innerWindowID:0 [http-on-opening-request]
[CONTENT-65603] >>> will-navigate (isDocument+isStart) innerWindowId:10737418242 request:https://example.org/browser/devtools/shared/commands/target/tests/simple_document.html [web-progress-listener]
[CONTENT-65603] > start http request:https://example.org/browser/devtools/shared/commands/target/tests/simple_document.html BrowsingContextID:50 innerWindowID:0 [document-on-opening-request]
[CONTENT-65603] + console-message with innerWindowID:10737418242 errorMessage:Error: late exception [nsIConsoleListener]
[CONTENT-65603] - WindowGlobal -- innerWindowID:10737418242 BrowsingContextID:50 [JSWindowActor.didDestroy]
[PARENT       ] + WindowGlobal -- innerWindowID:35 BrowsingContextID:50 location:about:blank [window-global-created]
[PARENT       ] - WindowGlobal -- innerWindowID:10737418242 BrowsingContextID:50 location:https://example.com/browser/devtools/shared/commands/target/tests/simple_document.html [window-global-destroyed]
[CONTENT-65657] + WindowGlobal -- innerWindowID:35 BrowsingContextID:50 [JSWindowActor.DOMWindowCreated]
[CONTENT-65657] >>> will-navigate (isDocument+isStart) innerWindowId:35 request:https://example.org/browser/devtools/shared/commands/target/tests/simple_document.html [web-progress-listener]
[PARENT       ] < end   http request:https://example.org/browser/devtools/shared/commands/target/tests/simple_document.html BrowsingContextID:50 innerWindowID:0 [http-on-stop-request]
[PARENT       ] + WindowGlobal -- innerWindowID:12884901889 BrowsingContextID:50 location:https://example.org/browser/devtools/shared/commands/target/tests/simple_document.html [window-global-created]
[PARENT       ] - WindowGlobal -- innerWindowID:35 BrowsingContextID:50 location:about:blank [window-global-destroyed]
[CONTENT-65657] + WindowGlobal -- innerWindowID:12884901889 BrowsingContextID:50 [JSWindowActor.DOMWindowCreated]
[CONTENT-65657] - WindowGlobal -- innerWindowID:35 BrowsingContextID:50 [JSWindowActor.didDestroy]
[CONTENT-65657] > start http request:https://example.org/browser/devtools/shared/commands/target/tests/simple_script.js BrowsingContextID:50 innerWindowID:12884901889 [http-on-opening-request]
[PARENT       ] > start http request:https://example.org/browser/devtools/shared/commands/target/tests/simple_script.js BrowsingContextID:50 innerWindowID:12884901889 [http-on-opening-request]
[PARENT       ] < end   http request:https://example.org/browser/devtools/shared/commands/target/tests/simple_script.js BrowsingContextID:50 innerWindowID:12884901889 [http-on-stop-request]
[CONTENT-65657] + console-message with innerWindowID:12884901889 errorMessage:Error: early error [nsIConsoleListener]
[CONTENT-65657] + console-message with innerWindowID:12884901889 errorMessage:An unsupported character encoding was declared for the HTML document using a meta tag. The declaration was ignored. [nsIConsoleListener]
<= Second page loaded
[CONTENT-65657] <<< navigate (isWindow+isStop) innerWindowId:12884901889 [web-progress-listener]

This is actually very similar to page reload, except that two processes are involved, process A for previous page and B for the new page.
And we get a transcient WindowGlobal being created for an about:blank document.

1. In the parent, the HTTP request is notified (via http-on-opening-request)
2. In the content "A", the WebProgressListener notifies about the load of a new document ("will-navigate"). And the HTTP request is also notified right after (via document-on-opening-request).
3. Still in the content "A", the previous document's unload listeners are also fired at this step. And the WindowGlobal of the previous page is destroyed in content A et right after in the parent process. (This was done slightly later in page reload. this may be racy?)
4. In parallel in the parent and right after in the content B, the transcient WindowGlobal for about:blank is created. This fires a new "will-navigate" for navigation from about:blank to the final destination.
5. In the parent, and because the request is fast to respond, the HTTP request is notified as completed (via http-on-stop-request) (this seems slightly delayed compared to page reload)
6. In the parent and right after in content "B", the final WindowGlobal is created and the transcient about:blank one is destroyed.

Then the last 3 steps are the exact same as page reload:
7) In both processes, we get notifications about the JavaScript file requests (<script src="...">)
8) In the content "B", we get notified about early messages (via nsIConsoleListener)
9) In the content "B", WebProgressListener notifies about the end of the load ("navigate")

=> The very first notification is the http-on-opening-request in the parent, followed by WebProgressListener's "will-navigate" in the content process A. It is important to highlight that "will-navigate" fires in the previous page's content process. And we have to be careful to ignore the "will-navigate" related to the transcient about:blank document.

Navigation with COOP headers, new process and new BrowsingContext:

Current tab is loaded in process 65657 with innerWindowId:12884901889
=> Load a third page with COOP header: https://example.com/browser/devtools/shared/commands/target/tests/coop_document.html
[PARENT       ] > start http request:https://example.com/browser/devtools/shared/commands/target/tests/coop_document.html BrowsingContextID:50 innerWindowID:0 [http-on-opening-request]
[CONTENT-65657] >>> will-navigate (isDocument+isStart) innerWindowId:12884901889 request:https://example.com/browser/devtools/shared/commands/target/tests/coop_document.html [web-progress-listener]
[CONTENT-65657] > start http request:https://example.com/browser/devtools/shared/commands/target/tests/coop_document.html BrowsingContextID:50 innerWindowID:0 [document-on-opening-request]
[PARENT       ] + new BrowsingContext with id:52 current innerWindowID:undefined [browsing-context-attached]
[PARENT       ] + WindowGlobal -- innerWindowID:37 BrowsingContextID:52 location:about:blank [window-global-created]
[CONTENT-65657] + console-message with innerWindowID:12884901889 errorMessage:Error: late exception [nsIConsoleListener]
[CONTENT-65657] - WindowGlobal -- innerWindowID:12884901889 BrowsingContextID:50 [JSWindowActor.didDestroy]
[PARENT       ] - WindowGlobal -- innerWindowID:12884901889 BrowsingContextID:50 location:https://example.org/browser/devtools/shared/commands/target/tests/simple_document.html [window-global-destroyed]
[CONTENT-65696] + WindowGlobal -- innerWindowID:37 BrowsingContextID:52 [JSWindowActor.DOMWindowCreated]
[CONTENT-65696] >>> will-navigate (isDocument+isStart) innerWindowId:37 request:https://example.com/browser/devtools/shared/commands/target/tests/coop_document.html [web-progress-listener]
[PARENT       ] < end   http request:https://example.com/browser/devtools/shared/commands/target/tests/coop_document.html BrowsingContextID:50 innerWindowID:0 [http-on-stop-request]
[PARENT       ] + WindowGlobal -- innerWindowID:15032385537 BrowsingContextID:52 location:https://example.com/browser/devtools/shared/commands/target/tests/coop_document.html [window-global-created]
[PARENT       ] - WindowGlobal -- innerWindowID:37 BrowsingContextID:52 location:about:blank [window-global-destroyed]
[CONTENT-65696] + WindowGlobal -- innerWindowID:15032385537 BrowsingContextID:52 [JSWindowActor.DOMWindowCreated]
[CONTENT-65696] - WindowGlobal -- innerWindowID:37 BrowsingContextID:52 [JSWindowActor.didDestroy]
[CONTENT-65696] > start http request:https://example.com/browser/devtools/shared/commands/target/tests/simple_script.js BrowsingContextID:52 innerWindowID:15032385537 [http-on-opening-request]
[PARENT       ] > start http request:https://example.com/browser/devtools/shared/commands/target/tests/simple_script.js BrowsingContextID:52 innerWindowID:15032385537 [http-on-opening-request]
[PARENT       ] < end   http request:https://example.com/browser/devtools/shared/commands/target/tests/simple_script.js BrowsingContextID:52 innerWindowID:15032385537 [http-on-stop-request]
[CONTENT-65696] + console-message with innerWindowID:15032385537 errorMessage:Error: early error [nsIConsoleListener]
[CONTENT-65696] + console-message with innerWindowID:15032385537 errorMessage:An unsupported character encoding was declared for the HTML document using a meta tag. The declaration was ignored. [nsIConsoleListener]
<= Third page loaded
[CONTENT-65696] <<< navigate (isWindow+isStop) innerWindowId:15032385537 [web-progress-listener]

1. As always, In the parent, the HTTP request is notified (via http-on-opening-request)
2. As always, In the content "A", the WebProgressListener notifies about the load of a new document ("will-navigate"). And the HTTP request is also notified right after (via document-on-opening-request).
3. New step specific to this scenario, in the parent, we get the new BrowsingContext (via browsing-context-attached)

Then, all the following steps are the same as navigation to another origin while keeping the same BrowsingContext:
4) Still in the content "A", the previous document's unload listeners are also fired at this step. And the WindowGlobal of the previous page is destroyed in content A et right after in the parent process. (This was done slightly later in page reload. this may be racy?)
4) In parallel in the parent and right after in the content B, the transcient WindowGlobal for about:blank is created. This issue a new "will-navigate" for navigation from about:blank to the final destination.
5) In the parent, and because the request is fast to respond, the HTTP request is notified as completed (via http-on-stop-request) (this seems slightly delayed compared to page reload)
6) In the parent and right after in content "B", the final WindowGlobal is created and the transcient about:blank one is destroyed.
And the last 3 steps are the exact same as page reload:
7) In both processes, we get notifications about the JavaScript file requests (<script src="...">)
8) In the content "B", we get notified about early messages (via nsIConsoleListener)
9) In the content "B", WebProgressListener notifies about the end of the load ("navigate")

=> The same conclusion about the two first events holds.

Some other conclusions:

• Only the first document request seems to happen before "will-navigate". Console message/Errors/Other requests all seem to be notified after the "will-navigate" event.
• Also, all these meaningful observables seem to come after the WindowGlobal is created. It sounds like we could depend on the TargetActor to be created as it is created as soon as the WindowGlobal is created? Only the first document request would need to be special cased.

Comment 22

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1632141 - [devtools] Emit will-navigate for server side generated targets and listen to will-navigate to clear frontend state on navigation.

Comment 23

[Alexandre Poirot [:ochameau]]

Out of comment 21 conclusion, I think it is best to keep listening to will-navigate or an equivalent of it.

The other option would be to listen to onTargetAvailable. It may work for for all but the first request (as will-navigate), but it is still significantly later and:

• may cause some races intests with a significant shift. While working on this topic, I saw that some tests were expecting will-navigate to be fired immediately and that the panel would be clear super early, almost as soon as we called browser.reload()!
• because we don't spawn one target for each reload/navigation, this would still require "something else", which defeat the whole purpose of unifying into a unique, reliable event.

Now, if we stick to listen to will-navigate, I see two flavors:

Keep BrowsingContextTargetActor's will-navigate.

But it means that we would have to remove this condition. So that will-navigate will also fire for server-side generated targets (i.e. when devtools.target-switching.server.enabled is true).

Also, there is a theoritical race where we could miss this event. It doesn't seem to happen at first sight. But it feels dangerous to listen to an event on the target that is right after going to be destroyed. When it is destroyed, it's EventEmitter interface is destroyed and should no longer transfer any event.

It also mean that each panel has to register "will-navigate" on each targetFront via onTargetAvailable.
This is the kind of task the ResourceWatcher is meant to hide away. Which leads me to the second option.

I gave a try to this option:
https://phabricator.services.mozilla.com/D112971
And try is happy with that:
https://treeherder.mozilla.org/jobs?repo=try&revision=223aafa9fcaac7008550f0bc5b777ec3b612b69f
And it should fix most server side target switching issues... even the top request navigation request seems to work decently.

Introduce DOCUMENT_EVENT's will-navigate.

In order to help listening to will-navigate on all the targets without manually registering an event listener.
And better align will-navigate with dom-loading, dom-interactive and dom-complete (which should become the replacement for navigate).
We could introduce a new DOCUMENT_EVENT's "will-navigate", which would fire on the target just before it is destroyed and notify that it navigates away.
We would then receive DOCUMENT_EVENT's "dom-loading" from the next target actor.

The main downside I saw while trying to implement that is the throttling of resources.
The throttling makes it so that the DOCUMENT_EVENT is significantly delayed compared to targetFront's will-navigate.
This makes the panel clearing very much delayed while a few tests expect to be cleared right away.

I also gave a try to this approach in:
https://bugzilla.mozilla.org/show_bug.cgi?id=1702511
Try is also happy:
https://treeherder.mozilla.org/jobs?repo=try&revision=e7183a12d37a5e2ec612401993ab506ce7a2f3ab
And may also be happy without the navigation request patches:
https://treeherder.mozilla.org/jobs?repo=try&revision=e977d71f83a87399abde76953f8becfdd7da4e9a
It required more changes in the console because of the migration from targetfront's event to resources.
See the various in-code comments in:
https://phabricator.services.mozilla.com/D112231

Comment 24

[Phabricator Automation]

Comment on attachment 9217421 [details]
Bug 1632141 - [devtools] Emit will-navigate for server side generated targets and listen to will-navigate to clear frontend state on navigation.

Revision D112971 was moved to bug 1706995. Setting attachment 9217421 [details] to obsolete.