Closed Bug 1866814 Opened 7 months ago Closed 3 months ago

Merge all JS Window/Process Actors specific per target type into a unique DevTools JS Process Actor in the watcher architecture

Categories

(DevTools :: Framework, task)

task

Tracking

(firefox126 fixed)

RESOLVED FIXED
126 Branch
Tracking Status
firefox126 --- fixed

People

(Reporter: ochameau, Assigned: ochameau)

References

(Blocks 1 open bug, Regressed 1 open bug)

Details

Attachments

(5 files)

In bug 1648499, bug 1651522, bug 1593937 and bug 1633712, lots of code has been duplicated to communicate with each target types (window global, workers, service workers, content processes).

This introduce some maintenance burden as quite some duplicated code.

Also, this may start being a performance bottleneck as we we have to pass a new IPC message / JS Actor message/query per target. This may send duplicated messages to the same content process as soon as we have many target in the same content process, which is very common. Ideally we should send messages once per content process.

Blocks: 1824726
  • The formers JSWindow Actors used independently to create Target Actor for WindowGlobals, Web workers and Service workers
    are all unified behind a single JSProcess Actor pair which was only used to create target actors for DOM Content Processes.

  • The DevToolsProcess JSProcess actor now actively monitor the currently watched target types
    in order to start and stop listeners specific to each target type.
    We no longer rely on JSWindow Actors to observe WindowGlobal instantiations.

  • watchedByDevTools is now consistantly set from each WindowGlobal's content process, via the WindowGlobalTargetActor.
    With the new setup, the parent process no longer track WindowGlobal/BrowsingContext's and so there is no
    natural place to flag them. While, in the content process, the target actor is an obvious place.
    There is just one trick in window global target watcher in order to also set this flag on initial about blank documents.

  • browser-element-host Session Data is now slightly more simple. It works like any other similar session data attribute.
    But we have to ignore any pending exception coming up from updateDomainSessionDataForServiceWorkers call as that be still pending
    while the toolbox closes, which make the JS Process Actor be unregistered,
    and ultimately make the underlying sendQuery promise be rejected.

  • The loader is made specific to each Watcher/connection and put in the connections map,
    so that we can better support content toolbox & browser toolbox usecases
    and we have a unique place to define the loader per watcher/connection.

Blocks: 1883847

The test wasn't selected the newly created tab and was trying to open devtools
on the blank tab.

This tweak helps run the test many times in a row with --run-until-failure.

Assignee: nobody → poirot.alex
Attachment #9391288 - Attachment description: Bug 1866814 - [devtools] Test against the right tab in browser_command_line_urls.js → Bug 1866814 - [devtools] Test against the right tab in browser_command_line_urls.js.
Status: NEW → ASSIGNED
Keywords: leave-open
Pushed by apoirot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9bba11cbefa7
[devtools] Test against the right tab in browser_command_line_urls.js. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/041a44007fb4
[devtools] Wait for the location to be updated in browser_command_line_urls.js. r=devtools-reviewers,nchevobbe
Pushed by apoirot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d27f783be149
[devtools] Merge all DevTools JS Actors into a single DOM Process JS Actor. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/f503f5d02d1e
[devtools] Rename WatcherRegistry to ParentProcessWatcherRegistry. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/5d59dd808307
[devtools] Document the new DevTools backend usage of JS Process Actors. r=devtools-reviewers,bomsy

Backed out for causing bc failures in browser_ext_devtools_inspectedWindow_targetSwitch.js

  • Backout link
  • Push with failures
  • Failure Log
  • Failure line: TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_devtools_inspectedWindow_targetSwitch.js | Test timed out -
    TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_devtools_inspectedWindow_targetSwitch.js | Uncaught exception received from previously timed out test bound - at resource://testing-common/BrowserTestUtils.sys.mjs:538 - Error: The window unloaded while we were waiting for the browser to load - this should never happen.
Flags: needinfo?(poirot.alex)

The WebExtension mochitest failure should be fixed now.

Flags: needinfo?(poirot.alex)
Keywords: leave-open
Pushed by apoirot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a3d3dd0da81d
[devtools] Merge all DevTools JS Actors into a single DOM Process JS Actor. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/2c19eeb928f3
[devtools] Rename WatcherRegistry to ParentProcessWatcherRegistry. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/52bb75377d06
[devtools] Document the new DevTools backend usage of JS Process Actors. r=devtools-reviewers,bomsy
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 126 Branch

== Change summary for alert #42098 (as of Tue, 02 Apr 2024 12:55:03 GMT) ==

Regressions:

Ratio Test Platform Options Absolute values (old vs new)
41% reload-debugger:content-process objects-with-stacks linux1804-64-qr 986.00 -> 1,394.00
39% reload-webconsole:content-process objects-with-stacks linux1804-64-qr 1,001.00 -> 1,396.00
39% reload-webconsole:content-process objects-with-stacks linux1804-64-shippable-qr 1,001.00 -> 1,396.00
39% reload-netmonitor:content-process objects-with-stacks linux1804-64-qr 1,006.00 -> 1,400.00
26% reload-debugger:content-process memory linux1804-64-qr 2,716,672.00 -> 3,417,088.00
10% reload-inspector:content-process objects-with-stacks linux1804-64-qr 4,038.56 -> 4,430.50

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=42098

Regressions: 1889991
Regressions: 1890115
Regressions: 1890247
Regressions: 1898490
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: