Bug 1470358: Deduplicate sheet insertion code between document and shadow root. r=heycam draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 22 Jun 2018 05:13:18 +0200
changeset 809439 db391e298008795d3c4f0d388c44acc7f19bb0fb
parent 809438 fe942005ce0606f0eb11ea1c155a63fd6068d12c
child 809440 8d59a43efbfd14c0979e9c141ba0b2f9ca166f54
child 809443 acb272c27fe32d83d5b731cdb62898f7afc36454
child 809511 5ece0c83074b8e7d7ec314fbc0208fb3b4a56b4e
push id113676
push userbmo:emilio@crisal.io
push dateFri, 22 Jun 2018 03:46:37 +0000
reviewersheycam
bugs1470358
milestone62.0a1
Bug 1470358: Deduplicate sheet insertion code between document and shadow root. r=heycam Summary: Sort of straight-forward cleanup. Test Plan: Covered by existing tests. Reviewers: heycam Tags: #secure-revision Bug #: 1470358 Differential Revision: https://phabricator.services.mozilla.com/D1763 MozReview-Commit-ID: 6NiHFo0Y75
dom/base/DocumentOrShadowRoot.cpp
dom/base/DocumentOrShadowRoot.h
dom/base/ShadowRoot.cpp
dom/base/ShadowRoot.h
dom/base/nsDocument.cpp
dom/base/nsIDocument.h
layout/style/Loader.cpp
layout/style/Loader.h
--- a/dom/base/DocumentOrShadowRoot.cpp
+++ b/dom/base/DocumentOrShadowRoot.cpp
@@ -30,24 +30,16 @@ DocumentOrShadowRoot::EnsureDOMStyleShee
 {
   if (!mDOMStyleSheets) {
     mDOMStyleSheets = new StyleSheetList(*this);
   }
   return *mDOMStyleSheets;
 }
 
 void
-DocumentOrShadowRoot::AppendSheet(StyleSheet& aSheet)
-{
-  aSheet.SetAssociatedDocumentOrShadowRoot(
-    this, StyleSheet::OwnedByDocumentOrShadowRoot);
-  mStyleSheets.AppendElement(&aSheet);
-}
-
-void
 DocumentOrShadowRoot::InsertSheetAt(size_t aIndex, StyleSheet& aSheet)
 {
   aSheet.SetAssociatedDocumentOrShadowRoot(
     this, StyleSheet::OwnedByDocumentOrShadowRoot);
   mStyleSheets.InsertElementAt(aIndex, &aSheet);
 }
 
 already_AddRefed<StyleSheet>
--- a/dom/base/DocumentOrShadowRoot.h
+++ b/dom/base/DocumentOrShadowRoot.h
@@ -174,17 +174,16 @@ public:
     return true;
   }
 
   void ReportEmptyGetElementByIdArg();
 
 protected:
   // Returns the reference to the sheet, if found in mStyleSheets.
   already_AddRefed<StyleSheet> RemoveSheet(StyleSheet& aSheet);
