From 43ec0f734f262517ab729b6dabdae43ac34774fa Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 1 Sep 2022 17:13:18 +0200 Subject: [PATCH] LibWeb: Make MutationRecord GC-allocated --- .../LibWeb/WrapperGenerator/IDLGenerators.cpp | 2 + .../LibWeb/Bindings/MainThreadVM.cpp | 4 +- .../Libraries/LibWeb/DOM/MutationObserver.cpp | 2 +- .../Libraries/LibWeb/DOM/MutationObserver.h | 8 +- .../Libraries/LibWeb/DOM/MutationRecord.cpp | 73 ++++++++----------- .../Libraries/LibWeb/DOM/MutationRecord.h | 49 ++++++++----- Userland/Libraries/LibWeb/DOM/Node.cpp | 2 +- Userland/Libraries/LibWeb/Forward.h | 1 - Userland/Libraries/LibWeb/idl_files.cmake | 2 +- 9 files changed, 72 insertions(+), 71 deletions(-) diff --git a/Meta/Lagom/Tools/CodeGenerators/LibWeb/WrapperGenerator/IDLGenerators.cpp b/Meta/Lagom/Tools/CodeGenerators/LibWeb/WrapperGenerator/IDLGenerators.cpp index fe6edc907c..e9ec6b244c 100644 --- a/Meta/Lagom/Tools/CodeGenerators/LibWeb/WrapperGenerator/IDLGenerators.cpp +++ b/Meta/Lagom/Tools/CodeGenerators/LibWeb/WrapperGenerator/IDLGenerators.cpp @@ -149,6 +149,8 @@ static bool impl_is_wrapper(Type const& type) return true; if (type.name == "DOMStringMap"sv) return true; + if (type.name == "MutationRecord"sv) + return true; return false; } diff --git a/Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp b/Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp index a9e230b89a..9964f7c4ce 100644 --- a/Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp +++ b/Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp @@ -15,7 +15,6 @@ #include #include #include -#include #include #include #include @@ -378,9 +377,8 @@ void queue_mutation_observer_microtask(DOM::Document& document) auto* wrapped_records = MUST(JS::Array::create(realm, 0)); for (size_t i = 0; i < records.size(); ++i) { auto& record = records.at(i); - auto* wrapped_record = Bindings::wrap(realm, record); auto property_index = JS::PropertyKey { i }; - MUST(wrapped_records->create_data_property(property_index, wrapped_record)); + MUST(wrapped_records->create_data_property(property_index, record.ptr())); } auto* wrapped_mutation_observer = Bindings::wrap(realm, mutation_observer); diff --git a/Userland/Libraries/LibWeb/DOM/MutationObserver.cpp b/Userland/Libraries/LibWeb/DOM/MutationObserver.cpp index 7a7e9e8114..d7f5851dbc 100644 --- a/Userland/Libraries/LibWeb/DOM/MutationObserver.cpp +++ b/Userland/Libraries/LibWeb/DOM/MutationObserver.cpp @@ -108,7 +108,7 @@ void MutationObserver::disconnect() } // https://dom.spec.whatwg.org/#dom-mutationobserver-takerecords -NonnullRefPtrVector MutationObserver::take_records() +Vector> MutationObserver::take_records() { // 1. Let records be a clone of this’s record queue. auto records = m_record_queue; diff --git a/Userland/Libraries/LibWeb/DOM/MutationObserver.h b/Userland/Libraries/LibWeb/DOM/MutationObserver.h index b222d9eaa4..57af34ce46 100644 --- a/Userland/Libraries/LibWeb/DOM/MutationObserver.h +++ b/Userland/Libraries/LibWeb/DOM/MutationObserver.h @@ -43,16 +43,16 @@ public: ExceptionOr observe(Node& target, MutationObserverInit options = {}); void disconnect(); - NonnullRefPtrVector take_records(); + Vector> take_records(); Vector>& node_list() { return m_node_list; } Vector> const& node_list() const { return m_node_list; } Bindings::CallbackType& callback() { return *m_callback; } - void enqueue_record(Badge, NonnullRefPtr mutation_record) + void enqueue_record(Badge, JS::NonnullGCPtr mutation_record) { - m_record_queue.append(move(mutation_record)); + m_record_queue.append(*mutation_record); } private: @@ -65,7 +65,7 @@ private: Vector> m_node_list; // https://dom.spec.whatwg.org/#concept-mo-queue - NonnullRefPtrVector m_record_queue; + Vector> m_record_queue; }; // https://dom.spec.whatwg.org/#registered-observer diff --git a/Userland/Libraries/LibWeb/DOM/MutationRecord.cpp b/Userland/Libraries/LibWeb/DOM/MutationRecord.cpp index 05ad9fd49d..bb66e5e5ea 100644 --- a/Userland/Libraries/LibWeb/DOM/MutationRecord.cpp +++ b/Userland/Libraries/LibWeb/DOM/MutationRecord.cpp @@ -1,57 +1,48 @@ /* * Copyright (c) 2022, Luke Wilde + * Copyright (c) 2022, Andreas Kling * * SPDX-License-Identifier: BSD-2-Clause */ +#include #include #include #include +#include namespace Web::DOM { -class MutationRecordImpl final : public MutationRecord { -public: - MutationRecordImpl(FlyString const& type, Node& target, NodeList& added_nodes, NodeList& removed_nodes, Node* previous_sibling, Node* next_sibling, String const& attribute_name, String const& attribute_namespace, String const& old_value) - : m_type(type) - , m_target(JS::make_handle(target)) - , m_added_nodes(added_nodes) - , m_removed_nodes(removed_nodes) - , m_previous_sibling(JS::make_handle(previous_sibling)) - , m_next_sibling(JS::make_handle(next_sibling)) - , m_attribute_name(attribute_name) - , m_attribute_namespace(attribute_namespace) - , m_old_value(old_value) - { - } - - virtual ~MutationRecordImpl() override = default; - - virtual FlyString const& type() const override { return m_type; } - virtual Node const* target() const override { return m_target.ptr(); } - virtual NodeList const* added_nodes() const override { return m_added_nodes; } - virtual NodeList const* removed_nodes() const override { return m_removed_nodes; } - virtual Node const* previous_sibling() const override { return m_previous_sibling.ptr(); } - virtual Node const* next_sibling() const override { return m_next_sibling.ptr(); } - virtual String const& attribute_name() const override { return m_attribute_name; } - virtual String const& attribute_namespace() const override { return m_attribute_namespace; } - virtual String const& old_value() const override { return m_old_value; } - -private: - FlyString m_type; - JS::Handle m_target; - JS::Handle m_added_nodes; - JS::Handle m_removed_nodes; - JS::Handle m_previous_sibling; - JS::Handle m_next_sibling; - String m_attribute_name; - String m_attribute_namespace; - String m_old_value; -}; - -NonnullRefPtr MutationRecord::create(FlyString const& type, Node& target, NodeList& added_nodes, NodeList& removed_nodes, Node* previous_sibling, Node* next_sibling, String const& attribute_name, String const& attribute_namespace, String const& old_value) +JS::NonnullGCPtr MutationRecord::create(HTML::Window& window, FlyString const& type, Node& target, NodeList& added_nodes, NodeList& removed_nodes, Node* previous_sibling, Node* next_sibling, String const& attribute_name, String const& attribute_namespace, String const& old_value) { - return adopt_ref(*new MutationRecordImpl(type, target, added_nodes, removed_nodes, previous_sibling, next_sibling, attribute_name, attribute_namespace, old_value)); + return *window.heap().allocate(window.realm(), window, type, target, added_nodes, removed_nodes, previous_sibling, next_sibling, attribute_name, attribute_namespace, old_value); +} + +MutationRecord::MutationRecord(HTML::Window& window, FlyString const& type, Node& target, NodeList& added_nodes, NodeList& removed_nodes, Node* previous_sibling, Node* next_sibling, String const& attribute_name, String const& attribute_namespace, String const& old_value) + : PlatformObject(window.realm()) + , m_type(type) + , m_target(JS::make_handle(target)) + , m_added_nodes(added_nodes) + , m_removed_nodes(removed_nodes) + , m_previous_sibling(JS::make_handle(previous_sibling)) + , m_next_sibling(JS::make_handle(next_sibling)) + , m_attribute_name(attribute_name) + , m_attribute_namespace(attribute_namespace) + , m_old_value(old_value) +{ + set_prototype(&window.ensure_web_prototype("MutationRecord")); +} + +MutationRecord::~MutationRecord() = default; + +void MutationRecord::visit_edges(Cell::Visitor& visitor) +{ + Base::visit_edges(visitor); + visitor.visit(m_target.ptr()); + visitor.visit(m_added_nodes.ptr()); + visitor.visit(m_removed_nodes.ptr()); + visitor.visit(m_previous_sibling.ptr()); + visitor.visit(m_next_sibling.ptr()); } } diff --git a/Userland/Libraries/LibWeb/DOM/MutationRecord.h b/Userland/Libraries/LibWeb/DOM/MutationRecord.h index 0c1b279400..e8905eb76f 100644 --- a/Userland/Libraries/LibWeb/DOM/MutationRecord.h +++ b/Userland/Libraries/LibWeb/DOM/MutationRecord.h @@ -6,33 +6,44 @@ #pragma once -#include -#include +#include namespace Web::DOM { // https://dom.spec.whatwg.org/#mutationrecord -// NOTE: This is implemented as a pure virtual interface with the actual implementation in the CPP file to prevent this circular dependency: Node.h -> MutationRecord.h -> MutationObserver.h -> Node.h -// This is also why this uses raw pointers and references, since using (NN)RP requires us to include the templated type, even in a header specifying a function return type. -class MutationRecord - : public RefCounted - , public Bindings::Wrappable { +class MutationRecord : public Bindings::PlatformObject { + WEB_PLATFORM_OBJECT(MutationRecord, Bindings::PlatformObject); + public: - using WrapperType = Bindings::MutationRecordWrapper; + static JS::NonnullGCPtr create(HTML::Window&, FlyString const& type, Node& target, NodeList& added_nodes, NodeList& removed_nodes, Node* previous_sibling, Node* next_sibling, String const& attribute_name, String const& attribute_namespace, String const& old_value); - static NonnullRefPtr create(FlyString const& type, Node& target, NodeList& added_nodes, NodeList& removed_nodes, Node* previous_sibling, Node* next_sibling, String const& attribute_name, String const& attribute_namespace, String const& old_value); + virtual ~MutationRecord() override; - virtual ~MutationRecord() override = default; + FlyString const& type() const { return m_type; } + Node const* target() const { return m_target; } + NodeList const* added_nodes() const { return m_added_nodes; } + NodeList const* removed_nodes() const { return m_removed_nodes; } + Node const* previous_sibling() const { return m_previous_sibling; } + Node const* next_sibling() const { return m_next_sibling; } + String const& attribute_name() const { return m_attribute_name; } + String const& attribute_namespace() const { return m_attribute_namespace; } + String const& old_value() const { return m_old_value; } - virtual FlyString const& type() const = 0; - virtual Node const* target() const = 0; - virtual NodeList const* added_nodes() const = 0; - virtual NodeList const* removed_nodes() const = 0; - virtual Node const* previous_sibling() const = 0; - virtual Node const* next_sibling() const = 0; - virtual String const& attribute_name() const = 0; - virtual String const& attribute_namespace() const = 0; - virtual String const& old_value() const = 0; +private: + MutationRecord(HTML::Window& window, FlyString const& type, Node& target, NodeList& added_nodes, NodeList& removed_nodes, Node* previous_sibling, Node* next_sibling, String const& attribute_name, String const& attribute_namespace, String const& old_value); + virtual void visit_edges(Cell::Visitor&) override; + + FlyString m_type; + JS::GCPtr m_target; + JS::GCPtr m_added_nodes; + JS::GCPtr m_removed_nodes; + JS::GCPtr m_previous_sibling; + JS::GCPtr m_next_sibling; + String m_attribute_name; + String m_attribute_namespace; + String m_old_value; }; } + +WRAPPER_HACK(MutationRecord, Web::DOM) diff --git a/Userland/Libraries/LibWeb/DOM/Node.cpp b/Userland/Libraries/LibWeb/DOM/Node.cpp index 24c4ba2b25..d6d05efab7 100644 --- a/Userland/Libraries/LibWeb/DOM/Node.cpp +++ b/Userland/Libraries/LibWeb/DOM/Node.cpp @@ -1368,7 +1368,7 @@ void Node::queue_mutation_record(FlyString const& type, String attribute_name, S for (auto& interested_observer : interested_observers) { // 1. Let record be a new MutationRecord object with its type set to type, target set to target, attributeName set to name, attributeNamespace set to namespace, oldValue set to mappedOldValue, // addedNodes set to addedNodes, removedNodes set to removedNodes, previousSibling set to previousSibling, and nextSibling set to nextSibling. - auto record = MutationRecord::create(type, *this, added_nodes, removed_nodes, previous_sibling, next_sibling, attribute_name, attribute_namespace, /* mappedOldValue */ interested_observer.value); + auto record = MutationRecord::create(window(), type, *this, added_nodes, removed_nodes, previous_sibling, next_sibling, attribute_name, attribute_namespace, /* mappedOldValue */ interested_observer.value); // 2. Enqueue record to observer’s record queue. interested_observer.key->enqueue_record({}, move(record)); diff --git a/Userland/Libraries/LibWeb/Forward.h b/Userland/Libraries/LibWeb/Forward.h index fa0d028ab4..7778d40efd 100644 --- a/Userland/Libraries/LibWeb/Forward.h +++ b/Userland/Libraries/LibWeb/Forward.h @@ -470,7 +470,6 @@ class IntersectionObserverWrapper; class LocationObject; class MessageChannelWrapper; class MutationObserverWrapper; -class MutationRecordWrapper; class OptionConstructor; class Path2DWrapper; class RangePrototype; diff --git a/Userland/Libraries/LibWeb/idl_files.cmake b/Userland/Libraries/LibWeb/idl_files.cmake index 6c0e0fc1a6..012dea5d09 100644 --- a/Userland/Libraries/LibWeb/idl_files.cmake +++ b/Userland/Libraries/LibWeb/idl_files.cmake @@ -38,7 +38,7 @@ libweb_js_wrapper(DOM/Element NO_INSTANCE) libweb_js_wrapper(DOM/Event NO_INSTANCE) libweb_js_wrapper(DOM/EventTarget NO_INSTANCE) libweb_js_wrapper(DOM/HTMLCollection) -libweb_js_wrapper(DOM/MutationRecord) +libweb_js_wrapper(DOM/MutationRecord NO_INSTANCE) libweb_js_wrapper(DOM/MutationObserver) libweb_js_wrapper(DOM/NamedNodeMap NO_INSTANCE) libweb_js_wrapper(DOM/Node NO_INSTANCE)