1892775 - Run document_start content script without await cssPromise when there is no css

Status: RESOLVED FIXED

(WebExtensions :: General, task)

Description

[Rob Wu [:robwu]]

Bug 1891259 removed a microtask checkpoint, which caused failures in extension tests related to content scripts at document_start, with multiple failures reporting readyState unexpectedly being "interactive" instead of "loading". Because of this, I suspect that this had something to do with await cssPromise (source code link) in the implementation, immediately before the actual content script execution. If anyone is interested in the low level details of how content scripts are scheduled synchronously, take a look at https://bugzilla.mozilla.org/show_bug.cgi?id=1792685#c8

For the record, the failures in https://bugzilla.mozilla.org/show_bug.cgi?id=1891259#c8 are in:

• toolkit/components/extensions/test/xpcshell/test_ext_contentscript.js
• toolkit/components/extensions/test/xpcshell/test_ext_contentScripts_register.js
• toolkit/components/extensions/test/xpcshell/test_ext_scripting_contentScripts.js

Although these failures are only in Android X-nofis (I pulled the logcat logs from CI and grep-filtered them to work around bug 1844829), I can reproduce locally on macOS with --disable-fission:

./mach test toolkit/components/extensions/test/xpcshell/test_ext_contentscript.js --log-mach-verbose --verbose --sequential --disable-fission

xpcshell.toml:toolkit/components/extensions/test/xpcshell/test_ext_contentscript.js
  FAIL test_contentscript_runAt - [test_contentscript_runAt : 157] readyState "interactive" is one of [loading] - false == true

When I make await cssPromise (source code link) conditional by wrapping it in if (cssPromise), then my specific test case passes. This confirms my suspicion that the patch here caused subtle behavioral changes with microtask scheduling.

I tested another way to make the error go away, and I did so by adding the following before await cssPromise:
if (!cssPromise) { context.contentWindow.document.blockParsing(Promise.resolve(), { blockScriptCreated: false, });
... with the same result. This latter is not the right fix but confirms that microtask scheduling was causing the test failures.

I'm going to fix this by making await cssPromise conditional. With this change, content scripts will be executed synchronously at document-element-inserted. This should be safe, since it used to be unsafe but got fixed in bug 1483602.

Comment 1

[Rob Wu [:robwu]]

Attachment: Bug 1892775 - Avoid unnecessary await cssPromise

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/864fdbcf591d
Avoid unnecessary await cssPromise r=willdurand

Comment 3

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/864fdbcf591d