Bug 1336011 - Fix Crash in InvalidArrayIndex_CRASH in mozilla::EditorBase::DeleteSelectionImpl

* EditorBase shouldn't refer mActionListeners directly in loops because it might be removed during a loop * Create an alias of the type of mEditorObservers * Create an alias of the type of mDocStateListeners Tag #1375
2026-05-26 14:54:25 +00:00 · 2020-04-15 01:55:25 -04:00
parent 5b8f93694a
commit 31048968fd
2 changed files with 124 additions and 71 deletions
@@ -1385,9 +1385,12 @@ EditorBase::CreateNode(nsIAtom* aTag,

  AutoRules beginRulesSniffing(this, EditAction::createNode, nsIEditor::eNext);

-  for (auto& listener : mActionListeners) {
-    listener->WillCreateNode(nsDependentAtomString(aTag),
-                             GetAsDOMNode(aParent), aPosition);
+  {
+    AutoActionListenerArray listeners(mActionListeners);
+    for (auto& listener : listeners) {
+      listener->WillCreateNode(nsDependentAtomString(aTag),
+                               GetAsDOMNode(aParent), aPosition);
+    }
  }

  nsCOMPtr<Element> ret;
@@ -1402,9 +1405,12 @@ EditorBase::CreateNode(nsIAtom* aTag,

  mRangeUpdater.SelAdjCreateNode(aParent, aPosition);

-  for (auto& listener : mActionListeners) {
-    listener->DidCreateNode(nsDependentAtomString(aTag), GetAsDOMNode(ret),
-                            GetAsDOMNode(aParent), aPosition, rv);
+  {
+    AutoActionListenerArray listeners(mActionListeners);
+    for (auto& listener : listeners) {
+      listener->DidCreateNode(nsDependentAtomString(aTag), GetAsDOMNode(ret),
+                              GetAsDOMNode(aParent), aPosition, rv);
+    }
  }

  return ret.forget();
@@ -1429,9 +1435,12 @@ EditorBase::InsertNode(nsIContent& aNode,
 {
  AutoRules beginRulesSniffing(this, EditAction::insertNode, nsIEditor::eNext);

-  for (auto& listener : mActionListeners) {
-    listener->WillInsertNode(aNode.AsDOMNode(), aParent.AsDOMNode(),
-                             aPosition);
+  {
+    AutoActionListenerArray listeners(mActionListeners);
+    for (auto& listener : listeners) {
+      listener->WillInsertNode(aNode.AsDOMNode(), aParent.AsDOMNode(),
+                               aPosition);
+    }
  }

  RefPtr<InsertNodeTransaction> transaction =
@@ -1440,9 +1449,12 @@ EditorBase::InsertNode(nsIContent& aNode,

  mRangeUpdater.SelAdjInsertNode(aParent.AsDOMNode(), aPosition);

-  for (auto& listener : mActionListeners) {
-    listener->DidInsertNode(aNode.AsDOMNode(), aParent.AsDOMNode(), aPosition,
-                            rv);
+  {
+    AutoActionListenerArray listeners(mActionListeners);
+    for (auto& listener : listeners) {
+      listener->DidInsertNode(aNode.AsDOMNode(), aParent.AsDOMNode(), aPosition,
+                              rv);
+    }
  }

  return rv;
@@ -1468,8 +1480,11 @@ EditorBase::SplitNode(nsIContent& aNode,
 {
  AutoRules beginRulesSniffing(this, EditAction::splitNode, nsIEditor::eNext);

-  for (auto& listener : mActionListeners) {
-    listener->WillSplitNode(aNode.AsDOMNode(), aOffset);
+  {
+    AutoActionListenerArray listeners(mActionListeners);
+    for (auto& listener : listeners) {
+      listener->WillSplitNode(aNode.AsDOMNode(), aOffset);
+    }
  }

  RefPtr<SplitNodeTransaction> transaction =
@@ -1482,9 +1497,12 @@ EditorBase::SplitNode(nsIContent& aNode,
  mRangeUpdater.SelAdjSplitNode(aNode, aOffset, newNode);

  nsresult rv = aResult.StealNSResult();
-  for (auto& listener : mActionListeners) {
-    listener->DidSplitNode(aNode.AsDOMNode(), aOffset, GetAsDOMNode(newNode),
-                           rv);
+  {
+    AutoActionListenerArray listeners(mActionListeners);
+    for (auto& listener : listeners) {
+      listener->DidSplitNode(aNode.AsDOMNode(), aOffset, GetAsDOMNode(newNode),
+                             rv);
+    }
  }
  // Note: result might be a success code, so we can't use Throw() to
  // set it on aResult.
@@ -1520,9 +1538,12 @@ EditorBase::JoinNodes(nsINode& aLeftNode,
  // Find the number of children of the lefthand node
  uint32_t oldLeftNodeLen = aLeftNode.Length();

-  for (auto& listener : mActionListeners) {
-    listener->WillJoinNodes(aLeftNode.AsDOMNode(), aRightNode.AsDOMNode(),
-                            parent->AsDOMNode());
+  {
+    AutoActionListenerArray listeners(mActionListeners);
+    for (auto& listener : listeners) {
+      listener->WillJoinNodes(aLeftNode.AsDOMNode(), aRightNode.AsDOMNode(),
+                              parent->AsDOMNode());
+    }
  }

  nsresult rv = NS_OK;
@@ -1535,9 +1556,12 @@ EditorBase::JoinNodes(nsINode& aLeftNode,
  mRangeUpdater.SelAdjJoinNodes(aLeftNode, aRightNode, *parent, offset,
                                (int32_t)oldLeftNodeLen);

-  for (auto& listener : mActionListeners) {
-    listener->DidJoinNodes(aLeftNode.AsDOMNode(), aRightNode.AsDOMNode(),
-                           parent->AsDOMNode(), rv);
+  {
+    AutoActionListenerArray listeners(mActionListeners);
+    for (auto& listener : listeners) {
+      listener->DidJoinNodes(aLeftNode.AsDOMNode(), aRightNode.AsDOMNode(),
+                             parent->AsDOMNode(), rv);
+    }
  }

  return rv;
@@ -1558,8 +1582,11 @@ EditorBase::DeleteNode(nsINode* aNode)
                               nsIEditor::ePrevious);

  // save node location for selection updating code.
-  for (auto& listener : mActionListeners) {
-    listener->WillDeleteNode(aNode->AsDOMNode());
+  {
+    AutoActionListenerArray listeners(mActionListeners);
+    for (auto& listener : listeners) {
+      listener->WillDeleteNode(aNode->AsDOMNode());
+    }
  }

  RefPtr<DeleteNodeTransaction> transaction;
@@ -1568,8 +1595,11 @@ EditorBase::DeleteNode(nsINode* aNode)
    rv = DoTransaction(transaction);
  }

-  for (auto& listener : mActionListeners) {
-    listener->DidDeleteNode(aNode->AsDOMNode(), rv);
+  {
+    AutoActionListenerArray listeners(mActionListeners);
+    for (auto& listener : listeners) {
+      listener->DidDeleteNode(aNode->AsDOMNode(), rv);
+    }
  }

  NS_ENSURE_SUCCESS(rv, rv);
@@ -1844,7 +1874,7 @@ void
 EditorBase::NotifyEditorObservers(NotificationForEditorObservers aNotification)
 {
  // Copy the observers since EditAction()s can modify mEditorObservers.
-  nsTArray<mozilla::OwningNonNull<nsIEditorObserver>> observers(mEditorObservers);
+  AutoEditorObserverArray observers(mEditorObservers);
  switch (aNotification) {
    case eNotifyEditorObserversOfEnd:
      mIsInEditAction = false;
@@ -2505,10 +2535,13 @@ EditorBase::InsertTextIntoTextNodeImpl(const nsAString& aStringToInsert,
  }

  // Let listeners know what's up
-  for (auto& listener : mActionListeners) {
-    listener->WillInsertText(
-      static_cast<nsIDOMCharacterData*>(insertedTextNode->AsDOMNode()),
-      insertedOffset, aStringToInsert);
+  {
+    AutoActionListenerArray listeners(mActionListeners);
+    for (auto& listener : listeners) {
+      listener->WillInsertText(
+        static_cast<nsIDOMCharacterData*>(insertedTextNode->AsDOMNode()),
+        insertedOffset, aStringToInsert);
+    }
  }

  // XXX We may not need these view batches anymore.  This is handled at a
@@ -2518,10 +2551,13 @@ EditorBase::InsertTextIntoTextNodeImpl(const nsAString& aStringToInsert,
  EndUpdateViewBatch();

  // let listeners know what happened
-  for (auto& listener : mActionListeners) {
-    listener->DidInsertText(
-      static_cast<nsIDOMCharacterData*>(insertedTextNode->AsDOMNode()),
-      insertedOffset, aStringToInsert, rv);
+  {
+    AutoActionListenerArray listeners(mActionListeners);
+    for (auto& listener : listeners) {
+      listener->DidInsertText(
+        static_cast<nsIDOMCharacterData*>(insertedTextNode->AsDOMNode()),
+        insertedOffset, aStringToInsert, rv);
+    }
  }

  // Added some cruft here for bug 43366.  Layout was crashing because we left
@@ -2583,8 +2619,7 @@ EditorBase::NotifyDocumentListeners(
    return NS_OK;
  }

-  nsTArray<OwningNonNull<nsIDocumentStateListener>>
-    listeners(mDocStateListeners);
+  AutoDocumentStateListenerArray listeners(mDocStateListeners);
  nsresult rv = NS_OK;

  switch (aNotificationType) {
@@ -2656,19 +2691,25 @@ EditorBase::DeleteText(nsGenericDOMDataNode& aCharData,
                               nsIEditor::ePrevious);

  // Let listeners know what's up
-  for (auto& listener : mActionListeners) {
-    listener->WillDeleteText(
-        static_cast<nsIDOMCharacterData*>(GetAsDOMNode(&aCharData)), aOffset,
-        aLength);
+  {
+    AutoActionListenerArray listeners(mActionListeners);
+    for (auto& listener : listeners) {
+      listener->WillDeleteText(
+          static_cast<nsIDOMCharacterData*>(GetAsDOMNode(&aCharData)), aOffset,
+          aLength);
+    }
  }

  nsresult rv = DoTransaction(transaction);

  // Let listeners know what happened
-  for (auto& listener : mActionListeners) {
-    listener->DidDeleteText(
-        static_cast<nsIDOMCharacterData*>(GetAsDOMNode(&aCharData)), aOffset,
-        aLength, rv);
+  {
+    AutoActionListenerArray listeners(mActionListeners);
+    for (auto& listener : listeners) {
+      listener->DidDeleteText(
+          static_cast<nsIDOMCharacterData*>(GetAsDOMNode(&aCharData)), aOffset,
+          aLength, rv);
+    }
  }

  return rv;
@@ -4034,17 +4075,20 @@ EditorBase::DeleteSelectionImpl(EDirection aAction,
  if (NS_SUCCEEDED(rv)) {
    AutoRules beginRulesSniffing(this, EditAction::deleteSelection, aAction);
    // Notify nsIEditActionListener::WillDelete[Selection|Text|Node]
-    if (!deleteNode) {
-      for (auto& listener : mActionListeners) {
-        listener->WillDeleteSelection(selection);
-      }
-    } else if (deleteCharData) {
-      for (auto& listener : mActionListeners) {
-        listener->WillDeleteText(deleteCharData, deleteCharOffset, 1);
-      }
-    } else {
-      for (auto& listener : mActionListeners) {
-        listener->WillDeleteNode(deleteNode->AsDOMNode());
+    {
+      AutoActionListenerArray listeners(mActionListeners);
+      if (!deleteNode) {
+        for (auto& listener : listeners) {
+          listener->WillDeleteSelection(selection);
+        }
+      } else if (deleteCharData) {
+        for (auto& listener : listeners) {
+          listener->WillDeleteText(deleteCharData, deleteCharOffset, 1);
+        }
+      } else {
+        for (auto& listener : listeners) {
+          listener->WillDeleteNode(deleteNode->AsDOMNode());
+        }
      }
    }

@@ -4052,17 +4096,20 @@ EditorBase::DeleteSelectionImpl(EDirection aAction,
    rv = DoTransaction(transaction);

    // Notify nsIEditActionListener::DidDelete[Selection|Text|Node]
-    if (!deleteNode) {
-      for (auto& listener : mActionListeners) {
-        listener->DidDeleteSelection(selection);
-      }
-    } else if (deleteCharData) {
-      for (auto& listener : mActionListeners) {
-        listener->DidDeleteText(deleteCharData, deleteCharOffset, 1, rv);
-      }
-    } else {
-      for (auto& listener : mActionListeners) {
-        listener->DidDeleteNode(deleteNode->AsDOMNode(), rv);
+    {
+      AutoActionListenerArray listeners(mActionListeners);
+      if (!deleteNode) {
+        for (auto& listener : mActionListeners) {
+          listener->DidDeleteSelection(selection);
+        }
+      } else if (deleteCharData) {
+        for (auto& listener : mActionListeners) {
+          listener->DidDeleteText(deleteCharData, deleteCharOffset, 1, rv);
+        }
+      } else {
+        for (auto& listener : mActionListeners) {
+          listener->DidDeleteNode(deleteNode->AsDOMNode(), rv);
+        }
      }
    }
  }