1580184 - Duplicated JSMs are displayed in the omniscient toolbox when reloading pages

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P1)

Description

[Alexandre Poirot [:ochameau]]

When enabling fission via fission.autostart, many JSMs get duplicated everytime you reload a page.

I think that it comes from this code, which only consumes new thread fronts and never cleanup sources:
https://searchfox.org/mozilla-central/source/devtools/client/debugger/src/client/firefox/commands.js#431-453

Comment 1

[Jason Laster [:jlast]]

Alex, can you give me a small STR? Is it about opening and closing devtools?

Comment 2

[Alexandre Poirot [:ochameau]]

I'm getting issues with reproducing this STR as I no longer see much sources, making it hard to know which content process to look into.
Simply opening the browser toolbox and then opening data: URI or any http URI won't appear in the source tree.
STR:

• toggle fission.autostart and omniscient pref to true
• restart firefox
• open the browser toolbox first
• open techno-barje.fr, or any data: URI (but you have to open each "no domain" section to confirm next step)
• sources do not appear

Then the STR as I recall it was to open multiple pages, and trigger the "Find in page" (CmdOrCtrl+F) for each page,
then I was seeing one FindBarChild.jsm per page I opened, whereas there should be only one per content process.
If you happen so see any .jsm file more than once in the a given process, that is a bug as they should be singleton per process.

Comment 3

[Jason Laster [:jlast]]

It looks like the Browser toolbox is missing content sources when I:

1. start firefox with fission.autostart and browsertoolbox.fission set to true
2. open the browser toolbox and content toolbox
3. go to https://firefox-dev.tools/debugger-examples/examples/todomvc/
4. navigate to https://firebugs.dev/

ER: see files like Meta.js
AR: missing files like Meta.js

Comment 4

[Alexandre Poirot [:ochameau]]

Attachment: duplicated sources

I'm now able to reproduce the original issue about FindBarChild.jsm. This is actually not related to fission.autostart preference.

Here is the STR:

• set devtools.browsertoolbox.fission to true
• open https://towering-blender.glitch.me/
• open the omniscient browser toolbox
• now reload the page a couple of time (this will break as there is a debugger statement, ensure resuming. I'm not sure this)
• In the source tree of the content process where this page is running (you can see its domain), open resource://gre and actors folder

ER: See only one FindBar.jsm module
AR: There is many.

Let's keep this bug for the duplicated sources and open another for any other issue.

Comment 5

[Alexandre Poirot [:ochameau]]

I'm now able to reproduce the original issue about FindBarChild.jsm. This is actually not related to fission.autostart preference.

Here is the STR:

• set devtools.browsertoolbox.fission to true
• open https://towering-blender.glitch.me/
• open the omniscient browser toolbox
• now reload the page a couple of time (this will break as there is a debugger statement, ensure resuming. I'm not sure this)
• In the source tree of the content process where this page is running (you can see its domain), open resource://gre and actors folder

ER: See only one FindBar.jsm module
AR: There is many.

Let's keep this bug for the duplicated sources and open another for any other issue.

Comment 6

[Brian Hackett [Laid off!]]

I can't reproduce this. Does this happen reliably for you with a fresh profile?

Comment 7

[Alexandre Poirot [:ochameau]]

It was reproducing easily when I submitted my last comment.
It no longer does.

It is now intermittent, and rarely reproduces.
I'm not able to reproduce FindBar.jsm duplication. I only got resource://gre/actors/ThumbnailsChild.jsm to be duplicated. But again, it doesn't always reproduce.

My last comment was without fission.autostart. But If I toggle this pref, it looks like it reproduces slightly more frequently. I was able to see FindBar.jsm duplicated once.
When I was demoing in Berlin, I was sharing my screen, so all this may be related to some race.

Comment 8

[Brian Hackett [Laid off!]]

I'm still having problems with the original STR, but see what I think is the same issue for other files. When I load the URL and expand all the source directories for the content process, I'll usually see a few source files duplicated. Additionally, expanding and collapsing the directory containing the duplicated files leads to some weird behavior, where there are more and more duplicate entries for the file, which don't disappear when the directory is closed. These two issues are interrelated; creating duplicate source tree entries ends up creating React list entries with the same key, which causes React to create multiple DOM elements with the same id and break its internal bookkeeping.

The duplicates arise because the source tree's addToTree method is able to create multiple identical entries for the same source. I've attached what I think is an appropriate fix, but this code is more confusing than I think it should be and further simplification would be nice.

Comment 9

[Brian Hackett [Laid off!]]

Attachment: Bug 1580184 - Avoid creating duplicate entries in the source tree, r=jlast.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/321a4d33f9b6
Avoid creating duplicate entries in the source tree, r=jlast.

Comment 11

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/321a4d33f9b6

Comment 12

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/321a4d33f9b6

Comment 13

[Cristian Fogel [:cfogel]]

In attempt to verify this issue, I was unable to reproduce the issue mentioned in the comments.
Considering the race condition status, I've lost it on both Windows 10 and an Ubuntu 18 with 71.0a1 (2019-10-02) and no duplicates came up as per comments 4/5.

In regards to this, clearing the QE+ flag.