Closed Bug 1565197 Opened 5 years ago Closed 3 years ago

[meta] Use JSWindowActor API instead of message manager in DevTools

Categories

(DevTools :: Framework, task)

task
Not set
normal

Tracking

(Fission Milestone:Future)

RESOLVED FIXED
Fission Milestone Future

People

(Reporter: ochameau, Unassigned)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Keywords: meta, Whiteboard: dt-fission-future)

For now, DevTools is using the Message Manager API in order to communicate with Tab's content processes, but also to reach Content Processes.

We know that, to support remoted frames (Fission), we will have to use the new API. JSWindowActor.
This API acknowledge the fact that a given tab may have ressources loaded in distinct processes. Some iframes of the tab's document can be running in distinct process than the tab top level document.
Also, when navigating from one URL to another in the same tab, the tab may change to another process. The new API may also help better support this scenario.

There is a couple of usages of the message manager in DevTools, here is a good searchfox query to see them:
https://searchfox.org/mozilla-central/search?q=.load(Frame%7CProcess)Script%5C(&case=false&regexp=true&path=devtools%2F
In this query there is a search for loadFrameScript, which is the main entrypoint of message manager usage.
I also highlighted the usages of loadProcessScript, but these usages may still be relevant?

I think that the most important usage to remove is the one related to Tab debugging and so, this one:
https://searchfox.org/mozilla-central/rev/f372470e10c8cb0691681603a1d6324dee5b3b8a/devtools/server/main.js#658

The JSWindowActor API is documented here:
https://searchfox.org/mozilla-central/source/dom/chrome-webidl/JSWindowActor.webidl
https://firefox-source-docs.mozilla.org/dom/dom/Fission.html

Bug 1565162 could be helpful as it demonstrate how to do similar refactoring in the brand new Remote agent codebase.

The key difference is that the message manager API was programatic. While JSWindowActor is declarative.
Message manager's loadFrameScript and addMessageListener/removeMessageListener have been replaces by a declaration:

# parent-script.jsm:
mm.loadFrameScript("resource://.../frame-script.js");
mm.sendAsyncMessage("foo");
mm.addMessageListener(data => {
  dump("Received "+data.name+"!\n");
});
# frame-script.js
addMessageListener("foo", () => sendAsyncMessage("bar"));

is being replaced by:

# parent-script.jsm:
ChromeUtils.registerWindowActor("Test", {
  parent: {
    moduleURI: "resource://.../TestParent.jsm",
  },
  child: {
    moduleURI: "resource://.../TestChild.jsm",
  },
  allFrames: true,
});
# TestParent.jsm
var EXPORTED_SYMBOLS = ["TestParent"];
class TestParent extends JSWindowActorParent {
  constructor() {
    super();
  }
  receiveMessage(data) {
    dump("received "+data.name+"!\n");
  }
}
# TestChild.jsm
var EXPORTED_SYMBOLS = ["TestChild"];
class TestChild extends JSWindowActorChild {
  constructor() {
    super();
  }
  receiveMessage(data) {
    if (data.name == "foo") {
      this.sendAsyncMessage("bar");
    }
  }
}

Note that there is something not documented that can be really hard to debug.
ChromeUtils.registerWindowActor first argument "Test" actually defines the name of the symbols to be exported by the two JSM! The symbol has to be made of this argument with "Parent" or "Child" suffix. So here, the JSM exported symbols have to be TestParent and TestChild. If you fail doing that you will have the following exception undefined is not a constructor.

Also, there is one important detail about TestChild and TestParent.
There is a 1 for 1 relation between the two. For each TestChild, there will be one TestParent. So, if the tab's document has remoted iframes, there will be as many pair of these two as there is remoted iframes!

In order to retrieve an actor from a particular top level document or frame, you have to use the BrowsingContext API like that:

// "Test" is the argument you passed to ChromeUtils.registerWindowActor function
// This will get you the actor for the top level document:
let actor = gBrowser.selectedBrowser.browsingContext.currentWindowGlobal.getActor("Test");
// actor is a `TestParent` instance
// Or for a (remoted?) iframe:
let actor = gBrowser.selectedBrowser.browsingContext.getChildren()[0].getActor("Test");

This example is from the parent process, but you can do similar access on the browsingContext object from the content process in order to retrieve the TestChild instance.
You can get a reference to the browsingContext from: docshell, frameloader and loadinfo objects.

One additional interesting detail about WindowGlobalParent.getActor():
When you call browsingContext.currentWindowGlobal.getActor(...), you will want to save around the returned actor instance.
If the browsing navigates to another document in the same process, the created actor will still be valid.
Actor isn't unique per document, but per browsing context loaded in a given process. This is similar to frame script.
More concretely, the issue is that browsingContext.currentWindowGlobal will be different when the tab navigates.
And so if you recall getActor another actor, in the same process will be instanciated and that may confuse you!

Later on, to support process switching, we may have to polish things again and cleanup'n forget the saved reference to the actor if the browsing context switches to another process. But that sounds like yet another story to look at!

Another interesting fact. The Child component, TestChild may implement a willDestroy and didDestroy methods to handle cleanup.
But only didDestroy seems to be consistantly called.

Depends on: 1565200
Whiteboard: dt-fission

If the browsing navigates to another document in the same process, the created actor will still be valid.

The behavior I see in my current patches seems different.
If I create an actor for a given browsingContext, the child/parent pair will be destroyed:

  • when reloading the page
  • when navigating to another page

I checked both with my current WIP actors as well as with the existing ContextMenuChild/ContextMenuParent. So that's going to be a major difference compared to our framescripts.

I am seeing a discrepancy between the behavior and the current documentation about JsWindowActor.
The willDestroy documentation states

willDestroy

This method will be called when we know that the JSWindowActor pair is going to be destroyed 
because the associated BrowsingContext is going away.  

But in practice, my actor pairs are destroyed whenever the target window/document is destroyed, for instance during a reload or a navigation.
And looking at the code it seems that actors are destroyed when the window global is destroyed (https://searchfox.org/mozilla-central/rev/34cb8d0a2a324043bcfc2c56f37b31abe7fb23a8/dom/ipc/WindowGlobalChild.cpp#225) (maybe I misunderstand this part of the code though, I don't have a good graps of this area).

What is supposed to be the lifecycle of JsWindowActors?
Are they destroyed when the Window global is destroyed or when the BrowsingContext is destroyed?

Kris, maybe you know (or know who we should ask) ?

Flags: needinfo?(kmaglione+bmo)

They are destroyed when the window global is destroyed

Flags: needinfo?(kmaglione+bmo)
No longer blocks: dt-fission
Depends on: 1597255

Tracking Fission DevTools bugs for Fission Nightly (M6)

Fission Milestone: --- → M6

dt-fission-reserve bugs do not need to block Fission Nightly (M6), but these bugs' summaries mention the word "Fission", so let's track them for Fission riding the trains to Beta (M7). We'll revisit these bugs before we ship Fission.

Fission Milestone: M6 → M7
Depends on: 1672390

Bulk change of all bugs with whiteboard tag of dt-fission to Fission MVP milestone.

Fission Milestone: M7 → MVP

Moving old "dt-fission" bugs to "dt-fission-future" because they don't block Fission MVP.

Fission Milestone: MVP → Future
Whiteboard: dt-fission → dt-fission-future

We still have legacy codepath which are using message managers (like toolboxes spawn from about:debugging),
but regular web toolboxes are now only using JSWindowActor APIs to communicate with the content processes!

Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.