1642012 - ExtensionProcessScript WebExtensionPolicy / stub initialization logic needs work

Status: NEW

(WebExtensions :: General, defect, P2)

Description

[Rob Wu [:robwu]]

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.

Comment 1

[Rob Wu [:robwu]]

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.

Comment 2

[Rob Wu [:robwu]]

Attachment: Bug 1642012 - Fix WebExtensionPolicy (stub) initialization

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.

Comment 3

[Rob Wu [:robwu]]

Attachment: Bug 1642012 - Fix WebExtensionPolicy stub init in existing processes + tests

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.

Comment 4

[Emma Malysz]

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