Open Bug 1642012 Opened 4 years ago Updated 3 years ago

ExtensionProcessScript WebExtensionPolicy / stub initialization logic needs work

Categories

(WebExtensions :: General, defect, P2)

defect

Tracking

(Not tracked)

People

(Reporter: robwu, Assigned: robwu, NeedInfo)

References

Details

Attachments

(2 files)

ExtensionProcessScript has a "change" event handler, but does not call Services.cpmm.sharedData.addEventListener, so it won't process changes to pending extensions: https://searchfox.org/mozilla-central/rev/8827278483c337667cdfb238112eb1be397dd102/toolkit/components/extensions/ExtensionProcessScript.jsm#279-286

Conversely, there is some logic (e.g. updateStubExtensions -> initStubPolicy -> construct active WebExtensionPolicy) that should be conditional on isContentProcess because it could otherwise be confused with the WebExtensionPolicy from the main process if extensions.webextensions.remote is false (which is unfortunately still the case on GeckoView - bug 1535365).
If the main process' policy is replaced, then policy.extension could become unset, which is unexpected (I started to look into this because I saw a patch that seems to suggest that policy.extension could be unset).

Need to take a closer look at ExtensionPolicyScript.jsm and fix the bugs there.

Severity: -- → S2
Type: task → defect
Priority: -- → P2
See Also: → 1651697
See Also: 1651697

This bug turns out to be (partly responsible for) causing bug 1651697 after all, because it is the only way for a WebExtensionPolicy instance to exist without an "extension" member but with a "instanceId" member, with the following sequence of events:

  1. Extension starts up and broadcasts Extension:Startup - https://searchfox.org/mozilla-central/rev/622dbd3409610ad3f71b56c9a6a92da905dab0aa/toolkit/components/extensions/Extension.jsm#2245
  2. Extension fails to load (due to bug 1651838), and uninitializes the extension - at this point WebExtensionPolicy is marked as inactive, as expected.
  3. Extension:Startup message is received in the main process, which calls initExtensionPolicy (which is supposed to only do something in a child process): https://searchfox.org/mozilla-central/rev/622dbd3409610ad3f71b56c9a6a92da905dab0aa/toolkit/components/extensions/ExtensionProcessScript.jsm#280
  4. Under normal circumstances, there is an active extension, so this code would be no-op: https://searchfox.org/mozilla-central/rev/622dbd3409610ad3f71b56c9a6a92da905dab0aa/toolkit/components/extensions/ExtensionProcessScript.jsm#193-195
    ... however, because the extension was unregistered at step 2, the !policy condition is matched, and a WebExtensionPolicy is initialized.
Blocks: 1651697

Fix ExtensionProcessScript.jsm:

  • Ensure that a WebExtensionPolicy stub is only initialized in the content
    process, because the main process already created by Extension.jsm

  • Ensure that a WebExtensionPolicy stub is correctly unregistered when
    startup failed.

  • Add TODO to note that WebExtensionPolicy stubs are not updated in
    existing processes.

Add sharedData "change" listener to fix initialization of
WebExtensionPolicy stubs in existing content processes.

And add unit tests to verify that the stubs are created as expected in
content processes, not only as a regression test for this, but also for
bug 1518863 where the logic was originally introduced.

Hey Rob, do you plan on taking a look at these patches?

Flags: needinfo?(rob)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: