LibWeb: Make NodeIterator GC-allocated

Author: https://github.com/awesomekling
Commit: bd629c45b5
Pull-request: https://github.com/SerenityOS/serenity/pull/14816
Reviewed-by: https://github.com/ADKaster
Reviewed-by: https://github.com/linusg ✅

Userland/Libraries/LibWeb/DOM/Document.cpp

@@ -1744,7 +1744,7 @@ ExceptionOr<Document::PrefixAndTagName> Document::validate_qualified_name(String
 }
 
 // https://dom.spec.whatwg.org/#dom-document-createnodeiterator
-NonnullRefPtr<NodeIterator> Document::create_node_iterator(Node& root, unsigned what_to_show, NodeFilter* filter)
+JS::NonnullGCPtr<NodeIterator> Document::create_node_iterator(Node& root, unsigned what_to_show, JS::GCPtr<NodeFilter> filter)
 {
     return NodeIterator::create(root, what_to_show, filter);
 }

Userland/Libraries/LibWeb/DOM/Document.h

@@ -350,7 +350,7 @@ public:
     };
     static ExceptionOr<PrefixAndTagName> validate_qualified_name(String const& qualified_name);
 
-    NonnullRefPtr<NodeIterator> create_node_iterator(Node& root, unsigned what_to_show, NodeFilter*);
+    JS::NonnullGCPtr<NodeIterator> create_node_iterator(Node& root, unsigned what_to_show, JS::GCPtr<NodeFilter>);
     NonnullRefPtr<TreeWalker> create_tree_walker(Node& root, unsigned what_to_show, NodeFilter*);
 
     void register_node_iterator(Badge<NodeIterator>, NodeIterator&);

Userland/Libraries/LibWeb/DOM/NodeIterator.cpp

@@ -6,6 +6,7 @@
 
 #include <LibWeb/Bindings/DOMExceptionWrapper.h>
 #include <LibWeb/Bindings/IDLAbstractOperations.h>
+#include <LibWeb/Bindings/NodeIteratorPrototype.h>
 #include <LibWeb/Bindings/NodeWrapper.h>
 #include <LibWeb/Bindings/NodeWrapperFactory.h>
 #include <LibWeb/DOM/Node.h>
