Bug 1445392: Make HTMLSlotElement slot change event stuff not linear. r?smaug draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 07 May 2018 19:09:19 +0200
changeset 792162 9df0ae05136917ef1272686ea6aa868af5b4eb9e
parent 792107 85b5b0ddb7e8d72e9f416458fd2ce9c9fa6118c4
child 792163 9dbd9fec54451984b7a530522a91885cd7ceecd7
push id109016
push userbmo:emilio@crisal.io
push dateMon, 07 May 2018 19:18:38 +0000
reviewerssmaug
bugs1445392
milestone61.0a1
Bug 1445392: Make HTMLSlotElement slot change event stuff not linear. r?smaug Also fix refcount churn in nsDOMMutationObserver and such. MozReview-Commit-ID: GwEB0LZY7Ss
dom/base/DocGroup.cpp
dom/base/DocGroup.h
dom/base/nsDOMMutationObserver.cpp
dom/html/HTMLSlotElement.cpp
dom/html/HTMLSlotElement.h
--- a/dom/base/DocGroup.cpp
+++ b/dom/base/DocGroup.cpp
@@ -154,27 +154,35 @@ DocGroup::AbstractMainThreadFor(TaskCate
 
 bool*
 DocGroup::GetValidAccessPtr()
 {
   return mTabGroup->GetValidAccessPtr();
 }
 
 void
-DocGroup::SignalSlotChange(const HTMLSlotElement* aSlot)
+DocGroup::SignalSlotChange(HTMLSlotElement& aSlot)
 {
-  if (mSignalSlotList.Contains(aSlot)) {
-    return;
-  }
-
-  mSignalSlotList.AppendElement(const_cast<HTMLSlotElement*>(aSlot));
+  MOZ_ASSERT(!mSignalSlotList.Contains(&aSlot));
+  mSignalSlotList.AppendElement(&aSlot);
 
   if (!sPendingDocGroups) {
     // Queue a mutation observer compound microtask.
     nsDOMMutationObserver::QueueMutationObserverMicroTask();
     sPendingDocGroups = new AutoTArray<RefPtr<DocGroup>, 2>;
   }
 
   sPendingDocGroups->AppendElement(this);
 }
 
+void
+DocGroup::MoveSignalSlotListTo(nsTArray<RefPtr<HTMLSlotElement>>& aDest)
+{
+  aDest.SetCapacity(aDest.Length() + mSignalSlotList.Length());
+  for (RefPtr<HTMLSlotElement>& slot : mSignalSlotList) {
+    slot->RemovedFromSignalSlotList();
+    aDest.AppendElement(Move(slot));
+  }
+  mSignalSlotList.Clear();
+}
+
 }
 }
--- a/dom/base/DocGroup.h
+++ b/dom/base/DocGroup.h
@@ -108,29 +108,21 @@ public:
     mTabGroup->ValidateAccess();
   }
 
   // Return a pointer that can be continually checked to see if access to this
   // DocGroup is valid. This pointer should live at least as long as the
   // DocGroup.
   bool* GetValidAccessPtr();
 
-  // Append aSlot to the list of signal slot list, if it's not in it already
-  // list, and queue a mutation observer microtask.
-  void SignalSlotChange(const mozilla::dom::HTMLSlotElement* aSlot);
+  // Append aSlot to the list of signal slot list, and queue a mutation observer
+  // microtask.
+  void SignalSlotChange(HTMLSlotElement& aSlot);
 
-  const nsTArray<RefPtr<HTMLSlotElement>>& SignalSlotList() const
-  {
-    return mSignalSlotList;
-  }
-
-  void ClearSignalSlotList()
-  {
-    mSignalSlotList.Clear();
-  }
+  void MoveSignalSlotListTo(nsTArray<RefPtr<HTMLSlotElement>>& aDest);
 
   // List of DocGroups that has non-empty signal slot list.
   static AutoTArray<RefPtr<DocGroup>, 2>* sPendingDocGroups;
 
 private:
   DocGroup(TabGroup* aTabGroup, const nsACString& aKey);
   ~DocGroup();
 
