LibJS: Implement WeakRef changes from 'Symbol as WeakMap Keys Proposal'

Author: https://github.com/IdanHo
Commit: https://github.com/SerenityOS/serenity/commit/53ed8decaf
Pull-request: https://github.com/SerenityOS/serenity/pull/14361
Reviewed-by: https://github.com/alimpfard ✅
Reviewed-by: https://github.com/linusg ✅

Userland/Libraries/LibJS/Runtime/WeakRef.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Idan Horowitz <idan.horowitz@serenityos.org>
+ * Copyright (c) 2021-2022, Idan Horowitz <idan.horowitz@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -8,26 +8,38 @@
 
 namespace JS {
 
-WeakRef* WeakRef::create(GlobalObject& global_object, Object* object)
+WeakRef* WeakRef::create(GlobalObject& global_object, Object& value)
 {
-    return global_object.heap().allocate<WeakRef>(global_object, object, *global_object.weak_ref_prototype());
+    return global_object.heap().allocate<WeakRef>(global_object, value, *global_object.weak_ref_prototype());
 }
 
-WeakRef::WeakRef(Object* object, Object& prototype)
+WeakRef* WeakRef::create(GlobalObject& global_object, Symbol& value)
+{
+    return global_object.heap().allocate<WeakRef>(global_object, value, *global_object.weak_ref_prototype());
+}
+
+WeakRef::WeakRef(Object& value, Object& prototype)
     : Object(prototype)
     , WeakContainer(heap())
-    , m_value(object)
+    , m_value(&value)
+    , m_last_execution_generation(vm().execution_generation())
+{
+}
+
+WeakRef::WeakRef(Symbol& value, Object& prototype)
+    : Object(prototype)
+    , WeakContainer(heap())
+    , m_value(&value)
     , m_last_execution_generation(vm().execution_generation())
 {
 }
 
 void WeakRef::remove_dead_cells(Badge<Heap>)
 {
-    VERIFY(m_value);
-    if (m_value->state() == Cell::State::Live)
+    if (m_value.visit([](Cell* cell) -> bool { return cell->state() == Cell::State::Live; }, [](Empty) -> bool { VERIFY_NOT_REACHED(); }))
         return;
 
-    m_value = nullptr;
+    m_value = Empty {};
     // This is an optimization, we deregister from the garbage collector early (even if we were not garbage collected ourself yet)
     // to reduce the garbage collection overhead, which we can do because a cleared weak ref cannot be reused.
     WeakContainer::deregister();
@@ -37,8 +49,10 @@ void WeakRef::visit_edges(Visitor& visitor)
 {
     Base::visit_edges(visitor);
 
-    if (vm().execution_generation() == m_last_execution_generation)
-        visitor.visit(m_value);
+    if (vm().execution_generation() == m_last_execution_generation) {
+        auto* cell = m_value.visit([](Cell* cell) -> Cell* { return cell; }, [](Empty) -> Cell* { return nullptr; });
+        visitor.visit(cell);
+    }
 }
 
 }

Userland/Libraries/LibJS/Runtime/WeakRef.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Idan Horowitz <idan.horowitz@serenityos.org>
+ * Copyright (c) 2021-2022, Idan Horowitz <idan.horowitz@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -18,12 +18,14 @@ class WeakRef final
     JS_OBJECT(WeakRef, Object);
 
 public:
-    static WeakRef* create(GlobalObject&, Object*);
+    static WeakRef* create(GlobalObject&, Object&);
+    static WeakRef* create(GlobalObject&, Symbol&);
 
-    explicit WeakRef(Object*, Object& prototype);
+    explicit WeakRef(Object&, Object& prototype);
+    explicit WeakRef(Symbol&, Object& prototype);
     virtual ~WeakRef() override = default;
 
-    Object* value() const { return m_value; };
+    auto const& value() const { return m_value; };
 
     void update_execution_generation() { m_last_execution_generation = vm().execution_generation(); };
 
@@ -32,7 +34,7 @@ public:
 private:
     virtual void visit_edges(Visitor&) override;
 
-    Object* m_value { nullptr };
+    Variant<Object*, Symbol*, Empty> m_value;
     u32 m_last_execution_generation { 0 };
 };
 

Userland/Libraries/LibJS/Runtime/WeakRefConstructor.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Idan Horowitz <idan.horowitz@serenityos.org>
+ * Copyright (c) 2021-2022, Idan Horowitz <idan.horowitz@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -42,9 +42,13 @@ ThrowCompletionOr<Object*> WeakRefConstructor::construct(FunctionObject& new_tar
     auto& global_object = this->global_object();
 
     auto target = vm.argument(0);
-    if (!target.is_object())
-        return vm.throw_completion<TypeError>(global_object, ErrorType::NotAnObject, target.to_string_without_side_effects());
-    return TRY(ordinary_create_from_constructor<WeakRef>(global_object, new_target, &GlobalObject::weak_ref_prototype, &target.as_object()));
+    if (!can_be_held_weakly(target))
+        return vm.throw_completion<TypeError>(global_object, ErrorType::CannotBeHeldWeakly, target.to_string_without_side_effects());
+
+    if (target.is_object())
+        return TRY(ordinary_create_from_constructor<WeakRef>(global_object, new_target, &GlobalObject::weak_ref_prototype, target.as_object()));
+    VERIFY(target.is_symbol());
+    return TRY(ordinary_create_from_constructor<WeakRef>(global_object, new_target, &GlobalObject::weak_ref_prototype, target.as_symbol()));
 }
 
 }

Userland/Libraries/LibJS/Runtime/WeakRefPrototype.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Idan Horowitz <idan.horowitz@serenityos.org>
+ * Copyright (c) 2021-2022, Idan Horowitz <idan.horowitz@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -30,7 +30,9 @@ JS_DEFINE_NATIVE_FUNCTION(WeakRefPrototype::deref)
     auto* weak_ref = TRY(typed_this_object(global_object));
 
     weak_ref->update_execution_generation();
-    return weak_ref->value() ?: js_undefined();
+    return weak_ref->value().visit(
+        [](Empty) -> Value { return js_undefined(); },
+        [](auto* value) -> Value { return value; });
 }
 
 }

Userland/Libraries/LibJS/Tests/builtins/WeakRef/WeakRef.js

@@ -8,7 +8,7 @@ describe("errors", () => {
         [-100, Infinity, NaN, 152n, undefined].forEach(value => {
             expect(() => {
                 new WeakRef(value);
-            }).toThrowWithMessage(TypeError, "is not an object");
+            }).toThrowWithMessage(TypeError, "cannot be held weakly");
         });
     });
     test("called without new", () => {
@@ -27,4 +27,9 @@ describe("normal behavior", () => {
         var a = new WeakRef({});
         expect(a instanceof WeakRef).toBeTrue();
     });
+
+    test("constructor with single symbol argument", () => {
+        var a = new WeakRef(Symbol());
+        expect(a instanceof WeakRef).toBeTrue();
+    });
 });

Userland/Libraries/LibJS/Tests/builtins/WeakRef/WeakRef.prototype.deref.js

@@ -3,13 +3,18 @@ test("length is 0", () => {
 });
 
 test("basic functionality", () => {
-    var original = { a: 1 };
-    var weakRef = new WeakRef(original);
+    var originalObject = { a: 1 };
+    var objectWeakRef = new WeakRef(originalObject);
 
-    expect(weakRef.deref()).toBe(original);
+    expect(objectWeakRef.deref()).toBe(originalObject);
+
+    var originalSymbol = { a: 1 };
+    var symbolWeakRef = new WeakRef(originalSymbol);
+
+    expect(symbolWeakRef.deref()).toBe(originalSymbol);
 });
 
-test("kept alive for current synchronous execution sequence", () => {
+test("object kept alive for current synchronous execution sequence", () => {
     var weakRef;
     {
         weakRef = new WeakRef({ a: 1 });
@@ -19,3 +24,14 @@ test("kept alive for current synchronous execution sequence", () => {
     // This is fine 🔥
     expect(weakRef.deref()).not.toBe(undefined);
 });
+
+test("symbol kept alive for current synchronous execution sequence", () => {
+    var weakRef;
+    {
+        weakRef = new WeakRef(Symbol("foo"));
+    }
+    weakRef.deref();
+    gc();
+    // This is fine 🔥
+    expect(weakRef.deref()).not.toBe(undefined);
+});