cppgc: Add basic operations for JSMember

The following adds support for JSMember through the existing
GlobalHandles implementation also used for TracedReference.

In addition, JSMember now supports set, clear, copy, move, comparison
and interaction with Local.

Bug: chromium:1056170
Change-Id: Ia50218bcfe4c056b3533a5b14eea954ade1da243
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2310357Reviewed-by: Omer Katz <omerkatz@chromium.org>
Reviewed-by: Anton Bikineev <bikineev@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69028}

include/v8-cppgc.h

@@ -19,41 +19,67 @@ namespace internal {
 
 class JSMemberBaseExtractor;
 
-// TODO(chromium:1056170): Provide implementation based on global handles.
-class JSMemberBase {
+class V8_EXPORT JSMemberBase {
  public:
   /**
    * Returns true if the reference is empty, i.e., has not been assigned
    * object.
    */
-  bool IsEmpty() const { return val_ == kNullAddress; }
+  bool IsEmpty() const { return val_ == nullptr; }
 
   /**
    * Clears the reference. IsEmpty() will return true after this call.
    */
-  V8_INLINE void Reset();
+  inline void Reset();
 
  private:
-  static internal::Address New(v8::Isolate* isolate, internal::Address* object,
-                               internal::Address* slot);
-  static void Delete(internal::Address* slot);
+  static internal::Address* New(v8::Isolate* isolate,
+                                internal::Address* object_slot,
+                                internal::Address** this_slot);
+  static void Delete(internal::Address* object);
+  static void Copy(const internal::Address* const* from_slot,
+                   internal::Address** to_slot);
+  static void Move(internal::Address** from_slot, internal::Address** to_slot);
 
   JSMemberBase() = default;
 
-  JSMemberBase(v8::Isolate* isolate, internal::Address* object)
-      : val_(New(isolate, object, &this->val_)) {}
+  JSMemberBase(v8::Isolate* isolate, internal::Address* object_slot)
+      : val_(New(isolate, object_slot, &val_)) {}
 
-  internal::Address val_ = kNullAddress;
+  inline JSMemberBase& CopyImpl(const JSMemberBase& other);
+  inline JSMemberBase& MoveImpl(JSMemberBase&& other);
+
+  // val_ points to a GlobalHandles node.
+  internal::Address* val_ = nullptr;
 
   template <typename T>
   friend class v8::JSMember;
   friend class v8::internal::JSMemberBaseExtractor;
 };
 
+JSMemberBase& JSMemberBase::CopyImpl(const JSMemberBase& other) {
+  if (this != &other) {
+    Reset();
+    if (!other.IsEmpty()) {
+      Copy(&other.val_, &val_);
+    }
+  }
+  return *this;
+}
+
+JSMemberBase& JSMemberBase::MoveImpl(JSMemberBase&& other) {
+  if (this != &other) {
+    // No call to Reset() as Move() will conditionally reset itself when needed,
+    // and otherwise reuse the internal meta data.
+    Move(&other.val_, &val_);
+  }
+  return *this;
+}
+
 void JSMemberBase::Reset() {
   if (IsEmpty()) return;
-  Delete(reinterpret_cast<internal::Address*>(val_));
-  val_ = kNullAddress;
+  Delete(val_);
+  val_ = nullptr;
 }
 
 }  // namespace internal
@@ -62,11 +88,9 @@ void JSMemberBase::Reset() {
  * A traced handle without destructor that clears the handle. The handle may
  * only be used in GarbageCollected objects and must be processed in a Trace()
  * method.
- *
- * TODO(chromium:1056170): Implementation.
  */
 template <typename T>
-class JSMember : public internal::JSMemberBase {
+class V8_EXPORT JSMember : public internal::JSMemberBase {
   static_assert(std::is_base_of<v8::Value, T>::value,
                 "JSMember only supports references to v8::Value");
 
@@ -76,15 +100,100 @@ class JSMember : public internal::JSMemberBase {
   template <typename U,
             typename = std::enable_if_t<std::is_base_of<T, U>::value>>
   JSMember(Isolate* isolate, Local<U> that)
-      : internal::JSMemberBase(isolate, that.val_) {}
+      : internal::JSMemberBase(isolate,
+                               reinterpret_cast<internal::Address*>(*that)) {}
+
+  JSMember(const JSMember& other) { CopyImpl(other); }
+
+  template <typename U,
+            typename = std::enable_if_t<std::is_base_of<T, U>::value>>
+  JSMember(const JSMember<U>& other) {  // NOLINT
+    CopyImpl(other);
+  }
+
+  JSMember(JSMember&& other) { MoveImpl(std::move(other)); }
+
+  template <typename U,
+            typename = std::enable_if_t<std::is_base_of<T, U>::value>>
+  JSMember(JSMember<U>&& other) {  // NOLINT
+    MoveImpl(std::move(other));
+  }
+
+  JSMember& operator=(const JSMember& other) { return CopyImpl(other); }
+
+  template <typename U,
+            typename = std::enable_if_t<std::is_base_of<T, U>::value>>
+  JSMember& operator=(const JSMember<U>& other) {
+    return CopyImpl(other);
+  }
+
+  JSMember& operator=(JSMember&& other) { return MoveImpl(other); }
 
-  // Heterogeneous construction.
-  // TODO(chromium:1056170): Implementation.
   template <typename U,
             typename = std::enable_if_t<std::is_base_of<T, U>::value>>
-  JSMember(const JSMember<U>& other) {}  // NOLINT
+  JSMember& operator=(JSMember<U>&& other) {
+    return MoveImpl(other);
+  }
+
+  T* operator->() const { return reinterpret_cast<T*>(val_); }
+  T* operator*() const { return reinterpret_cast<T*>(val_); }
+
+  using internal::JSMemberBase::Reset;
+
+  template <typename U,
+            typename = std::enable_if_t<std::is_base_of<T, U>::value>>
+  void Set(v8::Isolate* isolate, Local<U> that) {
+    Reset();
+    val_ = New(isolate, reinterpret_cast<internal::Address*>(*that), &val_);
+  }
 };
 
+template <typename T1, typename T2,
+          typename = std::enable_if_t<std::is_base_of<T2, T1>::value ||
+                                      std::is_base_of<T1, T2>::value>>
+inline bool operator==(const JSMember<T1>& lhs, const JSMember<T2>& rhs) {
+  v8::internal::Address* a = reinterpret_cast<v8::internal::Address*>(*lhs);
+  v8::internal::Address* b = reinterpret_cast<v8::internal::Address*>(*rhs);
+  if (a == nullptr) return b == nullptr;
+  if (b == nullptr) return false;
+  return *a == *b;
+}
+
+template <typename T1, typename T2,
+          typename = std::enable_if_t<std::is_base_of<T2, T1>::value ||
+                                      std::is_base_of<T1, T2>::value>>
+inline bool operator!=(const JSMember<T1>& lhs, const JSMember<T2>& rhs) {
+  return !(lhs == rhs);
+}
+
+template <typename T1, typename T2,
+          typename = std::enable_if_t<std::is_base_of<T2, T1>::value ||
+                                      std::is_base_of<T1, T2>::value>>
+inline bool operator==(const JSMember<T1>& lhs, const Local<T2>& rhs) {
+  v8::internal::Address* a = reinterpret_cast<v8::internal::Address*>(*lhs);
+  v8::internal::Address* b = reinterpret_cast<v8::internal::Address*>(*rhs);
+  if (a == nullptr) return b == nullptr;
+  if (b == nullptr) return false;
+  return *a == *b;
+}
+
+template <typename T1, typename T2,
+          typename = std::enable_if_t<std::is_base_of<T2, T1>::value ||
+                                      std::is_base_of<T1, T2>::value>>
+inline bool operator==(const Local<T1>& lhs, const JSMember<T2> rhs) {
+  return rhs == lhs;
+}
+
+template <typename T1, typename T2>
+inline bool operator!=(const JSMember<T1>& lhs, const T2& rhs) {
+  return !(lhs == rhs);
+}
+
+template <typename T1, typename T2>
+inline bool operator!=(const T1& lhs, const JSMember<T2>& rhs) {
+  return !(lhs == rhs);
+}
+
 class JSVisitor : public cppgc::Visitor {
  public:
   explicit JSVisitor(cppgc::Visitor::Key key) : cppgc::Visitor(key) {}

src/api/api.cc

@@ -990,29 +990,39 @@ i::Address* V8::GlobalizeTracedReference(i::Isolate* isolate, i::Address* obj,
 }
 
 // static
-i::Address i::JSMemberBase::New(v8::Isolate* isolate, i::Address* object,
-                                i::Address* slot) {
+i::Address* i::JSMemberBase::New(v8::Isolate* isolate, i::Address* object_slot,
+                                 i::Address** this_slot) {
   i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
   LOG_API(i_isolate, JSMemberBase, New);
 #ifdef DEBUG
-  Utils::ApiCheck((object != nullptr), "i::JSMemberBase::New",
+  Utils::ApiCheck((object_slot != nullptr), "i::JSMemberBase::New",
                   "the object must be not null");
-  Utils::ApiCheck((slot != nullptr), "i::JSMemberBase::New",
-                  "the address slot must be not null");
 #endif
   i::Handle<i::Object> result = i_isolate->global_handles()->CreateTraced(
-      *object, slot, false /* no destructor */);
+      *object_slot, reinterpret_cast<i::Address*>(this_slot),
+      false /* no destructor */);
 #ifdef VERIFY_HEAP
   if (i::FLAG_verify_heap) {
-    i::Object(*object).ObjectVerify(i_isolate);
+    i::Object(*object_slot).ObjectVerify(i_isolate);
   }
 #endif  // VERIFY_HEAP
-  return reinterpret_cast<i::Address>(result.location());
+  return result.location();
+}
+
+// static
+void i::JSMemberBase::Delete(i::Address* object) {
+  i::GlobalHandles::DestroyTraced(object);
+}
+
+// static
+void i::JSMemberBase::Copy(const i::Address* const* from_slot,
+                           i::Address** to_slot) {
+  i::GlobalHandles::CopyTracedGlobal(from_slot, to_slot);
 }
 
 // static
-void i::JSMemberBase::Delete(i::Address* slot) {
-  i::GlobalHandles::DestroyTraced(slot);
+void i::JSMemberBase::Move(i::Address** from_slot, i::Address** to_slot) {
+  i::GlobalHandles::MoveTracedGlobal(from_slot, to_slot);
 }
 
 i::Address* V8::CopyGlobalReference(i::Address* from) {

test/unittests/BUILD.gn

@@ -242,6 +242,7 @@ v8_source_set("unittests_sources") {
     "heap/heap-unittest.cc",
     "heap/heap-utils.h",
     "heap/item-parallel-job-unittest.cc",
+    "heap/js-member-unittest.cc",
     "heap/list-unittest.cc",
     "heap/local-heap-unittest.cc",
     "heap/marking-unittest.cc",

test/unittests/heap/js-member-unittest.cc

+// Copyright 2016 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "include/v8-cppgc.h"
+#include "test/unittests/test-utils.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace v8 {
+namespace internal {
+
+using JSMemberTest = TestWithIsolate;
+
+TEST_F(JSMemberTest, ResetFromLocal) {
+  v8::Local<v8::Context> context = v8::Context::New(v8_isolate());
+  v8::Context::Scope context_scope(context);
+  v8::JSMember<v8::Object> member;
+  {
+    v8::HandleScope handles(v8_isolate());
+    v8::Local<v8::Object> local =
+        v8::Local<v8::Object>::New(v8_isolate(), v8::Object::New(v8_isolate()));
+    EXPECT_TRUE(member.IsEmpty());
+    EXPECT_NE(member, local);
+    member.Set(v8_isolate(), local);
+    EXPECT_FALSE(member.IsEmpty());
+    EXPECT_EQ(member, local);
+  }
+}
+
+TEST_F(JSMemberTest, ConstructFromLocal) {
+  v8::Local<v8::Context> context = v8::Context::New(v8_isolate());
+  v8::Context::Scope context_scope(context);
+  {
+    v8::HandleScope handles(v8_isolate());
+    v8::Local<v8::Object> local =
+        v8::Local<v8::Object>::New(v8_isolate(), v8::Object::New(v8_isolate()));
+    v8::JSMember<v8::Object> member(v8_isolate(), local);
+    EXPECT_FALSE(member.IsEmpty());
+    EXPECT_EQ(member, local);
+  }
+}
+
+TEST_F(JSMemberTest, Reset) {
+  v8::Local<v8::Context> context = v8::Context::New(v8_isolate());
+  v8::Context::Scope context_scope(context);
+  {
+    v8::HandleScope handles(v8_isolate());
+    v8::Local<v8::Object> local =
+        v8::Local<v8::Object>::New(v8_isolate(), v8::Object::New(v8_isolate()));
+    v8::JSMember<v8::Object> member(v8_isolate(), local);
+    EXPECT_FALSE(member.IsEmpty());
+    EXPECT_EQ(member, local);
+    member.Reset();
+    EXPECT_TRUE(member.IsEmpty());
+    EXPECT_NE(member, local);
+  }
+}
+
+TEST_F(JSMemberTest, Copy) {
+  v8::Local<v8::Context> context = v8::Context::New(v8_isolate());
+  v8::Context::Scope context_scope(context);
+  {
+    v8::HandleScope handles(v8_isolate());
+    v8::Local<v8::Object> local =
+        v8::Local<v8::Object>::New(v8_isolate(), v8::Object::New(v8_isolate()));
+    v8::JSMember<v8::Object> member(v8_isolate(), local);
+    v8::JSMember<v8::Object> member_copy1(member);
+    v8::JSMember<v8::Object> member_copy2 = member;
+    EXPECT_EQ(member, local);
+    EXPECT_EQ(member_copy1, local);
+    EXPECT_EQ(member_copy2, local);
+  }
+}
+
+TEST_F(JSMemberTest, CopyHeterogenous) {
+  v8::Local<v8::Context> context = v8::Context::New(v8_isolate());
+  v8::Context::Scope context_scope(context);
+  {
+    v8::HandleScope handles(v8_isolate());
+    v8::Local<v8::Object> local =
+        v8::Local<v8::Object>::New(v8_isolate(), v8::Object::New(v8_isolate()));
+    v8::JSMember<v8::Object> member(v8_isolate(), local);
+    v8::JSMember<v8::Value> member_copy1(member);
+    v8::JSMember<v8::Value> member_copy2 = member;
+    EXPECT_EQ(member, local);
+    EXPECT_EQ(member_copy1, local);
+    EXPECT_EQ(member_copy2, local);
+  }
+}
+
+TEST_F(JSMemberTest, Move) {
+  v8::Local<v8::Context> context = v8::Context::New(v8_isolate());
+  v8::Context::Scope context_scope(context);
+  {
+    v8::HandleScope handles(v8_isolate());
+    v8::Local<v8::Object> local =
+        v8::Local<v8::Object>::New(v8_isolate(), v8::Object::New(v8_isolate()));
+    v8::JSMember<v8::Object> member(v8_isolate(), local);
+    v8::JSMember<v8::Object> member_moved1(std::move(member));
+    v8::JSMember<v8::Object> member_moved2 = std::move(member_moved1);
+    EXPECT_TRUE(member.IsEmpty());
+    EXPECT_TRUE(member_moved1.IsEmpty());
+    EXPECT_EQ(member_moved2, local);
+  }
+}
+
+TEST_F(JSMemberTest, MoveHeterogenous) {
+  v8::Local<v8::Context> context = v8::Context::New(v8_isolate());
+  v8::Context::Scope context_scope(context);
+  {
+    v8::HandleScope handles(v8_isolate());
+    v8::Local<v8::Object> local =
+        v8::Local<v8::Object>::New(v8_isolate(), v8::Object::New(v8_isolate()));
+    v8::JSMember<v8::Object> member1(v8_isolate(), local);
+    v8::JSMember<v8::Value> member_moved1(std::move(member1));
+    v8::JSMember<v8::Object> member2(v8_isolate(), local);
+    v8::JSMember<v8::Object> member_moved2 = std::move(member2);
+    EXPECT_TRUE(member1.IsEmpty());
+    EXPECT_EQ(member_moved1, local);
+    EXPECT_TRUE(member2.IsEmpty());
+    EXPECT_EQ(member_moved2, local);
+  }
+}
+
+TEST_F(JSMemberTest, Equality) {
+  v8::Local<v8::Context> context = v8::Context::New(v8_isolate());
+  v8::Context::Scope context_scope(context);
+  {
+    v8::HandleScope handles(v8_isolate());
+    v8::Local<v8::Object> local1 =
+        v8::Local<v8::Object>::New(v8_isolate(), v8::Object::New(v8_isolate()));
+    v8::JSMember<v8::Object> member1(v8_isolate(), local1);
+    v8::JSMember<v8::Object> member2(v8_isolate(), local1);
+    EXPECT_EQ(member1, member2);
+    EXPECT_EQ(member2, member1);
+    v8::Local<v8::Object> local2 =
+        v8::Local<v8::Object>::New(v8_isolate(), v8::Object::New(v8_isolate()));
+    v8::JSMember<v8::Object> member3(v8_isolate(), local2);
+    EXPECT_NE(member2, member3);
+    EXPECT_NE(member3, member2);
+  }
+}
+
+TEST_F(JSMemberTest, EqualityHeterogenous) {
+  v8::Local<v8::Context> context = v8::Context::New(v8_isolate());
+  v8::Context::Scope context_scope(context);
+  {
+    v8::HandleScope handles(v8_isolate());
+    v8::Local<v8::Object> local1 =
+        v8::Local<v8::Object>::New(v8_isolate(), v8::Object::New(v8_isolate()));
+    v8::JSMember<v8::Object> member1(v8_isolate(), local1);
+    v8::JSMember<v8::Value> member2(v8_isolate(), local1);
+    EXPECT_EQ(member1, member2);
+    EXPECT_EQ(member2, member1);
+    v8::Local<v8::Object> local2 =
+        v8::Local<v8::Object>::New(v8_isolate(), v8::Object::New(v8_isolate()));
+    v8::JSMember<v8::Object> member3(v8_isolate(), local2);
+    EXPECT_NE(member2, member3);
+    EXPECT_NE(member3, member2);
+  }
+}
+
+}  // namespace internal
+}  // namespace v8