From fc9618eabb37f5ad88628ee6198569a274828453 Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Tue, 11 Feb 2020 06:52:32 +0000 Subject: [PATCH] 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 | 22 ++++++++++++++ editor/libeditor/EditorUtils.h | 11 +++---- editor/libeditor/HTMLEditSubActionHandler.cpp | 30 +++++++------------ editor/libeditor/HTMLEditorDataTransfer.cpp | 3 +- 4 files changed, 37 insertions(+), 29 deletions(-) diff --git a/editor/libeditor/EditorUtils.cpp b/editor/libeditor/EditorUtils.cpp index 8fd920885bf3..36fd631d7bcd 100644 --- a/editor/libeditor/EditorUtils.cpp +++ b/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>& aArrayOfNodes) const; + /****************************************************************************** * mozilla::EditActionResult *****************************************************************************/ @@ -81,6 +85,24 @@ DOMIterator::DOMIterator(MOZ_GUARD_OBJECT_NOTIFIER_ONLY_PARAM_IN_IMPL) MOZ_GUARD_OBJECT_NOTIFIER_INIT; } +template +void DOMIterator::AppendAllNodesToArray( + nsTArray>& aArrayOfNodes) const { + for (; !mIter->IsDone(); mIter->Next()) { + if (NodeClass* node = NodeClass::FromNode(mIter->GetCurrentNode())) { + aArrayOfNodes.AppendElement(*node); + } + } +} + +template <> +void DOMIterator::AppendAllNodesToArray( + nsTArray>& aArrayOfNodes) const { + for (; !mIter->IsDone(); mIter->Next()) { + aArrayOfNodes.AppendElement(*mIter->GetCurrentNode()); + } +} + void DOMIterator::AppendList( const BoolDomIterFunctor& functor, nsTArray>& arrayOfNodes) const { diff --git a/editor/libeditor/EditorUtils.h b/editor/libeditor/EditorUtils.h index 0371b0b1b24b..5d1977dc564a 100644 --- a/editor/libeditor/EditorUtils.h +++ b/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 + void AppendAllNodesToArray( + nsTArray>& aArrayOfNodes) const; + void AppendList( const BoolDomIterFunctor& functor, nsTArray>& 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: /** diff --git a/editor/libeditor/HTMLEditSubActionHandler.cpp b/editor/libeditor/HTMLEditSubActionHandler.cpp index bf625e39dacc..18089ede81c1 100644 --- a/editor/libeditor/HTMLEditSubActionHandler.cpp +++ b/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> arrayOfNodes; - TrivialFunctor functor; + AutoTArray, 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> arrayOfBreaks; - BRNodeFunctor functor; + AutoTArray, 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 nextContent = &aMostAncestorToBeSplit; - for (OwningNonNull& brNode : arrayOfBreaks) { - EditorDOMPoint atBrNode(brNode); + for (OwningNonNull& 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(); } diff --git a/editor/libeditor/HTMLEditorDataTransfer.cpp b/editor/libeditor/HTMLEditorDataTransfer.cpp index 77191102f265..cebdc133196e 100644 --- a/editor/libeditor/HTMLEditorDataTransfer.cpp +++ b/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(