1951688 - Refactor content script/style injection to clearly separate compilation from execution

Status: RESOLVED FIXED

(WebExtensions :: General, task, P1)

Description

[Rob Wu [:robwu]]

In order to run content scripts in order (of extension declaration in the manifest), we need to ensure that the order of content script execution (and style injection) is stable.

The asynchronous part of this is mostly from compilation of content scripts and content styles; the application of an already-compiled content script/style is synchronous.

The purpose of this bug is to refactor the logic in ExtensionContent.sys.mjs, so that the async compilation is separate from the synchronous execution. This enables us to compile the scripts/styles, ahead of actual execution, while waiting for a suitable point of execution (bug 1792685 and bug 1920169).

I split this work off bug 1792685, because the logical changes are non-trivial and useful on its own. These patches are not expected to cause significant changes in user-observed behavior. On the other hand, the work of bug 1792685 may incur the risk of perceived regressions, if extension developers would rather have their script run sooner than waiting for them to run in order.

Comment 1

[Rob Wu [:robwu]]

Attachment: Bug 1951688 - Clarify role of DocumentManager

Adds several comments and move DocumentManager.lazyInit() calls to a
single place where it is called less often, but still when needed.

(historically, DocumentManager did much more than what it does now,
but all of that has moved into ExtensionPolicyService.cpp).

Comment 2

[Rob Wu [:robwu]]

Attachment: Bug 1951688 - Decouple content scripts' css compilation from injection

Before this patch, injectInto combined the logic for asynchronous
stylesheet compilation with the synchronous addSheet invocation.
It assumed that as soon as the stylesheet was compiled, that the
document was associated with the compiled stylesheet (in the cache) and
that the stylesheet was applied to the document.

With this patch, the compile and caching logic are combined, which
enables the actual application of the styles to be deferred.

In the pre-patch implementation, the addDocument/deleteDocument took
the url of the stylesheet, to look up the compiled stylesheet.
In the new implementation, the sheetPromise is used as a key. Since
this value is immediately available, this also removes any chance of
TOCTOU bugs in the maintenance of the cache.

Moreover, with the ability to look up the compiled sheets synchronously,
the cleanup() and removeCSS logic can be consolidated, because they are
nearly identical.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2559d12df950
Clarify role of DocumentManager r=zombie
https://hg.mozilla.org/integration/autoland/rev/e39a4974535c
Decouple content scripts' css compilation from injection r=zombie

Comment 4

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/2559d12df950
https://hg.mozilla.org/mozilla-central/rev/e39a4974535c