Reducer number of events between parent process code and soon-to-be privileged content proces toolbox
Categories
(DevTools :: Framework, task)
Tracking
(firefox111 fixed)
| Tracking | Status | |
|---|---|---|
| firefox111 | --- | fixed |
People
(Reporter: ochameau, Assigned: ochameau)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
-
"toolbox-unload"
Is fired in middle of the destruction codepath, near the end:
https://searchfox.org/mozilla-central/rev/8fd39dc89c68501a354446a8681ec8263d15e5aa/devtools/client/framework/toolbox.js#4171
And listened by the ToolboxHostManager:
https://searchfox.org/mozilla-central/rev/8fd39dc89c68501a354446a8681ec8263d15e5aa/devtools/client/framework/toolbox-host-manager.js#117
to destroy itself, with a delay.
Here it looks like we rather want to wait for full toolbox destruction (given the delay), so it makes sense to try to listen to "toolbox-destroyed". Which is fired the latest. -
"toolbox-created"
Was added only for a test. It would most likely change drasticaly once we move the toolbox OOP. It's actual precise time of firing will most likely no longer be relevant for the test verifying a race condition in many toggling of a unique toolbox. -
"toolbox-destroy"
Is only used from WebExt codebase over there:
https://searchfox.org/mozilla-central/rev/8fd39dc89c68501a354446a8681ec8263d15e5aa/browser/components/extensions/parent/ext-devtools.js#377
It can't easily move to toolbox-destruction as it queries toolbox.commands, which is already nullified when this event is fired.
Toolbox.commands will most likely no longer exists once the toolbox runs in another process, so we might want to early revisit this implementation.
|
Assignee | |
Updated•1 year ago
|
|
|
Assignee | |
Comment 1•1 year ago
|
||
This toolbox-unload was only fired for ToolboxHostManager own usage.
On top of that, ToolBoxHostManager was delaying its destruction by an additional
event loop. It indicates that this toolbox-unload event was too early.
So it rather makes sense to use the latest event in the destruction codepath.
i.e. toolbox's "destroyed" event.
I kept the event loop tick as toolbox-destroyed is still slightly too early.
The debugger is still going through pending async work in browser_dbg-breakpoints-in-evaled-sources.js.
This relates to the promise middleware which delay each promise action
by an event loop tick using setTimeout. setTimeout throws once we remove the host iframe.
I first tried to remove this event loop tick.
It highlighted two possible intermittents in two tests involving viewSourceInDebugger.
These tests weren't waiting for the full processing of opened sources by the Debugger frontend.
Using debugger shared-head helpers ensure waiting for full source selection.
|
||
Updated•1 year ago
|
|
|
Assignee | |
Comment 2•1 year ago
|
||
This event was only used in this test.
Unfortunately, I wasn't able to keep the same logic in the test while keeping it reliable.
I prefer to remove this test-only event and simplify the test to only ensure
we safely ignore subsequent calls to toggle.
What is hard to know from the test, when calling toggle in some following event loops
is if this will be ignored -or- close/open a toolbox...
|
|
Assignee | |
Comment 3•1 year ago
|
||
Pushed by apoirot@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8ddfebd080fb [devtools] Move manager destruction from toolbox-unload to destroyed. r=devtools-reviewers,bomsy https://hg.mozilla.org/integration/autoland/rev/9e752ce5baed [devtools] Remove "toolbox-created" event. r=devtools-reviewers,perftest-reviewers,sparky,jdescottes
|
||
Comment 5•1 year ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/8ddfebd080fb
https://hg.mozilla.org/mozilla-central/rev/9e752ce5baed
Description
•