1566520 - Don't initialize devtools during extension startup

Status: RESOLVED FIXED

(WebExtensions :: Developer Tools, task, P3)

Description

[Andrew Swan [:aswan]]

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)

Comment 1

[Andrew Swan [:aswan]]

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?

Comment 2

[Andrew Swan [:aswan]]

Revising the summary based on comment 1. Luca to follow up and evaluate exactly where this should be addressed.

Comment 3

[Luca Greco [:rpl] [:luca] [:lgreco]]

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&regexp=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

[Julian Descottes [:jdescottes]]

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.

Comment 5

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1566520 - Ensure DevToolsShim.getToolboxes returns an empty array if devtools are not yet initialized. r?jdescottes!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
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

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/ad7258b803c7

Comment 8

[Alex Cornestean]

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!