Don't leak FunctionTemplate when converting C++ callback

diff --git a/atom/common/native_mate_converters/callback.cc b/atom/common/native_mate_converters/callback.cc
new file mode 100644 (file)
index 0000000..0be90f6
--- /dev/null
+++ b/atom/common/native_mate_converters/callback.cc
@@ -0,0 +1,62 @@
+// Copyright (c) 2015 GitHub, Inc. All rights reserved.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include "atom/common/native_mate_converters/callback.h"
+
+namespace mate {
+
+namespace internal {
+
+namespace {
+
+struct TranslaterHolder {
+  Translater translater;
+};
+
+// Cached JavaScript version of |CallTranslater|.
+v8::Persistent<v8::FunctionTemplate> g_call_translater;
+
+void CallTranslater(v8::Local<v8::External> external, mate::Arguments* args) {
+  TranslaterHolder* holder = static_cast<TranslaterHolder*>(external->Value());
+  holder->translater.Run(args);
+}
+
+// func.bind(func, arg1).
+// NB(zcbenz): Using C++11 version crashes VS.
+v8::Local<v8::Value> BindFunctionWith(v8::Isolate* isolate,
+                                      v8::Local<v8::Context> context,
+                                      v8::Local<v8::Function> func,
+                                      v8::Local<v8::Value> arg1) {
+  v8::MaybeLocal<v8::Value> bind = func->Get(mate::StringToV8(isolate, "bind"));
+  CHECK(!bind.IsEmpty());
+  v8::Local<v8::Function> bind_func =
+      v8::Local<v8::Function>::Cast(bind.ToLocalChecked());
+  v8::Local<v8::Value> converted[] = { func, arg1 };
+  return bind_func->Call(
+      context, func, arraysize(converted), converted).ToLocalChecked();
+}
+
+}  // namespace
+
+v8::Local<v8::Value> CreateFunctionFromTranslater(
+    v8::Isolate* isolate, const Translater& translater) {
+  // The FunctionTemplate is cached.
+  if (g_call_translater.IsEmpty())
+    g_call_translater.Reset(
+        isolate,
+        mate::CreateFunctionTemplate(isolate, base::Bind(&CallTranslater)));
+
+  v8::Local<v8::FunctionTemplate> call_translater =
+      v8::Local<v8::FunctionTemplate>::New(isolate, g_call_translater);
+  TranslaterHolder* holder = new TranslaterHolder;
+  holder->translater = translater;
+  return BindFunctionWith(isolate,
+                          isolate->GetCurrentContext(),
+                          call_translater->GetFunction(),
+                          v8::External::New(isolate, holder));
+}
+
+}  // namespace internal
+
+}  // namespace mate

diff --git a/atom/common/native_mate_converters/callback.h b/atom/common/native_mate_converters/callback.h
index 68ea911..4349b99 100644 (file)
--- a/atom/common/native_mate_converters/callback.h
+++ b/atom/common/native_mate_converters/callback.h
@@ -20,6 +20,7 @@ namespace internal {
 
 typedef scoped_refptr<RefCountedPersistent<v8::Function> > SafeV8Function;
 
+// Helper to invoke a V8 function with C++ parameters.
 template <typename Sig>
 struct V8FunctionInvoker {};
 
@@ -81,13 +82,41 @@ struct V8FunctionInvoker<ReturnType(ArgTypes...)> {
   }
 };
 
+// Helper to pass a C++ funtion to JavaScript.
+using Translater = base::Callback<void(Arguments* args)>;
+v8::Local<v8::Value> CreateFunctionFromTranslater(
+    v8::Isolate* isolate, const Translater& translater);
+
+// Calls callback with Arguments.
+template <typename Sig>
+struct NativeFunctionInvoker {};
+
+template <typename ReturnType, typename... ArgTypes>
+struct NativeFunctionInvoker<ReturnType(ArgTypes...)> {
+  static void Go(base::Callback<ReturnType(ArgTypes...)> val, Arguments* args) {
+    using Indices = typename IndicesGenerator<sizeof...(ArgTypes)>::type;
+    Invoker<Indices, ArgTypes...> invoker(args, 0);
+    if (invoker.IsOK())
+      invoker.DispatchToCallback(val);
+  }
+};
+
+// Create a static function that accepts generic callback.
+template <typename Sig>
+Translater ConvertToTranslater(const base::Callback<Sig>& val) {
+  return base::Bind(&NativeFunctionInvoker<Sig>::Go, val);
+}
+
 }  // namespace internal
 
 template<typename Sig>
 struct Converter<base::Callback<Sig> > {
   static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
-                                    const base::Callback<Sig>& val) {
-    return CreateFunctionTemplate(isolate, val)->GetFunction();
+                                   const base::Callback<Sig>& val) {
+    // We don't use CreateFunctionTemplate here because it creates a new
+    // FunctionTemplate everytime, which is cached by V8 and causes leaks.
+    internal::Translater translater = internal::ConvertToTranslater(val);
+    return internal::CreateFunctionFromTranslater(isolate, translater);
   }
   static bool FromV8(v8::Isolate* isolate,
                      v8::Local<v8::Value> val,

diff --git a/filenames.gypi b/filenames.gypi
index a294a1e..65bf6b7 100644 (file)
--- a/filenames.gypi
+++ b/filenames.gypi
@@ -304,6 +304,7 @@
       'atom/common/native_mate_converters/accelerator_converter.h',
       'atom/common/native_mate_converters/blink_converter.cc',
       'atom/common/native_mate_converters/blink_converter.h',
+      'atom/common/native_mate_converters/callback.cc',
       'atom/common/native_mate_converters/callback.h',
       'atom/common/native_mate_converters/file_path_converter.h',
       'atom/common/native_mate_converters/gfx_converter.cc',