From f68ed6d25ba0d8327fabcb24cb6b70ef051c7da0 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sun, 11 Oct 2020 21:52:59 +0200 Subject: [PATCH] LibWeb: Make DOM Nodes keep their Document alive In addition to being reference-counted, all nodes that are part of a document must also keep the document alive. This is achieved by adding a second ref-count to the Document object and incrementing/decrementing it whenever a node is created/destroyed in that document. This brings us much closer to a proper DOM lifetime model, although the JS bindings still need more work. --- Libraries/LibWeb/DOM/Document.cpp | 10 ++++++++++ Libraries/LibWeb/DOM/Document.h | 18 ++++++++++++++++++ Libraries/LibWeb/DOM/Node.cpp | 9 +++++++++ Libraries/LibWeb/DOM/Node.h | 2 ++ Libraries/LibWeb/TreeNode.h | 17 ++++++----------- 5 files changed, 45 insertions(+), 11 deletions(-) diff --git a/Libraries/LibWeb/DOM/Document.cpp b/Libraries/LibWeb/DOM/Document.cpp index 0a999ea6e9..90526b6932 100644 --- a/Libraries/LibWeb/DOM/Document.cpp +++ b/Libraries/LibWeb/DOM/Document.cpp @@ -77,6 +77,16 @@ Document::~Document() { } +void Document::removed_last_ref() +{ + ASSERT(!ref_count()); + + if (m_referencing_node_count) + return; + + delete this; +} + Origin Document::origin() const { if (!m_url.is_valid()) diff --git a/Libraries/LibWeb/DOM/Document.h b/Libraries/LibWeb/DOM/Document.h index 1a7455d515..277e678b3c 100644 --- a/Libraries/LibWeb/DOM/Document.h +++ b/Libraries/LibWeb/DOM/Document.h @@ -177,9 +177,27 @@ public: const String& ready_state() const { return m_ready_state; } void set_ready_state(const String&); + void ref_from_node(Badge) + { + ++m_referencing_node_count; + } + + void unref_from_node(Badge) + { + ASSERT(m_referencing_node_count); + --m_referencing_node_count; + if (!m_referencing_node_count && !ref_count()) { + removed_last_ref(); + } + } + + void removed_last_ref(); + private: virtual RefPtr create_layout_node(const CSS::StyleProperties* parent_style) override; + unsigned m_referencing_node_count { 0 }; + OwnPtr m_style_resolver; RefPtr m_style_sheets; RefPtr m_hovered_node; diff --git a/Libraries/LibWeb/DOM/Node.cpp b/Libraries/LibWeb/DOM/Node.cpp index 2a1852fa5b..71a89fae03 100644 --- a/Libraries/LibWeb/DOM/Node.cpp +++ b/Libraries/LibWeb/DOM/Node.cpp @@ -206,4 +206,13 @@ Bindings::EventTargetWrapper* Node::create_wrapper(JS::GlobalObject& global_obje return wrap(global_object, *this); } +void Node::removed_last_ref() +{ + if (is(*this)) { + downcast(*this).removed_last_ref(); + return; + } + delete this; +} + } diff --git a/Libraries/LibWeb/DOM/Node.h b/Libraries/LibWeb/DOM/Node.h index a009cde6e8..df6e301647 100644 --- a/Libraries/LibWeb/DOM/Node.h +++ b/Libraries/LibWeb/DOM/Node.h @@ -65,6 +65,8 @@ public: virtual ~Node(); + void removed_last_ref(); + NodeType type() const { return m_type; } bool is_element() const { return type() == NodeType::ELEMENT_NODE; } bool is_text() const { return type() == NodeType::TEXT_NODE; } diff --git a/Libraries/LibWeb/TreeNode.h b/Libraries/LibWeb/TreeNode.h index 4512863f18..5f1fef9d1f 100644 --- a/Libraries/LibWeb/TreeNode.h +++ b/Libraries/LibWeb/TreeNode.h @@ -30,6 +30,7 @@ #include #include #include +#include namespace Web { @@ -46,17 +47,11 @@ public: { ASSERT(m_ref_count); if (!--m_ref_count) { - if (m_next_sibling) - m_next_sibling->m_previous_sibling = m_previous_sibling; - if (m_previous_sibling) - m_previous_sibling->m_next_sibling = m_next_sibling; - T* next_child; - for (auto* child = m_first_child; child; child = next_child) { - next_child = child->m_next_sibling; - child->m_parent = nullptr; - child->unref(); - } - delete static_cast(this); + if constexpr (IsBaseOf::value) + static_cast(this)->removed_last_ref(); + else + delete static_cast(this); + return; } } int ref_count() const { return m_ref_count; }