Closed Bug 1566520 Opened 5 months ago Closed 5 months ago

Don't initialize devtools during extension startup

Categories

(WebExtensions :: Developer Tools, task, P3)

task

Tracking

(firefox70 fixed)

RESOLVED FIXED
mozilla70
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)

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?

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

Flags: needinfo?(lgreco)
Summary: Defer building/starting devtools pages → Don't initialize devtools during extension startup

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)

Flags: needinfo?(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.

Flags: needinfo?(jdescottes)
Priority: -- → P3
Assignee: nobody → lgreco
Status: NEW → ASSIGNED
Type: enhancement → task
Flags: needinfo?(lgreco)
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
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70

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!

Flags: needinfo?(lgreco)
Flags: needinfo?(lgreco) → qe-verify-
You need to log in before you can comment on or make changes to this bug.