Replace `target.getInspector()` by `target.getFront("inspector")`
Categories
(DevTools :: Inspector, enhancement)
Tracking
(firefox70 fixed)
| Tracking | Status | |
|---|---|---|
| firefox70 | --- | fixed |
People
(Reporter: ochameau, Assigned: gl)
References
Details
Attachments
(1 file)
The inspector front should be retrieved like any other target-scoped actor, via target.getFront("inspector").
But because of some race condition leading to failures reported in bug 1493131, we had to add a dedicated workaround: target.getInspector().
We should get rid of that eventually, for these reasons:
- clarity:
It is surprising that the inspector is retrieved differently from all the other fronts. - sanity:
This hides some complexity in how the inspector is created/destroyed. It would be helpful to better understand why the inspector is complex and fix the underlying races. Note that bug 1567860 is already aiming at simplifying the destroy codepath of the inspector. So, this work may benefit from this other one. Or the other way around?
We have no proof yet that it will be a blocker for fission. But I suspect that the complexity of creation/destruction of the inspector front will be an issue when it comes to instanciate more than one.
Also. When looking into this, it is good to know one particularity of the inspector front.
It has an async constructor:
https://searchfox.org/mozilla-central/rev/4436573fa3d5c4311822090e188504c10c916c6f/devtools/shared/fronts/inspector.js#475-485
Only the inspector and performance fronts are using this async feature.
It means that when you call target.getFront("inspector") or target.getInspector(), we have to wait for that initialize method to complete. It may explain some issues...
Finally, this work is most likely bound to bug 1568150, focusing on cleaning up the destroying of this front. I have which work depends on which other one. It might be possible that both cleanup are tightly coupled and might need to land at the same time.
|
Assignee | |
Updated•5 years ago
|
|
|
Assignee | |
Comment 1•5 years ago
|
||
|
|
Assignee | |
Comment 2•5 years ago
|
||
Pushed by gluong@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/531e06bae805 Replace `target.getInspector()` by `target.getFront("inspector")`. r=ochameau
|
||
Comment 4•5 years ago
|
||
Backed out or failing browser_ext_devtools_panel
Failure log https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=260197866&repo=autoland&lineNumber=17489
Backout: https://hg.mozilla.org/integration/autoland/rev/99f5aa73e22800bbd18c1fe10a4a2814040ce53a
|
||
Updated•5 years ago
|
Pushed by gluong@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3df8752de015 Replace `target.getInspector()` by `target.getFront("inspector")`. r=ochameau
|
|
Assignee | |
Comment 6•5 years ago
|
||
(In reply to Andreea Pavel [:apavel] from comment #4)
Backed out or failing browser_ext_devtools_panel
Failure log https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=260197866&repo=autoland&lineNumber=17489
Backout: https://hg.mozilla.org/integration/autoland/rev/99f5aa73e22800bbd18c1fe10a4a2814040ce53a
Relanded. I ran into issues with rapid closing and reopening of the inspector toolbox with browser_ext_devtools_panel, where the target and iframeWindow could be null.
|
|
Assignee | |
Updated•5 years ago
|
|
||
Comment 7•5 years ago
|
||
| bugherder | ||
Description
•