LibJS: Make the GC marking phase cycle-proof

Don't visit cells that are already marked. This prevents the marking
phase from looping forever when two cells refer to each other.

Also do the marking directly from the CellVisitor, removing another
unnecessary phase of the collector. :^)

Libraries/LibJS/Cell.h

@@ -44,13 +44,10 @@ public:
 
     class Visitor {
     public:
-        virtual void did_visit(Cell*) = 0;
+        virtual void visit(Cell*) = 0;
     };
 
-    virtual void visit_graph(Visitor& visitor)
-    {
-        visitor.did_visit(this);
-    }
+    virtual void visit_children(Visitor&) {}
 
 private:
     bool m_mark { false };

Libraries/LibJS/Heap.cpp

@@ -63,11 +63,7 @@ void Heap::collect_garbage()
 {
     HashTable<Cell*> roots;
     collect_roots(roots);
-
-    HashTable<Cell*> live_cells;
-    visit_live_cells(roots, live_cells);
-
-    mark_live_cells(live_cells);
+    mark_live_cells(roots);
     sweep_dead_cells();
 }
 
@@ -83,48 +79,30 @@ void Heap::collect_roots(HashTable<Cell*>& roots)
 #endif
 }
 
-class LivenessVisitor final : public Cell::Visitor {
+class MarkingVisitor final : public Cell::Visitor {
 public:
-    LivenessVisitor(HashTable<Cell*>& live_cells)
-        : m_live_cells(live_cells)
+    MarkingVisitor() {}
+
+    virtual void visit(Cell* cell)
     {
-    }
-
-    virtual void did_visit(Cell* cell) override
-    {
-        m_live_cells.set(cell);
-    }
-
-private:
-    HashTable<Cell*>& m_live_cells;
-};
-
-void Heap::visit_live_cells(const HashTable<Cell*>& roots, HashTable<Cell*>& live_cells)
-{
-    LivenessVisitor visitor(live_cells);
-    for (auto* root : roots) {
-        root->visit_graph(visitor);
-    }
-
-#ifdef HEAP_DEBUG
-    dbg() << "visit_live_cells:";
-    for (auto* cell : live_cells) {
-        dbg() << "  @ " << cell;
-    }
-#endif
-}
-
-void Heap::mark_live_cells(const HashTable<Cell*>& live_cells)
-{
-#ifdef HEAP_DEBUG
-    dbg() << "mark_live_cells:";
-#endif
-    for (auto& cell : live_cells) {
+        if (cell->is_marked())
+            return;
 #ifdef HEAP_DEBUG
         dbg() << "  ! " << cell;
 #endif
         cell->set_marked(true);
+        cell->visit_children(*this);
     }
+};
+
+void Heap::mark_live_cells(const HashTable<Cell*>& roots)
+{
+#ifdef HEAP_DEBUG
+    dbg() << "mark_live_cells:";
+#endif
+    MarkingVisitor visitor;
+    for (auto* root : roots)
+        visitor.visit(root);
 }
 
 void Heap::sweep_dead_cells()
@@ -147,5 +125,4 @@ void Heap::sweep_dead_cells()
         });
     }
 }
-
 }

Libraries/LibJS/Heap.h

@@ -57,7 +57,6 @@ private:
     Cell* allocate_cell(size_t);
 
     void collect_roots(HashTable<Cell*>&);
-    void visit_live_cells(const HashTable<Cell*>& roots, HashTable<Cell*>& live_cells);
     void mark_live_cells(const HashTable<Cell*>& live_cells);
     void sweep_dead_cells();
 

Libraries/LibJS/Object.cpp

@@ -48,12 +48,12 @@ void Object::put(String property_name, Value value)
     m_properties.set(property_name, move(value));
 }
 
-void Object::visit_graph(Cell::Visitor& visitor)
+void Object::visit_children(Cell::Visitor& visitor)
 {
-    Cell::visit_graph(visitor);
+    Cell::visit_children(visitor);
     for (auto& it : m_properties) {
         if (it.value.is_object())
-            it.value.as_object()->visit_graph(visitor);
+            visitor.visit(it.value.as_object());
     }
 }
 

Libraries/LibJS/Object.h

@@ -44,7 +44,7 @@ public:
     virtual bool is_function() const { return false; }
 
     virtual const char* class_name() const override { return "Object"; }
-    virtual void visit_graph(Cell::Visitor&) override;
+    virtual void visit_children(Cell::Visitor&) override;
 
 private:
     HashMap<String, Value> m_properties;