Bug 1847546 - Don't do duplicate work on style elements from the innerHTML setter. r=smaug

Right now, when binding to the tree we queue a runnable to update the
stylesheet, even though mEnableUpdates is false.

Even though the redundant update is nowadays always cached, it's just
wasted work, and the code is simpler without it.

This will be tested by bug 1771113, which is what made me look at this.

We need to tweak a bit the dispatch of applicable state change events
for DevTools, because for a case like:

  div.attachShadow({ mode: "open" }).innerHTML = `<style>...</style>`;

Before we'd go through the stylesheet cache here due to the redundant
update:

  https://searchfox.org/mozilla-central/rev/fb43eb3bdf5b51000bc7dfe3474cbe56ca2ab63c/layout/style/SharedStyleSheetCache.cpp#161-165

But now we won't, and the code in StyleSheet.cpp wasn't quite correct /
didn't dispatch the event.

Nobody listens to style-sheet-applicable-state-changed, so remove that
code while at it.

Differential Revision: https://phabricator.services.mozilla.com/D185559

dom/base/Document.cpp

@@ -1361,7 +1361,6 @@ Document::Document(const char* aContentType)
       mQuirkSheetAdded(false),
       mContentEditableSheetAdded(false),
       mDesignModeSheetAdded(false),
-      mSSApplicableStateNotificationPending(false),
       mMayHaveTitleElement(false),
       mDOMLoadingSet(false),
       mDOMInteractiveSet(false),
@@ -7427,17 +7426,6 @@ void Document::StyleSheetApplicableStateChanged(StyleSheet& aSheet) {
       RemoveStyleSheetFromStyleSets(aSheet);
     }
   }
