Bug 1588688 - part 1: Remove `TrivialFunctor` and `BRNodeFunctor` for `DOMIterator` r=m_kato

`TrivialFunctor` returns always true for `DOMIterator::AppendList()`.  That
means that `DOMIterator` appends all nodes which are listed up by
`ContentIterator`.  So, it's reasonable to make `DOMIterator` have
`AppendAllNodesToArray()` because it's more self-documented and faster.

Additionally, `BRNodeFunctor` always returns true when nodes with which
`HTMLBRElement::FromNode()` returns non-nullptr.  So, we can make
`AppendAllNodesToArray()` a template class which just checks whether
every nodes are specific node type.

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

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

editor/libeditor/EditorUtils.cpp

@@ -9,6 +9,7 @@
 #include "mozilla/EditorDOMPoint.h"
 #include "mozilla/OwningNonNull.h"
 #include "mozilla/TextEditor.h"
+#include "mozilla/dom/HTMLBRElement.h"
 #include "mozilla/dom/Selection.h"
 #include "mozilla/dom/Text.h"
 #include "nsContentUtils.h"
@@ -26,6 +27,9 @@ namespace mozilla {
 
 using namespace dom;
 
+template void DOMIterator::AppendAllNodesToArray(
+    nsTArray<OwningNonNull<HTMLBRElement>>& aArrayOfNodes) const;
+
 /******************************************************************************
  * mozilla::EditActionResult
  *****************************************************************************/
@@ -81,6 +85,24 @@ DOMIterator::DOMIterator(MOZ_GUARD_OBJECT_NOTIFIER_ONLY_PARAM_IN_IMPL)
   MOZ_GUARD_OBJECT_NOTIFIER_INIT;
 }
 
