Closed Bug 1756495 Opened 10 months ago Closed 9 months ago

Dynamic content script registration may be incomplete when a new process spawns during the registration

Categories

(WebExtensions :: General, defect, P2)

defect

Tracking

(firefox99 fixed)

RESOLVED FIXED
99 Branch
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:

  1. Broadcast Extension:RegisterContentScript with the script options.
  2. Update the shared memory with dynamic content script registrations.

Examples where content scripts are being registered:

The registrations are used as follows:

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):

  1. Launch Firefox with any extension with host permissions for example.com (e.g. via web-ext run).
  2. Allow the extension access to private browsing mode via about:addons.
  3. 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).
  4. 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
  5. Switch to the devtools from step 3, and run the following snippet:
s = await browser.contentScripts.register({
  css: [{code:"*{background:cyan}"}],
  matches: ["*://example.com/"],
})
  1. The breakpoint should be triggered in the devtools from step 4.
  2. Open a private browsing window, visit about:processes and look at the pre-allocated processes.
  3. 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.
  1. Visit example.com in a private browsing window once more, and look at the background color.
  2. Return to the devtools from step 4/6, and resume execution.
  3. 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.
Whiteboard: [addons-jira]
Assignee: nobody → wdurand
Severity: -- → N/A
Priority: -- → P2
Attachment #9265196 - Attachment description: WIP: Bug 1756495 - Ensure script registration is complete when a new process is spawned during the registration. → Bug 1756495 - Ensure script registration is complete when a new process is spawned during the registration. r?robwu!
Pushed by wdurand@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d6a3e5603500
Ensure script registration is complete when a new process is spawned during the registration. r=robwu

Backed out for causing xpcshell failures on test_ext_contentscript_dynamic_registration.js

Backout link

Push with failures

Failure log

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

Flags: needinfo?(wdurand)
Pushed by wdurand@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9ae4e33f83bc
Ensure script registration is complete when a new process is spawned during the registration. r=robwu

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

Push with failures

Failure log

Pushed by wdurand@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b1d3464943fa
Ensure script registration is complete when a new process is spawned during the registration. r=robwu
Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → 99 Branch
Flags: needinfo?(wdurand)
You need to log in before you can comment on or make changes to this bug.