Bug 1441014: Deindent / Simplify some code for slot removal. r?smaug draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sun, 25 Feb 2018 17:33:28 +0100
changeset 759561 d145c295b1be6401de9981020ded18f02c1ce777
parent 759560 5f60ed0a1126e848128b5fef79160e2065b1d45b
push id100382
push userbmo:emilio@crisal.io
push dateSun, 25 Feb 2018 16:57:32 +0000
reviewerssmaug
bugs1441014
milestone60.0a1
Bug 1441014: Deindent / Simplify some code for slot removal. r?smaug MozReview-Commit-ID: LcbZSXnbVLL
dom/base/ShadowRoot.cpp
dom/base/ShadowRoot.h
--- a/dom/base/ShadowRoot.cpp
+++ b/dom/base/ShadowRoot.cpp
@@ -99,24 +99,23 @@ ShadowRoot::WrapObject(JSContext* aCx, J
 }
 
 void
 ShadowRoot::AddSlot(HTMLSlotElement* aSlot)
 {
   MOZ_ASSERT(aSlot);
 
   // Note that if name attribute missing, the slot is a default slot.
- nsAutoString name;
+  nsAutoString name;
   aSlot->GetName(name);
 
   nsTArray<HTMLSlotElement*>* currentSlots = mSlotMap.LookupOrAdd(name);
   MOZ_ASSERT(currentSlots);
 
-  HTMLSlotElement* oldSlot = currentSlots->IsEmpty() ?
-    nullptr : currentSlots->ElementAt(0);
+  HTMLSlotElement* oldSlot = currentSlots->SafeElementAt(0);
 
   TreeOrderComparator comparator;
   currentSlots->InsertElementSorted(aSlot, comparator);
 
   HTMLSlotElement* currentSlot = currentSlots->ElementAt(0);
   if (currentSlot != aSlot) {
     return;
   }
@@ -161,51 +160,53 @@ ShadowRoot::AddSlot(HTMLSlotElement* aSl
 void
 ShadowRoot::RemoveSlot(HTMLSlotElement* aSlot)
 {
   MOZ_ASSERT(aSlot);
 
   nsAutoString name;
   aSlot->GetName(name);
 
-  nsTArray<HTMLSlotElement*>* currentSlots = mSlotMap.Get(name);
-
-  if (currentSlots) {
-    if (currentSlots->Length() == 1) {
-      MOZ_ASSERT(currentSlots->ElementAt(0) == aSlot);
-      mSlotMap.Remove(name);
+  SlotArray* currentSlots = mSlotMap.Get(name);
+  MOZ_DIAGNOSTIC_ASSERT(currentSlots && currentSlots->Contains(aSlot),
+                        "Slot to deregister wasn't found?");
+  if (currentSlots->Length() == 1) {
+    MOZ_ASSERT(currentSlots->ElementAt(0) == aSlot);
+    mSlotMap.Remove(name);
 
-      if (aSlot->AssignedNodes().Length() > 0) {
-        aSlot->ClearAssignedNodes();
-        aSlot->EnqueueSlotChangeEvent();
-      }
-    } else {
-      bool doEnqueueSlotChange = false;
-      bool doReplaceSlot = currentSlots->ElementAt(0) == aSlot;
-      currentSlots->RemoveElement(aSlot);
-      HTMLSlotElement* replacementSlot = currentSlots->ElementAt(0);
+    if (!aSlot->AssignedNodes().IsEmpty()) {
+      aSlot->ClearAssignedNodes();
+      aSlot->EnqueueSlotChangeEvent();
+    }
+
+    return;
+  }
+
+  const bool wasFirstSlot = currentSlots->ElementAt(0) == aSlot;
+  currentSlots->RemoveElement(aSlot);
 
-      // Move assigned nodes from removed slot to the next slot in
-      // tree order with the same name.
-      if (doReplaceSlot) {
-        const nsTArray<RefPtr<nsINode>>& assignedNodes = aSlot->AssignedNodes();
-        while (assignedNodes.Length() > 0) {
-          nsINode* assignedNode = assignedNodes[0];
+  // Move assigned nodes from removed slot to the next slot in
+  // tree order with the same name.
+  if (!wasFirstSlot) {
+    return;
+  }
 
-          aSlot->RemoveAssignedNode(assignedNode);
-          replacementSlot->AppendAssignedNode(assignedNode);
-          doEnqueueSlotChange = true;
-        }
+  HTMLSlotElement* replacementSlot = currentSlots->ElementAt(0);
+  const nsTArray<RefPtr<nsINode>>& assignedNodes = aSlot->AssignedNodes();
+  bool slottedNodesChanged = !assignedNodes.IsEmpty();
+  while (!assignedNodes.IsEmpty()) {
+    nsINode* assignedNode = assignedNodes[0];
 
-        if (doEnqueueSlotChange) {
-          aSlot->EnqueueSlotChangeEvent();
-          replacementSlot->EnqueueSlotChangeEvent();
-        }
-      }
-    }
+    aSlot->RemoveAssignedNode(assignedNode);
+    replacementSlot->AppendAssignedNode(assignedNode);
+  }
+
+  if (slottedNodesChanged) {
+    aSlot->EnqueueSlotChangeEvent();
+    replacementSlot->EnqueueSlotChangeEvent();
   }
 }
 
 void
 ShadowRoot::StyleSheetChanged()
 {
   nsIDocument* doc = OwnerDoc();
 
--- a/dom/base/ShadowRoot.h
+++ b/dom/base/ShadowRoot.h
@@ -144,20 +144,21 @@ public:
 
   nsresult GetEventTargetParent(EventChainPreVisitor& aVisitor) override;
 
 protected:
   virtual ~ShadowRoot();
 
   ShadowRootMode mMode;
 
+  using SlotArray = AutoTArray<HTMLSlotElement*, 1>;
   // Map from name of slot to an array of all slots in the shadow DOM with with
   // the given name. The slots are stored as a weak pointer because the elements
   // are in the shadow tree and should be kept alive by its parent.
-  nsClassHashtable<nsStringHashKey, nsTArray<mozilla::dom::HTMLSlotElement*>> mSlotMap;
+  nsClassHashtable<nsStringHashKey, SlotArray> mSlotMap;
   nsXBLPrototypeBinding* mProtoBinding;
 
   // It is necessary to hold a reference to the associated nsXBLBinding
   // because the binding holds a reference on the nsXBLDocumentInfo that
   // owns |mProtoBinding|.
   RefPtr<nsXBLBinding> mAssociatedBinding;
 
   // Flag to indicate whether the descendants of this shadow root are part of the