1614096 - [meta] Move all devtools UI out of the parent process into a privileged content process

Status: NEW

(DevTools :: General, task, P3)

Description

[Markus Stange [:mstange]]

The devtools currently render inside the parent process. This is bad for performance reasons: It means that the devtools have the potential to slow down the operation of the entire browser as long as any devtools UI is displayed anywhere, even when the user isn't interacting with it at the moment. For example, scrolling regular web pages can jank at random times when the browser error console window is open in the background. Furthermore, interactions with the devtools themselves are less smooth than they could be, for example because async scrolling cannot fully be taken advantage of.
In my personal experience, I encounter devtools-related browser slowdowns mostly when I forget to close a devtools tab or window and focus on something else. Then, at some point, I get curious why my browser is randomly slower than usual, and then I find devtools code in the profiles.

Here's another example I just encountered: https://perfht.ml/37ajBUN
I happened to have about:debugging open in a background tab and had forgotten about it. But every 5 seconds, the whole browser would freeze for ~200ms, because about:debugging was running code on the main thread.

Rather than fixing these issues on a case-by-case basis, I think UI-related devtools code should move out of the parent process entirely. The parent process main thread is a critical resource that simply cannot be blocked for more than a few milliseconds at a time without impacting overall browser performance.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[Tom Ritter [:tjr]]

sec-want because when DevTool is out of the Parent Process we should be able to be more conservative with the JIT used there, which would eliminate attack surface.

Comment 3

[Alexandre Poirot [:ochameau]]