-  void AppendSheet(StyleSheet& aSheet);
   void InsertSheetAt(size_t aIndex, StyleSheet& aSheet);
 
   nsIContent* Retarget(nsIContent* aContent) const;
 
   /**
    * If focused element's subtree root is this document or shadow root, return
    * focused element, otherwise, get the shadow host recursively until the
    * shadow host's subtree root is this document or shadow root.
--- a/dom/base/ShadowRoot.cpp
+++ b/dom/base/ShadowRoot.cpp
@@ -321,58 +321,16 @@ ShadowRoot::InsertSheetAt(size_t aIndex,
 {
   DocumentOrShadowRoot::InsertSheetAt(aIndex, aSheet);
   if (aSheet.IsApplicable()) {
     InsertSheetIntoAuthorData(aIndex, aSheet);
   }
 }
 
 void
-ShadowRoot::AppendStyleSheet(StyleSheet& aSheet)
-{
-  DocumentOrShadowRoot::AppendSheet(aSheet);
-  if (aSheet.IsApplicable()) {
-    Servo_AuthorStyles_AppendStyleSheet(mServoStyles.get(), &aSheet);
-    if (mStyleRuleMap) {
-      mStyleRuleMap->SheetAdded(aSheet);
-    }
-    ApplicableRulesChanged();
-  }
-}
-
-void
-ShadowRoot::InsertSheet(StyleSheet* aSheet, nsIContent* aLinkingContent)
-{
-  nsCOMPtr<nsIStyleSheetLinkingElement>
-    linkingElement = do_QueryInterface(aLinkingContent);
-
-  // FIXME(emilio, bug 1410578): <link> should probably also be allowed here.
-  MOZ_ASSERT(linkingElement, "The only styles in a ShadowRoot should come "
-                             "from <style>.");
-
-  linkingElement->SetStyleSheet(aSheet); // This sets the ownerNode on the sheet
-
-  // Find the correct position to insert into the style sheet list (must
-  // be in tree order).
-  for (size_t i = 0; i <= SheetCount(); i++) {
-    if (i == SheetCount()) {
-      AppendStyleSheet(*aSheet);
-      return;
-    }
-
-    StyleSheet* sheet = SheetAt(i);
-    nsINode* sheetOwningNode = sheet->GetOwnerNode();
-    if (nsContentUtils::PositionIsBefore(aLinkingContent, sheetOwningNode)) {
-      InsertSheetAt(i, *aSheet);
-      return;
-    }
-  }
-}
-
-void
 ShadowRoot::InsertSheetIntoAuthorData(size_t aIndex, StyleSheet& aSheet)
 {
   MOZ_ASSERT(SheetAt(aIndex) == &aSheet);
   MOZ_ASSERT(aSheet.IsApplicable());
 
   if (mStyleRuleMap) {
     mStyleRuleMap->SheetAdded(aSheet);
   }
--- a/dom/base/ShadowRoot.h
+++ b/dom/base/ShadowRoot.h
@@ -63,37 +63,40 @@ public:
   {
     return mMode;
   }
   bool IsClosed() const
   {
     return mMode == ShadowRootMode::Closed;
   }
 
-  void InsertSheet(StyleSheet* aSheet, nsIContent* aLinkingContent);
   void RemoveSheet(StyleSheet* aSheet);
   void RuleAdded(StyleSheet&, css::Rule&);
   void RuleRemoved(StyleSheet&, css::Rule&);
   void RuleChanged(StyleSheet&, css::Rule*);
   void StyleSheetApplicableStateChanged(StyleSheet&, bool aApplicable);
 
   StyleSheetList* StyleSheets()
   {
     return &DocumentOrShadowRoot::EnsureDOMStyleSheets();
   }
 
   /**
    * Clones internal state, for example stylesheets, of aOther to 'this'.
    */
   void CloneInternalDataFrom(ShadowRoot* aOther);
+  void InsertSheetAt(size_t aIndex, StyleSheet&);
+
 private:
   void InsertSheetIntoAuthorData(size_t aIndex, StyleSheet&);
 
-  void InsertSheetAt(size_t aIndex, StyleSheet&);
-  void AppendStyleSheet(StyleSheet&);
+  void AppendStyleSheet(StyleSheet& aSheet)
+  {
+    InsertSheetAt(SheetCount(), aSheet);
+  }
 
   /**
    * Try to reassign an element to a slot and returns whether the assignment
    * changed.
    */
   void MaybeReassignElement(Element* aElement);
 
   /**
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -4165,29 +4165,16 @@ nsIDocument::NotifyStyleSheetRemoved(Sty
     DO_STYLESHEET_NOTIFICATION(StyleSheetChangeEvent,
                                "StyleSheetRemoved",
                                mDocumentSheet,
                                aDocumentSheet);
   }
 }
 
 void
-nsIDocument::AddStyleSheet(StyleSheet* aSheet)
-{
-  MOZ_ASSERT(aSheet);
-  DocumentOrShadowRoot::AppendSheet(*aSheet);
-
-  if (aSheet->IsApplicable()) {
-    AddStyleSheetToStyleSets(aSheet);
-  }
-
-  NotifyStyleSheetAdded(aSheet, true);
-}
-
-void
 nsIDocument::RemoveStyleSheetFromStyleSets(StyleSheet* aSheet)
 {
   if (nsIPresShell* shell = GetShell()) {
     shell->StyleSet()->RemoveDocStyleSheet(aSheet);
     shell->ApplicableStylesChanged();
   }
 }
 
@@ -4241,27 +4228,25 @@ nsIDocument::UpdateStyleSheets(nsTArray<
       }
 
       NotifyStyleSheetAdded(newSheet, true);
     }
   }
 }
 
 void
-nsIDocument::InsertStyleSheetAt(StyleSheet* aSheet, size_t aIndex)
-{
-  MOZ_ASSERT(aSheet);
-
-  DocumentOrShadowRoot::InsertSheetAt(aIndex, *aSheet);
-
-  if (aSheet->IsApplicable()) {
-    AddStyleSheetToStyleSets(aSheet);
-  }
-
-  NotifyStyleSheetAdded(aSheet, true);
+nsIDocument::InsertSheetAt(size_t aIndex, StyleSheet& aSheet)
+{
+  DocumentOrShadowRoot::InsertSheetAt(aIndex, aSheet);
+
+  if (aSheet.IsApplicable()) {
+    AddStyleSheetToStyleSets(&aSheet);
+  }
+
+  NotifyStyleSheetAdded(&aSheet, true);
 }
 
 
 void
 nsIDocument::SetStyleSheetApplicableState(StyleSheet* aSheet, bool aApplicable)
 {
   MOZ_ASSERT(aSheet, "null arg");
 
--- a/dom/base/nsIDocument.h
+++ b/dom/base/nsIDocument.h
@@ -1460,41 +1460,41 @@ public:
    * Style sheets are ordered, most significant last.
    */
 
   mozilla::dom::StyleSheetList* StyleSheets()
   {
     return &DocumentOrShadowRoot::EnsureDOMStyleSheets();
   }
 
