refactor: update gin_helper/function_template (#41683)

* refactor: update gin_helper/function_template Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com> * fix: crash in Node.js Worker threads Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com> --------- Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
electron · Mar 25, 2024 · 83777ac · 83777ac
1 parent f585a4e
commit 83777ac
Show file tree

Hide file tree

Showing 4 changed files with 267 additions and 110 deletions.
diff --git a/shell/common/gin_helper/callback.h b/shell/common/gin_helper/callback.h
@@ -132,8 +132,9 @@ template <typename ReturnType, typename... ArgTypes>
 struct NativeFunctionInvoker<ReturnType(ArgTypes...)> {
   static void Go(base::RepeatingCallback<ReturnType(ArgTypes...)> val,
                  gin::Arguments* args) {
-    using Indices = typename IndicesGenerator<sizeof...(ArgTypes)>::type;
-    Invoker<Indices, ArgTypes...> invoker(args, 0);
+    using Indices = std::index_sequence_for<ArgTypes...>;
+    Invoker<Indices, ArgTypes...> invoker(args,
+                                          {.holder_is_first_argument = false});
     if (invoker.IsOK())
       invoker.DispatchToCallback(val);
   }

diff --git a/shell/common/gin_helper/constructor.h b/shell/common/gin_helper/constructor.h
@@ -26,7 +26,8 @@ inline WrappableBase* InvokeFactory(
     gin::Arguments* args,
     const base::RepeatingCallback<WrappableBase*(P1)>& callback) {
   typename CallbackParamTraits<P1>::LocalType a1;
-  if (!gin_helper::GetNextArgument(args, 0, true, &a1))
+  if (!gin_helper::GetNextArgument(args, {.holder_is_first_argument = true}, 0,
+                                   &a1))
     return nullptr;
   return callback.Run(a1);
 }
@@ -37,8 +38,10 @@ inline WrappableBase* InvokeFactory(
     const base::RepeatingCallback<WrappableBase*(P1, P2)>& callback) {
   typename CallbackParamTraits<P1>::LocalType a1;
   typename CallbackParamTraits<P2>::LocalType a2;
-  if (!gin_helper::GetNextArgument(args, 0, true, &a1) ||
-      !gin_helper::GetNextArgument(args, 0, false, &a2))
+  if (!gin_helper::GetNextArgument(args, {.holder_is_first_argument = true}, 0,
+                                   &a1) ||
+      !gin_helper::GetNextArgument(args, {.holder_is_first_argument = false}, 0,
+                                   &a2))
     return nullptr;
   return callback.Run(a1, a2);
 }
@@ -50,9 +53,12 @@ inline WrappableBase* InvokeFactory(
   typename CallbackParamTraits<P1>::LocalType a1;
   typename CallbackParamTraits<P2>::LocalType a2;
   typename CallbackParamTraits<P3>::LocalType a3;
-  if (!gin_helper::GetNextArgument(args, 0, true, &a1) ||
-      !gin_helper::GetNextArgument(args, 0, false, &a2) ||
-      !gin_helper::GetNextArgument(args, 0, false, &a3))
+  if (!gin_helper::GetNextArgument(args, {.holder_is_first_argument = true}, 0,
+                                   &a1) ||
+      !gin_helper::GetNextArgument(args, {.holder_is_first_argument = false}, 0,
+                                   &a2) ||
+      !gin_helper::GetNextArgument(args, {.holder_is_first_argument = false}, 0,
+                                   &a3))
     return nullptr;
   return callback.Run(a1, a2, a3);
 }
@@ -65,10 +71,14 @@ inline WrappableBase* InvokeFactory(
   typename CallbackParamTraits<P2>::LocalType a2;
   typename CallbackParamTraits<P3>::LocalType a3;
   typename CallbackParamTraits<P4>::LocalType a4;
-  if (!gin_helper::GetNextArgument(args, 0, true, &a1) ||
-      !gin_helper::GetNextArgument(args, 0, false, &a2) ||
-      !gin_helper::GetNextArgument(args, 0, false, &a3) ||
-      !gin_helper::GetNextArgument(args, 0, false, &a4))
+  if (!gin_helper::GetNextArgument(args, {.holder_is_first_argument = true}, 0,
+                                   &a1) ||
+      !gin_helper::GetNextArgument(args, {.holder_is_first_argument = false}, 0,
+                                   &a2) ||
+      !gin_helper::GetNextArgument(args, {.holder_is_first_argument = false}, 0,
+                                   &a3) ||
+      !gin_helper::GetNextArgument(args, {.holder_is_first_argument = false}, 0,
+                                   &a4))
     return nullptr;
   return callback.Run(a1, a2, a3, a4);
 }
@@ -83,11 +93,16 @@ inline WrappableBase* InvokeFactory(
   typename CallbackParamTraits<P3>::LocalType a3;
   typename CallbackParamTraits<P4>::LocalType a4;
   typename CallbackParamTraits<P5>::LocalType a5;
-  if (!gin_helper::GetNextArgument(args, 0, true, &a1) ||
-      !gin_helper::GetNextArgument(args, 0, false, &a2) ||
-      !gin_helper::GetNextArgument(args, 0, false, &a3) ||
-      !gin_helper::GetNextArgument(args, 0, false, &a4) ||
-      !gin_helper::GetNextArgument(args, 0, false, &a5))
+  if (!gin_helper::GetNextArgument(args, {.holder_is_first_argument = true}, 0,
+                                   &a1) ||
+      !gin_helper::GetNextArgument(args, {.holder_is_first_argument = false}, 0,
+                                   &a2) ||
+      !gin_helper::GetNextArgument(args, {.holder_is_first_argument = false}, 0,
+                                   &a3) ||
+      !gin_helper::GetNextArgument(args, {.holder_is_first_argument = false}, 0,
+                                   &a4) ||
+      !gin_helper::GetNextArgument(args, {.holder_is_first_argument = false}, 0,
+                                   &a5))
     return nullptr;
   return callback.Run(a1, a2, a3, a4, a5);
 }
@@ -108,12 +123,18 @@ inline WrappableBase* InvokeFactory(
   typename CallbackParamTraits<P4>::LocalType a4;
   typename CallbackParamTraits<P5>::LocalType a5;
   typename CallbackParamTraits<P6>::LocalType a6;
-  if (!gin_helper::GetNextArgument(args, 0, true, &a1) ||
-      !gin_helper::GetNextArgument(args, 0, false, &a2) ||
-      !gin_helper::GetNextArgument(args, 0, false, &a3) ||
-      !gin_helper::GetNextArgument(args, 0, false, &a4) ||
-      !gin_helper::GetNextArgument(args, 0, false, &a5) ||
-      !gin_helper::GetNextArgument(args, 0, false, &a6))
+  if (!gin_helper::GetNextArgument(args, {.holder_is_first_argument = true}, 0,
+                                   &a1) ||
+      !gin_helper::GetNextArgument(args, {.holder_is_first_argument = false}, 0,
+                                   &a2) ||
+      !gin_helper::GetNextArgument(args, {.holder_is_first_argument = false}, 0,
+                                   &a3) ||
+      !gin_helper::GetNextArgument(args, {.holder_is_first_argument = false}, 0,
+                                   &a4) ||
+      !gin_helper::GetNextArgument(args, {.holder_is_first_argument = false}, 0,
+                                   &a5) ||
+      !gin_helper::GetNextArgument(args, {.holder_is_first_argument = false}, 0,
+                                   &a6))
     return nullptr;
   return callback.Run(a1, a2, a3, a4, a5, a6);
 }

diff --git a/shell/common/gin_helper/function_template.cc b/shell/common/gin_helper/function_template.cc
@@ -4,10 +4,34 @@
 
 #include "shell/common/gin_helper/function_template.h"
 
+#include "base/observer_list.h"
+#include "base/strings/strcat.h"
+
 namespace gin_helper {
 
+CallbackHolderBase::DisposeObserver::DisposeObserver(
+    gin::PerIsolateData* per_isolate_data,
+    CallbackHolderBase* holder)
+    : per_isolate_data_(per_isolate_data), holder_(*holder) {
+  if (per_isolate_data_)
+    per_isolate_data_->AddDisposeObserver(this);
+}
+CallbackHolderBase::DisposeObserver::~DisposeObserver() {
+  if (per_isolate_data_)
+    per_isolate_data_->RemoveDisposeObserver(this);
+}
+void CallbackHolderBase::DisposeObserver::OnBeforeDispose(
+    v8::Isolate* isolate) {
+  holder_->v8_ref_.Reset();
+}
+void CallbackHolderBase::DisposeObserver::OnDisposed() {
+  // The holder contains the observer, so the observer is destroyed here also.
+  delete &holder_.get();
+}
+
 CallbackHolderBase::CallbackHolderBase(v8::Isolate* isolate)
-    : v8_ref_(isolate, v8::External::New(isolate, this)) {
+    : v8_ref_(isolate, v8::External::New(isolate, this)),
+      dispose_observer_(gin::PerIsolateData::From(isolate), this) {
   v8_ref_.SetWeak(this, &CallbackHolderBase::FirstWeakCallback,
                   v8::WeakCallbackType::kParameter);
 }
@@ -33,4 +57,28 @@ void CallbackHolderBase::SecondWeakCallback(
   delete data.GetParameter();
 }
 
+void ThrowConversionError(gin::Arguments* args,
+                          const InvokerOptions& invoker_options,
+                          size_t index) {
+  if (index == 0 && invoker_options.holder_is_first_argument) {
+    // Failed to get the appropriate `this` object. This can happen if a
+    // method is invoked using Function.prototype.[call|apply] and passed an
+    // invalid (or null) `this` argument.
+    std::string error =
+        invoker_options.holder_type
+            ? base::StrCat({"Illegal invocation: Function must be "
+                            "called on an object of type ",
+                            invoker_options.holder_type})
+            : "Illegal invocation";
+    args->ThrowTypeError(error);
+  } else {
+    // Otherwise, this failed parsing on a different argument.
+    // Arguments::ThrowError() will try to include appropriate information.
+    // Ideally we would include the expected c++ type in the error message
+    // here, too (which we can access via typeid(ArgType).name()), however we
+    // compile with no-rtti, which disables typeid.
+    args->ThrowError();
+  }
+}
+
 }  // namespace gin_helper