Bug 1554498 - Don't use nsIMutationObserver for ShadowRoot. r=smaug

This penalizes a bit non-shadow-DOM content, in exchange of making Shadow DOM
slightly faster as well.

The biggest advantage of this is that all ContentRemoved notifications would see
the flattened tree before the changes, which is something a11y needs to be
correct.

XBL would still not be handled right by a11y, but that's not new and content
cannot do random stuff with XBL so it's not too bad.

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

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

dom/base/CharacterData.cpp

@@ -489,6 +489,11 @@ nsresult CharacterData::BindToTree(BindContext& aContext, nsINode& aParent) {
 
   UpdateEditableState(false);
 
+  // Ensure we only do these once, in the case we move the shadow host around.
+  if (aContext.SubtreeRootChanges()) {
+    HandleShadowDOMRelatedInsertionSteps(hadParent);
+  }
+
   MOZ_ASSERT(OwnerDoc() == aParent.OwnerDoc(), "Bound to wrong document");
   MOZ_ASSERT(IsInComposedDoc() == aContext.InComposedDoc());
   MOZ_ASSERT(IsInUncomposedDoc() == aContext.InUncomposedDoc());
@@ -506,6 +511,8 @@ void CharacterData::UnbindFromTree(bool aNullParent) {
   // Unset frame flags; if we need them again later, they'll get set again.
   UnsetFlags(NS_CREATE_FRAME_IF_NON_WHITESPACE | NS_REFRAME_IF_WHITESPACE);
 
+  HandleShadowDOMRelatedRemovalSteps(aNullParent);
+
   Document* document = GetComposedDoc();
 
   if (aNullParent) {

dom/base/Element.cpp

@@ -1721,6 +1721,7 @@ nsresult Element::BindToTree(BindContext& aContext, nsINode& aParent) {
     if (HasID()) {
       AddToIdTable(DoGetID());
     }
+    HandleShadowDOMRelatedInsertionSteps(hadParent);
   }
 
   if (MayHaveStyle() && !IsXULElement()) {
@@ -1801,6 +1802,8 @@ bool WillDetachFromShadowOnUnbind(const Element& aElement, bool aNullParent) {
 }
 
 void Element::UnbindFromTree(bool aNullParent) {
+  HandleShadowDOMRelatedRemovalSteps(aNullParent);
+
   const bool detachingFromShadow =
       WillDetachFromShadowOnUnbind(*this, aNullParent);
   // Make sure to only remove from the ID table if our subtree root is actually
@@ -2629,19 +2632,25 @@ nsresult Element::AfterSetAttr(int32_t aNamespaceID, nsAtom* aName,
                                const nsAttrValue* aOldValue,
                                nsIPrincipal* aMaybeScriptedPrincipal,
                                bool aNotify) {
-  if (aNamespaceID == kNameSpaceID_None && aName == nsGkAtoms::part) {
+  if (aNamespaceID == kNameSpaceID_None) {
-    bool isPart = !!aValue;
+    if (aName == nsGkAtoms::part) {
-    if (HasPartAttribute() != isPart) {
+      bool isPart = !!aValue;
-      SetHasPartAttribute(isPart);
+      if (HasPartAttribute() != isPart) {
-      if (ShadowRoot* shadow = GetContainingShadow()) {
+        SetHasPartAttribute(isPart);
-        if (isPart) {
+        if (ShadowRoot* shadow = GetContainingShadow()) {
-          shadow->PartAdded(*this);
+          if (isPart) {
-        } else {
+            shadow->PartAdded(*this);
-          shadow->PartRemoved(*this);
+          } else {
+            shadow->PartRemoved(*this);
+          }
         }
       }
+      MOZ_ASSERT(HasPartAttribute() == isPart);
+    } else if (aName == nsGkAtoms::slot && GetParent()) {
+      if (ShadowRoot* shadow = GetParent()->GetShadowRoot()) {
+        shadow->MaybeReassignElement(*this);
+      }
     }
-    MOZ_ASSERT(HasPartAttribute() == isPart);
   }
   return NS_OK;
 }

dom/base/ShadowRoot.cpp

@@ -36,15 +36,11 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(ShadowRoot, DocumentFragment)
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(ShadowRoot)
-  if (tmp->GetHost()) {
-    tmp->GetHost()->RemoveMutationObserver(tmp);
-  }
   DocumentOrShadowRoot::Unlink(tmp);
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END_INHERITED(DocumentFragment)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(ShadowRoot)
   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIContent)
-  NS_INTERFACE_MAP_ENTRY(nsIMutationObserver)
   NS_INTERFACE_MAP_ENTRY(nsIRadioGroupContainer)
 NS_INTERFACE_MAP_END_INHERITING(DocumentFragment)
 
@@ -69,19 +65,9 @@ ShadowRoot::ShadowRoot(Element* aElement, ShadowRootMode aMode,
 
   ExtendedDOMSlots()->mBindingParent = aElement;
   ExtendedDOMSlots()->mContainingShadow = this;
-
-  // Add the ShadowRoot as a mutation observer on the host to watch
-  // for mutations because the insertion points in this ShadowRoot
-  // may need to be updated when the host children are modified.
-  GetHost()->AddMutationObserver(this);
 }
 
 ShadowRoot::~ShadowRoot() {
-  if (auto* host = GetHost()) {
-    // mHost may have been unlinked.
-    host->RemoveMutationObserver(this);
-  }
-
   if (IsInComposedDoc()) {
     OwnerDoc()->RemoveComposedDocShadowRoot(*this);
   }
@@ -158,13 +144,11 @@ void ShadowRoot::Unattach() {
   MOZ_ASSERT(!HasSlots(), "Won't work!");
   if (!GetHost()) {
     // It is possible that we've been unlinked already. In such case host
-    // should have called Unbind and ShadowRoot's own unlink
+    // should have called Unbind and ShadowRoot's own unlink.
-    // RemoveMutationObserver.
     return;
   }
 
   Unbind();
-  GetHost()->RemoveMutationObserver(this);
   SetHost(nullptr);
 }
 
@@ -220,8 +204,8 @@ void ShadowRoot::AddSlot(HTMLSlotElement* aSlot) {
     while (assignedNodes.Length() > 0) {
       nsINode* assignedNode = assignedNodes[0];
 
-      oldSlot->RemoveAssignedNode(assignedNode);
+      oldSlot->RemoveAssignedNode(*assignedNode->AsContent());
-      aSlot->AppendAssignedNode(assignedNode);
+      aSlot->AppendAssignedNode(*assignedNode->AsContent());
       doEnqueueSlotChange = true;
     }
 
@@ -245,7 +229,7 @@ void ShadowRoot::AddSlot(HTMLSlotElement* aSlot) {
         continue;
       }
       doEnqueueSlotChange = true;
-      aSlot->AppendAssignedNode(child);
+      aSlot->AppendAssignedNode(*child);
     }
 
     if (doEnqueueSlotChange) {
@@ -299,8 +283,8 @@ void ShadowRoot::RemoveSlot(HTMLSlotElement* aSlot) {
   while (!assignedNodes.IsEmpty()) {
     nsINode* assignedNode = assignedNodes[0];
 
-    aSlot->RemoveAssignedNode(assignedNode);
+    aSlot->RemoveAssignedNode(*assignedNode->AsContent());
-    replacementSlot->AppendAssignedNode(assignedNode);
+    replacementSlot->AppendAssignedNode(*assignedNode->AsContent());
   }
 
   aSlot->EnqueueSlotChangeEvent();
@@ -488,7 +472,7 @@ void ShadowRoot::GetEventTargetParent(EventChainPreVisitor& aVisitor) {
   }
 }
 
-ShadowRoot::SlotAssignment ShadowRoot::SlotAssignmentFor(nsIContent* aContent) {
+ShadowRoot::SlotAssignment ShadowRoot::SlotAssignmentFor(nsIContent& aContent) {
   nsAutoString slotName;
   // Note that if slot attribute is missing, assign it to the first default
   // slot, if exists.
@@ -513,7 +497,7 @@ ShadowRoot::SlotAssignment ShadowRoot::SlotAssignmentFor(nsIContent* aContent) {
     // Seek through the host's explicit children until the
     // assigned content is found.
     while (currentContent && currentContent != assignedNodes[i]) {
-      if (currentContent == aContent) {
+      if (currentContent == &aContent) {
         insertionIndex.emplace(i);
         break;
       }
@@ -529,9 +513,9 @@ ShadowRoot::SlotAssignment ShadowRoot::SlotAssignmentFor(nsIContent* aContent) {
   return {slot, insertionIndex};
 }
 
-void ShadowRoot::MaybeReassignElement(Element* aElement) {
+void ShadowRoot::MaybeReassignElement(Element& aElement) {
-  MOZ_ASSERT(aElement->GetParent() == GetHost());
+  MOZ_ASSERT(aElement.GetParent() == GetHost());
-  HTMLSlotElement* oldSlot = aElement->GetAssignedSlot();
+  HTMLSlotElement* oldSlot = aElement.GetAssignedSlot();
   SlotAssignment assignment = SlotAssignmentFor(aElement);
 
   if (assignment.mSlot == oldSlot) {
@@ -541,7 +525,7 @@ void ShadowRoot::MaybeReassignElement(Element* aElement) {
 
   if (Document* doc = GetComposedDoc()) {
     if (RefPtr<PresShell> presShell = doc->GetPresShell()) {
-      presShell->SlotAssignmentWillChange(*aElement, oldSlot, assignment.mSlot);
+      presShell->SlotAssignmentWillChange(aElement, oldSlot, assignment.mSlot);
     }
   }
 
@@ -615,103 +599,56 @@ nsINode* ShadowRoot::CreateElementAndAppendChildAt(nsINode& aParentNode,
   return aParentNode.AppendChild(*node, rv);
 }
 
-void ShadowRoot::AttributeChanged(Element* aElement, int32_t aNameSpaceID,
+void ShadowRoot::MaybeUnslotHostChild(nsIContent& aChild) {
-                                  nsAtom* aAttribute, int32_t aModType,
+  // Need to null-check the host because we may be unlinked already.
-                                  const nsAttrValue* aOldValue) {
+  MOZ_ASSERT(!GetHost() || aChild.GetParent() == GetHost());
-  if (aNameSpaceID != kNameSpaceID_None || aAttribute != nsGkAtoms::slot) {
+
+  HTMLSlotElement* slot = aChild.GetAssignedSlot();
+  if (!slot) {
     return;
   }
 
-  if (aElement->GetParent() != GetHost()) {
+  MOZ_DIAGNOSTIC_ASSERT(!aChild.IsRootOfAnonymousSubtree(),
-    return;
+                        "How did aChild end up assigned to a slot?");
+  // If the slot is going to start showing fallback content, we need to tell
+  // layout about it.
+  if (slot->AssignedNodes().Length() == 1) {
+    InvalidateStyleAndLayoutOnSubtree(slot);
   }
 
-  MaybeReassignElement(aElement);
+  slot->RemoveAssignedNode(aChild);
+  slot->EnqueueSlotChangeEvent();
 }
 
-void ShadowRoot::ContentAppended(nsIContent* aFirstNewContent) {
+void ShadowRoot::MaybeSlotHostChild(nsIContent& aChild) {
-  for (nsIContent* content = aFirstNewContent; content;
+  MOZ_ASSERT(aChild.GetParent() == GetHost());
-       content = content->GetNextSibling()) {
+  // Check to ensure that the child not an anonymous subtree root because even
-    ContentInserted(content);
+  // though its parent could be the host it may not be in the host's child list.
-  }
+  if (aChild.IsRootOfAnonymousSubtree()) {
-}
-
-void ShadowRoot::ContentInserted(nsIContent* aChild) {
-  // Check to ensure that the child not an anonymous subtree root because
-  // even though its parent could be the host it may not be in the host's child
-  // list.
-  if (aChild->IsRootOfAnonymousSubtree()) {
     return;
   }
 
-  if (!aChild->IsSlotable()) {
+  if (!aChild.IsSlotable()) {
     return;
   }
 
-  if (aChild->GetParent() == GetHost()) {
+  SlotAssignment assignment = SlotAssignmentFor(aChild);
-    SlotAssignment assignment = SlotAssignmentFor(aChild);
+  if (!assignment.mSlot) {
-    if (!assignment.mSlot) {
-      return;
-    }
-
-    // Fallback content will go away, let layout know.
-    if (assignment.mSlot->AssignedNodes().IsEmpty()) {
-      InvalidateStyleAndLayoutOnSubtree(assignment.mSlot);
-    }
-
-    if (assignment.mIndex) {
-      assignment.mSlot->InsertAssignedNode(*assignment.mIndex, aChild);
-    } else {
-      assignment.mSlot->AppendAssignedNode(aChild);
-    }
-    assignment.mSlot->EnqueueSlotChangeEvent();
-
-    SlotStateChanged(assignment.mSlot);
     return;
   }
 
-  // If parent's root is a shadow root, and parent is a slot whose assigned
+  // Fallback content will go away, let layout know.
-  // nodes is the empty list, then run signal a slot change for parent.
+  if (assignment.mSlot->AssignedNodes().IsEmpty()) {
-  HTMLSlotElement* slot = HTMLSlotElement::FromNodeOrNull(aChild->GetParent());
+    InvalidateStyleAndLayoutOnSubtree(assignment.mSlot);
-  if (slot && slot->GetContainingShadow() == this &&
-      slot->AssignedNodes().IsEmpty()) {
-    slot->EnqueueSlotChangeEvent();
-  }
-}
-
-void ShadowRoot::ContentRemoved(nsIContent* aChild,
-                                nsIContent* aPreviousSibling) {
-  // Check to ensure that the child not an anonymous subtree root because
-  // even though its parent could be the host it may not be in the host's child
-  // list.
-  if (aChild->IsRootOfAnonymousSubtree()) {
-    return;
   }
 
-  if (!aChild->IsSlotable()) {
+  if (assignment.mIndex) {
-    return;
+    assignment.mSlot->InsertAssignedNode(*assignment.mIndex, aChild);
-  }
+  } else {
-
+    assignment.mSlot->AppendAssignedNode(aChild);
-  if (aChild->GetParent() == GetHost()) {
-    if (HTMLSlotElement* slot = aChild->GetAssignedSlot()) {
-      // If the slot is going to start showing fallback content, we need to tell
-      // layout about it.
-      if (slot->AssignedNodes().Length() == 1) {
-        InvalidateStyleAndLayoutOnSubtree(slot);
-      }
-      slot->RemoveAssignedNode(aChild);
-      slot->EnqueueSlotChangeEvent();
-    }
-    return;
-  }
-
-  // If parent's root is a shadow root, and parent is a slot whose assigned
-  // nodes is the empty list, then run signal a slot change for parent.
-  HTMLSlotElement* slot = HTMLSlotElement::FromNodeOrNull(aChild->GetParent());
-  if (slot && slot->GetContainingShadow() == this &&
-      slot->AssignedNodes().IsEmpty()) {
-    slot->EnqueueSlotChangeEvent();
   }
+  assignment.mSlot->EnqueueSlotChangeEvent();
+  SlotStateChanged(assignment.mSlot);
 }
 
 ServoStyleRuleMap& ShadowRoot::ServoStyleRuleMap() {

dom/base/ShadowRoot.h

@@ -39,7 +39,6 @@ class HTMLInputElement;
 
 class ShadowRoot final : public DocumentFragment,
                          public DocumentOrShadowRoot,
-                         public nsStubMutationObserver,
                          public nsIRadioGroupContainer {
  public:
   NS_IMPL_FROMNODE_HELPER(ShadowRoot, IsShadowRoot());
@@ -47,16 +46,20 @@ class ShadowRoot final : public DocumentFragment,
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(ShadowRoot, DocumentFragment)
   NS_DECL_ISUPPORTS_INHERITED
 
-  NS_DECL_NSIMUTATIONOBSERVER_ATTRIBUTECHANGED
-  NS_DECL_NSIMUTATIONOBSERVER_CONTENTAPPENDED
-  NS_DECL_NSIMUTATIONOBSERVER_CONTENTINSERTED
-  NS_DECL_NSIMUTATIONOBSERVER_CONTENTREMOVED
-
   ShadowRoot(Element* aElement, ShadowRootMode aMode,
              already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo);
 
   void AddSizeOfExcludingThis(nsWindowSizes&, size_t* aNodeSize) const final;
 
+  // Try to reassign an element to a slot.
+  void MaybeReassignElement(Element&);
+  // Called when an element is inserted as a direct child of our host. Tries to
+  // slot the child in one of our slots.
+  void MaybeSlotHostChild(nsIContent&);
+  // Called when a direct child of our host is removed. Tries to un-slot the
+  // child from the currently-assigned slot, if any.
+  void MaybeUnslotHostChild(nsIContent&);
+
   // Shadow DOM v1
   Element* Host() const {
     MOZ_ASSERT(GetHost(),
@@ -106,12 +109,6 @@ class ShadowRoot final : public DocumentFragment,
     InsertSheetAt(SheetCount(), aSheet);
   }
 
-  /**
-   * Try to reassign an element to a slot and returns whether the assignment
-   * changed.
-   */
-  void MaybeReassignElement(Element* aElement);
-
   /**
    * Represents the insertion point in a slot for a given node.
    */
@@ -131,7 +128,7 @@ class ShadowRoot final : public DocumentFragment,
    * It's the caller's responsibility to actually call InsertAssignedNode /
    * AppendAssignedNode in the slot as needed.
    */
-  SlotAssignment SlotAssignmentFor(nsIContent* aContent);
+  SlotAssignment SlotAssignmentFor(nsIContent&);
 
   /**
    * Explicitly invalidates the style and layout of the flattened-tree subtree

dom/base/nsIContent.h

@@ -492,6 +492,22 @@ class nsIContent : public nsINode {
    */
   inline nsIContent* GetFlattenedTreeParent() const;
 
+ protected:
+  // Handles getting inserted or removed directly under a <slot> element.
+  // This is meant to only be called from the two functions below.
+  inline void HandleInsertionToOrRemovalFromSlot();
+
+  // Handles Shadow-DOM-related state tracking. Meant to be called near the
+  // end of BindToTree(), only if the tree we're in actually changed, that is,
+  // after the subtree has been bound to the new parent.
+  inline void HandleShadowDOMRelatedInsertionSteps(bool aHadParent);
+
+  // Handles Shadow-DOM related state tracking. Meant to be called near the
+  // beginning of UnbindFromTree(), before the node has lost the reference to
+  // its parent.
+  inline void HandleShadowDOMRelatedRemovalSteps(bool aNullParent);
+
+ public:
   /**
    * API to check if this is a link that's traversed in response to user input
    * (e.g. a click event). Specializations for HTML/SVG/generic XML allow for

dom/base/nsIContentInlines.h

@@ -221,4 +221,51 @@ inline bool nsIContent::IsInAnonymousSubtree() const {
   return !bindingParent->GetShadowRoot();
 }
 
+inline void nsIContent::HandleInsertionToOrRemovalFromSlot() {
+  using mozilla::dom::HTMLSlotElement;
+
+  MOZ_ASSERT(GetParentElement());
+  if (!IsInShadowTree() || IsRootOfAnonymousSubtree()) {
+    return;
+  }
+  HTMLSlotElement* slot = HTMLSlotElement::FromNode(mParent);
+  if (!slot) {
+    return;
+  }
+  // If parent's root is a shadow root, and parent is a slot whose
+  // assigned nodes is the empty list, then run signal a slot change for
+  // parent.
+  if (slot->AssignedNodes().IsEmpty()) {
+    slot->EnqueueSlotChangeEvent();
+  }
+}
+
+inline void nsIContent::HandleShadowDOMRelatedInsertionSteps(bool aHadParent) {
+  using mozilla::dom::Element;
+  using mozilla::dom::ShadowRoot;
+
+  if (!aHadParent) {
+    if (Element* parentElement = Element::FromNode(mParent)) {
+      if (ShadowRoot* shadow = parentElement->GetShadowRoot()) {
+        shadow->MaybeSlotHostChild(*this);
+      }
+      HandleInsertionToOrRemovalFromSlot();
+    }
+  }
+}
+
+inline void nsIContent::HandleShadowDOMRelatedRemovalSteps(bool aNullParent) {
+  using mozilla::dom::Element;
+  using mozilla::dom::ShadowRoot;
+
+  if (aNullParent) {
+    if (Element* parentElement = Element::FromNode(mParent)) {
+      if (ShadowRoot* shadow = parentElement->GetShadowRoot()) {
+        shadow->MaybeUnslotHostChild(*this);
+      }
+      HandleInsertionToOrRemovalFromSlot();
+    }
+  }
+}
+
 #endif  // nsIContentInlines_h

dom/html/HTMLSlotElement.cpp

@@ -154,26 +154,25 @@ const nsTArray<RefPtr<nsINode>>& HTMLSlotElement::AssignedNodes() const {
   return mAssignedNodes;
 }
 
-void HTMLSlotElement::InsertAssignedNode(uint32_t aIndex, nsINode* aNode) {
+void HTMLSlotElement::InsertAssignedNode(uint32_t aIndex, nsIContent& aNode) {
-  MOZ_ASSERT(!aNode->AsContent()->GetAssignedSlot(), "Losing track of a slot");
+  MOZ_ASSERT(!aNode.GetAssignedSlot(), "Losing track of a slot");
-  mAssignedNodes.InsertElementAt(aIndex, aNode);
+  mAssignedNodes.InsertElementAt(aIndex, &aNode);
-  aNode->AsContent()->SetAssignedSlot(this);
+  aNode.SetAssignedSlot(this);
 }
 
-void HTMLSlotElement::AppendAssignedNode(nsINode* aNode) {
+void HTMLSlotElement::AppendAssignedNode(nsIContent& aNode) {
-  MOZ_ASSERT(!aNode->AsContent()->GetAssignedSlot(), "Losing track of a slot");
+  MOZ_ASSERT(!aNode.GetAssignedSlot(), "Losing track of a slot");
-  mAssignedNodes.AppendElement(aNode);
+  mAssignedNodes.AppendElement(&aNode);
-  aNode->AsContent()->SetAssignedSlot(this);
+  aNode.SetAssignedSlot(this);
 }
 
-void HTMLSlotElement::RemoveAssignedNode(nsINode* aNode) {
+void HTMLSlotElement::RemoveAssignedNode(nsIContent& aNode) {
   // This one runs from unlinking, so we can't guarantee that the slot pointer
   // hasn't been cleared.
-  MOZ_ASSERT(!aNode->AsContent()->GetAssignedSlot() ||
+  MOZ_ASSERT(!aNode.GetAssignedSlot() || aNode.GetAssignedSlot() == this,
-                 aNode->AsContent()->GetAssignedSlot() == this,
              "How exactly?");
-  mAssignedNodes.RemoveElement(aNode);
+  mAssignedNodes.RemoveElement(&aNode);
-  aNode->AsContent()->SetAssignedSlot(nullptr);
+  aNode.SetAssignedSlot(nullptr);
 }
 
 void HTMLSlotElement::ClearAssignedNodes() {

dom/html/HTMLSlotElement.h

@@ -54,9 +54,9 @@ class HTMLSlotElement final : public nsGenericHTMLElement {
 
   // Helper methods
   const nsTArray<RefPtr<nsINode>>& AssignedNodes() const;
-  void InsertAssignedNode(uint32_t aIndex, nsINode* aNode);
+  void InsertAssignedNode(uint32_t aIndex, nsIContent&);
-  void AppendAssignedNode(nsINode* aNode);
+  void AppendAssignedNode(nsIContent&);
-  void RemoveAssignedNode(nsINode* aNode);
+  void RemoveAssignedNode(nsIContent&);
   void ClearAssignedNodes();
 
   void EnqueueSlotChangeEvent();