-  /**
-   * Insert a sheet at a particular spot in the stylesheet list (zero-based)
-   * @param aSheet the sheet to insert
-   * @param aIndex the index to insert at.
-   * @throws no exceptions
-   */
-  void InsertStyleSheetAt(mozilla::StyleSheet* aSheet, size_t aIndex);
-
+  void InsertSheetAt(size_t aIndex, mozilla::StyleSheet&);
 
   /**
    * Replace the stylesheets in aOldSheets with the stylesheets in
    * aNewSheets. The two lists must have equal length, and the sheet
    * at positon J in the first list will be replaced by the sheet at
    * position J in the second list.  Some sheets in the second list
    * may be null; if so the corresponding sheets in the first list
    * will simply be removed.
    */
   void UpdateStyleSheets(
       nsTArray<RefPtr<mozilla::StyleSheet>>& aOldSheets,
       nsTArray<RefPtr<mozilla::StyleSheet>>& aNewSheets);
 
   /**
    * Add a stylesheet to the document
+   *
+   * TODO(emilio): This is only used by parts of editor that are no longer in
+   * use by m-c or c-c, so remove.
    */
-  void AddStyleSheet(mozilla::StyleSheet* aSheet);
+  void AddStyleSheet(mozilla::StyleSheet* aSheet)
+  {
+    MOZ_ASSERT(aSheet);
+    InsertSheetAt(SheetCount(), *aSheet);
+  }
 
   /**
    * Remove a stylesheet from the document
    */
   void RemoveStyleSheet(mozilla::StyleSheet* aSheet);
 
   /**
    * Notify the document that the applicable state of the sheet changed
--- a/layout/style/Loader.cpp
+++ b/layout/style/Loader.cpp
@@ -75,17 +75,17 @@ using namespace mozilla::dom;
  *
  * The CSS Loader gets requests to load various sorts of style sheets:
  * inline style from <style> elements, linked style, @import-ed child
  * sheets, non-document sheets.  The loader handles the following tasks:
  * 1) Creation of the actual style sheet objects: CreateSheet()
  * 2) setting of the right media, title, enabled state, etc on the
  *    sheet: PrepareSheet()
  * 3) Insertion of the sheet in the proper cascade order:
- *    InsertSheetInDoc() and InsertChildSheet()
+ *    InsertSheetInTree() and InsertChildSheet()
  * 4) Load of the sheet: LoadSheet() including security checks
  * 5) Parsing of the sheet: ParseSheet()
  * 6) Cleanup: SheetComplete()
  *
  * The detailed documentation for these functions is found with the
  * function implementations.
  *
  * The following helper object is used:
@@ -1145,91 +1145,101 @@ Loader::PrepareSheet(StyleSheet* aSheet,
   aSheet->SetMedia(mediaList);
 
   aSheet->SetTitle(aTitle);
   aSheet->SetEnabled(aIsAlternate == IsAlternate::No);
   return MediaListMatches(mediaList, mDocument);
 }
 
 /**
- * InsertSheetInDoc handles ordering of sheets in the document.  Here
- * we have two types of sheets -- those with linking elements and
- * those without.  The latter are loaded by Link: headers.
+ * InsertSheetInTree handles ordering of sheets in the document or shadow root.
+ *
+ * Here we have two types of sheets -- those with linking elements and
+ * those without.  The latter are loaded by Link: headers, and are only added to
+ * the document.
+ *
  * The following constraints are observed:
  * 1) Any sheet with a linking element comes after all sheets without
  *    linking elements
  * 2) Sheets without linking elements are inserted in the order in
  *    which the inserting requests come in, since all of these are
  *    inserted during header data processing in the content sink
  * 3) Sheets with linking elements are ordered based on document order
  *    as determined by CompareDocumentPosition.
  */
