LibWeb: Break reference cycles so DOM::Document actually gets deleted

When a document reaches ref_count==0, we will now remove all of the
descendant nodes from the document, and also break all the explicit
links (such as the currently hovered element.)

Basically, DOM nodes will keep the document alive even after the
document reaches ref_count==0. This allows JS wrappers to stay alive
and keep the document alive as well. This matches the behavior of
at least some other browsers.

This patch also adds a bunch of sanity checking assertions around
DOM teardown, to help catch mistakes in the future.

Fixes #3771.

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<Node> 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<LayoutNode> layout_nodes;
 
-    for_each_in_subtree([&](auto& node) {
+    m_layout_root->for_each_in_subtree([&](auto& layout_node) {
-        if (node.layout_node())
+        layout_nodes.append(layout_node);
-            layout_nodes.append(*node.layout_node());
         return IterationDecision::Continue;
     });
 

Libraries/LibWeb/DOM/Document.h

@@ -179,16 +179,12 @@ public:
 
     void ref_from_node(Badge<Node>)
     {
-        ++m_referencing_node_count;
+        increment_referencing_node_count();
     }
 
     void unref_from_node(Badge<Node>)
     {
-        ASSERT(m_referencing_node_count);
+        decrement_referencing_node_count();
-        --m_referencing_node_count;
-        if (!m_referencing_node_count && !ref_count()) {
-            removed_last_ref();
-        }
     }
 
     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<CSS::StyleResolver> m_style_resolver;

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<Document>(*this).removed_last_ref();
         return;
     }
+    m_deletion_has_begun = true;
     delete this;
 }
 

Libraries/LibWeb/TreeNode.h

@@ -39,18 +39,22 @@ class TreeNode : public Weakable<T> {
 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<DOM::Node, T>::value)
+            if constexpr (IsBaseOf<DOM::Node, T>::value) {
+                m_in_removed_last_ref = true;
                 static_cast<T*>(this)->removed_last_ref();
-            else
+            } else {
                 delete static_cast<T*>(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 };