Revert "[api, global-handles] Fix moving weak Global<T>"

This reverts commit 584f0b43.

Reason for revert: Breaks MSAN build - https://ci.chromium.org/p/v8/builders/luci.v8.ci/V8%20Linux%20-%20arm64%20-%20sim%20-%20MSAN/24872

Original change's description:
> [api, global-handles] Fix moving weak Global<T>
> 
> v8::Global may be used as a weak reference. In the case this reference is a
> simple phantom reference, we need to update the internal state to be able to
> clear the right slot once the object refered to is dead.
> 
> Bug: chromium:924220
> Change-Id: I2ab7c3afcbe22988791faef406c284db03a43caf
> Reviewed-on: https://chromium-review.googlesource.com/c/1430101
> Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#59040}

TBR=ulan@chromium.org,mlippautz@chromium.org

Change-Id: I19c3e929962203df4e1f24191d054180723b1c9d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:924220
Reviewed-on: https://chromium-review.googlesource.com/c/1430833Reviewed-by: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59046}

include/v8.h

@@ -758,7 +758,6 @@ class Global : public PersistentBase<T> {
    * A Global with no storage cell.
    */
   V8_INLINE Global() : PersistentBase<T>(nullptr) {}
-
   /**
    * Construct a Global from a Local.
    * When the Local is non-empty, a new storage cell is created
@@ -769,7 +768,6 @@ class Global : public PersistentBase<T> {
       : PersistentBase<T>(PersistentBase<T>::New(isolate, *that)) {
     TYPE_CHECK(T, S);
   }
-
   /**
    * Construct a Global from a PersistentBase.
    * When the Persistent is non-empty, a new storage cell is created
@@ -780,20 +778,26 @@ class Global : public PersistentBase<T> {
       : PersistentBase<T>(PersistentBase<T>::New(isolate, that.val_)) {
     TYPE_CHECK(T, S);
   }
-
   /**
    * Move constructor.
    */
-  V8_INLINE Global(Global&& other);
-
+  V8_INLINE Global(Global&& other) : PersistentBase<T>(other.val_) {
+    other.val_ = nullptr;
+  }
   V8_INLINE ~Global() { this->Reset(); }
-
   /**
    * Move via assignment.
    */
   template <class S>
-  V8_INLINE Global& operator=(Global<S>&& rhs);
-
+  V8_INLINE Global& operator=(Global<S>&& rhs) {  // NOLINT
+    TYPE_CHECK(T, S);
+    if (this != &rhs) {
+      this->Reset();
+      this->val_ = rhs.val_;
+      rhs.val_ = nullptr;
+    }
+    return *this;
+  }
   /**
    * Pass allows returning uniques from functions, etc.
    */
@@ -818,6 +822,7 @@ class Global : public PersistentBase<T> {
 template <class T>
 using UniquePersistent = Global<T>;
 
+
  /**
  * A stack-allocated class that governs a number of local handles.
  * After a handle scope has been created, all local handles will be
@@ -8560,9 +8565,7 @@ class V8_EXPORT V8 {
 
   static internal::Address* GlobalizeReference(internal::Isolate* isolate,
                                                internal::Address* handle);
-  static void MoveGlobalReference(internal::Address** from,
-                                  internal::Address** to);
-  static internal::Address* CopyGlobalReference(internal::Address* from);
+  static internal::Address* CopyPersistent(internal::Address* handle);
   static void DisposeGlobal(internal::Address* global_handle);
   static void MakeWeak(internal::Address* location, void* data,
                        WeakCallbackInfo<void>::Callback weak_callback,
@@ -8582,8 +8585,6 @@ class V8_EXPORT V8 {
   static void FromJustIsNothing();
   static void ToLocalEmpty();
   static void InternalFieldOutOfBounds(int index);
-  template <class T>
-  friend class Global;
   template <class T> friend class Local;
   template <class T>
   friend class MaybeLocal;
@@ -9511,7 +9512,7 @@ void Persistent<T, M>::Copy(const Persistent<S, M2>& that) {
   this->Reset();
   if (that.IsEmpty()) return;
   internal::Address* p = reinterpret_cast<internal::Address*>(that.val_);
-  this->val_ = reinterpret_cast<T*>(V8::CopyGlobalReference(p));
+  this->val_ = reinterpret_cast<T*>(V8::CopyPersistent(p));
   M::Copy(that, this);
 }
 
@@ -9644,32 +9645,6 @@ uint16_t PersistentBase<T>::WrapperClassId() const {
   return *reinterpret_cast<uint16_t*>(addr);
 }
 
-template <class T>
-Global<T>::Global(Global&& other) : PersistentBase<T>(other.val_) {
-  if (other.val_ != nullptr) {
-    V8::MoveGlobalReference(reinterpret_cast<internal::Address**>(&other.val_),
-                            reinterpret_cast<internal::Address**>(&this->val_));
-    other.val_ = nullptr;
-  }
-}
-
-template <class T>
-template <class S>
-Global<T>& Global<T>::operator=(Global<S>&& rhs) {
-  TYPE_CHECK(T, S);
-  if (this != &rhs) {
-    this->Reset();
-    if (rhs.val_ != nullptr) {
-      this->val_ = rhs.val_;
-      V8::MoveGlobalReference(
-          reinterpret_cast<internal::Address**>(&rhs.val_),
-          reinterpret_cast<internal::Address**>(&this->val_));
-      rhs.val_ = nullptr;
-    }
-  }
-  return *this;
-}
-
 template <typename T>
 ReturnValue<T>::ReturnValue(internal::Address* slot) : value_(slot) {}
 

src/api.cc

@@ -1012,15 +1012,11 @@ i::Address* V8::GlobalizeReference(i::Isolate* isolate, i::Address* obj) {
   return result.location();
 }
 
-i::Address* V8::CopyGlobalReference(i::Address* from) {
-  i::Handle<i::Object> result = i::GlobalHandles::CopyGlobal(from);
+i::Address* V8::CopyPersistent(i::Address* obj) {
+  i::Handle<i::Object> result = i::GlobalHandles::CopyGlobal(obj);
   return result.location();
 }
 
-void V8::MoveGlobalReference(internal::Address** from, internal::Address** to) {
-  i::GlobalHandles::MoveGlobal(from, to);
-}
-
 void V8::RegisterExternallyReferencedObject(i::Address* location,
                                             i::Isolate* isolate) {
   isolate->heap()->RegisterExternallyReferencedObject(location);

src/global-handles.cc

@@ -625,20 +625,6 @@ Handle<Object> GlobalHandles::CopyGlobal(Address* location) {
   return global_handles->Create(*location);
 }
 
-void GlobalHandles::MoveGlobal(Address** from, Address** to) {
-  DCHECK_NOT_NULL(*from);
-  DCHECK_NOT_NULL(*to);
-  DCHECK_EQ(*from, *to);
-  Node* node = Node::FromLocation(*from);
-  if (node->IsPhantomResetHandle()) {
-    node->set_parameter(to);
-  }
-
-  // - Strong handles do not require fixups.
-  // - Weak handles with finalizers and callbacks are too general to fix up. For
-  //   those the callers need to ensure consistency.
-}
-
 void GlobalHandles::Destroy(Address* location) {
   if (location != nullptr) {
     NodeSpace<Node>::Release(Node::FromLocation(location));

src/global-handles.h

@@ -40,10 +40,7 @@ enum WeaknessType {
 // callbacks and finalizers attached to them.
 class GlobalHandles final {
  public:
-  // Move a global handle.
-  static void MoveGlobal(Address** from, Address** to);
-
-  // Copy a global handle.
+  // Copy a global handle
   static Handle<Object> CopyGlobal(Address* location);
 
   // Destroy a global handle.

test/cctest/test-global-handles.cc

@@ -69,14 +69,6 @@ void ConstructJSObject(v8::Isolate* isolate, v8::Local<v8::Context> context,
   CHECK(!flag_and_persistent->handle.IsEmpty());
 }
 
-void ConstructJSObject(v8::Isolate* isolate, v8::Global<v8::Object>* global) {
-  v8::HandleScope scope(isolate);
-  v8::Local<v8::Object> object(v8::Object::New(isolate));
-  CHECK(!object.IsEmpty());
-  *global = v8::Global<v8::Object>(isolate, object);
-  CHECK(!global->IsEmpty());
-}
-
 void ConstructJSApiObject(v8::Isolate* isolate, v8::Local<v8::Context> context,
                           FlagAndPersistent* flag_and_persistent) {
   v8::HandleScope handle_scope(isolate);
@@ -540,32 +532,5 @@ TEST(SecondPassPhantomCallbacks) {
   CHECK(fp.flag);
 }
 
-TEST(MoveStrongGlobal) {
-  CcTest::InitializeVM();
-  v8::Isolate* isolate = CcTest::isolate();
-  v8::HandleScope scope(isolate);
-
-  v8::Global<v8::Object>* global = new Global<v8::Object>();
-  ConstructJSObject(isolate, global);
-  InvokeMarkSweep();
-  v8::Global<v8::Object> global2(std::move(*global));
-  delete global;
-  InvokeMarkSweep();
-}
-
-TEST(MoveWeakGlobal) {
-  CcTest::InitializeVM();
-  v8::Isolate* isolate = CcTest::isolate();
-  v8::HandleScope scope(isolate);
-
-  v8::Global<v8::Object>* global = new Global<v8::Object>();
-  ConstructJSObject(isolate, global);
-  InvokeMarkSweep();
-  global->SetWeak();
-  v8::Global<v8::Object> global2(std::move(*global));
-  delete global;
-  InvokeMarkSweep();
-}
-
 }  // namespace internal
 }  // namespace v8