-nsresult
-Loader::InsertSheetInDoc(StyleSheet* aSheet,
-                         nsIContent* aLinkingContent,
-                         nsIDocument* aDocument)
+void
+Loader::InsertSheetInTree(StyleSheet& aSheet, nsIContent* aLinkingContent)
 {
-  LOG(("css::Loader::InsertSheetInDoc"));
-  MOZ_ASSERT(aSheet, "Nothing to insert");
-  MOZ_ASSERT(aDocument, "Must have a document to insert into");
+  LOG(("css::Loader::InsertSheetInTree"));
+  MOZ_ASSERT(mDocument, "Must have a document to insert into");
+  MOZ_ASSERT(!aLinkingContent ||
+             aLinkingContent->IsInUncomposedDoc() ||
+             aLinkingContent->IsInShadowTree(),
+             "Why would we insert it anywhere?");
+
+  nsCOMPtr<nsIStyleSheetLinkingElement> linkingElement =
+    do_QueryInterface(aLinkingContent);
+  if (linkingElement) {
+    linkingElement->SetStyleSheet(&aSheet);
+  }
+
+  ShadowRoot* shadow =
+    aLinkingContent ? aLinkingContent->GetContainingShadow() : nullptr;
+
+  auto& target = shadow
+    ? static_cast<DocumentOrShadowRoot&>(*shadow)
+    : static_cast<DocumentOrShadowRoot&>(*mDocument);
 
   // XXX Need to cancel pending sheet loads for this element, if any
 
-  int32_t sheetCount = aDocument->SheetCount();
+  int32_t sheetCount = target.SheetCount();
 
   /*
    * Start the walk at the _end_ of the list, since in the typical
    * case we'll just want to append anyway.  We want to break out of
    * the loop when insertionPoint points to just before the index we
    * want to insert at.  In other words, when we leave the loop
    * insertionPoint is the index of the stylesheet that immediately
    * precedes the one we're inserting.
    */
-  int32_t insertionPoint;
-  for (insertionPoint = sheetCount - 1; insertionPoint >= 0; --insertionPoint) {
-    StyleSheet* curSheet = aDocument->SheetAt(insertionPoint);
-    NS_ASSERTION(curSheet, "There must be a sheet here!");
-    nsCOMPtr<nsINode> sheetOwner = curSheet->GetOwnerNode();
+  int32_t insertionPoint = sheetCount - 1;
+  for (; insertionPoint >= 0; --insertionPoint) {
+    nsINode* sheetOwner = target.SheetAt(insertionPoint)->GetOwnerNode();
     if (sheetOwner && !aLinkingContent) {
       // Keep moving; all sheets with a sheetOwner come after all
       // sheets without a linkingNode
       continue;
     }
 
     if (!sheetOwner) {
-      // Aha!  The current sheet has no sheet owner, so we want to
-      // insert after it no matter whether we have a linkingNode
+      // Aha!  The current sheet has no sheet owner, so we want to insert after
+      // it no matter whether we have a linking content or not.
       break;
     }
 
-    NS_ASSERTION(aLinkingContent != sheetOwner,
-                 "Why do we still have our old sheet?");
+    MOZ_ASSERT(aLinkingContent != sheetOwner,
+               "Why do we still have our old sheet?");
 
     // Have to compare
     if (nsContentUtils::PositionIsBefore(sheetOwner, aLinkingContent)) {
       // The current sheet comes before us, and it better be the first
       // such, because now we break
       break;
     }
   }
 
-  ++insertionPoint; // adjust the index to the spot we want to insert in
+  ++insertionPoint;
 
-  // XXX <meta> elements do not implement nsIStyleSheetLinkingElement;
-  // need to fix this for them to be ordered correctly.
-  nsCOMPtr<nsIStyleSheetLinkingElement>
-    linkingElement = do_QueryInterface(aLinkingContent);
-  if (linkingElement) {
-    linkingElement->SetStyleSheet(aSheet); // This sets the ownerNode on the sheet
+  if (shadow) {
+    shadow->InsertSheetAt(insertionPoint, aSheet);
+  } else {
+    mDocument->InsertSheetAt(insertionPoint, aSheet);
   }
 
-  aDocument->InsertStyleSheetAt(aSheet, insertionPoint);
-  LOG(("  Inserting into document at position %d", insertionPoint));
-
-  return NS_OK;
+  LOG(("  Inserting into target (doc: %d) at position %d", target.AsNode().IsDocument(), insertionPoint));
 }
 
 /**
  * InsertChildSheet handles ordering of @import-ed sheet in their
  * parent sheets.  Here we want to just insert based on order of the
  * @import rules that imported the sheets.  In theory we can't just
  * append to the end because the CSSOM can insert @import rules.  In
  * practice, we get the call to load the child sheet before the CSSOM
@@ -1913,24 +1923,17 @@ Loader::LoadInlineStyle(const SheetInfo&
   NS_ASSERTION(state == eSheetNeedsParser,
                "Inline sheets should not be cached");
 
   LOG(("  Sheet is alternate: %d", static_cast<int>(isAlternate)));
 
   auto matched =
     PrepareSheet(sheet, aInfo.mTitle, aInfo.mMedia, nullptr, isAlternate);
 
-  if (aInfo.mContent->IsInShadowTree()) {
-    aInfo.mContent->GetContainingShadow()->InsertSheet(sheet, aInfo.mContent);
-  } else {
-    rv = InsertSheetInDoc(sheet, aInfo.mContent, mDocument);
-    if (NS_FAILED(rv)) {
-      return Err(rv);
-    }
-  }
+  InsertSheetInTree(*sheet, aInfo.mContent);
 
   nsIPrincipal* principal = aInfo.mContent->NodePrincipal();
   if (aInfo.mTriggeringPrincipal) {
     // The triggering principal may be an expanded principal, which is safe to
     // use for URL security checks, but not as the loader principal for a
     // stylesheet. So treat this as principal inheritance, and downgrade if
     // necessary.
     principal =
@@ -2030,24 +2033,17 @@ Loader::LoadStyleLink(const SheetInfo& a
     return Err(rv);
   }
 
   LOG(("  Sheet is alternate: %d", static_cast<int>(isAlternate)));
 
   auto matched =
     PrepareSheet(sheet, aInfo.mTitle, aInfo.mMedia, nullptr, isAlternate);
 
-  if (aInfo.mContent && aInfo.mContent->IsInShadowTree()) {
-    aInfo.mContent->GetContainingShadow()->InsertSheet(sheet, aInfo.mContent);
-  } else {
-    rv = InsertSheetInDoc(sheet, aInfo.mContent, mDocument);
-    if (NS_FAILED(rv)) {
-      return Err(rv);
-    }
-  }
+  InsertSheetInTree(*sheet, aInfo.mContent);
 
   nsCOMPtr<nsIStyleSheetLinkingElement> owningElement(
     do_QueryInterface(aInfo.mContent));
 
   if (state == eSheetComplete) {
     LOG(("  Sheet already complete: 0x%p", sheet.get()));
     if (aObserver || !mObservers.IsEmpty() || owningElement) {
       rv = PostLoadEvent(aInfo.mURI,
--- a/layout/style/Loader.h
+++ b/layout/style/Loader.h
@@ -503,19 +503,18 @@ private:
   //
   // This method will set the sheet's enabled state based on aIsAlternate
   MediaMatched PrepareSheet(StyleSheet* aSheet,
                             const nsAString& aTitle,
                             const nsAString& aMediaString,
                             dom::MediaList* aMediaList,
                             IsAlternate);
 
-  nsresult InsertSheetInDoc(StyleSheet* aSheet,
-                            nsIContent* aLinkingContent,
-                            nsIDocument* aDocument);
+  // Inserts a style sheet in a document or a ShadowRoot.
+  void InsertSheetInTree(StyleSheet& aSheet, nsIContent* aLinkingContent);
 
   nsresult InsertChildSheet(StyleSheet* aSheet,
                             StyleSheet* aParentSheet);
 
   nsresult InternalLoadNonDocumentSheet(
     nsIURI* aURL,
     bool aIsPreload,
     SheetParsingMode aParsingMode,