Backed out changeset 702857fa260e (bug 1673885) for wpt failures on ttwf-cssom-doc-ext-load-count.html CLOSED TREE

dom/base/LinkStyle.cpp

@@ -69,14 +69,6 @@ LinkStyle::LinkStyle()
 
 LinkStyle::~LinkStyle() { LinkStyle::SetStyleSheet(nullptr); }
 
-StyleSheet* LinkStyle::GetSheetForBindings() const {
-  if (!StaticPrefs::dom_expose_incomplete_stylesheets() && mStyleSheet &&
-      !mStyleSheet->IsComplete()) {
-    return nullptr;
-  }
-  return mStyleSheet;
-}
-
 void LinkStyle::GetTitleAndMediaForElement(const Element& aSelf,
                                            nsString& aTitle, nsString& aMedia) {
   // Only honor title as stylesheet name for elements in the document (that is,
@@ -210,18 +202,14 @@ Result<LinkStyle::Update, nsresult> LinkStyle::DoUpdateStyleSheet(
                "there should not be a old document and old "
                "ShadowRoot simultaneously.");
 
-    // We're removing the link element from the document or shadow tree, unload
-    // the stylesheet.
-    //
-    // We want to do this even if updates are disabled, since otherwise a sheet
-    // with a stale linking element pointer will be hanging around -- not good!
-    if (mStyleSheet->IsComplete() ||
-        StaticPrefs::dom_expose_incomplete_stylesheets()) {
-      if (aOldShadowRoot) {
-        aOldShadowRoot->RemoveStyleSheet(*mStyleSheet);
-      } else {
-        aOldDocument->RemoveStyleSheet(*mStyleSheet);
-      }
+    // We're removing the link element from the document or shadow tree,
+    // unload the stylesheet.  We want to do this even if updates are
+    // disabled, since otherwise a sheet with a stale linking element pointer
+    // will be hanging around -- not good!
+    if (aOldShadowRoot) {
+      aOldShadowRoot->RemoveStyleSheet(*mStyleSheet);
+    } else {
+      aOldDocument->RemoveStyleSheet(*mStyleSheet);
     }
 
     SetStyleSheet(nullptr);
@@ -252,20 +240,17 @@ Result<LinkStyle::Update, nsresult> LinkStyle::DoUpdateStyleSheet(
   }
 
   if (mStyleSheet) {
-    if (mStyleSheet->IsComplete() ||
-        StaticPrefs::dom_expose_incomplete_stylesheets()) {
-      if (thisContent.IsInShadowTree()) {
-        ShadowRoot* containingShadow = thisContent.GetContainingShadow();
-        // Could be null only during unlink.
-        if (MOZ_LIKELY(containingShadow)) {
-          containingShadow->RemoveStyleSheet(*mStyleSheet);
-        }
-      } else {
-        doc->RemoveStyleSheet(*mStyleSheet);
+    if (thisContent.IsInShadowTree()) {
+      ShadowRoot* containingShadow = thisContent.GetContainingShadow();
+      // Could be null only during unlink.
+      if (MOZ_LIKELY(containingShadow)) {
+        containingShadow->RemoveStyleSheet(*mStyleSheet);
       }
+    } else {
+      doc->RemoveStyleSheet(*mStyleSheet);
     }
 
-    SetStyleSheet(nullptr);
+    LinkStyle::SetStyleSheet(nullptr);
   }
 
   if (!info) {

dom/base/LinkStyle.h

@@ -220,9 +220,6 @@ class LinkStyle {
 
   StyleSheet* GetSheet() const { return mStyleSheet; }
 
-  /** JS can only observe the sheet once fully loaded */
-  StyleSheet* GetSheetForBindings() const;
-
  protected:
   LinkStyle();
   virtual ~LinkStyle();

dom/webidl/LinkStyle.webidl

@@ -8,6 +8,6 @@
  */
 
 interface mixin LinkStyle {
-  [BinaryName="sheetForBindings"] readonly attribute StyleSheet? sheet;
+      readonly attribute StyleSheet? sheet;
 };
 

dom/xml/ProcessingInstruction.cpp

@@ -58,9 +58,9 @@ ProcessingInstruction::ProcessingInstruction(
 
 ProcessingInstruction::~ProcessingInstruction() = default;
 
-StyleSheet* ProcessingInstruction::GetSheetForBindings() const {
+StyleSheet* ProcessingInstruction::GetSheet() const {
   if (const auto* linkStyle = LinkStyle::FromNode(*this)) {
-    return linkStyle->GetSheetForBindings();
+    return linkStyle->GetSheet();
   }
   return nullptr;
 }

dom/xml/ProcessingInstruction.h

@@ -38,7 +38,7 @@ class ProcessingInstruction : public CharacterData {
   void GetTarget(nsAString& aTarget) { aTarget = NodeName(); }
   // This is the WebIDL API for LinkStyle, even though only
   // XMLStylesheetProcessingInstruction actually implements LinkStyle.
-  StyleSheet* GetSheetForBindings() const;
+  StyleSheet* GetSheet() const;
 
   NS_IMPL_FROMNODE_HELPER(ProcessingInstruction, IsProcessingInstruction())
 

layout/style/Loader.cpp

@@ -276,7 +276,6 @@ SheetLoadData::SheetLoadData(Loader* aLoader, const nsAString& aTitle,
       mPendingChildren(0),
       mSyncLoad(aSyncLoad),
       mIsNonDocumentSheet(false),
-      mIsChildSheet(aSheet->GetParentSheet()),
       mIsLoading(false),
       mIsBeingParsed(false),
       mIsCancelled(false),
@@ -318,7 +317,6 @@ SheetLoadData::SheetLoadData(Loader* aLoader, nsIURI* aURI, StyleSheet* aSheet,
       mPendingChildren(0),
       mSyncLoad(aParentData && aParentData->mSyncLoad),
       mIsNonDocumentSheet(aParentData && aParentData->mIsNonDocumentSheet),
-      mIsChildSheet(aSheet->GetParentSheet()),
       mIsLoading(false),
       mIsBeingParsed(false),
       mIsCancelled(false),
@@ -343,7 +341,6 @@ SheetLoadData::SheetLoadData(Loader* aLoader, nsIURI* aURI, StyleSheet* aSheet,
   MOZ_ASSERT(mTriggeringPrincipal);
   MOZ_ASSERT(!mUseSystemPrincipal || mSyncLoad,
              "Shouldn't use system principal for async loads");
-  MOZ_ASSERT_IF(aParentData, mIsChildSheet);
 }
 
 SheetLoadData::SheetLoadData(
@@ -361,7 +358,6 @@ SheetLoadData::SheetLoadData(
       mPendingChildren(0),
       mSyncLoad(aSyncLoad),
       mIsNonDocumentSheet(true),
-      mIsChildSheet(false),
       mIsLoading(false),
       mIsBeingParsed(false),
       mIsCancelled(false),
@@ -386,7 +382,6 @@ SheetLoadData::SheetLoadData(
   MOZ_ASSERT(mLoader, "Must have a loader!");
   MOZ_ASSERT(!mUseSystemPrincipal || mSyncLoad,
              "Shouldn't use system principal for async loads");
-  MOZ_ASSERT(!aSheet->GetParentSheet(), "Shouldn't be used for child loads");
 }
 
 SheetLoadData::~SheetLoadData() {
@@ -443,11 +438,7 @@ void SheetLoadData::FireLoadEvent(nsIThreadInternal* aThread) {
   RefPtr<SheetLoadData> kungFuDeathGrip(this);
   aThread->RemoveObserver(this);
 
-  // Now fire the event.
-  //
-  // NOTE(emilio): A bit weird that we fire the event even if the node is no
-  // longer in the tree, or the sheet that just loaded / errored is not the
-  // current node.sheet, but...
+  // Now fire the event
   nsCOMPtr<nsINode> node = mOwningNode;
   MOZ_ASSERT(node, "How did that happen???");
 
@@ -1043,14 +1034,20 @@ Loader::MediaMatched Loader::PrepareSheet(
  * 3) Sheets with linking elements are ordered based on document order
  *    as determined by CompareDocumentPosition.
  */
-void Loader::InsertSheetInTree(StyleSheet& aSheet, nsINode* aOwningNode) {
+void Loader::InsertSheetInTree(StyleSheet& aSheet,
+                               nsIContent* aLinkingContent) {
   LOG(("css::Loader::InsertSheetInTree"));
   MOZ_ASSERT(mDocument, "Must have a document to insert into");
-  MOZ_ASSERT(!aOwningNode || aOwningNode->IsInUncomposedDoc() ||
-                 aOwningNode->IsInShadowTree(),
+  MOZ_ASSERT(!aLinkingContent || aLinkingContent->IsInUncomposedDoc() ||
+                 aLinkingContent->IsInShadowTree(),
              "Why would we insert it anywhere?");
+
+  if (auto* linkStyle = LinkStyle::FromNodeOrNull(aLinkingContent)) {
+    linkStyle->SetStyleSheet(&aSheet);
+  }
+
   ShadowRoot* shadow =
-      aOwningNode ? aOwningNode->GetContainingShadow() : nullptr;
+      aLinkingContent ? aLinkingContent->GetContainingShadow() : nullptr;
 
   auto& target = shadow ? static_cast<DocumentOrShadowRoot&>(*shadow)
                         : static_cast<DocumentOrShadowRoot&>(*mDocument);
@@ -1070,7 +1067,7 @@ void Loader::InsertSheetInTree(StyleSheet& aSheet, nsINode* aOwningNode) {
   int32_t insertionPoint = sheetCount - 1;
   for (; insertionPoint >= 0; --insertionPoint) {
     nsINode* sheetOwner = target.SheetAt(insertionPoint)->GetOwnerNode();
-    if (sheetOwner && !aOwningNode) {
+    if (sheetOwner && !aLinkingContent) {
       // Keep moving; all sheets with a sheetOwner come after all
       // sheets without a linkingNode
       continue;
@@ -1082,11 +1079,11 @@ void Loader::InsertSheetInTree(StyleSheet& aSheet, nsINode* aOwningNode) {
       break;
     }
 
-    MOZ_ASSERT(aOwningNode != sheetOwner,
+    MOZ_ASSERT(aLinkingContent != sheetOwner,
                "Why do we still have our old sheet?");
 
     // Have to compare
-    if (nsContentUtils::PositionIsBefore(sheetOwner, aOwningNode)) {
+    if (nsContentUtils::PositionIsBefore(sheetOwner, aLinkingContent)) {
       // The current sheet comes before us, and it better be the first
       // such, because now we break
       break;
@@ -1698,12 +1695,7 @@ Result<Loader::LoadSheetResult, nsresult> Loader::LoadInlineStyle(
   auto matched = PrepareSheet(*sheet, aInfo.mTitle, aInfo.mMedia, nullptr,
                               isAlternate, aInfo.mIsExplicitlyEnabled);
 
-  if (auto* linkStyle = LinkStyle::FromNodeOrNull(aInfo.mContent)) {
-    linkStyle->SetStyleSheet(sheet);
-  }
-  if (StaticPrefs::dom_expose_incomplete_stylesheets() || sheet->IsComplete()) {
-    InsertSheetInTree(*sheet, aInfo.mContent);
-  }
+  InsertSheetInTree(*sheet, aInfo.mContent);
 
   Completed completed;
   if (sheetFromCache) {
@@ -1801,12 +1793,7 @@ Result<Loader::LoadSheetResult, nsresult> Loader::LoadStyleLink(
   auto matched = PrepareSheet(*sheet, aInfo.mTitle, aInfo.mMedia, nullptr,
                               isAlternate, aInfo.mIsExplicitlyEnabled);
 
-  if (auto* linkStyle = LinkStyle::FromNodeOrNull(aInfo.mContent)) {
-    linkStyle->SetStyleSheet(sheet);
-  }
-  if (StaticPrefs::dom_expose_incomplete_stylesheets() || sheet->IsComplete()) {
-    InsertSheetInTree(*sheet, aInfo.mContent);
-  }
+  InsertSheetInTree(*sheet, aInfo.mContent);
 
   // We may get here with no content for Link: headers for example.
   MOZ_ASSERT(!aInfo.mContent || LinkStyle::FromNode(*aInfo.mContent),

layout/style/Loader.h

@@ -515,7 +515,7 @@ class Loader final {
                             IsAlternate, IsExplicitlyEnabled);
 
   // Inserts a style sheet in a document or a ShadowRoot.
-  void InsertSheetInTree(StyleSheet& aSheet, nsINode* aOwningNode);
+  void InsertSheetInTree(StyleSheet& aSheet, nsIContent* aLinkingContent);
   // Inserts a style sheet into a parent style sheet.
   void InsertChildSheet(StyleSheet& aSheet, StyleSheet& aParentSheet);
 

layout/style/SharedStyleSheetCache.cpp

@@ -337,43 +337,6 @@ void SharedStyleSheetCache::LoadCompletedInternal(
       MOZ_ASSERT(data->mSheet->IsConstructed() ||
                      !data->mSheet->HasForcedUniqueInner(),
                  "should not get a forced unique inner during parsing");
-      // Insert the sheet into the tree now the sheet has loaded, but only if
-      // the sheet is still relevant, and if this is a top-level sheet.
-      const bool needInsertIntoTree = [&] {
-        if (StaticPrefs::dom_expose_incomplete_stylesheets()) {
-          // No need to do that, it's already done. This is technically a bit
-          // racy, but having to reload if you hit an in-progress load while
-          // switching the pref from about:config is not a big deal.
-          return false;
-        }
-        if (!data->mLoader->GetDocument()) {
-          // Not a document load, nothing to do.
-          return false;
-        }
-        if (data->mIsPreload != css::Loader::IsPreload::No) {
-          // Preloads are not supposed to be observable.
-          return false;
-        }
-        if (data->mSheet->IsConstructed()) {
-          // Constructable sheets are not in the regular stylesheet tree.
-          return false;
-        }
-        if (data->mIsChildSheet) {
-          // A child sheet, those will get exposed from the parent, no need to
-          // insert them into the tree.
-          return false;
-        }
-        if (data->mOwningNode != data->mSheet->GetOwnerNode()) {
-          // The sheet was already removed from the tree and is no longer the
-          // current sheet of the owning node, we can bail.
-          return false;
-        }
-        return true;
-      }();
-
-      if (needInsertIntoTree) {
-        data->mLoader->InsertSheetInTree(*data->mSheet, data->mOwningNode);
-      }
       data->mSheet->SetComplete();
       data->ScheduleLoadEventIfNeeded();
     } else if (data->mSheet->IsApplicable()) {

layout/style/SheetLoadData.h

@@ -137,10 +137,6 @@ class SheetLoadData final : public PreloaderBase,
   // proceed even if we have no document.
   const bool mIsNonDocumentSheet : 1;
 
-  // Whether this stylesheet is for a child sheet load. This is necessary
-  // because the sheet could be detached mid-load by CSSOM.
-  const bool mIsChildSheet : 1;
-
   // mIsLoading is true from the moment we are placed in the loader's
   // "loading datas" table (right after the async channel is opened)
   // to the moment we are removed from said table (due to the load

layout/style/test/test_load_events_on_stylesheets.html

@@ -119,9 +119,13 @@ link.onerror = function() { --pendingEventCounter;
 }
 document.body.appendChild(link);
 
+// Make sure we have that last stylesheet
+is(document.styleSheets.length, 7, "Should have seven stylesheets here");
+
+// Make sure that the sixth stylesheet is all loaded
 // If we ever switch away from sync loading of already-complete sheets, this
 // test will need adjusting
-checkSheetComplete(link.sheet, 1);
+checkSheetComplete(document.styleSheets[6], 1);
 
 ++pendingEventCounter;
 link = document.createElement("link");

modules/libpref/init/StaticPrefList.yaml

@@ -2088,16 +2088,6 @@
   value: true
   mirror: always
 
-# Whether the LinkStyle.sheet and DocumentOrShadowRoot.styleSheets accessor
-# exposes in-progress-loading stylesheets.
-#
-# Historical behavior is `true`, but other browsers (and also a bit of common
-# sense) match `false`.
-- name: dom.expose-incomplete-stylesheets
-  type: bool
-  value: false
-  mirror: always
-
 # Whether the Large-Allocation header is enabled.
 - name: dom.largeAllocationHeader.enabled
   type: bool