Closed Bug 1580184 Opened 5 years ago Closed 5 years ago

Duplicated JSMs are displayed in the omniscient toolbox when reloading pages

Categories

(DevTools :: Debugger, defect, P1)

defect

Tracking

(firefox72 fixed)

RESOLVED FIXED
Firefox 72
Tracking Status
firefox72 --- fixed

People

(Reporter: ochameau, Assigned: bhackett1024)

References

(Blocks 1 open bug)

Details

(Whiteboard: dt-fission-m1)

Attachments

(2 files)

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

Priority: -- → P2
Whiteboard: dt-fission

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

Flags: needinfo?(poirot.alex)

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.

Flags: needinfo?(poirot.alex)

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

Attached image duplicated sources (deleted) —

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.

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.

Summary: Duplicated sources are displayed when reloading pages and fission → Duplicated JSMs are displayed in the omniscient toolbox when reloading pages

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

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.

Assignee: nobody → bhackett1024
Status: NEW → ASSIGNED
Priority: P2 → P1

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.

Pushed by bhackett@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/321a4d33f9b6 Avoid creating duplicate entries in the source tree, r=jlast.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 72
Flags: qe-verify+
Whiteboard: dt-fission → dt-fission dt-fission-m1

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.

Flags: qe-verify+
Whiteboard: dt-fission dt-fission-m1 → dt-fission-m1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: