1766822 - Optional permission changes are not propagated to prelaunched processes

Status: RESOLVED FIXED

(WebExtensions :: General, defect, P1)

Description

[Rob Wu [:robwu]]

Correct propagation of (optional) permissions is very important to ensure that content scripts can run when the user approves an optional permission (bug 1745819). This was previously not done correctly for new processes, which got addressed in bug 1760526. The fix is not complete, however:

When a test was added that opened multiple tabs (described in this review comment, implemented in D144811 as toolkit/components/extensions/test/mochitest/test_ext_scripting_permissions.html as part of bug 1760451, the test failed unexpectedly: the test got stuck because the registered content script did not run after granting an optional permission.

Setting dom.ipc.processPrelaunch.enabled to false is a work-around to the issue.

The issue is caused by the fact that we rely on BrowserExtensionContent to update policy.allowedOrigins, after the construction of WebExtensionPolicy from sharedData in ExtensionProcessScript, but the BrowserExtensionContent instance is lazily instantiated, via one of the extensions.get(policy) calls. In case of prelaunched processes, there is the possibility that the permission got updated before the construction of BrowserExtensionContent, and consequently the bookkeeping of policy.allowedOrigins goes wrong.

Comment 1

[William Durand [:willdurand] [OOO until Aug. 25]]

Attachment: Bug 1766822 - Add a test case to reveal the bug. r?robwu!

This new test file contains a test case that exposes an issue described
in Bug 1766822. This test file hangs after having loaded example.org
with no particular CLI flag passed to mach test.

Note that when we pass --disable-fission, the test file passes in most
cases but not all the time with --verify, which is why I didn't enable
more prefs (e.g., dom.ipc.processPrelaunch.enabled).

Comment 2

[Rob Wu [:robwu]]

Attachment: Bug 1766822 - Refactor, introduce updateAllowedOrigins

Introduce common updateAllowedOrigins helper to avoid repetition of
policy.allowedOrigins-updating logic in four places.

While there are no user-visible behavioral changes, the patch does fix a
small issue: previously policy.allowedOrigins could have multiple
MatchPattern instances with exactly the same pattern due to the
implementation in ExtensionChild, now the helper ensures that all
patterns are unique.

Comment 3

[Rob Wu [:robwu]]

Attachment: Bug 1766822 - Decouple WebExtensionPolicy update from BrowserExtensionContent

This ensures that WebExtensionPolicy is properly initialized even when
ExtensionProcessScript has not instantiated BrowserExtensionContent yet.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4d2c16b03c1f
Add a test case to reveal the bug. r=robwu
https://hg.mozilla.org/integration/autoland/rev/23866e9385ac
Refactor, introduce updateAllowedOrigins r=willdurand
https://hg.mozilla.org/integration/autoland/rev/6b15391a53cf
Decouple WebExtensionPolicy update from BrowserExtensionContent r=willdurand,zombie

Comment 5

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/4d2c16b03c1f
https://hg.mozilla.org/mozilla-central/rev/23866e9385ac
https://hg.mozilla.org/mozilla-central/rev/6b15391a53cf