859371 - Nuke panel's global when the toolbox closes

Status: NEW

(DevTools :: General, enhancement, P3)

Description

[Dave Camp (:dcamp)]

Right now the old compartments stay around, which isn't great at all.

Comment 1

[Alexandre Poirot [:ochameau]]

When the toolbox closes, there is a set of ressources we could ensure removing.
The compartment / sandbox nuke feature is a great way to ensure everything get really removed without any chance of keeping a reference, which would keep almost everything alive.

These collectable ressources are the toolbox and panels documents.
We open an iframe against toolbox.xul, in which we load one iframe per tool, like devtools/client/inspector/index.xhtml.
We also instanciates "browser loaders" for each of these documents.
These loaders are going to load modules without using Cu.Sandbox. Instead, it will create a scope object out of each panel's global window object and evaluate these modules with such scope/global. So that the modules are bound to these documents lifecycle / globals.
Cu.nukeSandbox throws as these modules don't have any Sandbox. Instead, their global is a made-up JS object from the panels documents.

The documents aren't nuked either as they are loaded via the system principal.
So that the auto-nuking of documents stops at this check:
https://searchfox.org/mozilla-central/rev/07f7390618692fa4f2a674a96b9b677df3a13450/dom/base/WindowDestroyedEvent.cpp#110
if (obj && !js::IsSystemRealm(js::GetNonCCWObjectRealm(obj))) {

Jan, I saw that bug 1512260 recently shuffled the nuking code.
What do you think about trying to somehow get the devtools document to be nuked?
From what I undertood from bug 1512260, nuking system principal globals may not be an option?
If that's not an option, is switching to content privileges the only option?
If there is a third option, it is not clear how to change / flag the compartment/global used by document?
For sandboxes, it is easy as you cann pass arguments to Cu.Sandbox, but for documents?
I think it is fine trying to load the DevTools in a distinct compartment, as soon as everything is loaded in the same compartment.
Your recent work allowed to automagically see that happening!

Comment 2

[Jan de Mooij [:jandem]]

The way it works now is that we no longer nuke compartments, only individual realms/globals. Nuking a global cuts all cross-compartment wrappers targeting that global, but other globals within its compartment can still hold non-wrapper references to it (same-compartment realms don't have a clear boundary between them).

Nuking system-principal globals should still be fine, especially if everything is in its own compartment and you nuke all those globals. As you noticed, we don't nuke system globals in WindowDestroyedEvent::Run. Maybe we could change that, I'm not sure.

We could add a new function to Cu to nuke all globals in the caller's compartment. Would something like that work for you?

Comment 3

[Alexandre Poirot [:ochameau]]

Let me try to describe what are all the globals involved in DevTools. All of them are using the system principal and runs in the same compartment.

• The documents:
  • The toolbox: chrome://devtools/content/framework/toolbox.xul
    This is the main UI of devtools, where all the panels are displayed: console, debugger, ...
    Each panel is in its dedicated iframe, created in toolbox.xul
    • webconsole: chrome://devtools/content/webconsole/index.html
    • inspector: chrome://devtools/content/inspector/index.xhtml
    • debugger: chrome://devtools/content/debugger/index.html
    • ...
• The modules:
  All modules are loaded via a custom module loader, implemented by resource://devtools/shared/base-loader.js.
  • The main loader: resource://devtools/shared/Loader.jsm
    This is used to load always-alive modules. Modules whose lifetime is bound to Firefox one.
    Once they are loaded, they are never destroyed and are meant to be used until Firefox closes.
    This loader creates only one Sandbox via Cu.Sandbox() and then use a custom JS object of this Sandbox as global for each of its module.
    We then populate this JS object with some custom globals, for each module.
  • The browser loaders: resource://devtools/client/shared/browser-loader.js
    There is one browser loader per panel. Modules loaded by these loaders are bound to each panel lifetime.
    This loader doesn't involve any Sandbox. Instead, it creates an empty object from the panel's global and populate this JS object with some custom globals, for each module.

Thanks to your work to use the same compartment for all/most system principals contexts, everything here (documents and modules) are in the same compartment. The main privileged compartment used by almost all Firefox! It also means that there is no cross compartment wrapper involved anywhere here.
In such context, nuking one global would be pointless, right?
If we would like to benefit from the nuking of wrappers, it would first mean that we have to pull off the things we want to nuke in a distinct compartment. Am I still right?
The things having a limited lifecycle and that we would like to nuke are the devtools documents. Then my first blocker in pursuing this project would be to be able to have this document loaded in a distinct compartment. AFAIK, there isn't any freshCompartment equivalent for documents. I have absolutely no clue on how to tell toolbox.xul to load in a distinct compartment. And then, have the other child documents (console, inspector,...) load in this same distinct compartment.
Then, yes, I imagine that "a new function to Cu to nuke all globals in the caller's compartment" would allow to nuke the devtools documents and its related loaders.

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Alex, this seems to be outdated, can we close it?

Honza

Comment 5

[Alexandre Poirot [:ochameau]]

No, it is still relevant. Last year discussion is still meaningful.

It would still be a way to better cleanup frontend leftovers on toolbox close.

Comment 6

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Great, thanks for the update, Alex.

Honza