1696862 - With Fission enabled Multi-Process Browser Toolbox hangs with 100% CPU load when trying to inspect browser element of WebExtension sidebar

Status: RESOLVED FIXED

(DevTools :: Inspector, defect, P3)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:88.0) Gecko/20100101 Firefox/88.0 ID:20210302134918

Here the steps to reproduce:

1. Temporary install the extension from https://github.com/whimboo/perfchaser via about:debugging
2. Open the Multi-Process Browser Toolbox
3. Select the Inspector and choose webext-panels.xhtml to be inspected
4. Open the stack element with id webext-panels-stack
5. Open the contained browser element

After step 5 the MPBT should freeze, if not immediately switch to the debugger pane and back to the inspector.

Comment 1

[Julian Descottes [:jdescottes]]

Thanks for the report! FWIW step3 can be skipped and still trigger the same issue.

We fail to find (& create I guess) the target corresponding to the remote <browser> element that we try to expand in the STRs.
The corresponding nodeFront has remoteFrame=true and browsingContextID set to the correct value. But WatcherFront::getBrowsingContextTarget doesn't have a target corresponding to the browsingContextID of the remote browser and therefore we fallback on the parent target where we can't find the correct children of the browser element.

https://searchfox.org/mozilla-central/rev/5a66c4b4a41ab78a87c30c9db0d93c732c534402/devtools/client/fronts/watcher.js#103-131

    // If we could not find a browsing context target for the provided id, the
    // browsing context might not be the topmost browsing context of a given
    // process. For now we only create targets for the top browsing context of
    // each process, so we recursively check the parent browsing context ids
    // until we find a valid target.
    const parentBrowsingContextID = await this.getParentBrowsingContextID(id);
    if (parentBrowsingContextID && parentBrowsingContextID !== id) {
      return this.getBrowsingContextTarget(parentBrowsingContextID);
    }

Comment 2

[Julian Descottes [:jdescottes]]

whimboo: The bug is definitely valid, but I was curious to know why you wanted to debug your webextension via the MBT instead of the "webextension" toolbox you can get from about:debugging? Was it just for a random reason or is there something that doesn't work for you with the about:debugging workflow?

Comment 3

[Julian Descottes [:jdescottes]]

(whimboo: see question in comment above)

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Oh, I just didn't know that this should be done via about:debugging. Will remember that. Thanks.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Julien, I saw the same behavior just now again but with other steps that are kinda worrying:

I wanted to debug some code of the browser and as such opened the MPBT. Once it has been opened I selected the debugger panel to navigate to the wanted .js file. But the process immediately froze and I can only see the beach ball. The page sources list was actually empty.

Could this be the same problem? Note that I have Fission enabled, similar to the original issue on that bug.

Comment 6

[Julian Descottes [:jdescottes]]

Thanks for the info!

Could you file another bug for the Debugger issue, especially if you have reproducible STRs?

I believe they are 2 different issues because the initial problem is that the inspector UI fails to retrieve the children of the remote <browser> element. Long story short, it gets confused about which target it should use to fetch the <browser>'s children, and ends up retrieving the children of the parent <browser> element. This leads to a cycle in the DOM tree it tries to represent, which leads to several infinite recursion issues in the codebase.

We could try to do a quick workaround to at least verify that the two issues are unrelated. I'll try to provide a quick patch during the day, which we can apply locally to verify that the debugger issue is separate. Note: the workaround will not allow to expand the <browser> element, because for now I don't think we have a way to create a Target for it. I just plan to bail out and return an empty list of children.

Comment 7

[Julian Descottes [:jdescottes]]

Attachment: Bug 1696862 - Workaround to avoid inspector hang with MBT when inspecting webextension browser el

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Sorry, but after a restart of Firefox the hang of the MPBT doesn't happen anymore when opening it for the debugger. As such I don't have a chance to verify the change of behavior with your attached patch.

Comment 9

[Julian Descottes [:jdescottes]]

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #8)

Sorry, but after a restart of Firefox the hang of the MPBT doesn't happen anymore when opening it for the debugger. As such I don't have a chance to verify the change of behavior with your attached patch.

No worries, let us know if it comes back.

Let's block this bug on Bug 1642599. :nchevobbe will attempt to remove the webextension frame filter at https://searchfox.org/mozilla-central/rev/aa9a7136835deb0eeba00c62bb50a4a0e2cdea2d/devtools/server/actors/watcher/target-helpers/utils.js#31-34

This would allow us to get a valid target for the remote browser element of a webextension.
We might keep this bug to add a small inspector test case, unless we add one directly in Bug 1642599.

Comment 10

[Nicolas Chevobbe [:nchevobbe]]

I'll try to do that fix and add a test in this bug directly :)

Comment 11

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1696862 - [devtools] Don't filter out webextensions in shouldNotifyWindowGlobal. r=jdescottes,ochameau.

This prevented existing webextension documents targets to not be created, which
caused a few issues (couldn't expand webextension browser element in the inspector,
missing webextension messages in the browser console when watcher support is enabled, …).

A test is added for the inspector test case (for the console, a test already exists).

Depends on D108394

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0893961d7bc2
[devtools] Don't filter out webextensions in shouldNotifyWindowGlobal. r=jdescottes.

Comment 13

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/0893961d7bc2