1578243 - Make the network monitor support target switching

Status: RESOLVED FIXED

(DevTools :: Netmonitor, enhancement, P1)

Description

[Alexandre Poirot [:ochameau]]

Bug 1565263 landed the TargetList API which helps support switching to another target (as well as supporting the additional remote iframe target).
But we should followup in each tool in order to start using this new API.
The very first goal is to:

• stop memoizing the target/targetFront and may be also the target scoped front (inspector, console, thread, storage, ...). Instead we should use toolbox.targetList.targetFront in order to query for the current top level target front.
• support target switching by using TargetList.watchTargets. In a first iteration we would only care about the top level target. In order to do so, we can check for the isTopLevel argument being passed to the two callbacks register to watchTargets:

  this._toolbox.targetList.watchTargets([this._toolbox.targetList.TYPES.FRAME],
    ({ type, targetFront, isTopLevel }) => {
      if (isTopLevel) {
        // A new top level target is available
        // This will be fired on toolbox opening, for the first one,
        // And then, evertime we navigate to another process.
        // For now, you would need `devtools.target-switching.enabled` to be set to true
        // And navigate from any http website to about:robots, which loads into the parent process. Or enable Fission and navigate between two distinct top level domains.
      }
    },
    ({ type, targetFront, isTopLevel }) => {
      if (isTopLevel) {
        // A top level target has been destroyed
      }
    }

See bug 1565263 for how we migrated the console or bug 1578242 for the inspector.

Comment 1

[Alexandre Poirot [:ochameau]]

I think that it would be about reviewing all usages of tabTarget from here:
https://searchfox.org/mozilla-central/rev/62a130ba0ac80f75175e4b65536290b52391f116/devtools/client/netmonitor/src/connector/firefox-connector.js#55
in order to:

• Stop passing tabTarget. So we should refactor all the callsites to remove it.
• Instead, if we have to, we should query the target from toolbox.targetList.targetFront.
• But, we most likely should use the targetFront delivered via the TargetList API.

So from FirefoxConnector.connect method, we should do something like:

connect() {
  const {targetList} = connection.toolbox;
  targetList.watchTargets([targetList.TYPE.FRAME], ({type, targetFront, isTopLevel}) => {
    // Ignore additional targets for now
    if (!isTopLevel) {
      return;
    }
    this.webConsoleFront = this.tabTarget.getFront("console");
    this.dataProvider = new FirefoxDataProvider({
      webConsoleFront: this.webConsoleFront,
      actions: this.actions,
      owner: this.owner,
    });
    // The rest of connect method
  }, ({type, targetFront, isTopLevel) => {
    // Here, we should probably try to destroy `this.dataProvider` and everything we created in the previous callback.
  });
}

Comment 2

[Chris Peterson [:cpeterson]]

Tracking dt-fission-m2 bugs for Fission Nightly (M6)

Comment 3

[Alexandre Poirot [:ochameau]]

A few additional note which may help here as Netmonitor is a bit special compared to all other panels.

NetMonitorAPI is being instantiated and managed by the toolbox:
https://searchfox.org/mozilla-central/rev/923eec8d2fb8078ebc7a33a9e1ce73eac01f7446/devtools/client/framework/toolbox.js#3930-3952
And that is the thing that calls FirefoxConnector.connect:
https://searchfox.org/mozilla-central/rev/923eec8d2fb8078ebc7a33a9e1ce73eac01f7446/devtools/client/netmonitor/src/api.js#62-104
We most likely have to destroy the api in Toolbox.onTargetDestroyed and may be do some other action to accomodate target switching.

And you might want to review all the usages of the api instance:
https://searchfox.org/mozilla-central/search?q=NetMonitorAPI()&case=false&regexp=false&path=
This is being used from many places: the console, har export, har automation, the toolbox and last but not least, the netmonitor panel.
I'm wondering if we should communicate to these callsites that we switched to another target and need to stop using the previous api instance?

Comment 4

[Daisuke Akatsuka (:daisuke)]

Attachment: Bug 1578243: Refer current target instead of tabTarget. r?jdescottes!,ochameau!

Comment 5

[Daisuke Akatsuka (:daisuke)]

Attachment: Bug 1578243: Reflect the switching of the top-level target. r?jdescottes!,ochameau!

Depends on D57754

Comment 6

[Daisuke Akatsuka (:daisuke)]

Attachment: Bug 1578243: Add a test for target switching. r?jdescottes!,ochameau!

Depends on D57755

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0b22ee0b6115
Refer current target instead of tabTarget. r=jdescottes,ochameau
https://hg.mozilla.org/integration/autoland/rev/c9979d1a8ca6
Reflect the switching of the top-level target. r=jdescottes,ochameau
https://hg.mozilla.org/integration/autoland/rev/12fb5e522dd3
Add a test for target switching. r=jdescottes,ochameau

Comment 8

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/0b22ee0b6115
https://hg.mozilla.org/mozilla-central/rev/c9979d1a8ca6
https://hg.mozilla.org/mozilla-central/rev/12fb5e522dd3