Dynamic content script registration may be incomplete when a new process spawns during the registration
Categories
(WebExtensions :: General, defect, P2)
Tracking
(firefox99 fixed)
Tracking | Status | |
---|---|---|
firefox99 | --- | fixed |
People
(Reporter: robwu, Assigned: willdurand)
References
Details
(Whiteboard: [addons-jira])
Attachments
(1 file)
A dynamically registered content script may not actually be registered correctly when a new process spawns during the registration process. Consequently, such content scripts may not actually execute in web pages that are hosted in that content process.
The current implementation of dynamic content script registrations has the following logic to register scripts:
- Broadcast
Extension:RegisterContentScript
with the script options. - Update the shared memory with dynamic content script registrations.
Examples where content scripts are being registered:
- https://searchfox.org/mozilla-central/rev/211649f071259c4c733b4cafa94c44481c5caacc/toolkit/components/extensions/parent/ext-contentScripts.js#198-205
- https://searchfox.org/mozilla-central/rev/211649f071259c4c733b4cafa94c44481c5caacc/toolkit/components/extensions/parent/ext-userScripts.js#130-137
- https://phabricator.services.mozilla.com/D134154?id=541346#C4776460NL193
The registrations are used as follows:
- In existing processes, the registration is received via the
Extension:RegisterContentScript
messages and processed at https://searchfox.org/mozilla-central/rev/211649f071259c4c733b4cafa94c44481c5caacc/toolkit/components/extensions/ExtensionProcessScript.jsm#249-280 - In new processes, the shared memory is read at https://searchfox.org/mozilla-central/rev/211649f071259c4c733b4cafa94c44481c5caacc/toolkit/components/extensions/ExtensionProcessScript.jsm#178.
An issue occurs when a new process spawns between the (1) Extension:RegisterContentScript
broadcast and the (2) shared memory update. In that case, the content script is not correctly registered and will therefore not run.
To fix this issue, the two should be reversed, and the implementation has to be revised to account for the swapped logic.
STR (simulation):
- Launch Firefox with any extension with host permissions for example.com (e.g. via
web-ext run
). - Allow the extension access to private browsing mode via
about:addons
. - Visit about:debugging, find the extension and inspect its background page (or click on the
manifest.json
link to open a new tab and open the devtools for it). - Open the Browser Content Toolbox, search for ExtensionProcessScript.jsm and put a breakpoint at the
Extension:RegisterContentScript
handler to simulate a slow-to-respond process, at https://searchfox.org/mozilla-central/rev/211649f071259c4c733b4cafa94c44481c5caacc/toolkit/components/extensions/ExtensionProcessScript.jsm#253 - Switch to the devtools from step 3, and run the following snippet:
s = await browser.contentScripts.register({
css: [{code:"*{background:cyan}"}],
matches: ["*://example.com/"],
})
- The breakpoint should be triggered in the devtools from step 4.
- Open a private browsing window, visit
about:processes
and look at the pre-allocated processes. - Repeat this step until every preallocated process that was initially shown at step 7 has disappeared (e.g. a few times): Visit
example.com
in a private window and close the window.
- This step is needed, because we want to test the behavior with a new process spawn. There may be pre-allocated processes that aren't rendering content, but are already processing messages.
- Visit example.com in a private browsing window once more, and look at the background color.
- Return to the devtools from step 4/6, and resume execution.
- Refresh the tab from step 9, and look at the background color.
Expected:
- At step 8, 9 and 11, the background color should always be cyan.
Actual:
- At step 8, the first few attempts may indeed have a cyan background.
- At step 9, the background doesn't change, which means that the content script isn't registered in the process.
- At step 11, the background doesn't change, which means that the issue is persistent.
Reporter | ||
Updated•3 years ago
|
Updated•3 years ago
|
Assignee | ||
Comment 1•3 years ago
|
||
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Updated•3 years ago
|
Comment 3•3 years ago
|
||
Backed out for causing xpcshell failures on test_ext_contentscript_dynamic_registration.js
Failure line(s): TEST-UNEXPECTED-FAIL | xpcshell.ini:toolkit/components/extensions/test/xpcshell/test_ext_contentscript_dynamic_registration.js | test_contentScripts_register - [test_contentScripts_register : 96] expected PID 1 to not be in [0, 1]) - false == true
Comment 5•3 years ago
|
||
Backed out changeset 9ae4e33f83bc (bug 1756495) for causing xpc failures in xpcshell/test_ext_contentscript_dynamic_registration.js
Backout link: https://hg.mozilla.org/integration/autoland/rev/2b935e00ff63f8a937e135ae77f04679ae97e08c
Comment 7•3 years ago
|
||
bugherder |
Assignee | ||
Updated•3 years ago
|
Description
•