diff --git a/Libraries/LibWeb/DOM/Document.cpp b/Libraries/LibWeb/DOM/Document.cpp index afe8aa30b6..3fa820aa10 100644 --- a/Libraries/LibWeb/DOM/Document.cpp +++ b/Libraries/LibWeb/DOM/Document.cpp @@ -81,10 +81,51 @@ Document::~Document() void Document::removed_last_ref() { ASSERT(!ref_count()); + ASSERT(!m_deletion_has_begun); - if (m_referencing_node_count) + if (m_referencing_node_count) { + // The document has reached ref_count==0 but still has nodes keeping it alive. + // At this point, sever all the node links we control. + // If nodes remain elsewhere (e.g JS wrappers), they will keep the document alive. + + // NOTE: This makes sure we stay alive across for the duration of the cleanup below. + increment_referencing_node_count(); + + m_focused_element = nullptr; + m_hovered_node = nullptr; + m_pending_parsing_blocking_script = nullptr; + m_inspected_node = nullptr; + m_scripts_to_execute_when_parsing_has_finished.clear(); + m_scripts_to_execute_as_soon_as_possible.clear(); + m_associated_inert_template_document = nullptr; + + m_interpreter = nullptr; + + { + // Gather up all the descendants of this document and prune them from the tree. + // FIXME: This could definitely be more elegant. + NonnullRefPtrVector descendants; + for_each_in_subtree([&](auto& node) { + if (&node != this) + descendants.append(node); + return IterationDecision::Continue; + }); + + for (auto& node : descendants) { + ASSERT(&node.document() == this); + ASSERT(!node.is_document()); + if (node.parent()) + node.parent()->remove_child(node); + } + } + + m_in_removed_last_ref = false; + decrement_referencing_node_count(); return; + } + m_in_removed_last_ref = false; + m_deletion_has_begun = true; delete this; } @@ -222,9 +263,8 @@ void Document::tear_down_layout_tree() NonnullRefPtrVector layout_nodes; - for_each_in_subtree([&](auto& node) { - if (node.layout_node()) - layout_nodes.append(*node.layout_node()); + m_layout_root->for_each_in_subtree([&](auto& layout_node) { + layout_nodes.append(layout_node); return IterationDecision::Continue; }); diff --git a/Libraries/LibWeb/DOM/Document.h b/Libraries/LibWeb/DOM/Document.h index 848989cd48..b4fb5d073d 100644 --- a/Libraries/LibWeb/DOM/Document.h +++ b/Libraries/LibWeb/DOM/Document.h @@ -179,16 +179,12 @@ public: void ref_from_node(Badge) { - ++m_referencing_node_count; + increment_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(); - } + decrement_referencing_node_count(); } void removed_last_ref(); @@ -200,6 +196,23 @@ private: void tear_down_layout_tree(); + void increment_referencing_node_count() + { + ASSERT(!m_deletion_has_begun); + ++m_referencing_node_count; + } + + void decrement_referencing_node_count() + { + ASSERT(!m_deletion_has_begun); + ASSERT(m_referencing_node_count); + --m_referencing_node_count; + if (!m_referencing_node_count && !ref_count()) { + m_deletion_has_begun = true; + delete this; + } + } + unsigned m_referencing_node_count { 0 }; OwnPtr m_style_resolver; diff --git a/Libraries/LibWeb/DOM/Node.cpp b/Libraries/LibWeb/DOM/Node.cpp index 0e54aaddbb..c81bebf7b8 100644 --- a/Libraries/LibWeb/DOM/Node.cpp +++ b/Libraries/LibWeb/DOM/Node.cpp @@ -54,14 +54,18 @@ Node::Node(Document& document, NodeType type) , m_document(&document) , m_type(type) { - m_document->ref_from_node({}); + if (!is_document()) + m_document->ref_from_node({}); } Node::~Node() { + ASSERT(m_deletion_has_begun); if (layout_node() && layout_node()->parent()) layout_node()->parent()->remove_child(*layout_node()); - m_document->unref_from_node({}); + + if (!is_document()) + m_document->unref_from_node({}); } const HTML::HTMLAnchorElement* Node::enclosing_link_element() const @@ -222,6 +226,7 @@ void Node::removed_last_ref() downcast(*this).removed_last_ref(); return; } + m_deletion_has_begun = true; delete this; } diff --git a/Libraries/LibWeb/TreeNode.h b/Libraries/LibWeb/TreeNode.h index 5221e260f9..c4c5999811 100644 --- a/Libraries/LibWeb/TreeNode.h +++ b/Libraries/LibWeb/TreeNode.h @@ -39,18 +39,22 @@ class TreeNode : public Weakable { public: void ref() { + ASSERT(!m_in_removed_last_ref); ASSERT(m_ref_count); ++m_ref_count; } void unref() { + ASSERT(!m_in_removed_last_ref); ASSERT(m_ref_count); if (!--m_ref_count) { - if constexpr (IsBaseOf::value) + if constexpr (IsBaseOf::value) { + m_in_removed_last_ref = true; static_cast(this)->removed_last_ref(); - else + } else { delete static_cast(this); + } return; } } @@ -280,6 +284,9 @@ public: protected: TreeNode() { } + bool m_deletion_has_begun { false }; + bool m_in_removed_last_ref { false }; + private: int m_ref_count { 1 }; T* m_parent { nullptr };