Closed Bug 1660186 Opened 4 years ago Closed 4 years ago

Components can run in the devtools process when they shouldn't

Categories

(Thunderbird :: General, defect)

defect

Tracking

(thunderbird_esr78 wontfix)

RESOLVED FIXED
82 Branch
Tracking Status
thunderbird_esr78 --- wontfix

People

(Reporter: darktrojan, Assigned: darktrojan)

References

Details

Attachments

(2 files)

I've just discovered that by opening the developer toolbox and doing certain things, I can cause the devtools process to do things it shouldn't be able to do. In my case it was starting up the address book manager, loading all of the address books and attempting to contact the servers of my CardDAV directories. (Four username/password dialogs appeared, when I currently have three such directories set up. This is how I established what was happening, there were four set up when my devtools profile was created, and it still has the prefs from then.) It may also explain why I've been seeing GMail authorisation dialogs.

I think I can solve this by setting ProcessSelector.MAIN_PROCESS_ONLY in the components.conf file of each component – a step I was planning anyway on the route to e10s.

That does not stop components loading in the devtools process. :-(

I know of no other way to do this. The environment variable is set as the toolbox process is
started, so it should be a reliable indicator.

Not registering with the observer service prevents many unnecessary and unwanted components from
starting, and should prevent the bug as first discovered.

Target Milestone: --- → 82 Branch

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/00ba88f1df55
Avoid starting mail components when in the developer toolbox process. r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED

Geoff - the implementation in your patch enables users/programs to easily disable components by just setting one environment variable. Is that desirable?

If it is not, consider also checking the argv, as the toolbox also includes a unique flag: https://searchfox.org/mozilla-central/rev/73a14f1b367948faa571ed2fe5d7eb29460787c1/devtools/client/framework/browser-toolbox/Launcher.jsm#249-250

... and also consider adding a regression test to ensure that this continues to work despite potential future refactors.

No, that isn't desirable, but it does fail spectacularly. You make a good point, I should add a second check, even if it is one that's prone to regressing every time the URL changes.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---

I should add that the reason I didn't use the command line arguments earlier is that I haven't figured out how to get hold of them.

A way to do so is by registering a nsICommandLineHandler.
Here is how the chrome param is handled: https://searchfox.org/comm-central/rev/68e09f5b6f83daeee9db9955de2d3334fe45d19a/mozilla/browser/components/BrowserContentHandler.jsm#443

I know you can do that but it happens way too late. Perhaps our whole start-up sequence is out of order, that wouldn't surprise me.

This way of testing for the devtools process is less prone to failure (malicious or otherwise).

Comment on attachment 9172338 [details]
Bug 1660186 - Avoid starting mail components when in the developer toolbox process, take 2. r?mkmelin

Thanks for the idea Rob.

Attachment #9172338 - Flags: feedback?(rob)

Comment on attachment 9172338 [details]
Bug 1660186 - Avoid starting mail components when in the developer toolbox process, take 2. r?mkmelin

I provided feedback on Phabricator @ https://phabricator.services.mozilla.com/D88384#inline-503343

Attachment #9172338 - Flags: feedback?(rob) → feedback+

Pushed by thunderbird@calypsoblue.org:
https://hg.mozilla.org/comm-central/rev/31541aadcddb
Avoid starting mail components when in the developer toolbox process, take 2. r=mkmelin,robwu

Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/e495b3715362
follow-up - Remove two modules from the start-up performance test whitelist. rs=bustage-fix
No longer depends on: 1732515
Regressions: 1732515
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: