commit d2478ad5f94d857d1d6c49c13c97b10f678e2908
parent 3c6c65bb6a00d9fa32ed3f4bd677ed2d0c272ef5
Author: Frédéric Wang <fwang@igalia.com>
Date: Mon, 8 Dec 2025 14:05:52 +0000
Bug 1997818 - Improve parser blocking for ScriptElement::MaybeProcessScript(). r=smaug
When processing a non-trusted inline script, we must run a TrustedTypes
default policy callback on the source text, which in particular may:
- Generally be executed asynchronously in a script runner (thus
requiring to block the parser in the meantime).
- Modify the script type, for example between classic and module (thus
changing the expected blocking behavior for the parser).
Currently, ScriptElement::MaybeProcessScript() returns a boolean
telling callers whether they need to block the parser. Most callers
ignore it but for those that don't, the behavior described above make it
impossible to know the correct desired returned value in advance for
non-trusted inline scripts. Current code always always tell callers not
to block the parser, which is incorrect and causing assertion failures.
This patch adds an optional nsIParser argument to MaybeProcessScript(),
which becomes responsible of blocking the parser when needed. For
external and trusted inline scripts we can continue to try and process
the script immediately and use the result to determine whether to block
the parser. Similarly, for non-trusted inline scripts in context where
it is safe to run script, we can run the default policy callback before
the processing attempt and blocking decision.
For non-trusted inline scripts with a default policy callback that need
to be executed in a script runner, we always block the parser while
waiting that script runner to be executed and only decide to unblock it
after the attempt to process the script.
Differential Revision: https://phabricator.services.mozilla.com/D272372
Diffstat:
10 files changed, 87 insertions(+), 66 deletions(-)
diff --git a/dom/html/HTMLScriptElement.cpp b/dom/html/HTMLScriptElement.cpp
@@ -65,7 +65,7 @@ nsresult HTMLScriptElement::BindToTree(BindContext& aContext,
NS_ENSURE_SUCCESS(rv, rv);
if (IsInComposedDoc()) {
- MaybeProcessScript();
+ MaybeProcessScript(nullptr /* aParser */);
}
return NS_OK;
diff --git a/dom/prototype/PrototypeDocumentContentSink.cpp b/dom/prototype/PrototypeDocumentContentSink.cpp
@@ -455,8 +455,7 @@ void PrototypeDocumentContentSink::CloseElement(Element* aElement,
{
nsAutoMicroTask mt;
}
- DebugOnly<bool> block = sele->AttemptToExecute();
- MOZ_ASSERT(!block, "<script type=module> shouldn't block the parser");
+ sele->AttemptToExecute(nullptr /* aParser */);
}
}
}
diff --git a/dom/script/ScriptElement.cpp b/dom/script/ScriptElement.cpp
@@ -91,7 +91,7 @@ ScriptElement::ScriptEvaluated(nsresult aResult, nsIScriptElement* aElement,
void ScriptElement::CharacterDataChanged(nsIContent* aContent,
const CharacterDataChangeInfo& aInfo) {
UpdateTrustWorthiness(aInfo.mMutationEffectOnScript);
- MaybeProcessScript();
+ MaybeProcessScript(nullptr /* aParser */);
}
void ScriptElement::AttributeChanged(Element* aElement, int32_t aNameSpaceID,
@@ -115,7 +115,7 @@ void ScriptElement::AttributeChanged(Element* aElement, int32_t aNameSpaceID,
if (mParserCreated == NOT_FROM_PARSER && aModType == AttrModType::Addition) {
auto* cont = GetAsContent();
if (cont->IsInComposedDoc()) {
- MaybeProcessScript();
+ MaybeProcessScript(nullptr /* aParser */);
}
}
}
@@ -123,13 +123,13 @@ void ScriptElement::AttributeChanged(Element* aElement, int32_t aNameSpaceID,
void ScriptElement::ContentAppended(nsIContent* aFirstNewContent,
const ContentAppendInfo& aInfo) {
UpdateTrustWorthiness(aInfo.mMutationEffectOnScript);
- MaybeProcessScript();
+ MaybeProcessScript(nullptr /* aParser */);
}
void ScriptElement::ContentInserted(nsIContent* aChild,
const ContentInsertInfo& aInfo) {
UpdateTrustWorthiness(aInfo.mMutationEffectOnScript);
- MaybeProcessScript();
+ MaybeProcessScript(nullptr /* aParser */);
}
void ScriptElement::ContentWillBeRemoved(nsIContent* aChild,
@@ -137,7 +137,7 @@ void ScriptElement::ContentWillBeRemoved(nsIContent* aChild,
UpdateTrustWorthiness(aInfo.mMutationEffectOnScript);
}
-bool ScriptElement::MaybeProcessScript() {
+bool ScriptElement::MaybeProcessScript(nsCOMPtr<nsIParser> aParser) {
nsIContent* cont = GetAsContent();
NS_ASSERTION(cont->DebugGetSlots()->mMutationObservers.contains(this),
@@ -151,30 +151,58 @@ bool ScriptElement::MaybeProcessScript() {
// https://html.spec.whatwg.org/#prepare-the-script-element
// The spec says we should calculate "source text" of inline scripts at the
// beginning of the "Prepare the script element" algorithm.
- // - If this is an inline script that is not trusted (i.e. we must execute the
- // Trusted Type default policy callback to obtain a trusted "source text")
- // then we must wrap the GetTrustedTypesCompliantInlineScriptText call in a
- // script runner.
- // - If it is an inline script that is trusted, we will actually retrieve the
- // "source text" lazily for performance reasons (see bug 1376651) so we just
- // use a void string.
- // - If it is an external script, we similarly just pass a void string.
- if (!HasExternalScriptContent() && !mIsTrusted) {
- if (!TrustedTypeUtils::CanSkipTrustedTypesEnforcement(
- *GetAsContent()->AsElement())) {
- // TODO(bug 1997818): Ensure we properly block the parser.
- nsContentUtils::AddScriptRunner(NS_NewRunnableFunction(
- "ScriptElement::MaybeProcessScript",
- [self = RefPtr<nsIScriptElement>(this)]()
- MOZ_CAN_RUN_SCRIPT_BOUNDARY_LAMBDA {
- nsString sourceText;
- self->GetTrustedTypesCompliantInlineScriptText(sourceText);
- ((ScriptElement*)self.get())->MaybeProcessScript(sourceText);
- }));
- return false;
+ if (HasExternalScriptContent() || mIsTrusted ||
+ TrustedTypeUtils::CanSkipTrustedTypesEnforcement(
+ *GetAsContent()->AsElement())) {
+ // - If it is an inline script that is trusted, we will actually retrieve
+ // the "source text" lazily for performance reasons (see bug 1376651) so we
+ // just pass a void string to MaybeProcessScript().
+ // - If it is an external script, we actually don't need the "source text"
+ // and can similarly pass a void string to MaybeProcessScript().
+ bool block = MaybeProcessScript(VoidString());
+ if (block && aParser) {
+ aParser->BlockParser();
+ }
+ return block;
+ }
+
+ // This is an inline script that is not trusted (i.e. we must execute the
+ // Trusted Type default policy callback to obtain a trusted "source text").
+ if (nsContentUtils::IsSafeToRunScript()) {
+ // - If it is safe to run script in this context, we run the default policy
+ // callback and pass the returned "source text" to MaybeProcessScript().
+ bool block =
+ ([self = RefPtr<nsIScriptElement>(this)]()
+ MOZ_CAN_RUN_SCRIPT_BOUNDARY_LAMBDA {
+ nsString sourceText;
+ self->GetTrustedTypesCompliantInlineScriptText(sourceText);
+ return static_cast<ScriptElement*>(self.get())
+ ->MaybeProcessScript(sourceText);
+ })();
+ if (block && aParser) {
+ aParser->BlockParser();
}
+ return block;
}
- return MaybeProcessScript(VoidString());
+
+ // - The default policy callback must be wrapped in a script runner. So we
+ // need to block the parser at least until we can get the "source text".
+ nsContentUtils::AddScriptRunner(NS_NewRunnableFunction(
+ "ScriptElement::MaybeProcessScript",
+ [self = RefPtr<nsIScriptElement>(this), aParser]()
+ MOZ_CAN_RUN_SCRIPT_BOUNDARY_LAMBDA {
+ nsString sourceText;
+ self->GetTrustedTypesCompliantInlineScriptText(sourceText);
+ bool block = static_cast<ScriptElement*>(self.get())
+ ->MaybeProcessScript(sourceText);
+ if (!block && aParser) {
+ aParser->UnblockParser();
+ }
+ }));
+ if (aParser) {
+ aParser->BlockParser();
+ }
+ return true;
}
bool ScriptElement::MaybeProcessScript(const nsAString& aSourceText) {
diff --git a/dom/script/ScriptElement.h b/dom/script/ScriptElement.h
@@ -12,6 +12,8 @@
#include "nsIScriptLoaderObserver.h"
#include "nsStubMutationObserver.h"
+class nsIParser;
+
namespace mozilla::dom {
/**
@@ -47,7 +49,7 @@ class ScriptElement : public nsIScriptElement, public nsStubMutationObserver {
*/
virtual bool HasExternalScriptContent() = 0;
- virtual bool MaybeProcessScript() override;
+ virtual bool MaybeProcessScript(nsCOMPtr<nsIParser> aParser) override;
virtual MOZ_CAN_RUN_SCRIPT nsresult
GetTrustedTypesCompliantInlineScriptText(nsString& aSourceText) override;
diff --git a/dom/script/nsIScriptElement.cpp b/dom/script/nsIScriptElement.cpp
@@ -73,6 +73,17 @@ already_AddRefed<nsIParser> nsIScriptElement::GetCreatorParser() {
return parser.forget();
}
+bool nsIScriptElement::AttemptToExecute(nsCOMPtr<nsIParser> aParser) {
+ mDoneAddingChildren = true;
+ bool block = MaybeProcessScript(aParser);
+ if (!mAlreadyStarted) {
+ // Need to lose parser-insertedness here to allow another script to cause
+ // execution later.
+ LoseParserInsertedness();
+ }
+ return block;
+}
+
mozilla::dom::ReferrerPolicy nsIScriptElement::GetReferrerPolicy() {
return mozilla::dom::ReferrerPolicy::_empty;
}
diff --git a/dom/script/nsIScriptElement.h b/dom/script/nsIScriptElement.h
@@ -213,19 +213,12 @@ class nsIScriptElement : public nsIScriptLoaderObserver {
* This method is called when the parser finishes creating the script
* element's children, if any are present.
*
- * @return whether the parser will be blocked while this script is being
- * loaded
+ * @param aParser If non-null, a parser that can be blocked until the script
+ * becomes available.
+ * @return whether a non-null aParser would be blocked while this script is
+ * being loaded.
*/
- bool AttemptToExecute() {
- mDoneAddingChildren = true;
- bool block = MaybeProcessScript();
- if (!mAlreadyStarted) {
- // Need to lose parser-insertedness here to allow another script to cause
- // execution later.
- LoseParserInsertedness();
- }
- return block;
- }
+ bool AttemptToExecute(nsCOMPtr<nsIParser> aParser);
/**
* Get the CORS mode of the script element
@@ -277,7 +270,7 @@ class nsIScriptElement : public nsIScriptLoaderObserver {
* @return whether the parser will be blocked while this script is being
* loaded
*/
- virtual bool MaybeProcessScript() = 0;
+ virtual bool MaybeProcessScript(nsCOMPtr<nsIParser> aParser) = 0;
/**
* Since we've removed the XPCOM interface to HTML elements, we need a way to
diff --git a/dom/svg/SVGScriptElement.cpp b/dom/svg/SVGScriptElement.cpp
@@ -191,7 +191,7 @@ nsresult SVGScriptElement::BindToTree(BindContext& aContext, nsINode& aParent) {
NS_ENSURE_SUCCESS(rv, rv);
if (IsInComposedDoc()) {
- MaybeProcessScript();
+ MaybeProcessScript(nullptr /* aParser */);
}
return NS_OK;
diff --git a/dom/xml/nsXMLContentSink.cpp b/dom/xml/nsXMLContentSink.cpp
@@ -642,17 +642,12 @@ nsresult nsXMLContentSink::CloseElement(nsIContent* aContent) {
// Now tell the script that it's ready to go. This may execute the script
// or return true, or neither if the script doesn't need executing.
- bool block = sele->AttemptToExecute();
- if (mParser) {
- if (block) {
- GetParser()->BlockParser();
- }
+ bool block = sele->AttemptToExecute(GetParser());
- // If the parser got blocked, make sure to return the appropriate rv.
- // I'm not sure if this is actually needed or not.
- if (!mParser->IsParserEnabled()) {
- block = true;
- }
+ // If the parser got blocked, make sure to return the appropriate rv.
+ // I'm not sure if this is actually needed or not.
+ if (mParser && !mParser->IsParserEnabled()) {
+ block = true;
}
return block ? NS_ERROR_HTMLPARSER_BLOCK : NS_OK;
diff --git a/dom/xslt/xslt/txMozillaXMLOutput.cpp b/dom/xslt/xslt/txMozillaXMLOutput.cpp
@@ -270,7 +270,7 @@ nsresult txMozillaXMLOutput::endElement() {
{
nsAutoMicroTask mt;
}
- bool block = sele->AttemptToExecute();
+ bool block = sele->AttemptToExecute(nullptr /* aParser */);
// If the act of insertion evaluated the script, we're fine.
// Else, add this script element to the array of loading scripts.
if (block) {
diff --git a/parser/html/nsHtml5TreeOpExecutor.cpp b/parser/html/nsHtml5TreeOpExecutor.cpp
@@ -912,9 +912,7 @@ void nsHtml5TreeOpExecutor::RunScript(nsIContent* aScriptElement,
MOZ_ASSERT(sele->GetScriptDeferred() || sele->GetScriptAsync() ||
sele->GetScriptIsModule() || sele->GetScriptIsImportMap() ||
aScriptElement->AsElement()->HasAttr(nsGkAtoms::nomodule));
- DebugOnly<bool> block = sele->AttemptToExecute();
- MOZ_ASSERT(!block,
- "Defer, async, module, importmap, or nomodule tried to block.");
+ sele->AttemptToExecute(nullptr /* aParser */);
return;
}
@@ -927,15 +925,10 @@ void nsHtml5TreeOpExecutor::RunScript(nsIContent* aScriptElement,
// Copied from nsXMLContentSink
// Now tell the script that it's ready to go. This may execute the script
// or return true, or neither if the script doesn't need executing.
- bool block = sele->AttemptToExecute();
+ bool block = sele->AttemptToExecute(GetParser());
// If the act of insertion evaluated the script, we're fine.
- // Else, block the parser till the script has loaded.
- if (block) {
- if (mParser) {
- GetParser()->BlockParser();
- }
- } else {
+ if (!block) {
// mParser may have been nulled out by now, but the flusher deals
// If this event isn't needed, it doesn't do anything. It is sometimes