+template <class NodeClass>
+void DOMIterator::AppendAllNodesToArray(
+    nsTArray<OwningNonNull<NodeClass>>& aArrayOfNodes) const {
+  for (; !mIter->IsDone(); mIter->Next()) {
+    if (NodeClass* node = NodeClass::FromNode(mIter->GetCurrentNode())) {
+      aArrayOfNodes.AppendElement(*node);
+    }
+  }
+}
+
+template <>
+void DOMIterator::AppendAllNodesToArray(
+    nsTArray<OwningNonNull<nsINode>>& aArrayOfNodes) const {
+  for (; !mIter->IsDone(); mIter->Next()) {
+    aArrayOfNodes.AppendElement(*mIter->GetCurrentNode());
+  }
+}
+
 void DOMIterator::AppendList(
     const BoolDomIterFunctor& functor,
     nsTArray<OwningNonNull<nsINode>>& arrayOfNodes) const {

editor/libeditor/EditorUtils.h

@@ -739,7 +739,6 @@ class BoolDomIterFunctor {
 class MOZ_RAII DOMIterator {
  public:
   explicit DOMIterator(MOZ_GUARD_OBJECT_NOTIFIER_ONLY_PARAM);
-
   explicit DOMIterator(nsINode& aNode MOZ_GUARD_OBJECT_NOTIFIER_PARAM);
   virtual ~DOMIterator() = default;
 
@@ -747,6 +746,10 @@ class MOZ_RAII DOMIterator {
   nsresult Init(const RawRangeBoundary& aStartRef,
                 const RawRangeBoundary& aEndRef);
 
+  template <class NodeClass>
+  void AppendAllNodesToArray(
+      nsTArray<OwningNonNull<NodeClass>>& aArrayOfNodes) const;
+
   void AppendList(
       const BoolDomIterFunctor& functor,
       nsTArray<mozilla::OwningNonNull<nsINode>>& arrayOfNodes) const;
@@ -770,12 +773,6 @@ class MOZ_RAII DOMSubtreeIterator final : public DOMIterator {
       delete;
 };
 
-class TrivialFunctor final : public BoolDomIterFunctor {
- public:
-  // Used to build list of all nodes iterator covers
-  virtual bool operator()(nsINode* aNode) const override { return true; }
-};
-
 class EditorUtils final {
  public:
   /**

editor/libeditor/HTMLEditSubActionHandler.cpp

@@ -103,13 +103,6 @@ class TableCellAndListItemFunctor final : public BoolDomIterFunctor {
   }
 };
 
-class BRNodeFunctor final : public BoolDomIterFunctor {
- public:
-  virtual bool operator()(nsINode* aNode) const override {
-    return aNode->IsHTMLElement(nsGkAtoms::br);
-  }
-};
-
 class EmptyEditableFunctor final : public BoolDomIterFunctor {
  public:
   explicit EmptyEditableFunctor(HTMLEditor* aHTMLEditor)
@@ -3232,14 +3225,13 @@ EditActionResult HTMLEditor::HandleDeleteNonCollapsedSelection(
     AutoRangeArray arrayOfRanges(SelectionRefPtr());
     for (auto& range : arrayOfRanges.mRanges) {
       // Build a list of nodes in the range
-      nsTArray<OwningNonNull<nsINode>> arrayOfNodes;
-      TrivialFunctor functor;
+      AutoTArray<OwningNonNull<nsINode>, 10> arrayOfNodes;
       DOMSubtreeIterator iter;
       nsresult rv = iter.Init(*range);
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return result.SetResult(rv);
       }
-      iter.AppendList(functor, arrayOfNodes);
+      iter.AppendAllNodesToArray(arrayOfNodes);
 
       // Now that we have the list, delete non-table elements
       int32_t listCount = arrayOfNodes.Length();
@@ -7727,7 +7719,7 @@ nsresult HTMLEditor::CollectEditTargetNodes(
       return rv;
     }
     if (aOutArrayOfNodes.IsEmpty()) {
-      iter.AppendList(TrivialFunctor(), aOutArrayOfNodes);
+      iter.AppendAllNodesToArray(aOutArrayOfNodes);
     } else {
       // We don't want duplicates in aOutArrayOfNodes, so we use an
       // iterator/functor that only return nodes that are not already in
@@ -7972,21 +7964,20 @@ nsresult HTMLEditor::SplitElementsAtEveryBRElement(
   MOZ_ASSERT(IsEditActionDataAvailable());
 
   // First build up a list of all the break nodes inside the inline container.
-  nsTArray<OwningNonNull<nsINode>> arrayOfBreaks;
-  BRNodeFunctor functor;
+  AutoTArray<OwningNonNull<HTMLBRElement>, 24> arrayOfBRElements;
   DOMIterator iter(aMostAncestorToBeSplit);
-  iter.AppendList(functor, arrayOfBreaks);
+  iter.AppendAllNodesToArray(arrayOfBRElements);
 
   // If there aren't any breaks, just put inNode itself in the array
-  if (arrayOfBreaks.IsEmpty()) {
+  if (arrayOfBRElements.IsEmpty()) {
     aOutArrayOfNodes.AppendElement(aMostAncestorToBeSplit);
     return NS_OK;
   }
 
   // Else we need to bust up aMostAncestorToBeSplit along all the breaks
   nsCOMPtr<nsIContent> nextContent = &aMostAncestorToBeSplit;
-  for (OwningNonNull<nsINode>& brNode : arrayOfBreaks) {
-    EditorDOMPoint atBrNode(brNode);
+  for (OwningNonNull<HTMLBRElement>& brElement : arrayOfBRElements) {
+    EditorDOMPoint atBrNode(brElement);
     if (NS_WARN_IF(!atBrNode.IsSet())) {
       return NS_ERROR_FAILURE;
     }
@@ -8009,15 +8000,14 @@ nsresult HTMLEditor::SplitElementsAtEveryBRElement(
 
     // Move break outside of container and also put in node list
     EditorDOMPoint atNextNode(splitNodeResult.GetNextNode());
-    nsresult rv = MoveNodeWithTransaction(MOZ_KnownLive(*brNode->AsContent()),
-                                          atNextNode);
+    nsresult rv = MoveNodeWithTransaction(brElement, atNextNode);
     if (NS_WARN_IF(Destroyed())) {
       return NS_ERROR_EDITOR_DESTROYED;
     }
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
-    aOutArrayOfNodes.AppendElement(*brNode);
+    aOutArrayOfNodes.AppendElement(brElement);
 
     nextContent = splitNodeResult.GetNextNode();
   }

editor/libeditor/HTMLEditorDataTransfer.cpp

@@ -2730,12 +2730,11 @@ void HTMLEditor::CreateListOfNodesToPaste(
   }
 
   // Now use a subtree iterator over the range to create a list of nodes
-  TrivialFunctor functor;
   DOMSubtreeIterator iter;
   if (NS_WARN_IF(NS_FAILED(iter.Init(*docFragRange)))) {
     return;
   }
-  iter.AppendList(functor, outNodeList);
+  iter.AppendAllNodesToArray(outNodeList);
 }
 
 void HTMLEditor::GetListAndTableParents(