-
-  PostStyleSheetApplicableStateChangeEvent(aSheet);
-
-  if (!mSSApplicableStateNotificationPending) {
-    MOZ_RELEASE_ASSERT(NS_IsMainThread());
-    nsCOMPtr<nsIRunnable> notification = NewRunnableMethod(
-        "Document::NotifyStyleSheetApplicableStateChanged", this,
-        &Document::NotifyStyleSheetApplicableStateChanged);
-    mSSApplicableStateNotificationPending =
-        NS_SUCCEEDED(Dispatch(TaskCategory::Other, notification.forget()));
-  }
 }
 
 void Document::PostStyleSheetApplicableStateChangeEvent(StyleSheet& aSheet) {
@@ -7462,16 +7450,6 @@ void Document::PostStyleSheetApplicableStateChangeEvent(StyleSheet& aSheet) {
   asyncDispatcher->PostDOMEvent();
 }
 
-void Document::NotifyStyleSheetApplicableStateChanged() {
-  mSSApplicableStateNotificationPending = false;
-  nsCOMPtr<nsIObserverService> observerService =
-      mozilla::services::GetObserverService();
-  if (observerService) {
-    observerService->NotifyObservers(
-        ToSupports(this), "style-sheet-applicable-state-changed", nullptr);
-  }
-}
-
 static int32_t FindSheet(const nsTArray<RefPtr<StyleSheet>>& aSheets,
                          nsIURI* aSheetURI) {
   for (int32_t i = aSheets.Length() - 1; i >= 0; i--) {

dom/base/Document.h

@@ -4761,10 +4761,6 @@ class Document : public nsINode,
   // Whether we have a designmode.css stylesheet in the style set.
   bool mDesignModeSheetAdded : 1;
 
-  // Keeps track of whether we have a pending
-  // 'style-sheet-applicable-state-changed' notification.
-  bool mSSApplicableStateNotificationPending : 1;
-
   // True if this document has ever had an HTML or SVG <title> element
   // bound to it
   bool mMayHaveTitleElement : 1;

dom/base/LinkStyle.cpp

@@ -63,9 +63,7 @@ LinkStyle::SheetInfo::SheetInfo(
 }
 
 LinkStyle::SheetInfo::~SheetInfo() = default;
-
-LinkStyle::LinkStyle()
-    : mUpdatesEnabled(true), mLineNumber(1), mColumnNumber(1) {}
+LinkStyle::LinkStyle() = default;
 
 LinkStyle::~LinkStyle() { LinkStyle::SetStyleSheet(nullptr); }
 
@@ -197,6 +195,14 @@ Result<LinkStyle::Update, nsresult> LinkStyle::UpdateStyleSheetInternal(
                             aForceUpdate);
 }
 
+void LinkStyle::BindToTree() {
+  if (mUpdatesEnabled) {
+    nsContentUtils::AddScriptRunner(NS_NewRunnableFunction(
+        "LinkStyle::BindToTree",
+        [this, pin = RefPtr{&AsContent()}] { UpdateStyleSheetInternal(); }));
+  }
+}
+
 Result<LinkStyle::Update, nsresult> LinkStyle::DoUpdateStyleSheet(
     Document* aOldDocument, ShadowRoot* aOldShadowRoot,
     nsICSSLoaderObserver* aObserver, ForceUpdate aForceUpdate) {

dom/base/LinkStyle.h

@@ -171,13 +171,16 @@ class LinkStyle {
   Result<Update, nsresult> UpdateStyleSheet(nsICSSLoaderObserver*);
 
   /**
-   * Tells this element whether to update the stylesheet when the
-   * element's properties change.
-   *
-   * @param aEnableUpdates update on changes or not.
+   * Tells this element whether to update the stylesheet when the element's
+   * properties change. This is used by the parser until it has all content etc,
+   * and to guarantee that the right observer is used.
    */
-  void SetEnableUpdates(bool aEnableUpdates) {
-    mUpdatesEnabled = aEnableUpdates;
+  void DisableUpdates() { mUpdatesEnabled = false; }
+  Result<Update, nsresult> EnableUpdatesAndUpdateStyleSheet(
+      nsICSSLoaderObserver* aObserver) {
+    MOZ_ASSERT(!mUpdatesEnabled);
+    mUpdatesEnabled = true;
+    return UpdateStyleSheet(aObserver);
   }
 
   /**
@@ -275,11 +278,13 @@ class LinkStyle {
                                               nsICSSLoaderObserver*,
                                               ForceUpdate);
 
+  void BindToTree();
+
   RefPtr<mozilla::StyleSheet> mStyleSheet;
   nsCOMPtr<nsIPrincipal> mTriggeringPrincipal;
-  bool mUpdatesEnabled;
-  uint32_t mLineNumber;
-  uint32_t mColumnNumber;
+  bool mUpdatesEnabled = true;
+  uint32_t mLineNumber = 1;
+  uint32_t mColumnNumber = 1;
 };
 
 }  // namespace mozilla::dom

dom/html/HTMLLinkElement.cpp

@@ -90,10 +90,7 @@ nsresult HTMLLinkElement::BindToTree(BindContext& aContext, nsINode& aParent) {
     TryDNSPrefetchOrPreconnectOrPrefetchOrPreloadOrPrerender();
   }
 
-  void (HTMLLinkElement::*update)() =
-      &HTMLLinkElement::UpdateStyleSheetInternal;
-  nsContentUtils::AddScriptRunner(
-      NewRunnableMethod("dom::HTMLLinkElement::BindToTree", this, update));
+  LinkStyle::BindToTree();
 
   if (IsInUncomposedDoc() &&
       AttrValueIs(kNameSpaceID_None, nsGkAtoms::rel, nsGkAtoms::localization,

dom/html/HTMLStyleElement.cpp

@@ -83,12 +83,7 @@ void HTMLStyleElement::ContentChanged(nsIContent* aContent) {
 nsresult HTMLStyleElement::BindToTree(BindContext& aContext, nsINode& aParent) {
   nsresult rv = nsGenericHTMLElement::BindToTree(aContext, aParent);
   NS_ENSURE_SUCCESS(rv, rv);
-
-  void (HTMLStyleElement::*update)() =
-      &HTMLStyleElement::UpdateStyleSheetInternal;
-  nsContentUtils::AddScriptRunner(
-      NewRunnableMethod("dom::HTMLStyleElement::BindToTree", this, update));
-
+  LinkStyle::BindToTree();
   return rv;
 }
 
@@ -144,15 +139,12 @@ void HTMLStyleElement::SetTextContentInternal(const nsAString& aTextContent,
     }
   }
 
-  SetEnableUpdates(false);
+  DisableUpdates();
 
   aError = nsContentUtils::SetNodeTextContent(this, aTextContent, true);
-
-  SetEnableUpdates(true);
-
   mTriggeringPrincipal = aScriptedPrincipal;
 
-  Unused << UpdateStyleSheetInternal(nullptr, nullptr);
+  Unused << EnableUpdatesAndUpdateStyleSheet(nullptr);
 }
 
 void HTMLStyleElement::SetDevtoolsAsTriggeringPrincipal() {

dom/prototype/PrototypeDocumentContentSink.cpp

@@ -381,7 +381,7 @@ nsresult PrototypeDocumentContentSink::InsertXMLStylesheetPI(
     XMLStylesheetProcessingInstruction* aPINode) {
   // We want to be notified when the style sheet finishes loading, so
   // disable style sheet loading for now.
-  aPINode->SetEnableUpdates(false);
+  aPINode->DisableUpdates();
   aPINode->OverrideBaseURI(mCurrentPrototype->GetURI());
 
   ErrorResult rv;
@@ -391,11 +391,9 @@ nsresult PrototypeDocumentContentSink::InsertXMLStylesheetPI(
     return rv.StealNSResult();
   }
 
-  aPINode->SetEnableUpdates(true);
-
   // load the stylesheet if necessary, passing ourselves as
   // nsICSSObserver
-  auto result = aPINode->UpdateStyleSheet(this);
+  auto result = aPINode->EnableUpdatesAndUpdateStyleSheet(this);
   if (result.isErr()) {
     // Ignore errors from UpdateStyleSheet; we don't want failure to
     // do that to break the XUL document load.  But do propagate out

dom/svg/SVGStyleElement.cpp

@@ -62,12 +62,7 @@ NS_IMPL_ELEMENT_CLONE_WITH_INIT(SVGStyleElement)
 nsresult SVGStyleElement::BindToTree(BindContext& aContext, nsINode& aParent) {
   nsresult rv = SVGStyleElementBase::BindToTree(aContext, aParent);
   NS_ENSURE_SUCCESS(rv, rv);
-
-  void (SVGStyleElement::*update)() =
-      &SVGStyleElement::UpdateStyleSheetInternal;
-  nsContentUtils::AddScriptRunner(
-      NewRunnableMethod("dom::SVGStyleElement::BindToTree", this, update));
-
+  LinkStyle::BindToTree();
   return rv;
 }
 

dom/xml/nsXMLContentSink.cpp

@@ -520,7 +520,7 @@ nsresult nsXMLContentSink::CreateElement(
       aNodeInfo->Equals(nsGkAtoms::style, kNameSpaceID_SVG)) {
     if (auto* linkStyle = LinkStyle::FromNode(*content)) {
       if (aFromParser) {
-        linkStyle->SetEnableUpdates(false);
+        linkStyle->DisableUpdates();
       }
       if (!aNodeInfo->Equals(nsGkAtoms::link, kNameSpaceID_XHTML)) {
         linkStyle->SetLineNumber(aFromParser ? aLineNumber : 0);
@@ -595,9 +595,8 @@ nsresult nsXMLContentSink::CloseElement(nsIContent* aContent) {
       nodeInfo->Equals(nsGkAtoms::style, kNameSpaceID_XHTML) ||
       nodeInfo->Equals(nsGkAtoms::style, kNameSpaceID_SVG)) {
     if (auto* linkStyle = LinkStyle::FromNode(*aContent)) {
-      linkStyle->SetEnableUpdates(true);
-      auto updateOrError =
-          linkStyle->UpdateStyleSheet(mRunsToCompletion ? nullptr : this);
+      auto updateOrError = linkStyle->EnableUpdatesAndUpdateStyleSheet(
+          mRunsToCompletion ? nullptr : this);
       if (updateOrError.isErr()) {
         rv = updateOrError.unwrapErr();
       } else if (updateOrError.unwrap().ShouldBlock() && !mRunsToCompletion) {
@@ -1181,7 +1180,7 @@ nsXMLContentSink::HandleProcessingInstruction(const char16_t* aTarget,
 
   auto* linkStyle = LinkStyle::FromNode(*node);
   if (linkStyle) {
-    linkStyle->SetEnableUpdates(false);
+    linkStyle->DisableUpdates();
     mPrettyPrintXML = false;
   }
 
@@ -1192,9 +1191,8 @@ nsXMLContentSink::HandleProcessingInstruction(const char16_t* aTarget,
   if (linkStyle) {
     // This is an xml-stylesheet processing instruction... but it might not be
     // a CSS one if the type is set to something else.
-    linkStyle->SetEnableUpdates(true);
-    auto updateOrError =
-        linkStyle->UpdateStyleSheet(mRunsToCompletion ? nullptr : this);
+    auto updateOrError = linkStyle->EnableUpdatesAndUpdateStyleSheet(
+        mRunsToCompletion ? nullptr : this);
     if (updateOrError.isErr()) {
       return updateOrError.unwrapErr();
     }

dom/xslt/xslt/txMozillaXMLOutput.cpp

@@ -281,8 +281,8 @@ nsresult txMozillaXMLOutput::endElement() {
   if (mCreatingNewDocument) {
     // Handle all sorts of stylesheets
     if (auto* linkStyle = LinkStyle::FromNode(*mCurrentNode)) {
-      linkStyle->SetEnableUpdates(true);
-      auto updateOrError = linkStyle->UpdateStyleSheet(mNotifier);
+      auto updateOrError =
+          linkStyle->EnableUpdatesAndUpdateStyleSheet(mNotifier);
       if (mNotifier && updateOrError.isOk() &&
           updateOrError.unwrap().ShouldBlock()) {
         mNotifier->AddPendingStylesheet();
@@ -345,7 +345,7 @@ nsresult txMozillaXMLOutput::processingInstruction(const nsString& aTarget,
   if (mCreatingNewDocument) {
     linkStyle = LinkStyle::FromNode(*pi);
     if (linkStyle) {
-      linkStyle->SetEnableUpdates(false);
+      linkStyle->DisableUpdates();
     }
   }
 
@@ -356,8 +356,7 @@ nsresult txMozillaXMLOutput::processingInstruction(const nsString& aTarget,
   }
 
   if (linkStyle) {
-    linkStyle->SetEnableUpdates(true);
-    auto updateOrError = linkStyle->UpdateStyleSheet(mNotifier);
+    auto updateOrError = linkStyle->EnableUpdatesAndUpdateStyleSheet(mNotifier);
     if (mNotifier && updateOrError.isOk() &&
         updateOrError.unwrap().ShouldBlock()) {
       mNotifier->AddPendingStylesheet();
@@ -487,7 +486,7 @@ nsresult txMozillaXMLOutput::startElementInternal(nsAtom* aPrefix,
   if (mCreatingNewDocument) {
     // Handle all sorts of stylesheets
     if (auto* linkStyle = LinkStyle::FromNodeOrNull(mOpenedElement)) {
-      linkStyle->SetEnableUpdates(false);
+      linkStyle->DisableUpdates();
     }
   }
 

layout/style/StyleSheet.cpp

@@ -302,12 +302,21 @@ void StyleSheet::SetComplete() {
 
 void StyleSheet::ApplicableStateChanged(bool aApplicable) {
   MOZ_ASSERT(aApplicable == IsApplicable());
-  auto Notify = [this](DocumentOrShadowRoot& target) {
+  Document* docToPostEvent = nullptr;
+  auto Notify = [&](DocumentOrShadowRoot& target) {
     nsINode& node = target.AsNode();
     if (ShadowRoot* shadow = ShadowRoot::FromNode(node)) {
       shadow->StyleSheetApplicableStateChanged(*this);
+      MOZ_ASSERT(!docToPostEvent || !shadow->IsInComposedDoc() ||
+                 docToPostEvent == shadow->GetComposedDoc());
+      if (!docToPostEvent) {
+        docToPostEvent = shadow->GetComposedDoc();
+      }
     } else {
-      node.AsDocument()->StyleSheetApplicableStateChanged(*this);
+      Document* doc = node.AsDocument();
+      MOZ_ASSERT(!docToPostEvent || docToPostEvent == doc);
+      doc->StyleSheetApplicableStateChanged(*this);
+      docToPostEvent = doc;
     }
   };
 
@@ -326,6 +335,10 @@ void StyleSheet::ApplicableStateChanged(bool aApplicable) {
       Notify(*adopter);
     }
   }
+
+  if (docToPostEvent) {
+    docToPostEvent->PostStyleSheetApplicableStateChangeEvent(*this);
+  }
 }
 
 void StyleSheet::SetDisabled(bool aDisabled) {

parser/html/nsHtml5DocumentBuilder.cpp

@@ -32,7 +32,7 @@ nsresult nsHtml5DocumentBuilder::Init(mozilla::dom::Document* aDoc,
   return nsContentSink::Init(aDoc, aURI, aContainer, aChannel);
 }
 
-nsHtml5DocumentBuilder::~nsHtml5DocumentBuilder() {}
+nsHtml5DocumentBuilder::~nsHtml5DocumentBuilder() = default;
 
 nsresult nsHtml5DocumentBuilder::MarkAsBroken(nsresult aReason) {
   mBroken = aReason;
@@ -47,28 +47,14 @@ void nsHtml5DocumentBuilder::UpdateStyleSheet(nsIContent* aElement) {
     return;
   }
 
-  // Break out of the doc update created by Flush() to zap a runnable
-  // waiting to call UpdateStyleSheet without the right observer
-  EndDocUpdate();
-
-  if (MOZ_UNLIKELY(!mParser)) {
-    // EndDocUpdate ran stuff that called nsIParser::Terminate()
-    return;
-  }
-
-  linkStyle->SetEnableUpdates(true);
-
-  auto updateOrError =
-      linkStyle->UpdateStyleSheet(mRunsToCompletion ? nullptr : this);
+  auto updateOrError = linkStyle->EnableUpdatesAndUpdateStyleSheet(
+      mRunsToCompletion ? nullptr : this);
 
   if (updateOrError.isOk() && updateOrError.unwrap().ShouldBlock() &&
       !mRunsToCompletion) {
     ++mPendingSheetCount;
     mScriptLoader->AddParserBlockingScriptExecutionBlocker();
   }
-
-  // Re-open update
-  BeginDocUpdate();
 }
 
 void nsHtml5DocumentBuilder::SetDocumentMode(nsHtml5DocumentMode m) {

parser/html/nsHtml5TreeOperation.cpp

@@ -474,7 +474,7 @@ nsIContent* nsHtml5TreeOperation::CreateHTMLElement(
 
   if (willExecuteScript) {  // This will cause custom element constructors to
                             // run
-    mozilla::dom::AutoSetThrowOnDynamicMarkupInsertionCounter
+    AutoSetThrowOnDynamicMarkupInsertionCounter
         throwOnDynamicMarkupInsertionCounter(document);
     nsHtml5AutoPauseUpdate autoPauseContentUpdate(aBuilder);
     { nsAutoMicroTask mt; }
@@ -491,7 +491,7 @@ nsIContent* nsHtml5TreeOperation::CreateHTMLElement(
 
     if (MOZ_UNLIKELY(aName == nsGkAtoms::style || aName == nsGkAtoms::link)) {
       if (auto* linkStyle = dom::LinkStyle::FromNode(*newContent)) {
-        linkStyle->SetEnableUpdates(false);
+        linkStyle->DisableUpdates();
       }
     }
 
@@ -517,7 +517,7 @@ nsIContent* nsHtml5TreeOperation::CreateHTMLElement(
 
     if (MOZ_UNLIKELY(aName == nsGkAtoms::style || aName == nsGkAtoms::link)) {
       if (auto* linkStyle = dom::LinkStyle::FromNode(*newContent)) {
-        linkStyle->SetEnableUpdates(false);
+        linkStyle->DisableUpdates();
       }
     }
 
@@ -564,7 +564,7 @@ nsIContent* nsHtml5TreeOperation::CreateSVGElement(
 
   if (MOZ_UNLIKELY(aName == nsGkAtoms::style)) {
     if (auto* linkStyle = dom::LinkStyle::FromNode(*newContent)) {
-      linkStyle->SetEnableUpdates(false);
+      linkStyle->DisableUpdates();
     }
   }