Skip to content

Commit

Permalink
Deploy smart more pointers in NodeIterator
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=263215

Reviewed by Chris Dumez.

* Source/WebCore/dom/NodeIterator.cpp:
(WebCore::NodeIterator::NodePointer::moveToNext):
(WebCore::NodeIterator::NodePointer::moveToPrevious):
(WebCore::NodeIterator::nextNode):
(WebCore::NodeIterator::previousNode):
(WebCore::NodeIterator::updateForNodeRemoval const):
* Source/WebCore/dom/NodeIterator.h:
* Source/WebCore/dom/Traversal.h:
(WebCore::NodeIteratorBase::protectedRoot const):

Canonical link: https://commits.webkit.org/269398@main
  • Loading branch information
rniwa committed Oct 17, 2023
1 parent 236dd9d commit e859d6e
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 14 deletions.
33 changes: 19 additions & 14 deletions Source/WebCore/dom/NodeIterator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,29 +46,31 @@ inline void NodeIterator::NodePointer::clear()

inline bool NodeIterator::NodePointer::moveToNext(Node& root)
{
if (!node)
auto currentNode = protectedNode();
if (!currentNode)
return false;
if (isPointerBeforeNode) {
isPointerBeforeNode = false;
return true;
}
node = NodeTraversal::next(*node, &root);
node = NodeTraversal::next(*currentNode, &root);
return node;
}

inline bool NodeIterator::NodePointer::moveToPrevious(Node& root)
{
if (!node)
auto currentNode = protectedNode();
if (!currentNode)
return false;
if (!isPointerBeforeNode) {
isPointerBeforeNode = true;
return true;
}
if (node == &root) {
if (currentNode == &root) {
node = nullptr;
return false;
}
node = NodeTraversal::previous(*node);
node = NodeTraversal::previous(*currentNode);
return node;
}

Expand All @@ -94,7 +96,8 @@ ExceptionOr<RefPtr<Node>> NodeIterator::nextNode()
RefPtr<Node> result;

m_candidateNode = m_referenceNode;
while (m_candidateNode.moveToNext(root())) {
auto root = protectedRoot();
while (m_candidateNode.moveToNext(root)) {
// NodeIterators treat the DOM tree as a flat list of nodes.
// In other words, FILTER_REJECT does not pass over descendants
// of the rejected node. Hence, FILTER_REJECT is the same as FILTER_SKIP.
Expand Down Expand Up @@ -123,7 +126,8 @@ ExceptionOr<RefPtr<Node>> NodeIterator::previousNode()
RefPtr<Node> result;

m_candidateNode = m_referenceNode;
while (m_candidateNode.moveToPrevious(root())) {
auto root = protectedRoot();
while (m_candidateNode.moveToPrevious(root)) {
// NodeIterators treat the DOM tree as a flat list of nodes.
// In other words, FILTER_REJECT does not pass over descendants
// of the rejected node. Hence, FILTER_REJECT is the same as FILTER_SKIP.
Expand Down Expand Up @@ -159,20 +163,21 @@ void NodeIterator::updateForNodeRemoval(Node& removedNode, NodePointer& referenc

// Iterator is not affected if the removed node is the reference node and is the root.
// or if removed node is not the reference node, or the ancestor of the reference node.
if (!removedNode.isDescendantOf(root()))
auto root = protectedRoot();
if (!removedNode.isDescendantOf(root))
return;
bool willRemoveReferenceNode = &removedNode == referenceNode.node;
bool willRemoveReferenceNodeAncestor = referenceNode.node && referenceNode.node->isDescendantOf(removedNode);
if (!willRemoveReferenceNode && !willRemoveReferenceNodeAncestor)
return;

if (referenceNode.isPointerBeforeNode) {
Node* node = NodeTraversal::next(removedNode, &root());
RefPtr node = NodeTraversal::next(removedNode, root.ptr());
if (node) {
// Move out from under the node being removed if the new reference
// node is a descendant of the node being removed.
while (node && node->isDescendantOf(removedNode))
node = NodeTraversal::next(*node, &root());
node = NodeTraversal::next(*node, root.ptr());
if (node)
referenceNode.node = node;
} else {
Expand All @@ -194,7 +199,7 @@ void NodeIterator::updateForNodeRemoval(Node& removedNode, NodePointer& referenc
}
}
} else {
Node* node = NodeTraversal::previous(removedNode);
RefPtr node = NodeTraversal::previous(removedNode);
if (node) {
// Move out from under the node being removed if the reference node is
// a descendant of the node being removed.
Expand All @@ -203,18 +208,18 @@ void NodeIterator::updateForNodeRemoval(Node& removedNode, NodePointer& referenc
node = NodeTraversal::previous(*node);
}
if (node)
referenceNode.node = node;
referenceNode.node = WTFMove(node);
} else {
// FIXME: This branch doesn't appear to have any LayoutTests.
node = NodeTraversal::next(removedNode, &root());
node = NodeTraversal::next(removedNode, root.ptr());
// Move out from under the node being removed if the reference node is
// a descendant of the node being removed.
if (willRemoveReferenceNodeAncestor) {
while (node && node->isDescendantOf(removedNode))
node = NodeTraversal::previous(*node);
}
if (node)
referenceNode.node = node;
referenceNode.node = WTFMove(node);
}
}
}
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/dom/NodeIterator.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ class NodeIterator final : public ScriptWrappable, public RefCounted<NodeIterato

NodePointer() = default;
NodePointer(Node&, bool);
RefPtr<Node> protectedNode() const { return node; }

void clear();
bool moveToNext(Node& root);
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/dom/Traversal.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class NodeIteratorBase {
public:
Node& root() { return m_root.get(); }
const Node& root() const { return m_root.get(); }
Ref<Node> protectedRoot() const { return m_root; }

unsigned whatToShow() const { return m_whatToShow; }
NodeFilter* filter() const { return m_filter.get(); }
Expand Down

0 comments on commit e859d6e

Please sign in to comment.