(In reply to Tom Ritter [:tjr] from comment #2)

sec-want because when DevTool is out of the Parent Process we should be able to be more conservative with the JIT used there, which would eliminate attack surface.

Tom, Could we possibly disable the JIT? I was wondering how much we benefit from it? We have document and common js modules. I don't know if JIT is enabled in both cases. We have DAMP test on talos to quickly assert the impact if that's easy to turn it on/off.

On a more positive side, I've built a proof of concept.
The tools renders, that's already a big deal as we might still involve legacy xul codepaths.
I suspect we will have various edgecases with tooltips, popups.
For example, get crashes when the autocomplete popup kicks in.

A significant part of this PoC is to make the UI communicate with the parent process via a JSWindow Actor
in order to "connect" to the DevTools Server which has to run from the parent process.

But the real blocker will most likely be about tests.
Most of our tests are browser mochitests. They all expect DevTools UI documents to run in the parent process and have raw access to the Toolbox, all the panels (inspector, console,...) and their DOM documents.
Migrating them to use SpecialPowser.spawn will be an epic quest.

I'll clean up a few things in my PoC and try to shape a plan to land a preffed off version of it.

Comment 4

[Tom Ritter [:tjr]]

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

(In reply to Tom Ritter [:tjr] from comment #2)

sec-want because when DevTool is out of the Parent Process we should be able to be more conservative with the JIT used there, which would eliminate attack surface.

Tom, Could we possibly disable the JIT?

Yeah - that would be the goal - to disable it entirely =)

Comment 5

[Alex Thayer [:alexical] (she/her)]

(In reply to Tom Ritter [:tjr] from comment #4)

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

(In reply to Tom Ritter [:tjr] from comment #2)

sec-want because when DevTool is out of the Parent Process we should be able to be more conservative with the JIT used there, which would eliminate attack surface.

Tom, Could we possibly disable the JIT?

Yeah - that would be the goal - to disable it entirely =)

Devtools is a pretty performance-sensitive area of the browser frontend. I would expect unilaterally disabling the JIT across all of it would severely degrade the experience for users. Am I missing something or do you have any more detail on this idea?

Comment 6

[Tom Ritter [:tjr]]

(In reply to Doug Thayer [:dthayer] (he/him) from comment #5)

(In reply to Tom Ritter [:tjr] from comment #4)

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

(In reply to Tom Ritter [:tjr] from comment #2)

sec-want because when DevTool is out of the Parent Process we should be able to be more conservative with the JIT used there, which would eliminate attack surface.

Tom, Could we possibly disable the JIT?

Yeah - that would be the goal - to disable it entirely =)

Devtools is a pretty performance-sensitive area of the browser frontend. I would expect unilaterally disabling the JIT across all of it would severely degrade the experience for users. Am I missing something or do you have any more detail on this idea?

We wouldn't/won't disable the JIT for DevTools. We haven't disabled any of the JIT levels in the parent process for this reason. But if DevTools goes into a Privileged Content Process, and its no longer running in the parent process, we can disable some or all JIT levels in the parent. (But leave them in the Devtools process.)

Comment 7

[Alex Thayer [:alexical] (she/her)]

Ah okay - I must have interpreted "there" as referring to devtools instead of the parent process. Thank you for clarifying!

Side note: have we looked into what the performance losses look like for disabling the JIT in the parent process (once we ignore devtools)?

Comment 8

[Alexandre Poirot [:ochameau]]

Quick overview about this task.

Bug 1791057 highlights that's DevTools seems to run overall fine in a privileged content process.
There is probably a tail of small breakages around Tooltip/Context menus/... The length of this tail remains to be investigated.

But there is a daunting task, which prevented me from pushing on bug 1791057 patches:
adapting the 2500+ browser mochitests to work with OOP DevTools.
This task is similar to what we had to be for e10s and the massive migration of tests to ContentTask.spawn.

The browser mochitests run by default from the parent process.
Things like gDevTools and its famous const toolbox = await gDevTools.showToolboxForTab(tab, "webconsole") would still work fine.
But it won't be able to return the toolbox object anymore as that's an object which will now be in a content process.
The vast majority of the tests are then running assertion on that toolbox object and the related panels, via const panel = toolbox.getPanel("webconsole").

Let's take a simple test:
https://searchfox.org/mozilla-central/rev/5ff3324fe989a19c6fa5ac5b923089ef4ce2ebb2/devtools/client/styleeditor/test/browser_styleeditor_loading.js#9-36

add_task(async function () {
  // launch Style Editor right when the tab is created (before load)
  // this checks that the Style Editor still launches correctly when it is
  // opened *while* the page is still loading. The Style Editor should not
  // signal that it is loaded until the accompanying content page is loaded.
  const tabAdded = addTab(TESTCASE_URI);
  const tab = gBrowser.selectedTab;
  const styleEditorLoaded = gDevTools.showToolboxForTab(tab, {
    toolId: "styleeditor",
  });

  await Promise.all([tabAdded, styleEditorLoaded]);

  const toolbox = gDevTools.getToolboxForTab(tab);
  const panel = toolbox.getPanel("styleeditor");
  const { panelWindow } = panel;

  ok(
    !getRootElement(panel).classList.contains("loading"),
    "style editor root element does not have 'loading' class name anymore"
  );

  let button = panelWindow.document.querySelector(".style-editor-newButton");
  ok(!button.hasAttribute("disabled"), "new style sheet button is enabled");

  button = panelWindow.document.querySelector(".style-editor-importButton");
  ok(!button.hasAttribute("disabled"), "import button is enabled");
});

Everything except the addTab and showToolbox can be executed from the parent process as-is.
Everything else has to be moved within SpecialPowers.spawn call or some equivalent of it.
Ideally, we would automate such refactoring. But it isn't clear if that's something we can easily automate.
If we can't, it sounds like a daunting manual task.

Comment 9

[Tom Ritter [:tjr]]

Well that's exciting! My question would be - how important is it that all the tests run in the Content Process? It wouldn't be that difficult to have the tests run in the parent (with the JIT enabled) but us to ship Devtools in the Content Process with the JIT disabled in the Parent.

Obviously there shouldn't be no coverage of what we're actually shipping, but would getting a fraction of them cut over (maybe chosen strategically) give enough confidence that we'd be comfortable running the rest the in the parent?