@@ -14,7 +15,8 @@
 namespace Web::DOM {
 
 NodeIterator::NodeIterator(Node& root)
-    : m_root(root)
+    : PlatformObject(root.document().preferred_window_object().ensure_web_prototype<Bindings::NodeIteratorPrototype>("NodeIterator"))
+    , m_root(root)
     , m_reference({ root })
 {
     root.document().register_node_iterator({}, *this);
@@ -25,22 +27,29 @@ NodeIterator::~NodeIterator()
     m_root->document().unregister_node_iterator({}, *this);
 }
 
+void NodeIterator::visit_edges(Cell::Visitor& visitor)
+{
+    Base::visit_edges(visitor);
+    visitor.visit(m_filter.ptr());
+}
+
 // https://dom.spec.whatwg.org/#dom-document-createnodeiterator
-NonnullRefPtr<NodeIterator> NodeIterator::create(Node& root, unsigned what_to_show, NodeFilter* filter)
+JS::NonnullGCPtr<NodeIterator> NodeIterator::create(Node& root, unsigned what_to_show, JS::GCPtr<NodeFilter> filter)
 {
     // 1. Let iterator be a new NodeIterator object.
     // 2. Set iterator’s root and iterator’s reference to root.
     // 3. Set iterator’s pointer before reference to true.
-    auto iterator = adopt_ref(*new NodeIterator(root));
+    auto& window_object = root.document().preferred_window_object();
+    auto* iterator = window_object.heap().allocate<NodeIterator>(window_object.realm(), root);
 
     // 4. Set iterator’s whatToShow to whatToShow.
     iterator->m_what_to_show = what_to_show;
 
     // 5. Set iterator’s filter to filter.
-    iterator->m_filter = JS::make_handle(filter);
+    iterator->m_filter = filter;
 
     // 6. Return iterator.
-    return iterator;
+    return *iterator;
 }
 
 // https://dom.spec.whatwg.org/#dom-nodeiterator-detach
@@ -117,13 +126,9 @@ JS::ThrowCompletionOr<RefPtr<Node>> NodeIterator::traverse(Direction direction)
 // https://dom.spec.whatwg.org/#concept-node-filter
 JS::ThrowCompletionOr<NodeFilter::Result> NodeIterator::filter(Node& node)
 {
-    VERIFY(wrapper());
-    auto& vm = wrapper()->vm();
-    auto& realm = *vm.current_realm();
-
     // 1. If traverser’s active flag is set, then throw an "InvalidStateError" DOMException.
     if (m_active)
-        return JS::throw_completion(wrap(realm, InvalidStateError::create("NodeIterator is already active")));
+        return JS::throw_completion(wrap(shape().realm(), InvalidStateError::create("NodeIterator is already active")));
 
     // 2. Let n be node’s nodeType attribute value − 1.
     auto n = node.node_type() - 1;
@@ -133,7 +138,7 @@ JS::ThrowCompletionOr<NodeFilter::Result> NodeIterator::filter(Node& node)
         return NodeFilter::FILTER_SKIP;
 
     // 4. If traverser’s filter is null, then return FILTER_ACCEPT.
-    if (!m_filter.cell())
+    if (!m_filter)
         return NodeFilter::FILTER_ACCEPT;
 
     // 5. Set traverser’s active flag.
@@ -141,7 +146,7 @@ JS::ThrowCompletionOr<NodeFilter::Result> NodeIterator::filter(Node& node)
 
     // 6. Let result be the return value of call a user object’s operation with traverser’s filter, "acceptNode", and « node ».
     //    If this throws an exception, then unset traverser’s active flag and rethrow the exception.
-    auto result = Bindings::IDL::call_user_object_operation(m_filter->callback(), "acceptNode", {}, wrap(realm, node));
+    auto result = Bindings::IDL::call_user_object_operation(m_filter->callback(), "acceptNode", {}, wrap(shape().realm(), node));
     if (result.is_abrupt()) {
         m_active = false;
         return result;
@@ -151,7 +156,7 @@ JS::ThrowCompletionOr<NodeFilter::Result> NodeIterator::filter(Node& node)
     m_active = false;
 
     // 8. Return result.
-    auto result_value = TRY(result.value()->to_i32(vm));
+    auto result_value = TRY(result.value()->to_i32(vm()));
     return static_cast<NodeFilter::Result>(result_value);
 }
 

Userland/Libraries/LibWeb/DOM/NodeIterator.h

@@ -6,29 +6,29 @@
 
 #pragma once
 
-#include <AK/RefCounted.h>
+#include <LibJS/Runtime/Object.h>
-#include <LibWeb/Bindings/Wrappable.h>
 #include <LibWeb/DOM/NodeFilter.h>
 
 namespace Web::DOM {
 
 // https://dom.spec.whatwg.org/#nodeiterator
-class NodeIterator
+class NodeIterator final : public Bindings::PlatformObject {
-    : public RefCounted<NodeIterator>
+    JS_OBJECT(NodeIterator, Bindings::PlatformObject);
-    , public Bindings::Wrappable {
-public:
-    using WrapperType = Bindings::NodeIteratorWrapper;
 
+public:
+    static JS::NonnullGCPtr<NodeIterator> create(Node& root, unsigned what_to_show, JS::GCPtr<NodeFilter>);
+
+    NodeIterator(Node& root);
     virtual ~NodeIterator() override;
 
-    static NonnullRefPtr<NodeIterator> create(Node& root, unsigned what_to_show, NodeFilter*);
+    NodeIterator& impl() { return *this; }
 
     NonnullRefPtr<Node> root() { return m_root; }
     NonnullRefPtr<Node> reference_node() { return m_reference.node; }
     bool pointer_before_reference_node() const { return m_reference.is_before_node; }
     unsigned what_to_show() const { return m_what_to_show; }
 
-    NodeFilter* filter() { return m_filter.cell(); }
+    NodeFilter* filter() { return m_filter.ptr(); }
 
     JS::ThrowCompletionOr<RefPtr<Node>> next_node();
     JS::ThrowCompletionOr<RefPtr<Node>> previous_node();
@@ -38,7 +38,7 @@ public:
     void run_pre_removing_steps(Node&);
 
 private:
-    NodeIterator(Node& root);
+    virtual void visit_edges(Cell::Visitor&) override;
 
     enum class Direction {
         Next,
@@ -72,10 +72,15 @@ private:
     unsigned m_what_to_show { 0 };
 
     // https://dom.spec.whatwg.org/#concept-traversal-filter
-    JS::Handle<DOM::NodeFilter> m_filter;
+    JS::GCPtr<DOM::NodeFilter> m_filter;
 
     // https://dom.spec.whatwg.org/#concept-traversal-active
     bool m_active { false };
 };
 
 }
+
+namespace Web::Bindings {
+inline JS::Object* wrap(JS::Realm&, Web::DOM::NodeIterator& object) { return &object; }
+using NodeIteratorWrapper = Web::DOM::NodeIterator;
+}

Userland/Libraries/LibWeb/DOM/NodeIterator.idl

@@ -1,7 +1,7 @@
 #import <DOM/Node.idl>
 #import <DOM/NodeFilter.idl>
 
-[Exposed=Window]
+[Exposed=Window, NoInstanceWrapper]
 interface NodeIterator {
 
     [SameObject] readonly attribute Node root;

Userland/Libraries/LibWeb/Forward.h

@@ -564,8 +564,6 @@ class MessagePortWrapper;
 class MouseEventWrapper;
 class MutationObserverWrapper;
 class MutationRecordWrapper;
-class NodeFilterWrapper;
-class NodeIteratorWrapper;
 class NodeListWrapper;
 class NodeWrapper;
 class OptionConstructor;

Userland/Libraries/LibWeb/idl_files.cmake

@@ -42,7 +42,7 @@ libweb_js_wrapper(DOM/MutationRecord)
 libweb_js_wrapper(DOM/MutationObserver)
 libweb_js_wrapper(DOM/NamedNodeMap NO_INSTANCE)
 libweb_js_wrapper(DOM/Node)
-libweb_js_wrapper(DOM/NodeIterator)
+libweb_js_wrapper(DOM/NodeIterator NO_INSTANCE)
 libweb_js_wrapper(DOM/NodeList)
 libweb_js_wrapper(DOM/ProcessingInstruction)
 libweb_js_wrapper(DOM/Range)