Bug 1617746 - Deal with duplicate adopted stylesheets correctly. r=nordzilla

This also fixes some other bits, like mAdoptedStyleSheets.ElementAt(0) (which
really should be "the first applicable adopted stylesheet") and so on. Added a
test for that.

The new invariant also implies that a DocumentOrShadowRoot can't appear twice in
StyleSheet::mAdopters.

Also clears adopted stylesheets in Document::ResetStylesheetsToURI, as it's the
right thing to do (if you hit that code path with constructable sheets you'd end
up asserting, as we'd call FillStyleSetDocumentSheets after and find sheets in
there). I don't quite have a test-case for that though.

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

--HG--
extra : moz-landing-system : lando

dom/base/Document.cpp

@@ -2705,71 +2705,35 @@ size_t Document::FindDocStyleSheetInsertionPoint(const StyleSheet& aSheet) {
   return index;
 }
 
-void Document::AppendAdoptedStyleSheet(StyleSheet& aSheet) {
-  DocumentOrShadowRoot::InsertAdoptedSheetAt(mAdoptedStyleSheets.Length(),
-                                             aSheet);
-  if (aSheet.IsApplicable()) {
-    AddStyleSheetToStyleSets(aSheet);
-  }
-}
-
-void Document::RemoveDocStyleSheetsFromStyleSets() {
-  MOZ_ASSERT(mStyleSetFilled);
-  // The stylesheets should forget us
-  //
-  // FIXME: Should also deal with adopted stylesheets, probably.
-  for (StyleSheet* sheet : Reversed(mStyleSheets)) {
-    sheet->ClearAssociatedDocumentOrShadowRoot();
-    if (sheet->IsApplicable()) {
-      mStyleSet->RemoveStyleSheet(*sheet);
-    }
-    // XXX Tell observers?
-  }
-}
-
-static void RemoveStyleSheetsFromStyleSets(
-    ServoStyleSet* aSet, const nsTArray<RefPtr<StyleSheet>>& aSheets) {
-  // The stylesheets should forget us
-  for (StyleSheet* sheet : Reversed(aSheets)) {
-    sheet->ClearAssociatedDocumentOrShadowRoot();
-    if (sheet->IsApplicable()) {
-      aSet->RemoveStyleSheet(*sheet);
-    }
-  }
-}
-
 void Document::ResetStylesheetsToURI(nsIURI* aURI) {
   MOZ_ASSERT(aURI);
 
-  if (mStyleSetFilled) {
-    // Skip removing style sheets from the style set if we know we haven't
-    // filled the style set.  (This allows us to avoid calling
-    // GetStyleBackendType() too early.)
-    RemoveDocStyleSheetsFromStyleSets();
-    RemoveStyleSheetsFromStyleSets(mStyleSet.get(),
-                                   mAdditionalSheets[eAgentSheet]);
-    RemoveStyleSheetsFromStyleSets(mStyleSet.get(),
-                                   mAdditionalSheets[eUserSheet]);
-    RemoveStyleSheetsFromStyleSets(mStyleSet.get(),
-                                   mAdditionalSheets[eAuthorSheet]);
+  ClearAdoptedStyleSheets();
 
-    if (nsStyleSheetService* sheetService =
-            nsStyleSheetService::GetInstance()) {
-      RemoveStyleSheetsFromStyleSets(mStyleSet.get(),
-                                     *sheetService->AuthorStyleSheets());
+  auto ClearSheetList = [&](nsTArray<RefPtr<StyleSheet>>& aSheetList) {
+    for (auto& sheet : Reversed(aSheetList)) {
+      sheet->ClearAssociatedDocumentOrShadowRoot();
+      if (mStyleSetFilled) {
+        mStyleSet->RemoveStyleSheet(*sheet);
+      }
+    }
+    aSheetList.Clear();
+  };
+  ClearSheetList(mStyleSheets);
+  for (auto& sheets : mAdditionalSheets) {
+    ClearSheetList(sheets);
+  }
+  if (mStyleSetFilled) {
+    if (auto* ss = nsStyleSheetService::GetInstance()) {
+      for (auto& sheet : Reversed(*ss->AuthorStyleSheets())) {
+        MOZ_ASSERT(!sheet->GetAssociatedDocumentOrShadowRoot());
+        if (sheet->IsApplicable()) {
+          mStyleSet->RemoveStyleSheet(*sheet);
+        }
+      }
     }
   }
 
-  // Release all the sheets
-  mStyleSheets.Clear();
-  for (auto& sheets : mAdditionalSheets) {
-    sheets.Clear();
-  }
-
-  // NOTE:  We don't release the catalog sheets.  It doesn't really matter
-  // now, but it could in the future -- in which case not releasing them
-  // is probably the right thing to do.
-
   // Now reset our inline style and attribute sheets.
   if (mAttrStyleSheet) {
     mAttrStyleSheet->Reset();
@@ -2928,19 +2892,23 @@ void Document::FillStyleSetDocumentSheets() {
   MOZ_ASSERT(mStyleSet->SheetCount(StyleOrigin::Author) == 0,
              "Style set already has document sheets?");
 
-  // Sheets are added in reverse order to avoid worst-case
-  // time complexity when looking up the index of a sheet
+  // Sheets are added in reverse order to avoid worst-case time complexity when
+  // looking up the index of a sheet.
+  //
+  // Note that usually appending is faster (rebuilds less stuff in the
+  // styleset), but in this case it doesn't matter since we're filling the
+  // styleset from scratch anyway.
   for (StyleSheet* sheet : Reversed(mStyleSheets)) {
     if (sheet->IsApplicable()) {
       mStyleSet->AddDocStyleSheet(*sheet);
     }
   }
 
-  for (StyleSheet* sheet : Reversed(mAdoptedStyleSheets)) {
-    if (sheet->IsApplicable()) {
-      mStyleSet->AddDocStyleSheet(*sheet);
+  EnumerateUniqueAdoptedStyleSheetsBackToFront([&](StyleSheet& aSheet) {
+    if (aSheet.IsApplicable()) {
+      mStyleSet->AddDocStyleSheet(aSheet);
     }
-  }
+  });
 
   nsStyleSheetService* sheetService = nsStyleSheetService::GetInstance();
   for (StyleSheet* sheet : *sheetService->AuthorStyleSheets()) {
@@ -16149,34 +16117,5 @@ Document::RecomputeContentBlockingAllowListPrincipal(
   return copy.forget();
 }
 
-// https://wicg.github.io/construct-stylesheets/#dom-documentorshadowroot-adoptedstylesheets
-void Document::SetAdoptedStyleSheets(
-    const Sequence<OwningNonNull<StyleSheet>>& aAdoptedStyleSheets,
-    ErrorResult& aRv) {
-  // Step 1 is a variable declaration
-
-  // 2.1 Check if all sheets are constructed, else throw NotAllowedError
-  // 2.2 Check if all sheets' constructor documents match the
-  // DocumentOrShadowRoot's node document, else throw NotAlloweError
-  EnsureAdoptedSheetsAreValid(aAdoptedStyleSheets, aRv);
-  if (aRv.Failed()) {
-    return;
-  }
-
-  // 3. Set the adopted style sheets to the new sheets
-  for (const RefPtr<StyleSheet>& sheet : mAdoptedStyleSheets) {
-    if (sheet->IsApplicable()) {
-      RemoveStyleSheetFromStyleSets(*sheet);
-    }
-    sheet->RemoveAdopter(*this);
-  }
-  mAdoptedStyleSheets.ClearAndRetainStorage();
-  mAdoptedStyleSheets.SetCapacity(aAdoptedStyleSheets.Length());
-  for (const OwningNonNull<StyleSheet>& sheet : aAdoptedStyleSheets) {
-    sheet->AddAdopter(*this);
-    AppendAdoptedStyleSheet(*sheet);
-  }
-}
-
 }  // namespace dom
 }  // namespace mozilla

dom/base/Document.h

@@ -475,6 +475,8 @@ class Document : public nsINode,
                  public nsStubMutationObserver,
                  public DispatcherTrait,
                  public SupportsWeakPtr<Document> {
+  friend class DocumentOrShadowRoot;
+
  protected:
   explicit Document(const char* aContentType);
   virtual ~Document();
@@ -1726,8 +1728,6 @@ class Document : public nsINode,
     return mAdditionalSheets[eAuthorSheet].SafeElementAt(0);
   }
 
-  void AppendAdoptedStyleSheet(StyleSheet& aSheet);
-
   /**
    * Returns the index that aSheet should be inserted at to maintain document
    * ordering.
@@ -3882,10 +3882,6 @@ class Document : public nsINode,
 
   already_AddRefed<Promise> AddCertException(bool aIsTemporary);
 
-  void SetAdoptedStyleSheets(
-      const Sequence<OwningNonNull<StyleSheet>>& aAdoptedStyleSheets,
-      ErrorResult& aRv);
-
  protected:
   void DoUpdateSVGUseElementShadowTrees();
 

dom/base/DocumentOrShadowRoot.cpp

@@ -78,11 +78,6 @@ void DocumentOrShadowRoot::InsertSheetAt(size_t aIndex, StyleSheet& aSheet) {
   mStyleSheets.InsertElementAt(aIndex, &aSheet);
 }
 
-void DocumentOrShadowRoot::InsertAdoptedSheetAt(size_t aIndex,
-                                                StyleSheet& aSheet) {
-  mAdoptedStyleSheets.InsertElementAt(aIndex, &aSheet);
-}
-
 already_AddRefed<StyleSheet> DocumentOrShadowRoot::RemoveSheet(
     StyleSheet& aSheet) {
   auto index = mStyleSheets.IndexOf(&aSheet);
@@ -95,13 +90,24 @@ already_AddRefed<StyleSheet> DocumentOrShadowRoot::RemoveSheet(
   return sheet.forget();
 }
 
+void DocumentOrShadowRoot::RemoveSheetFromStylesIfApplicable(
+    StyleSheet& aSheet) {
+  if (!aSheet.IsApplicable()) {
+    return;
+  }
+  if (mKind == Kind::Document) {
+    AsNode().AsDocument()->RemoveStyleSheetFromStyleSets(aSheet);
+  } else {
+    MOZ_ASSERT(AsNode().IsShadowRoot());
+    static_cast<ShadowRoot&>(AsNode()).RemoveSheetFromStyles(aSheet);
+  }
+}
+
 // https://wicg.github.io/construct-stylesheets/#dom-documentorshadowroot-adoptedstylesheets
-void DocumentOrShadowRoot::EnsureAdoptedSheetsAreValid(
+void DocumentOrShadowRoot::SetAdoptedStyleSheets(
     const Sequence<OwningNonNull<StyleSheet>>& aAdoptedStyleSheets,
     ErrorResult& aRv) {
-  nsTHashtable<nsPtrHashKey<const StyleSheet>> set(
-      aAdoptedStyleSheets.Length());
-
+  Document& doc = *AsNode().OwnerDoc();
   for (const OwningNonNull<StyleSheet>& sheet : aAdoptedStyleSheets) {
     // 2.1 Check if all sheets are constructed, else throw NotAllowedError
     if (!sheet->IsConstructed()) {
@@ -111,23 +117,51 @@ void DocumentOrShadowRoot::EnsureAdoptedSheetsAreValid(
     }
     // 2.2 Check if all sheets' constructor documents match the
     // DocumentOrShadowRoot's node document, else throw NotAlloweError
-    if (!sheet->ConstructorDocumentMatches(AsNode().OwnerDoc())) {
+    if (!sheet->ConstructorDocumentMatches(doc)) {
       return aRv.ThrowNotAllowedError(
           "Each adopted style sheet's constructor document must match the "
           "document or shadow root's node document");
     }
+  }
 
-    // FIXME(nordzilla): This is temporary code to disallow duplicate sheets.
-    // This exists to ensure that the fuzzers aren't blocked.
-    // This code will eventually be removed.
-    // https://bugzilla.mozilla.org/show_bug.cgi?id=1617302
-    if (!set.EnsureInserted(sheet.get())) {
-      return aRv.ThrowNotAllowedError(
-          "Temporarily disallowing duplicate stylesheets.");
+  auto* shadow = ShadowRoot::FromNode(AsNode());
+  MOZ_ASSERT((mKind == Kind::ShadowRoot) == !!shadow);
+
+  ClearAdoptedStyleSheets();
+
+  // 3. Set the adopted style sheets to the new sheets
+  mAdoptedStyleSheets.SetCapacity(aAdoptedStyleSheets.Length());
+
+  AdoptedStyleSheetSet set(aAdoptedStyleSheets.Length());
+  for (const OwningNonNull<StyleSheet>& sheet : aAdoptedStyleSheets) {
+    if (MOZ_UNLIKELY(!set.EnsureInserted(sheet.get()))) {
+      // The idea is that this case is rare, so we pay the price of removing the
+      // old sheet from the styles and append it later rather than the other way
+      // around.
+      RemoveSheetFromStylesIfApplicable(*sheet);
+    } else {
+      sheet->AddAdopter(*this);
+    }
+    mAdoptedStyleSheets.AppendElement(sheet);
+    if (sheet->IsApplicable()) {
+      if (mKind == Kind::Document) {
+        doc.AddStyleSheetToStyleSets(*sheet);
+      } else {
+        shadow->InsertSheetIntoAuthorData(mAdoptedStyleSheets.Length() - 1,
+                                          *sheet, mAdoptedStyleSheets);
+      }
     }
   }
 }
 
+void DocumentOrShadowRoot::ClearAdoptedStyleSheets() {
+  EnumerateUniqueAdoptedStyleSheetsBackToFront([&](StyleSheet& aSheet) {
+    RemoveSheetFromStylesIfApplicable(aSheet);
+    aSheet.RemoveAdopter(*this);
+  });
+  mAdoptedStyleSheets.Clear();
+}
+
 Element* DocumentOrShadowRoot::GetElementById(const nsAString& aElementId) {
   if (MOZ_UNLIKELY(aElementId.IsEmpty())) {
     nsContentUtils::ReportEmptyGetElementByIdArg(AsNode().OwnerDoc());
@@ -662,7 +696,9 @@ nsRadioGroupStruct* DocumentOrShadowRoot::GetOrCreateRadioGroup(
 int32_t DocumentOrShadowRoot::StyleOrderIndexOfSheet(
     const StyleSheet& aSheet) const {
   if (aSheet.IsConstructed()) {
-    int32_t index = mAdoptedStyleSheets.IndexOf(&aSheet);
+    // NOTE: constructable sheets can have duplicates, so we need to start
+    // looking from behind.
+    int32_t index = mAdoptedStyleSheets.LastIndexOf(&aSheet);
     return (index < 0) ? index : index + SheetCount();
   }
   return mStyleSheets.IndexOf(&aSheet);
@@ -679,26 +715,27 @@ void DocumentOrShadowRoot::Traverse(DocumentOrShadowRoot* tmp,
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mDOMStyleSheets)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mAdoptedStyleSheets)
 
-  auto NoteSheets = [tmp, &cb = cb](nsTArray<RefPtr<StyleSheet>>& sheetList) {
-    for (StyleSheet* sheet : sheetList) {
-      if (!sheet->IsApplicable()) {
-        continue;
-      }
-      // The style set or mServoStyles keep more references to it if the sheet
-      // is applicable.
-      if (tmp->mKind == Kind::ShadowRoot) {
-        NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mServoStyles->sheets[i]");
-        cb.NoteXPCOMChild(sheet);
-      } else if (tmp->AsNode().AsDocument()->StyleSetFilled()) {
-        NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(
-            cb, "mStyleSet->mRawSet.stylist.stylesheets.author[i]");
-        cb.NoteXPCOMChild(sheet);
-      }
+  auto NoteSheetIfApplicable = [&](StyleSheet& aSheet) {
+    if (!aSheet.IsApplicable()) {
+      return;
+    }
+    // The style set or mServoStyles keep more references to it if the sheet
+    // is applicable.
+    if (tmp->mKind == Kind::ShadowRoot) {
+      NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mServoStyles->sheets[i]");
+      cb.NoteXPCOMChild(&aSheet);
+    } else if (tmp->AsNode().AsDocument()->StyleSetFilled()) {
+      NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(
+          cb, "mStyleSet->mRawSet.stylist.stylesheets.author[i]");
+      cb.NoteXPCOMChild(&aSheet);
     }
   };
 
-  NoteSheets(tmp->mStyleSheets);
-  NoteSheets(tmp->mAdoptedStyleSheets);
+  for (auto& sheet : tmp->mStyleSheets) {
+    NoteSheetIfApplicable(*sheet);
+  }
+
+  tmp->EnumerateUniqueAdoptedStyleSheetsBackToFront(NoteSheetIfApplicable);
 
   for (auto iter = tmp->mIdentifierMap.ConstIter(); !iter.Done(); iter.Next()) {
     iter.Get()->Traverse(&cb);

dom/base/DocumentOrShadowRoot.h

@@ -221,15 +221,33 @@ class DocumentOrShadowRoot {
 
   nsIContent* Retarget(nsIContent* aContent) const;
 
+  void SetAdoptedStyleSheets(
+      const Sequence<OwningNonNull<StyleSheet>>& aAdoptedStyleSheets,
+      ErrorResult& aRv);
+
+  // This is needed because ServoStyleSet / ServoAuthorData don't deal with
+  // duplicate stylesheets (and it's unclear we'd want to support that as it'd
+  // be a bunch of duplicate work), while adopted stylesheets do need to deal
+  // with them.
+  template <typename Callback>
+  void EnumerateUniqueAdoptedStyleSheetsBackToFront(Callback aCallback) {
+    AdoptedStyleSheetSet set(mAdoptedStyleSheets.Length());
+    for (StyleSheet* sheet : Reversed(mAdoptedStyleSheets)) {
+      if (MOZ_UNLIKELY(!set.EnsureInserted(sheet))) {
+        continue;
+      }
+      aCallback(*sheet);
+    }
+  }
+
  protected:
+  using AdoptedStyleSheetSet = nsTHashtable<nsPtrHashKey<const StyleSheet>>;
+  void RemoveSheetFromStylesIfApplicable(StyleSheet&);
+  void ClearAdoptedStyleSheets();
+
   // Returns the reference to the sheet, if found in mStyleSheets.
   already_AddRefed<StyleSheet> RemoveSheet(StyleSheet& aSheet);
   void InsertSheetAt(size_t aIndex, StyleSheet& aSheet);
-  void InsertAdoptedSheetAt(size_t aIndex, StyleSheet& aSheet);
-
-  void EnsureAdoptedSheetsAreValid(
-      const Sequence<OwningNonNull<StyleSheet>>& aAdoptedStyleSheets,
-      ErrorResult& aRv);
 
   void AddSizeOfExcludingThis(nsWindowSizes&) const;
   void AddSizeOfOwnedSheetArrayExcludingThis(

dom/base/ShadowRoot.cpp

@@ -376,11 +376,17 @@ void ShadowRoot::InsertSheetAt(size_t aIndex, StyleSheet& aSheet) {
   }
 }
 
-void ShadowRoot::InsertAdoptedSheetAt(size_t aIndex, StyleSheet& aSheet) {
-  DocumentOrShadowRoot::InsertAdoptedSheetAt(aIndex, aSheet);
-  if (aSheet.IsApplicable()) {
-    InsertSheetIntoAuthorData(aIndex, aSheet, mAdoptedStyleSheets);
+StyleSheet* FirstApplicableAdoptedStyleSheet(
+    const nsTArray<RefPtr<StyleSheet>>& aList) {
+  size_t i = 0;
+  for (StyleSheet* sheet : aList) {
+    // Deal with duplicate sheets by only considering the last one.
+    if (sheet->IsApplicable() && MOZ_LIKELY(aList.LastIndexOf(sheet) == i)) {
+      return sheet;
+    }
+    i++;
   }
+  return nullptr;
 }
 
 void ShadowRoot::InsertSheetIntoAuthorData(
@@ -388,6 +394,7 @@ void ShadowRoot::InsertSheetIntoAuthorData(
     const nsTArray<RefPtr<StyleSheet>>& aList) {
   MOZ_ASSERT(aSheet.IsApplicable());
   MOZ_ASSERT(aList[aIndex] == &aSheet);
+  MOZ_ASSERT(aList.LastIndexOf(&aSheet) == aIndex);
   MOZ_ASSERT(&aList == &mAdoptedStyleSheets || &aList == &mStyleSheets);
 
   if (!mServoStyles) {
@@ -398,32 +405,45 @@ void ShadowRoot::InsertSheetIntoAuthorData(
     mStyleRuleMap->SheetAdded(aSheet);
   }
 
+  auto changedOnExit =
+      mozilla::MakeScopeExit([&] { ApplicableRulesChanged(); });
+
   for (size_t i = aIndex + 1; i < aList.Length(); ++i) {
     StyleSheet* beforeSheet = aList.ElementAt(i);
     if (!beforeSheet->IsApplicable()) {
       continue;
     }
 
+    // If this is a duplicate adopted stylesheet that is not in the right
+    // position (the last one) then we skip over it. Otherwise we're done.
+    if (&aList == &mAdoptedStyleSheets &&
+        MOZ_UNLIKELY(aList.LastIndexOf(beforeSheet) != i)) {
+      continue;
+    }
+
     Servo_AuthorStyles_InsertStyleSheetBefore(mServoStyles.get(), &aSheet,
                                               beforeSheet);
-    ApplicableRulesChanged();
     return;
   }
 
   if (mAdoptedStyleSheets.IsEmpty() || &aList == &mAdoptedStyleSheets) {
     Servo_AuthorStyles_AppendStyleSheet(mServoStyles.get(), &aSheet);
-  } else {
-    Servo_AuthorStyles_InsertStyleSheetBefore(mServoStyles.get(), &aSheet,
-                                              mAdoptedStyleSheets.ElementAt(0));
+    return;
+  }
+
+  if (auto* before = FirstApplicableAdoptedStyleSheet(mAdoptedStyleSheets)) {
+    Servo_AuthorStyles_InsertStyleSheetBefore(mServoStyles.get(), &aSheet,
+                                              before);
+  } else {
+    Servo_AuthorStyles_AppendStyleSheet(mServoStyles.get(), &aSheet);
   }
-  ApplicableRulesChanged();
 }
 
 // FIXME(emilio): This needs to notify document observers and such,
 // presumably.
 void ShadowRoot::StyleSheetApplicableStateChanged(StyleSheet& aSheet) {
   auto& sheetList = aSheet.IsConstructed() ? mAdoptedStyleSheets : mStyleSheets;
-  int32_t index = sheetList.IndexOf(&aSheet);
+  int32_t index = sheetList.LastIndexOf(&aSheet);
   if (index < 0) {
     // NOTE(emilio): @import sheets are handled in the relevant RuleAdded
     // notification, which only notifies after the sheet is loaded.
@@ -446,29 +466,22 @@ void ShadowRoot::StyleSheetApplicableStateChanged(StyleSheet& aSheet) {
   }
 }
 
-void ShadowRoot::ClearAdoptedStyleSheets() {
-  for (const RefPtr<StyleSheet>& sheet : mAdoptedStyleSheets) {
-    RemoveSheetFromStyles(*sheet);
-    sheet->RemoveAdopter(*this);
-  }
-  mAdoptedStyleSheets.Clear();
-}
-
 void ShadowRoot::RemoveSheetFromStyles(StyleSheet& aSheet) {
-  if (aSheet.IsApplicable()) {
-    MOZ_ASSERT(mServoStyles);
-    if (mStyleRuleMap) {
-      mStyleRuleMap->SheetRemoved(aSheet);
-    }
-    Servo_AuthorStyles_RemoveStyleSheet(mServoStyles.get(), &aSheet);
-    ApplicableRulesChanged();
+  MOZ_ASSERT(aSheet.IsApplicable());
+  MOZ_ASSERT(mServoStyles);
+  if (mStyleRuleMap) {
+    mStyleRuleMap->SheetRemoved(aSheet);
   }
+  Servo_AuthorStyles_RemoveStyleSheet(mServoStyles.get(), &aSheet);
+  ApplicableRulesChanged();
 }
 
 void ShadowRoot::RemoveSheet(StyleSheet& aSheet) {
   RefPtr<StyleSheet> sheet = DocumentOrShadowRoot::RemoveSheet(aSheet);
   MOZ_ASSERT(sheet);
-  RemoveSheetFromStyles(*sheet);
+  if (sheet->IsApplicable()) {
+    RemoveSheetFromStyles(*sheet);
+  }
 }
 
 void ShadowRoot::AddToIdTable(Element* aElement, nsAtom* aId) {
@@ -714,29 +727,3 @@ nsresult ShadowRoot::Clone(dom::NodeInfo* aNodeInfo, nsINode** aResult) const {
   *aResult = nullptr;
   return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
 }
-
-// https://wicg.github.io/construct-stylesheets/#dom-documentorshadowroot-adoptedstylesheets
-void ShadowRoot::SetAdoptedStyleSheets(
-    const Sequence<OwningNonNull<StyleSheet>>& aAdoptedStyleSheets,
-    ErrorResult& aRv) {
-  // Step 1 is a variable declaration
-
-  // 2.1 Check if all sheets are constructed, else throw NotAllowedError
-  // 2.2 Check if all sheets' constructor documents match the
-  // DocumentOrShadowRoot's node document, else throw NotAlloweError
-  EnsureAdoptedSheetsAreValid(aAdoptedStyleSheets, aRv);
-  if (aRv.Failed()) {
-    return;
-  }
-
-  // 3. Set the adopted style sheets to the new sheets
-  // TODO(nordzilla): There are optimizations that can be made here
-  // in the case of only appending new sheets.
-  // See https://bugzilla.mozilla.org/show_bug.cgi?id=1611236
-  ClearAdoptedStyleSheets();
-  mAdoptedStyleSheets.SetCapacity(aAdoptedStyleSheets.Length());
-  for (const OwningNonNull<StyleSheet>& sheet : aAdoptedStyleSheets) {
-    sheet->AddAdopter(*this);
-    AppendAdoptedStyleSheet(*sheet);
-  }
-}

dom/base/ShadowRoot.h

@@ -41,6 +41,8 @@ class HTMLInputElement;
 class ShadowRoot final : public DocumentFragment,
                          public DocumentOrShadowRoot,
                          public nsIRadioGroupContainer {
+  friend class DocumentOrShadowRoot;
+
  public:
   NS_IMPL_FROMNODE_HELPER(ShadowRoot, IsShadowRoot());
 
@@ -86,7 +88,6 @@ class ShadowRoot final : public DocumentFragment,
    */
   void CloneInternalDataFrom(ShadowRoot* aOther);
   void InsertSheetAt(size_t aIndex, StyleSheet&);
-  void InsertAdoptedSheetAt(size_t aIndex, StyleSheet&);
 
   // Calls UnbindFromTree for each of our kids, and also flags us as no longer
   // being connected.
@@ -110,10 +111,6 @@ class ShadowRoot final : public DocumentFragment,
     InsertSheetAt(SheetCount(), aSheet);
   }
 
-  void AppendAdoptedStyleSheet(StyleSheet& aSheet) {
-    InsertAdoptedSheetAt(AdoptedSheetCount(), aSheet);
-  }
-
   /**
    * Represents the insertion point in a slot for a given node.
    */
@@ -256,12 +253,6 @@ class ShadowRoot final : public DocumentFragment,
     return DocumentOrShadowRoot::SetValueMissingState(aName, aValue);
   }
 
-  void SetAdoptedStyleSheets(
-      const Sequence<OwningNonNull<StyleSheet>>& aAdoptedStyleSheets,
-      ErrorResult& aRv);
-
-  void ClearAdoptedStyleSheets();
-
  protected:
   // FIXME(emilio): This will need to become more fine-grained.
   void ApplicableRulesChanged();

layout/inspector/InspectorUtils.cpp

@@ -90,6 +90,9 @@ void InspectorUtils::GetAllStyleSheets(GlobalObject& aGlobalObject,
   for (size_t i = 0; i < aDocument.SheetCount(); i++) {
     aResult.AppendElement(aDocument.SheetAt(i));
   }
+
+  // FIXME(emilio, bug 1617948): This doesn't deal with adopted stylesheets, and
+  // it should. It should also handle duplicates correctly when it does.
 }
 
 bool InspectorUtils::IsIgnorableWhitespace(CharacterData& aDataNode) {

layout/style/StyleSheet.h

@@ -394,14 +394,15 @@ class StyleSheet final : public nsICSSLoaderObserver, public nsWrapperCache {
   bool IsConstructed() const { return !!mConstructorDocument; }
 
   // Ture if the sheet's constructor document matches the given document
-  bool ConstructorDocumentMatches(dom::Document* aDocument) const {
-    return mConstructorDocument == aDocument;
+  bool ConstructorDocumentMatches(dom::Document& aDocument) const {
+    return mConstructorDocument == &aDocument;
   }
 
   // Add a document or shadow root to the list of adopters.
   // Adopters will be notified when styles are changed.
   void AddAdopter(dom::DocumentOrShadowRoot& aAdopter) {
     MOZ_ASSERT(IsConstructed());
+    MOZ_ASSERT(!mAdopters.Contains(&aAdopter));
     mAdopters.AppendElement(&aAdopter);
   }
 

testing/web-platform/meta/css/cssom/CSSStyleSheet-constructable-disabled-regular-sheet-insertion.html.ini

@@ -0,0 +1,2 @@
+[CSSStyleSheet-constructable-disabled-regular-sheet-insertion.html]
+  prefs: [layout.css.constructable-stylesheets.enabled:true]

testing/web-platform/meta/css/cssom/CSSStyleSheet-constructable-duplicate.html.ini

@@ -0,0 +1,2 @@
+[CSSStyleSheet-constructable-duplicate.html]
+  prefs: [layout.css.constructable-stylesheets.enabled:true]

testing/web-platform/tests/css/cssom/CSSStyleSheet-constructable-disabled-regular-sheet-insertion.html

@@ -0,0 +1,22 @@
+<!doctype html>
+<title>Shouldn't crash / assert when inserting a stylesheet after there are disabled constructable sheets</title>
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<link rel="help" href="https://wicg.github.io/construct-stylesheets/">
+<script src = '/resources/testharness.js'></script>
+<script src = '/resources/testharnessreport.js'></script>
+<div id="host"></div>
+<script>
+test(function() {
+  let sheet = new CSSStyleSheet({ disabled: true });
+  sheet.replaceSync("div { color: red }");
+
+  let root = document.getElementById("host").attachShadow({ mode: "open" });
+  root.adoptedStyleSheets = [sheet];
+  let style = document.createElement("style");
+  root.innerHTML = `
+    <style>div { color: green }</style>
+    <div>Should be green</div>
+  `;
+  assert_equals(getComputedStyle(root.querySelector("div")).color, "rgb(0, 128, 0)", "Should insert the sheet at the right position and not crash");
+});
+</script>

testing/web-platform/tests/css/cssom/CSSStyleSheet-constructable-duplicate.html

@@ -0,0 +1,20 @@
+<!doctype html>
+<title>Cascade order of a stylesheet for duplicate sheets.</title>
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<link rel="help" href="https://wicg.github.io/construct-stylesheets/">
+<script src = '/resources/testharness.js'></script>
+<script src = '/resources/testharnessreport.js'></script>
+<div></div>
+<script>
+test(function() {
+  let sheets = [];
+
+  for (let i = 0; i < 2; ++i) {
+    sheets.push(new CSSStyleSheet());
+    sheets[i].replaceSync("div { z-index: " + i + " }");
+  }
+
+  document.adoptedStyleSheets = [sheets[1], sheets[0], sheets[1]];
+  assert_equals(getComputedStyle(document.querySelector("div")).zIndex, "1", "duplicate stylesheet should take right position in the cascade");
+});
+</script>