LibJS: Remove "uprooting" mechanism from garbage collector

The Heap::uproot_cell() API was used to implement markAsGarbage() which
was used in 3 tests to forcibly destroy a value, even if it had
references on the stack or elsewhere.

This patch rewrites the 3 tests that used this mechanism to be
structured in a way that allows garbage collection to collect the values
as intended without hacks. And now that the uprooting mechanism is no
longer needed, it's uprooted as well.

This fixes 3 test-js tests in bytecode mode. :^)

Tests/LibJS/test-js.cpp

@@ -49,34 +49,6 @@ TESTJS_GLOBAL_FUNCTION(get_weak_map_size, getWeakMapSize)
     return JS::Value(weak_map.values().size());
 }
 
-TESTJS_GLOBAL_FUNCTION(mark_as_garbage, markAsGarbage)
-{
-    auto argument = vm.argument(0);
-    if (!argument.is_string())
-        return vm.throw_completion<JS::TypeError>(JS::ErrorType::NotAString, TRY_OR_THROW_OOM(vm, argument.to_string_without_side_effects()));
-
-    auto& variable_name = argument.as_string();
-
-    // In native functions we don't have a lexical environment so get the outer via the execution stack.
-    auto outer_environment = vm.execution_context_stack().last_matching([&](auto& execution_context) {
-        return execution_context->lexical_environment != nullptr;
-    });
-    if (!outer_environment.has_value())
-        return vm.throw_completion<JS::ReferenceError>(JS::ErrorType::UnknownIdentifier, TRY(variable_name.deprecated_string()));
-
-    auto reference = TRY(vm.resolve_binding(TRY(variable_name.deprecated_string()), outer_environment.value()->lexical_environment));
-
-    auto value = TRY(reference.get_value(vm));
-
-    if (!can_be_held_weakly(value))
-        return vm.throw_completion<JS::TypeError>(JS::ErrorType::CannotBeHeldWeakly, DeprecatedString::formatted("Variable with name {}", TRY(variable_name.deprecated_string())));
-
-    vm.heap().uproot_cell(&value.as_cell());
-    TRY(reference.delete_(vm));
-
-    return JS::js_undefined();
-}
-
 TESTJS_GLOBAL_FUNCTION(detach_array_buffer, detachArrayBuffer)
 {
     auto array_buffer = vm.argument(0);

Userland/Libraries/LibJS/Heap/Heap.cpp

@@ -276,11 +276,6 @@ void Heap::mark_live_cells(HashTable<Cell*> const& roots)
         bytecode_interpreter->visit_edges(visitor);
 
     visitor.mark_all_live_cells();
-
-    for (auto& inverse_root : m_uprooted_cells)
-        inverse_root->set_marked(false);
-
-    m_uprooted_cells.clear();
 }
 
 bool Heap::cell_must_survive_garbage_collection(Cell const& cell)
@@ -427,11 +422,6 @@ void Heap::undefer_gc(Badge<DeferGC>)
     }
 }
 
-void Heap::uproot_cell(Cell* cell)
-{
-    m_uprooted_cells.append(cell);
-}
-
 void register_safe_function_closure(void* base, size_t size)
 {
     if (!s_custom_ranges_for_conservative_scan) {

Userland/Libraries/LibJS/Heap/Heap.h

@@ -76,8 +76,6 @@ public:
 
     BlockAllocator& block_allocator() { return m_block_allocator; }
 
-    void uproot_cell(Cell* cell);
-
 private:
     static bool cell_must_survive_garbage_collection(Cell const&);
 
@@ -112,8 +110,6 @@ private:
     MarkedVectorBase::List m_marked_vectors;
     WeakContainer::List m_weak_containers;
 
-    Vector<GCPtr<Cell>> m_uprooted_cells;
-
     BlockAllocator m_block_allocator;
 
     size_t m_gc_deferrals { 0 };

Userland/Libraries/LibJS/Tests/builtins/FinalizationRegistry/FinalizationRegistry.prototype.cleanupSome.js

@@ -3,9 +3,7 @@ test("length is 0", () => {
 });
 
 function registerInDifferentScope(registry) {
-    const target = {};
-    registry.register(target, {});
-    return target;
+    registry.register({}, {});
 }
 
 test("basic functionality", () => {
@@ -20,8 +18,7 @@ test("basic functionality", () => {
 
     expect(count).toBe(0);
 
-    const target = registerInDifferentScope(registry);
-    markAsGarbage("target");
+    registerInDifferentScope(registry);
     gc();
 
     registry.cleanupSome(increment);

Userland/Libraries/LibJS/Tests/builtins/WeakMap/WeakMap.prototype.set.js

@@ -1,3 +1,9 @@
+function registerInDifferentScope(registry) {
+    const target = {};
+    registry.register(target, {});
+    eval("");
+}
+
 test("basic functionality", () => {
     expect(WeakMap.prototype.set).toHaveLength(2);
 
@@ -21,25 +27,26 @@ test("invalid values", () => {
     });
 });
 
+function setObjectKey(weakMap) {
+    expect(weakMap.set({ e: 3 }, 1)).toBe(weakMap);
+}
+
+function setSymbolKey(weakMap) {
+    expect(weakMap.set(Symbol("foo"), "bar")).toBe(weakMap);
+}
+
 test("automatic removal of garbage-collected values", () => {
     const weakMap = new WeakMap();
-    const objectKey = { e: 3 };
 
-    expect(weakMap.set(objectKey, 1)).toBe(weakMap);
+    setObjectKey(weakMap);
     expect(getWeakMapSize(weakMap)).toBe(1);
 
-    markAsGarbage("objectKey");
     gc();
-
     expect(getWeakMapSize(weakMap)).toBe(0);
 
-    const symbolKey = Symbol("foo");
-
-    expect(weakMap.set(symbolKey, "bar")).toBe(weakMap);
+    setSymbolKey(weakMap);
     expect(getWeakMapSize(weakMap)).toBe(1);
 
-    markAsGarbage("symbolKey");
     gc();
-
     expect(getWeakMapSize(weakMap)).toBe(0);
 });

Userland/Libraries/LibJS/Tests/builtins/WeakSet/WeakSet.prototype.add.js

@@ -16,25 +16,26 @@ test("invalid values", () => {
     });
 });
 
+function addObjectItem(weakSet) {
+    weakSet.add({ a: 1 });
+}
+
+function addSymbolItem(weakSet) {
+    weakSet.add(Symbol("foo"));
+}
+
 test("automatic removal of garbage-collected values", () => {
     const weakSet = new WeakSet();
-    const objectItem = { a: 1 };
 
-    expect(weakSet.add(objectItem)).toBe(weakSet);
+    addObjectItem(weakSet);
     expect(getWeakSetSize(weakSet)).toBe(1);
 
-    markAsGarbage("objectItem");
     gc();
-
     expect(getWeakSetSize(weakSet)).toBe(0);
 
-    const symbolItem = Symbol("foo");
-
-    expect(weakSet.add(symbolItem)).toBe(weakSet);
+    addSymbolItem(weakSet);
     expect(getWeakSetSize(weakSet)).toBe(1);
 
-    markAsGarbage("symbolItem");
     gc();
-
     expect(getWeakSetSize(weakSet)).toBe(0);
 });