--- a/dom/base/nsDOMMutationObserver.cpp
+++ b/dom/base/nsDOMMutationObserver.cpp
@@ -895,27 +895,25 @@ nsDOMMutationObserver::HandleMutation()
   mCallback->Call(this, mutations, *this);
 }
 
 void
 nsDOMMutationObserver::HandleMutationsInternal(AutoSlowOperation& aAso)
 {
   nsTArray<RefPtr<nsDOMMutationObserver> >* suppressedObservers = nullptr;
 
-  // Let signalList be a copy of unit of related similar-origin browsing
-  // contexts' signal slot list.
+  // This loop implements:
+  //  * Let signalList be a copy of unit of related similar-origin browsing
+  //    contexts' signal slot list.
+  //  * Empty unit of related similar-origin browsing contexts' signal slot
+  //    list.
   nsTArray<RefPtr<HTMLSlotElement>> signalList;
   if (DocGroup::sPendingDocGroups) {
-    for (uint32_t i = 0; i < DocGroup::sPendingDocGroups->Length(); ++i) {
-      DocGroup* docGroup = DocGroup::sPendingDocGroups->ElementAt(i);
-      signalList.AppendElements(docGroup->SignalSlotList());
-
-      // Empty unit of related similar-origin browsing contexts' signal slot
-      // list.
-      docGroup->ClearSignalSlotList();
+    for (DocGroup* docGroup : *DocGroup::sPendingDocGroups) {
+      docGroup->MoveSignalSlotListTo(signalList);
     }
     delete DocGroup::sPendingDocGroups;
     DocGroup::sPendingDocGroups = nullptr;
   }
 
   if (sScheduledMutationObservers) {
     AutoTArray<RefPtr<nsDOMMutationObserver>, 4>* observers =
       sScheduledMutationObservers;
--- a/dom/html/HTMLSlotElement.cpp
+++ b/dom/html/HTMLSlotElement.cpp
@@ -29,19 +29,17 @@ NS_NewHTMLSlotElement(already_AddRefed<m
 namespace mozilla {
 namespace dom {
 
 HTMLSlotElement::HTMLSlotElement(already_AddRefed<mozilla::dom::NodeInfo>& aNodeInfo)
   : nsGenericHTMLElement(aNodeInfo)
 {
 }
 
-HTMLSlotElement::~HTMLSlotElement()
-{
-}
+HTMLSlotElement::~HTMLSlotElement() = default;
 
 NS_IMPL_ADDREF_INHERITED(HTMLSlotElement, nsGenericHTMLElement)
 NS_IMPL_RELEASE_INHERITED(HTMLSlotElement, nsGenericHTMLElement)
 
 NS_IMPL_CYCLE_COLLECTION_INHERITED(HTMLSlotElement,
                                    nsGenericHTMLElement,
                                    mAssignedNodes)
 
@@ -211,24 +209,29 @@ HTMLSlotElement::ClearAssignedNodes()
                node->AsContent()->GetAssignedSlot() == this, "How exactly?");
     node->AsContent()->SetAssignedSlot(nullptr);
   }
 
   mAssignedNodes.Clear();
 }
 
 void
-HTMLSlotElement::EnqueueSlotChangeEvent() const
+HTMLSlotElement::EnqueueSlotChangeEvent()
 {
+  if (mInSignalSlotList) {
+    return;
+  }
+
   DocGroup* docGroup = OwnerDoc()->GetDocGroup();
   if (!docGroup) {
     return;
   }
 
-  docGroup->SignalSlotChange(this);
+  mInSignalSlotList = true;
+  docGroup->SignalSlotChange(*this);
 }
 
 void
 HTMLSlotElement::FireSlotChangeEvent()
 {
   nsContentUtils::DispatchTrustedEvent(OwnerDoc(),
                                        static_cast<nsIContent*>(this),
                                        NS_LITERAL_STRING("slotchange"), true,
--- a/dom/html/HTMLSlotElement.h
+++ b/dom/html/HTMLSlotElement.h
@@ -58,23 +58,34 @@ public:
 
   // Helper methods
   const nsTArray<RefPtr<nsINode>>& AssignedNodes() const;
   void InsertAssignedNode(uint32_t aIndex, nsINode* aNode);
   void AppendAssignedNode(nsINode* aNode);
   void RemoveAssignedNode(nsINode* aNode);
   void ClearAssignedNodes();
 
-  void EnqueueSlotChangeEvent() const;
+  void EnqueueSlotChangeEvent();
+  void RemovedFromSignalSlotList()
+  {
+    MOZ_ASSERT(mInSignalSlotList);
+    mInSignalSlotList = false;
+  }
+
   void FireSlotChangeEvent();
 
 protected:
   virtual ~HTMLSlotElement();
-  virtual JSObject*
-  WrapNode(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) override;
+  JSObject* WrapNode(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) final;
 
   nsTArray<RefPtr<nsINode>> mAssignedNodes;
+
+  // Whether we're in the signal slot list of our unit of related similar-origin
+  // browsing contexts.
+  //
+  // https://dom.spec.whatwg.org/#signal-slot-list
+  bool mInSignalSlotList = false;
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_HTMLSlotElement_h