refactor: SafeV8Function to be backed by cppgc (#50397)

* refactor: SafeV8Function to be backed by cppgc

* spec: focus renderer before attempting paste

* spec: remove listeners to prevent leak on failed tests

shell/common/api/electron_api_testing.cc

@@ -2,13 +2,17 @@
 // Use of this source code is governed by the MIT license that can be
 // found in the LICENSE file.
 
+#include <optional>
+
 #include "base/command_line.h"
 #include "base/dcheck_is_on.h"
 #include "base/logging.h"
+#include "base/no_destructor.h"
 #include "content/browser/network_service_instance_impl.h"  // nogncheck
 #include "content/public/browser/network_service_instance.h"
 #include "content/public/common/content_switches.h"
 #include "shell/common/callback_util.h"
+#include "shell/common/gin_converters/callback_converter.h"
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/gin_helper/promise.h"
 #include "shell/common/node_includes.h"
@@ -17,6 +21,93 @@
 #if DCHECK_IS_ON()
 namespace {
 
+class CallbackTestingHelper final {
+ public:
+  void HoldRepeatingCallback(const base::RepeatingClosure& callback) {
+    repeating_callback_ = callback;
+  }
+
+  bool CopyHeldRepeatingCallback() {
+    if (!repeating_callback_)
+      return false;
+
+    repeating_callback_copy_ = *repeating_callback_;
+    return true;
+  }
+
+  bool InvokeHeldRepeatingCallback(v8::Isolate* isolate) {
+    if (!repeating_callback_)
+      return false;
+
+    return InvokeRepeatingCallback(isolate, *repeating_callback_);
+  }
+
+  bool InvokeCopiedRepeatingCallback(v8::Isolate* isolate) {
+    if (!repeating_callback_copy_)
+      return false;
+
+    return InvokeRepeatingCallback(isolate, *repeating_callback_copy_);
+  }
+
+  void HoldOnceCallback(base::OnceClosure callback) {
+    once_callback_ = std::move(callback);
+  }
+
+  bool InvokeHeldOnceCallback(v8::Isolate* isolate) {
+    if (!once_callback_)
+      return false;
+
+    base::OnceClosure callback = std::move(*once_callback_);
+    once_callback_.reset();
+    return InvokeOnceCallback(isolate, std::move(callback));
+  }
+
+  void ClearPrimaryHeldRepeatingCallback() { repeating_callback_.reset(); }
+
+  int GetHeldRepeatingCallbackCount() const {
+    return (repeating_callback_ ? 1 : 0) + (repeating_callback_copy_ ? 1 : 0);
+  }
+
+  void ClearAllHeldCallbacks() {
+    repeating_callback_.reset();
+    repeating_callback_copy_.reset();
+    once_callback_.reset();
+  }
+
+ private:
+  bool InvokeRepeatingCallback(v8::Isolate* isolate,
+                               const base::RepeatingClosure& callback) {
+    v8::TryCatch try_catch(isolate);
+    callback.Run();
+    if (try_catch.HasCaught()) {
+      try_catch.Reset();
+      return false;
+    }
+
+    return true;
+  }
+
+  bool InvokeOnceCallback(v8::Isolate* isolate, base::OnceClosure callback) {
+    v8::TryCatch try_catch(isolate);
+    std::move(callback).Run();
+    if (try_catch.HasCaught()) {
+      try_catch.Reset();
+      return false;
+    }
+
+    return true;
+  }
+
+  std::optional<base::RepeatingClosure> repeating_callback_;
+  std::optional<base::RepeatingClosure> repeating_callback_copy_;
+  std::optional<base::OnceClosure> once_callback_;
+};
+
+CallbackTestingHelper& GetCallbackTestingHelper() {
+  static base::NoDestructor<CallbackTestingHelper> helper;
+  return *helper;
+}
+
 void Log(int severity, std::string text) {
   switch (severity) {
     case logging::LOGGING_VERBOSE:
@@ -57,6 +148,44 @@ v8::Local<v8::Promise> SimulateNetworkServiceCrash(v8::Isolate* isolate) {
   return handle;
 }
 
+void HoldRepeatingCallbackForTesting(const base::RepeatingClosure& callback) {
+  GetCallbackTestingHelper().HoldRepeatingCallback(callback);
+}
+
+bool CopyHeldRepeatingCallbackForTesting() {
+  return GetCallbackTestingHelper().CopyHeldRepeatingCallback();
+}
+
+bool InvokeHeldRepeatingCallbackForTesting(gin::Arguments* args) {
+  return GetCallbackTestingHelper().InvokeHeldRepeatingCallback(
+      args->isolate());
+}
+
+bool InvokeCopiedRepeatingCallbackForTesting(gin::Arguments* args) {
+  return GetCallbackTestingHelper().InvokeCopiedRepeatingCallback(
+      args->isolate());
+}
+
+void HoldOnceCallbackForTesting(base::OnceClosure callback) {
+  GetCallbackTestingHelper().HoldOnceCallback(std::move(callback));
+}
+
+bool InvokeHeldOnceCallbackForTesting(gin::Arguments* args) {
+  return GetCallbackTestingHelper().InvokeHeldOnceCallback(args->isolate());
+}
+
+void ClearPrimaryHeldRepeatingCallbackForTesting() {
+  GetCallbackTestingHelper().ClearPrimaryHeldRepeatingCallback();
+}
+
+int GetHeldRepeatingCallbackCountForTesting() {
+  return GetCallbackTestingHelper().GetHeldRepeatingCallbackCount();
+}
+
+void ClearHeldCallbacksForTesting() {
+  GetCallbackTestingHelper().ClearAllHeldCallbacks();
+}
+
 void Initialize(v8::Local<v8::Object> exports,
                 v8::Local<v8::Value> unused,
                 v8::Local<v8::Context> context,
@@ -66,6 +195,22 @@ void Initialize(v8::Local<v8::Object> exports,
   dict.SetMethod("log", &Log);
   dict.SetMethod("getLoggingDestination", &GetLoggingDestination);
   dict.SetMethod("simulateNetworkServiceCrash", &SimulateNetworkServiceCrash);
+  dict.SetMethod("holdRepeatingCallbackForTesting",
+                 &HoldRepeatingCallbackForTesting);
+  dict.SetMethod("copyHeldRepeatingCallbackForTesting",
+                 &CopyHeldRepeatingCallbackForTesting);
+  dict.SetMethod("invokeHeldRepeatingCallbackForTesting",
+                 &InvokeHeldRepeatingCallbackForTesting);
+  dict.SetMethod("invokeCopiedRepeatingCallbackForTesting",
+                 &InvokeCopiedRepeatingCallbackForTesting);
+  dict.SetMethod("clearPrimaryHeldRepeatingCallbackForTesting",
+                 &ClearPrimaryHeldRepeatingCallbackForTesting);
+  dict.SetMethod("getHeldRepeatingCallbackCountForTesting",
+                 &GetHeldRepeatingCallbackCountForTesting);
+  dict.SetMethod("holdOnceCallbackForTesting", &HoldOnceCallbackForTesting);
+  dict.SetMethod("invokeHeldOnceCallbackForTesting",
+                 &InvokeHeldOnceCallbackForTesting);
+  dict.SetMethod("clearHeldCallbacksForTesting", &ClearHeldCallbacksForTesting);
 }
 
 }  // namespace

shell/common/gin_helper/callback.cc

@@ -4,12 +4,32 @@
 
 #include "shell/common/gin_helper/callback.h"
 
-#include "content/public/browser/browser_thread.h"
 #include "gin/dictionary.h"
-#include "shell/common/process_util.h"
+#include "gin/persistent.h"
+#include "v8/include/cppgc/allocation.h"
+#include "v8/include/v8-cppgc.h"
+#include "v8/include/v8-traced-handle.h"
 
 namespace gin_helper {
 
+class SafeV8FunctionHandle final
+    : public cppgc::GarbageCollected<SafeV8FunctionHandle> {
+ public:
+  SafeV8FunctionHandle(v8::Isolate* isolate, v8::Local<v8::Value> value)
+      : v8_function_(isolate, value.As<v8::Function>()) {}
+
+  void Trace(cppgc::Visitor* visitor) const { visitor->Trace(v8_function_); }
+
+  [[nodiscard]] bool IsAlive() const { return !v8_function_.IsEmpty(); }
+
+  v8::Local<v8::Function> NewHandle(v8::Isolate* isolate) const {
+    return v8_function_.Get(isolate);
+  }
+
+ private:
+  v8::TracedReference<v8::Function> v8_function_;
+};
+
 namespace {
 
 struct TranslatorHolder {
@@ -71,46 +91,19 @@ void CallTranslator(v8::Local<v8::External> external,
 
 }  // namespace
 
-// Destroy the class on UI thread when possible.
-struct DeleteOnUIThread {
-  template <typename T>
-  static void Destruct(const T* x) {
-    if (electron::IsBrowserProcess() &&
-        !content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)) {
-      content::GetUIThreadTaskRunner({})->DeleteSoon(FROM_HERE, x);
-    } else {
-      delete x;
-    }
-  }
-};
-
-// Like v8::Global, but ref-counted.
-template <typename T>
-class RefCountedGlobal
-    : public base::RefCountedThreadSafe<RefCountedGlobal<T>, DeleteOnUIThread> {
- public:
-  RefCountedGlobal(v8::Isolate* isolate, v8::Local<v8::Value> value)
-      : handle_(isolate, value.As<T>()) {}
-
-  [[nodiscard]] bool IsAlive() const { return !handle_.IsEmpty(); }
-
-  v8::Local<T> NewHandle(v8::Isolate* isolate) const {
-    return v8::Local<T>::New(isolate, handle_);
-  }
-
- private:
-  v8::Global<T> handle_;
-};
-
 SafeV8Function::SafeV8Function(v8::Isolate* isolate, v8::Local<v8::Value> value)
-    : v8_function_(new RefCountedGlobal<v8::Function>(isolate, value)) {}
+    : v8_function_(
+          gin::WrapPersistent(cppgc::MakeGarbageCollected<SafeV8FunctionHandle>(
+              isolate->GetCppHeap()->GetAllocationHandle(),
+              isolate,
+              value))) {}
 
 SafeV8Function::SafeV8Function(const SafeV8Function& other) = default;
 
 SafeV8Function::~SafeV8Function() = default;
 
 bool SafeV8Function::IsAlive() const {
-  return v8_function_.get() && v8_function_->IsAlive();
+  return v8_function_ && v8_function_->IsAlive();
 }
 
 v8::Local<v8::Function> SafeV8Function::NewHandle(v8::Isolate* isolate) const {

shell/common/gin_helper/callback.h

@@ -12,14 +12,14 @@
 #include "shell/common/gin_converters/std_converter.h"
 #include "shell/common/gin_helper/function_template.h"
 #include "shell/common/gin_helper/locker.h"
+#include "v8/include/cppgc/persistent.h"
 #include "v8/include/v8-function.h"
 #include "v8/include/v8-microtask-queue.h"
 // Implements safe conversions between JS functions and base::RepeatingCallback.
 
 namespace gin_helper {
 
-template <typename T>
-class RefCountedGlobal;
+class SafeV8FunctionHandle;
 
 // Manages the V8 function with RAII.
 class SafeV8Function {
@@ -32,7 +32,7 @@ class SafeV8Function {
   v8::Local<v8::Function> NewHandle(v8::Isolate* isolate) const;
 
  private:
-  scoped_refptr<RefCountedGlobal<v8::Function>> v8_function_;
+  cppgc::Persistent<SafeV8FunctionHandle> v8_function_;
 };
 
 // Helper to invoke a V8 function with C++ parameters.