Bug 1777925: Replaced MutationObserver array container type with linked list. r=smaug

Deletion of mutation observers from a list resulted in O(n^2) behavior and could lead to massive freezes.
This is resolved by using a LinkedList instead, reducing complexity to O(n).

A safely iterable doubly linked list was implemented based on `mozilla::DoublyLinkedList`,
allowing to insert and remove elements while iterating the list.

Due to the nature of `mozilla::DoublyLinkedList`, every Mutation Observer now inherits `mozilla::DoublyLinkedListElement<T>`.
This implies that a Mutation Observer can only be part of one DoublyLinkedList.
This conflicts with some Mutation Observers, which are being added to multiple `nsINode`s.
To continue supporting this, new MutationObserver base classes `nsMultiMutationObserver` and `nsStubMultiMutationObserver` are introduced,
which create `MutationObserverWrapper` objects each time they are added to a `nsINode`.
The wrapper objects forward every call to the actual observer.

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

dom/base/MutationObservers.cpp

@@ -56,11 +56,11 @@ static inline nsINode* ForEachAncestorObserver(nsINode* aNode,
   nsINode* last;
   nsINode* node = aNode;
   do {
-    nsAutoTObserverArray<nsIMutationObserver*, 1>* observers =
+    mozilla::SafeDoublyLinkedList<nsIMutationObserver>* observers =
         node->GetMutationObservers();
-    if (observers && !observers->IsEmpty()) {
-      for (nsIMutationObserver* obs : observers->ForwardRange()) {
-        aFunc(obs);
+    if (observers && !observers->isEmpty()) {
+      for (auto iter = observers->begin(); iter != observers->end(); ++iter) {
+        aFunc(&*iter);
       }
     }
     last = node;

dom/base/MutationObservers.h

@@ -7,10 +7,10 @@
 #ifndef DOM_BASE_MUTATIONOBSERVERS_H_
 #define DOM_BASE_MUTATIONOBSERVERS_H_
 
+#include "mozilla/DoublyLinkedList.h"
 #include "nsIContent.h"  // for use in inline function (NotifyParentChainChanged)
 #include "nsIMutationObserver.h"  // for use in inline function (NotifyParentChainChanged)
 #include "nsINode.h"
-#include "nsTObserverArray.h"
 
 class nsAtom;
 class nsAttrValue;
@@ -120,11 +120,12 @@ class MutationObservers {
    * @see nsIMutationObserver::ParentChainChanged
    */
   static inline void NotifyParentChainChanged(nsIContent* aContent) {
-    nsAutoTObserverArray<nsIMutationObserver*, 1>* observers =
+    mozilla::SafeDoublyLinkedList<nsIMutationObserver>* observers =
         aContent->GetMutationObservers();
-    if (observers && !observers->IsEmpty()) {
-      NS_OBSERVER_ARRAY_NOTIFY_OBSERVERS(*observers, ParentChainChanged,
-                                         (aContent));
+    if (observers && !observers->isEmpty()) {
+      for (auto iter = observers->begin(); iter != observers->end(); ++iter) {
+        iter->ParentChainChanged(aContent);
+      }
     }
   }
 

dom/base/nsIMutationObserver.h

@@ -10,6 +10,7 @@
 #include "nsISupports.h"
 
 #include "mozilla/Assertions.h"
+#include "mozilla/DoublyLinkedList.h"
 
 class nsAttrValue;
 class nsAtom;
@@ -92,7 +93,11 @@ struct CharacterDataChangeInfo {
  * Mutation observer interface
  *
  * See nsINode::AddMutationObserver, nsINode::RemoveMutationObserver for how to
- * attach or remove your observers.
+ * attach or remove your observers. nsINode stores mutation observers using a
+ * mozilla::SafeDoublyLinkedList, which is a specialization of the
+ * DoublyLinkedList allowing for adding/removing elements while iterating.
+ * If a mutation observer is intended to be added to multiple nsINode instances,
+ * derive from nsMultiMutationObserver.
  *
  * WARNING: During these notifications, you are not allowed to perform
  * any mutations to the current or any other document, or start a
@@ -102,7 +107,11 @@ struct CharacterDataChangeInfo {
  * done from an async event, as the notification might not be
  * surrounded by BeginUpdate/EndUpdate calls.
  */
-class nsIMutationObserver : public nsISupports {
+class nsIMutationObserver
+    : public nsISupports,
+      mozilla::DoublyLinkedListElement<nsIMutationObserver> {
+  friend struct mozilla::GetDoublyLinkedListElement<nsIMutationObserver>;
+
  public:
   NS_DECLARE_STATIC_IID_ACCESSOR(NS_IMUTATION_OBSERVER_IID)
 

dom/base/nsINode.cpp

@@ -651,12 +651,19 @@ DocumentOrShadowRoot* nsINode::GetUncomposedDocOrConnectedShadowRoot() const {
   return nullptr;
 }
 
+mozilla::SafeDoublyLinkedList<nsIMutationObserver>*
+nsINode::GetMutationObservers() {
+  return HasSlots() ? &GetExistingSlots()->mMutationObservers : nullptr;
+}
+
 void nsINode::LastRelease() {
   nsINode::nsSlots* slots = GetExistingSlots();
   if (slots) {
-    if (!slots->mMutationObservers.IsEmpty()) {
-      NS_OBSERVER_ARRAY_NOTIFY_OBSERVERS(slots->mMutationObservers,
-                                         NodeWillBeDestroyed, (this));
+    if (!slots->mMutationObservers.isEmpty()) {
+      for (auto iter = slots->mMutationObservers.begin();
+           iter != slots->mMutationObservers.end(); ++iter) {
+        iter->NodeWillBeDestroyed(this);
+      }
     }
 
     if (IsContent()) {
@@ -3524,6 +3531,29 @@ ParentObject nsINode::GetParentObject() const {
   return p;
 }
 
+void nsINode::AddMutationObserver(
+    nsMultiMutationObserver* aMultiMutationObserver) {
+  if (aMultiMutationObserver) {
+    NS_ASSERTION(!aMultiMutationObserver->ContainsNode(this),
+                 "Observer already in the list");
+    aMultiMutationObserver->AddMutationObserverToNode(this);
+  }
+}
+
+void nsINode::AddMutationObserverUnlessExists(
+    nsMultiMutationObserver* aMultiMutationObserver) {
+  if (aMultiMutationObserver && !aMultiMutationObserver->ContainsNode(this)) {
+    aMultiMutationObserver->AddMutationObserverToNode(this);
+  }
+}
+
+void nsINode::RemoveMutationObserver(
+    nsMultiMutationObserver* aMultiMutationObserver) {
+  if (aMultiMutationObserver) {
+    aMultiMutationObserver->RemoveMutationObserverFromNode(this);
+  }
+}
+
 NS_IMPL_ISUPPORTS(nsNodeWeakReference, nsIWeakReference)
 
 nsNodeWeakReference::nsNodeWeakReference(nsINode* aNode)

dom/base/nsINode.h

@@ -7,15 +7,16 @@
 #ifndef nsINode_h___
 #define nsINode_h___
 
+#include "mozilla/DoublyLinkedList.h"
 #include "mozilla/Likely.h"
 #include "mozilla/UniquePtr.h"
 #include "nsCOMPtr.h"              // for member, local
 #include "nsGkAtoms.h"             // for nsGkAtoms::baseURIProperty
 #include "mozilla/dom/NodeInfo.h"  // member (in nsCOMPtr)
 #include "nsIWeakReference.h"
+#include "nsIMutationObserver.h"
 #include "nsNodeInfoManager.h"  // for use in NodePrincipal()
 #include "nsPropertyTable.h"    // for typedefs
-#include "nsTObserverArray.h"   // for member
 #include "mozilla/ErrorResult.h"
 #include "mozilla/LinkedList.h"
 #include "mozilla/MemoryReporting.h"
@@ -44,7 +45,7 @@ class nsIContent;
 class nsIContentSecurityPolicy;
 class nsIFrame;
 class nsIHTMLCollection;
-class nsIMutationObserver;
+class nsMultiMutationObserver;
 class nsINode;
 class nsINodeList;
 class nsIPrincipal;
@@ -1106,12 +1107,16 @@ class nsINode : public mozilla::dom::EventTarget {
    */
   void AddMutationObserver(nsIMutationObserver* aMutationObserver) {
     nsSlots* s = Slots();
-    NS_ASSERTION(s->mMutationObservers.IndexOf(aMutationObserver) ==
-                     nsTArray<int>::NoIndex,
-                 "Observer already in the list");
-    s->mMutationObservers.AppendElement(aMutationObserver);
+    if (aMutationObserver) {
+      NS_ASSERTION(!s->mMutationObservers.contains(aMutationObserver),
+                   "Observer already in the list");
+
+      s->mMutationObservers.pushBack(aMutationObserver);
+    }
   }
 
+  void AddMutationObserver(nsMultiMutationObserver* aMultiMutationObserver);
+
   /**
    * Same as above, but only adds the observer if its not observing
    * the node already.
@@ -1121,9 +1126,14 @@ class nsINode : public mozilla::dom::EventTarget {
    */
   void AddMutationObserverUnlessExists(nsIMutationObserver* aMutationObserver) {
     nsSlots* s = Slots();
-    s->mMutationObservers.AppendElementUnlessExists(aMutationObserver);
+    if (aMutationObserver &&
+        !s->mMutationObservers.contains(aMutationObserver)) {
+      s->mMutationObservers.pushBack(aMutationObserver);
+    }
   }
 
+  void AddMutationObserverUnlessExists(
+      nsMultiMutationObserver* aMultiMutationObserver);
   /**
    * Same as AddMutationObserver, but for nsIAnimationObservers.  This
    * additionally records on the document that animation observers have
@@ -1145,13 +1155,13 @@ class nsINode : public mozilla::dom::EventTarget {
   void RemoveMutationObserver(nsIMutationObserver* aMutationObserver) {
     nsSlots* s = GetExistingSlots();
     if (s) {
-      s->mMutationObservers.RemoveElement(aMutationObserver);
+      s->mMutationObservers.remove(aMutationObserver);
     }
   }
 
-  nsAutoTObserverArray<nsIMutationObserver*, 1>* GetMutationObservers() {
-    return HasSlots() ? &GetExistingSlots()->mMutationObservers : nullptr;
-  }
+  void RemoveMutationObserver(nsMultiMutationObserver* aMultiMutationObserver);
+
+  mozilla::SafeDoublyLinkedList<nsIMutationObserver>* GetMutationObservers();
 
   /**
    * Helper methods to access ancestor node(s) of type T.
@@ -1273,7 +1283,7 @@ class nsINode : public mozilla::dom::EventTarget {
     /**
      * A list of mutation observers
      */
-    nsAutoTObserverArray<nsIMutationObserver*, 1> mMutationObservers;
+    mozilla::SafeDoublyLinkedList<nsIMutationObserver> mMutationObservers;
 
     /**
      * An object implementing NodeList for this content (childNodes)

dom/base/nsRange.cpp

@@ -141,7 +141,7 @@ nsRange::nsRange(nsINode* aNode)
       mNextStartRef(nullptr),
       mNextEndRef(nullptr) {
   // printf("Size of nsRange: %zu\n", sizeof(nsRange));
-  static_assert(sizeof(nsRange) <= 192,
+  static_assert(sizeof(nsRange) <= 208,
                 "nsRange size shouldn't be increased as far as possible");
 }
 

dom/base/nsStubMutationObserver.cpp

@@ -12,6 +12,181 @@
  */
 
 #include "nsStubMutationObserver.h"
+#include "mozilla/RefCountType.h"
+#include "nsISupports.h"
+#include "nsINode.h"
+
+/******************************************************************************
+ * nsStubMutationObserver
+ *****************************************************************************/
 
 NS_IMPL_NSIMUTATIONOBSERVER_CORE_STUB(nsStubMutationObserver)
 NS_IMPL_NSIMUTATIONOBSERVER_CONTENT(nsStubMutationObserver)
+
+/******************************************************************************
+ * MutationObserverWrapper
+ *****************************************************************************/
+
+/**
+ * @brief Wrapper class for a mutation observer that observes multiple nodes.
+ *
+ * This wrapper implements all methods of the nsIMutationObserver interface
+ * and forwards all calls to its owner, which is an instance of
+ * nsMultiMutationObserver.
+ *
+ * This class holds a reference to the owner and AddRefs/Releases its owner
+ * as well to ensure lifetime.
+ */
+class MutationObserverWrapper final : public nsIMutationObserver {
+ public:
+  NS_DECL_ISUPPORTS
+
+  explicit MutationObserverWrapper(nsMultiMutationObserver* aOwner)
+      : mOwner(aOwner) {}
+
+  void CharacterDataWillChange(nsIContent* aContent,
+                               const CharacterDataChangeInfo& aInfo) override {
+    MOZ_ASSERT(mOwner);
+    mOwner->CharacterDataWillChange(aContent, aInfo);
+  }
+
+  void CharacterDataChanged(nsIContent* aContent,
+                            const CharacterDataChangeInfo& aInfo) override {
+    MOZ_ASSERT(mOwner);
+    mOwner->CharacterDataChanged(aContent, aInfo);
+  }
+
+  void AttributeWillChange(mozilla::dom::Element* aElement,
+                           int32_t aNameSpaceID, nsAtom* aAttribute,
+                           int32_t aModType) override {
+    MOZ_ASSERT(mOwner);
+    mOwner->AttributeWillChange(aElement, aNameSpaceID, aAttribute, aModType);
+  }
+
+  void AttributeChanged(mozilla::dom::Element* aElement, int32_t aNameSpaceID,
+                        nsAtom* aAttribute, int32_t aModType,
+                        const nsAttrValue* aOldValue) override {
+    MOZ_ASSERT(mOwner);
+    mOwner->AttributeChanged(aElement, aNameSpaceID, aAttribute, aModType,
+                             aOldValue);
+  }
+
+  void NativeAnonymousChildListChange(nsIContent* aContent,
+                                      bool aIsRemove) override {
+    MOZ_ASSERT(mOwner);
+    mOwner->NativeAnonymousChildListChange(aContent, aIsRemove);
+  }
+
+  void AttributeSetToCurrentValue(mozilla::dom::Element* aElement,
+                                  int32_t aNameSpaceID,
+                                  nsAtom* aAttribute) override {
+    MOZ_ASSERT(mOwner);
+    mOwner->AttributeSetToCurrentValue(aElement, aNameSpaceID, aAttribute);
+  }
+
+  void ContentAppended(nsIContent* aFirstNewContent) override {
+    MOZ_ASSERT(mOwner);
+    mOwner->ContentAppended(aFirstNewContent);
+  }
+
+  void ContentInserted(nsIContent* aChild) override {
+    MOZ_ASSERT(mOwner);
+    mOwner->ContentInserted(aChild);
+  }
+
+  void ContentRemoved(nsIContent* aChild,
+                      nsIContent* aPreviousSibling) override {
+    MOZ_ASSERT(mOwner);
+    mOwner->ContentRemoved(aChild, aPreviousSibling);
+  }
+
+  void NodeWillBeDestroyed(const nsINode* aNode) override {
+    MOZ_ASSERT(mOwner);
+    RefPtr<nsMultiMutationObserver> owner = mOwner;
+    owner->NodeWillBeDestroyed(aNode);
+    owner->mWrapperForNode.Remove(const_cast<nsINode*>(aNode));
+    mOwner = nullptr;
+    ReleaseWrapper();
+  }
+
+  void ParentChainChanged(nsIContent* aContent) override {
+    MOZ_ASSERT(mOwner);
+    mOwner->ParentChainChanged(aContent);
+  }
+
+  MozExternalRefCountType AddRefWrapper() {
+    NS_LOG_ADDREF(this, mRefCnt, "MutationObserverWrapper", sizeof(*this));
+    return ++mRefCnt;
+  }
+
+  MozExternalRefCountType ReleaseWrapper() {
+    --mRefCnt;
+    NS_LOG_RELEASE(this, mRefCnt, "MutationObserverWrapper");
+    if (mRefCnt == 0) {
+      mRefCnt = 1;
+      auto refCnt = MozExternalRefCountType(mRefCnt);
+      delete this;
+      return refCnt;
+    }
+    return mRefCnt;
+  }
+
+ private:
+  ~MutationObserverWrapper() = default;
+  nsMultiMutationObserver* mOwner{nullptr};
+};
+
+NS_IMPL_QUERY_INTERFACE(MutationObserverWrapper, nsIMutationObserver);
+
+MozExternalRefCountType MutationObserverWrapper::AddRef() {
+  MOZ_ASSERT(mOwner);
+  AddRefWrapper();
+  mOwner->AddRef();
+  return mRefCnt;
+}
+
+MozExternalRefCountType MutationObserverWrapper::Release() {
+  MOZ_ASSERT(mOwner);
+  mOwner->Release();
+  return ReleaseWrapper();
+}
+
+/******************************************************************************
+ * nsMultiMutationObserver
+ *****************************************************************************/
+
+void nsMultiMutationObserver::AddMutationObserverToNode(nsINode* aNode) {
+  if (!aNode) {
+    return;
+  }
+  if (mWrapperForNode.Contains(aNode)) {
+    return;
+  }
+  auto* newWrapper = new MutationObserverWrapper{this};
+  newWrapper->AddRefWrapper();
+  mWrapperForNode.InsertOrUpdate(aNode, newWrapper);
+  aNode->AddMutationObserver(newWrapper);
+}
+
+void nsMultiMutationObserver::RemoveMutationObserverFromNode(nsINode* aNode) {
+  if (!aNode) {
+    return;
+  }
+
+  if (auto obs = mWrapperForNode.MaybeGet(aNode); obs.isSome()) {
+    aNode->RemoveMutationObserver(*obs);
+    mWrapperForNode.Remove(aNode);
+    (*obs)->ReleaseWrapper();
+  }
+}
+
+bool nsMultiMutationObserver::ContainsNode(const nsINode* aNode) const {
+  return mWrapperForNode.Contains(const_cast<nsINode*>(aNode));
+}
+
+/******************************************************************************
+ * nsStubMultiMutationObserver
+ *****************************************************************************/
+
+NS_IMPL_NSIMUTATIONOBSERVER_CORE_STUB(nsStubMultiMutationObserver)
+NS_IMPL_NSIMUTATIONOBSERVER_CONTENT(nsStubMultiMutationObserver)

dom/base/nsStubMutationObserver.h

@@ -14,6 +14,7 @@
 #ifndef nsStubMutationObserver_h_
 #define nsStubMutationObserver_h_
 
+#include "nsTHashMap.h"
 #include "nsIMutationObserver.h"
 
 /**
@@ -32,4 +33,51 @@ class nsStubMutationObserver : public nsIMutationObserver {
   NS_DECL_NSIMUTATIONOBSERVER
 };
 
+class MutationObserverWrapper;
+
+/**
+ * @brief Base class for MutationObservers that are used by multiple nodes.
+ *
+ * Mutation Observers are stored inside of a nsINode using a DoublyLinkedList,
+ * restricting the number of nodes a mutation observer can be inserted to one.
+ *
+ * To allow a mutation observer to be used by several nodes, this class
+ * provides a MutationObserverWrapper which implements the nsIMutationObserver
+ * interface and forwards all method calls to this class. For each node this
+ * mutation observer will be used for, a wrapper object is created.
+ */
+class nsMultiMutationObserver : public nsIMutationObserver {
+ public:
+  /**
+   * Adds the mutation observer to aNode by creating a MutationObserverWrapper
+   * and inserting it into aNode.
+   * Does nothing if there already is a mutation observer for aNode.
+   */
+  void AddMutationObserverToNode(nsINode* aNode);
+
+  /**
+   * Removes the mutation observer from aNode.
+   * Does nothing if there is no mutation observer for aNode.
+   */
+  void RemoveMutationObserverFromNode(nsINode* aNode);
+
+  /**
+   * Returns true if there is already a mutation observer for aNode.
+   */
+  bool ContainsNode(const nsINode* aNode) const;
+
+ private:
+  friend class MutationObserverWrapper;
+  nsTHashMap<nsINode*, MutationObserverWrapper*> mWrapperForNode;
+};
+
+/**
+ * Convenience class that provides support for multiple nodes and has
+ * default implementations for nsIMutationObserver.
+ */
+class nsStubMultiMutationObserver : public nsMultiMutationObserver {
+ public:
+  NS_DECL_NSIMUTATIONOBSERVER
+};
+
 #endif /* !defined(nsStubMutationObserver_h_) */

dom/l10n/L10nMutations.cpp

@@ -7,6 +7,7 @@
 #include "L10nMutations.h"
 #include "mozilla/dom/DocumentInlines.h"
 #include "nsRefreshDriver.h"
+#include "DOMLocalization.h"
 #include "mozilla/intl/Localization.h"
 
 using namespace mozilla;

dom/l10n/L10nMutations.h

@@ -12,18 +12,17 @@
 #include "nsRefreshObservers.h"
 #include "nsStubMutationObserver.h"
 #include "nsTHashSet.h"
-#include "mozilla/dom/DOMLocalization.h"
 
 class nsRefreshDriver;
 
 namespace mozilla::dom {
-
+class DOMLocalization;
 /**
  * L10nMutations manage observing roots for localization
  * changes and coalescing pending translations into
  * batches - one per animation frame.
  */
-class L10nMutations final : public nsStubMutationObserver,
+class L10nMutations final : public nsStubMultiMutationObserver,
                             public nsARefreshObserver {
  public:
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS

dom/script/ScriptElement.cpp

@@ -89,7 +89,7 @@ void ScriptElement::ContentInserted(nsIContent* aChild) {
 bool ScriptElement::MaybeProcessScript() {
   nsCOMPtr<nsIContent> cont = do_QueryInterface((nsIScriptElement*)this);
 
-  NS_ASSERTION(cont->DebugGetSlots()->mMutationObservers.Contains(this),
+  NS_ASSERTION(cont->DebugGetSlots()->mMutationObservers.contains(this),
                "You forgot to add self as observer");
 
   if (mAlreadyStarted || !mDoneAddingChildren || !cont->GetComposedDoc() ||

editor/libeditor/HTMLAnonymousNodeEditor.cpp

@@ -71,11 +71,18 @@ static int32_t GetCSSFloatValue(nsComputedDOMStyle* aComputedStyle,
   return NS_SUCCEEDED(rv) ? val : 0;
 }
 
-class ElementDeletionObserver final : public nsStubMutationObserver {
+/******************************************************************************
+ * mozilla::ElementDeletionObserver
+ *****************************************************************************/
+
+class ElementDeletionObserver final : public nsStubMultiMutationObserver {
  public:
   ElementDeletionObserver(nsIContent* aNativeAnonNode,
                           Element* aObservedElement)
-      : mNativeAnonNode(aNativeAnonNode), mObservedElement(aObservedElement) {}
+      : mNativeAnonNode(aNativeAnonNode), mObservedElement(aObservedElement) {
+    AddMutationObserverToNode(aNativeAnonNode);
+    AddMutationObserverToNode(aObservedElement);
+  }
 
   NS_DECL_ISUPPORTS
   NS_DECL_NSIMUTATIONOBSERVER_PARENTCHAINCHANGED
@@ -121,6 +128,10 @@ void ElementDeletionObserver::NodeWillBeDestroyed(const nsINode* aNode) {
   NS_RELEASE_THIS();
 }
 
+/******************************************************************************
+ * mozilla::HTMLEditor
+ *****************************************************************************/
+
 ManualNACPtr HTMLEditor::CreateAnonymousElement(nsAtom* aTag,
                                                 nsIContent& aParentContent,
                                                 const nsAString& aAnonClass,
@@ -198,8 +209,6 @@ ManualNACPtr HTMLEditor::CreateAnonymousElement(nsAtom* aTag,
   auto* observer = new ElementDeletionObserver(newNativeAnonymousContent,
                                                aParentContent.AsElement());
   NS_ADDREF(observer);  // NodeWillBeDestroyed releases.
-  aParentContent.AddMutationObserver(observer);
-  newNativeAnonymousContent->AddMutationObserver(observer);
 
 #ifdef DEBUG
   // Editor anonymous content gets passed to PostRecreateFramesFor... which

layout/style/ServoBindings.toml

@@ -384,6 +384,8 @@ opaque-types = [
     "mozilla::WeakPtr",
     "nsWritingIterator_reference", "nsReadingIterator_reference",
     "nsTObserverArray",  # <- Inherits from nsAutoTObserverArray<T, 0>
+    "mozilla::DoublyLinkedList",
+    "mozilla::SafeDoublyLinkedList",
     "nsTHashtable",  # <- Inheriting from inner typedefs that clang
                      #    doesn't expose properly.
     "nsTBaseHashSet", # <- Ditto

mfbt/DoublyLinkedList.h

@@ -149,7 +149,7 @@ class DoublyLinkedList final {
     T* operator->() const { return mCurrent; }
 
     Iterator& operator++() {
-      mCurrent = ElementAccess::Get(mCurrent).mNext;
+      mCurrent = mCurrent ? ElementAccess::Get(mCurrent).mNext : nullptr;
       return *this;
     }
 
@@ -370,6 +370,209 @@ class DoublyLinkedList final {
   }
 };
 
+/**
+ * @brief Double linked list that allows insertion/removal during iteration.
+ *
+ * This class uses the mozilla::DoublyLinkedList internally and keeps
+ * track of created iterator instances by putting them on a simple list on stack
+ * (compare nsTAutoObserverArray).
+ * This allows insertion or removal operations to adjust iterators and therefore
+ * keeping them valid during iteration.
+ */
+template <typename T, typename ElementAccess = GetDoublyLinkedListElement<T>>
+class SafeDoublyLinkedList {
+ public:
+  /**
+   * @brief Iterator class for SafeDoublyLinkedList.
+   *
+   * The iterator contains two iterators of the underlying list:
+   *  - mCurrent points to the current list element of the iterator.
+   *  - mNext points to the next element of the list.
+   *
+   * When removing an element from the list, mCurrent and mNext may
+   * be adjusted:
+   *  - If mCurrent is the element to be deleted, it is set to empty. mNext can
+   *    still be used to advance to the next element.
+   *  - If mNext is the element to be deleted, it is set to its next element
+   *    (or to empty if mNext is the last element of the list).
+   */
+  class SafeIterator {
+    using BaseIterator = typename DoublyLinkedList<T, ElementAccess>::Iterator;
+    friend class SafeDoublyLinkedList<T, ElementAccess>;
+
+   public:
+    using iterator_category = std::forward_iterator_tag;
+    using value_type = T;
+    using difference_type = std::ptrdiff_t;
+    using pointer = T*;
+    using const_pointer = const T*;
+    using reference = T&;
+    using const_reference = const T&;
+
+    SafeIterator() = default;
+    SafeIterator(SafeIterator const& aOther)
+        : SafeIterator(aOther.mCurrent, aOther.mList) {}
+
+    SafeIterator(BaseIterator aBaseIter,
+                 SafeDoublyLinkedList<T, ElementAccess>* aList)
+        : mCurrent(aBaseIter),
+          mNext(aBaseIter ? ++aBaseIter : BaseIterator()),
+          mList(aList) {
+      if (mList) {
+        mNextIterator = mList->mIter;
+        mList->mIter = this;
+      }
+    }
+    ~SafeIterator() {
+      if (mList) {
+        MOZ_ASSERT(mList->mIter == this,
+                   "Iterators must currently be destroyed in opposite order "
+                   "from the construction order. It is suggested that you "
+                   "simply put them on the stack");
+        mList->mIter = mNextIterator;
+      }
+    }
+
+    SafeIterator& operator++() {
+      mCurrent = mNext;
+      if (mNext) {
+        ++mNext;
+      }
+      return *this;
+    }
+
+    pointer operator->() { return &*mCurrent; }
+    const_pointer operator->() const { return &*mCurrent; }
+    reference operator*() { return *mCurrent; }
+    const_reference operator*() const { return *mCurrent; }
+
+    pointer current() { return mCurrent ? &*mCurrent : nullptr; }
+    const_pointer current() const { return mCurrent ? &*mCurrent : nullptr; }
+
+    explicit operator bool() const { return bool(mCurrent); }
+    bool operator==(SafeIterator const& other) const {
+      return mCurrent == other.mCurrent;
+    }
+    bool operator!=(SafeIterator const& other) const {
+      return mCurrent != other.mCurrent;
+    }
+
+    BaseIterator& next() { return mNext; }  // mainly needed for unittests.
+   private:
+    /**
+     * Base list iterator pointing to the current list element of the iteration.
+     * If element mCurrent points to gets removed, the iterator will be set to
+     * empty. mNext keeps the iterator valid.
+     */
+    BaseIterator mCurrent{nullptr};
+    /**
+     * Base list iterator pointing to the next list element of the iteration.
+     * If element mCurrent points to gets removed, mNext is still valid.
+     * If element mNext points to gets removed, mNext advances, keeping this
+     * iterator valid.
+     */
+    BaseIterator mNext{nullptr};
+
+    /**
+     * Next element in the stack-allocated list of iterators stored in the
+     * SafeLinkedList object.
+     */
+    SafeIterator* mNextIterator{nullptr};
+    SafeDoublyLinkedList<T, ElementAccess>* mList{nullptr};
+
+    void setNext(T* aElm) { mNext = BaseIterator(aElm); }
+    void setCurrent(T* aElm) { mCurrent = BaseIterator(aElm); }
+  };
+
+ private:
+  using BaseListType = DoublyLinkedList<T, ElementAccess>;
+  friend class SafeIterator;
+
+ public:
+  SafeDoublyLinkedList() = default;
+
+  bool isEmpty() const { return mList.isEmpty(); }
+  bool contains(T* aElm) {
+    for (auto iter = mList.begin(); iter != mList.end(); ++iter) {
+      if (&*iter == aElm) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  SafeIterator begin() { return SafeIterator(mList.begin(), this); }
+  SafeIterator begin() const { return SafeIterator(mList.begin(), this); }
+  SafeIterator cbegin() const { return begin(); }
+
+  SafeIterator end() { return SafeIterator(); }
+  SafeIterator end() const { return SafeIterator(); }
+  SafeIterator cend() const { return SafeIterator(); }
+
+  void pushFront(T* aElm) { mList.pushFront(aElm); }
+
+  void pushBack(T* aElm) {
+    mList.pushBack(aElm);
+    auto* iter = mIter;
+    while (iter) {
+      if (!iter->mNext) {
+        iter->setNext(aElm);
+      }
+      iter = iter->mNextIterator;
+    }
+  }
+
+  T* popFront() {
+    T* firstElm = mList.popFront();
+    auto* iter = mIter;
+    while (iter) {
+      if (iter->current() == firstElm) {
+        iter->setCurrent(nullptr);
+      }
+      iter = iter->mNextIterator;
+    }
+
+    return firstElm;
+  }
+
+  T* popBack() {
+    T* lastElm = mList.popBack();
+    auto* iter = mIter;
+    while (iter) {
+      if (iter->current() == lastElm) {
+        iter->setCurrent(nullptr);
+      } else if (iter->mNext && &*(iter->mNext) == lastElm) {
+        iter->setNext(nullptr);
+      }
+      iter = iter->mNextIterator;
+    }
+
+    return lastElm;
+  }
+
+  void remove(T* aElm) {
+    if (!mList.ElementProbablyInList(aElm)) {
+      return;
+    }
+    auto* iter = mIter;
+    while (iter) {
+      if (iter->mNext && &*(iter->mNext) == aElm) {
+        ++(iter->mNext);
+      }
+      if (iter->current() == aElm) {
+        iter->setCurrent(nullptr);
+      }
+      iter = iter->mNextIterator;
+    }
+
+    mList.remove(aElm);
+  }
+
+ private:
+  BaseListType mList;
+  SafeIterator* mIter{nullptr};
+};
+
 }  // namespace mozilla
 
 #endif  // mozilla_DoublyLinkedList_h

mfbt/tests/TestDoublyLinkedList.cpp

@@ -232,8 +232,73 @@ static void TestCustomAccessor() {
   }
 }
 
+static void TestSafeDoubleLinkedList() {
+  mozilla::SafeDoublyLinkedList<SomeClass> list;
+  auto* elt1 = new SomeClass(0);
+  auto* elt2 = new SomeClass(0);
+  auto* elt3 = new SomeClass(0);
+  auto* elt4 = new SomeClass(0);
+  list.pushBack(elt1);
+  list.pushBack(elt2);
+  list.pushBack(elt3);
+  auto iter = list.begin();
+
+  // basic tests for iterator validity
+  MOZ_RELEASE_ASSERT(
+      &*iter == elt1,
+      "iterator returned by begin() must point to the first element!");
+  MOZ_RELEASE_ASSERT(
+      &*(iter.next()) == elt2,
+      "iterator returned by begin() must have the second element as 'next'!");
+  list.remove(elt2);
+  MOZ_RELEASE_ASSERT(
+      &*(iter.next()) == elt3,
+      "After removal of the 2nd element 'next' must point to the 3rd element!");
+  ++iter;
+  MOZ_RELEASE_ASSERT(
+      &*iter == elt3,
+      "After advancing one step the current element must be the 3rd one!");
+  MOZ_RELEASE_ASSERT(!iter.next(), "This is the last element of the list!");
+  list.pushBack(elt4);
+  MOZ_RELEASE_ASSERT(&*(iter.next()) == elt4,
+                     "After adding an element at the end of the list the "
+                     "iterator must be updated!");
+
+  // advance to last element, then remove last element
+  ++iter;
+  list.popBack();
+  MOZ_RELEASE_ASSERT(bool(iter) == false,
+                     "After removing the last element, the iterator pointing "
+                     "to the last element must be empty!");
+
+  // iterate the whole remaining list, increment values
+  for (auto& el : list) {
+    el.incr();
+  }
+  MOZ_RELEASE_ASSERT(elt1->mValue == 1);
+  MOZ_RELEASE_ASSERT(elt2->mValue == 0);
+  MOZ_RELEASE_ASSERT(elt3->mValue == 1);
+  MOZ_RELEASE_ASSERT(elt4->mValue == 0);
+
+  // Removing the first element of the list while iterating must empty the
+  // iterator
+  for (auto it = list.begin(); it != list.end(); ++it) {
+    MOZ_RELEASE_ASSERT(bool(it) == true, "The iterator must contain a value!");
+    list.popFront();
+    MOZ_RELEASE_ASSERT(
+        bool(it) == false,
+        "After removing the first element, the iterator must be empty!");
+  }
+
+  delete elt1;
+  delete elt2;
+  delete elt3;
+  delete elt4;
+}
+
 int main() {
   TestDoublyLinkedList();
   TestCustomAccessor();
+  TestSafeDoubleLinkedList();
   return 0;
 }

toolkit/components/satchel/nsFormFillController.h

@@ -15,6 +15,7 @@
 #include "nsIDOMEventListener.h"
 #include "nsIFormAutoComplete.h"
 #include "nsCOMPtr.h"
+#include "nsStubMutationObserver.h"
 #include "nsTHashMap.h"
 #include "nsInterfaceHashtable.h"
 #include "nsIDocShell.h"
@@ -40,7 +41,7 @@ class nsFormFillController final : public nsIFormFillController,
                                    public nsIFormAutoCompleteObserver,
                                    public nsIDOMEventListener,
                                    public nsIObserver,
-                                   public nsIMutationObserver {
+                                   public nsMultiMutationObserver {
  public:
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_NSIFORMFILLCONTROLLER

widget/cocoa/nsMenuGroupOwnerX.h

@@ -10,7 +10,7 @@
 
 #include "mozilla/WeakPtr.h"
 
-#include "nsIMutationObserver.h"
+#include "nsStubMutationObserver.h"
 #include "nsHashKeys.h"
 #include "nsIObserver.h"
 #include "nsTHashMap.h"
@@ -42,7 +42,7 @@ enum {
 // The menu group owner observes DOM mutations, notifies registered nsChangeObservers, and manages
 // command registration.
 // There is one owner per menubar, and one per standalone native menu.
-class nsMenuGroupOwnerX : public nsIMutationObserver, public nsIObserver {
+class nsMenuGroupOwnerX : public nsMultiMutationObserver, public nsIObserver {
  public:
   // Both parameters can be null.
   nsMenuGroupOwnerX(mozilla::dom::Element* aElement, nsMenuBarX* aMenuBarIfMenuBar);