Don't initialize devtools during extension startup
Categories
(WebExtensions :: Developer Tools, task, P3)
Tracking
(firefox70 fixed)
Tracking | Status | |
---|---|---|
firefox70 | --- | fixed |
People
(Reporter: aswan, Assigned: rpl)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
If an extension defines a devtools_page, the page is created and started from onManifestEntry()
. We should use logic similar to what we use for background pages to defer this until much later in startup.
(fwiw, AdBlock Plus with >9 million users has a devtools_page so this does impact a significant number of users)
Reporter | ||
Comment 1•5 years ago
|
||
Hm, I think I confused the build()
methods on the actual page and the page definition object here:
https://searchfox.org/mozilla-central/rev/22b330ecb3edba1536a54887060cbdd09db21c59/browser/components/extensions/parent/ext-devtools.js#382
Still, this call causes devtools to be initialized:
https://searchfox.org/mozilla-central/rev/22b330ecb3edba1536a54887060cbdd09db21c59/browser/components/extensions/parent/ext-devtools.js#310
https://searchfox.org/mozilla-central/rev/22b330ecb3edba1536a54887060cbdd09db21c59/devtools/startup/DevToolsShim.jsm#319
Perhaps this is more of a devtools bug than extensions, but it seems like getToolboxes() should not have the side effect of initializing devtools if it hasn't already been initialized. Seems like it could just return [] in this case?
Reporter | ||
Comment 2•5 years ago
|
||
Revising the summary based on comment 1. Luca to follow up and evaluate exactly where this should be addressed.
Assignee | ||
Comment 3•5 years ago
|
||
Hey Julian,
it seems that one option to fix this issue could be to just call DevToolsShim.isInitialized() from ext-devtools.js and avoid calling DevToolsShim.getToolboxes if the devtools are not yet initialized.
Currently DevToolsShim.isInitialized
seems to only be called internally by the shim itself and in a test case (https://searchfox.org/mozilla-central/search?q=DevToolsShim.isInitialized&case=false®exp=false&path=), would you prefer to actually handle it in a DevToolsShim.getToolboxes
method?
(e.g. by defining the DevToolsShim.getToolboxes()
method so that it calls this.isInitialized() internally and returns an empty array if the devtools are not yet initialized)
Comment 4•5 years ago
|
||
Both options sound ok to me. Calling isInitialized()
is probably the easiest to get started, but if you want to modify the DevToolsShim more so that it doesn't initialize devtools for other webextension methods, we can also look at that.
Updated•5 years ago
|
Assignee | ||
Comment 5•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Pushed by luca.greco@alcacoop.it: https://hg.mozilla.org/integration/autoland/rev/ad7258b803c7 Ensure DevToolsShim.getToolboxes returns an empty array if devtools are not yet initialized. r=jdescottes
Comment 7•5 years ago
|
||
bugherder |
Comment 8•5 years ago
|
||
Hello,
Will this fix require manual validation? If yes, please provide some steps to reproduce in order to correctly test it and also, please set the "qe-verify+" flag. Otherwise, could the "qe-verify-" flag be added? Thanks!
Assignee | ||
Updated•5 years ago
|
Description
•