diff --git a/atom/common/native_mate_converters/callback.cc b/atom/common/native_mate_converters/callback.cc index 87faa3df3c..cd9838c1ed 100644 --- a/atom/common/native_mate_converters/callback.cc +++ b/atom/common/native_mate_converters/callback.cc @@ -5,6 +5,9 @@ #include "atom/common/native_mate_converters/callback.h" #include "atom/browser/atom_browser_main_parts.h" +#include "content/public/browser/browser_thread.h" + +using content::BrowserThread; namespace mate { @@ -56,31 +59,72 @@ v8::Local BindFunctionWith(v8::Isolate* isolate, } // namespace +// Destroy the class on UI thread when possible. +struct DeleteOnUIThread { + template + static void Destruct(const T* x) { + if (Locker::IsBrowserProcess() && + !BrowserThread::CurrentlyOn(BrowserThread::UI)) { + BrowserThread::DeleteSoon(BrowserThread::UI, FROM_HERE, x); + } else { + delete x; + } + } +}; + +// Like v8::Global, but ref-counted. +template +class RefCountedGlobal : public base::RefCountedThreadSafe, + DeleteOnUIThread> { + public: + RefCountedGlobal(v8::Isolate* isolate, v8::Local value) + : handle_(isolate, v8::Local::Cast(value)), + weak_factory_(this) { + // In browser process, we need to ensure the V8 handle is destroyed before + // the JavaScript env gets destroyed. + if (Locker::IsBrowserProcess() && atom::AtomBrowserMainParts::Get()) { + atom::AtomBrowserMainParts::Get()->RegisterDestructionCallback( + base::Bind(&RefCountedGlobal::FreeHandle, + weak_factory_.GetWeakPtr())); + } + } + + bool IsAlive() const { + return !handle_.IsEmpty(); + } + + v8::Local NewHandle(v8::Isolate* isolate) const { + return v8::Local::New(isolate, handle_); + } + + private: + void FreeHandle() { + handle_.Reset(); + } + + v8::Global handle_; + base::WeakPtrFactory> weak_factory_; + + DISALLOW_COPY_AND_ASSIGN(RefCountedGlobal); +}; + SafeV8Function::SafeV8Function(v8::Isolate* isolate, v8::Local value) - : v8_function_(new RefCountedPersistent(isolate, value)), - weak_factory_(this) { - Init(); + : v8_function_(new RefCountedGlobal(isolate, value)) { } SafeV8Function::SafeV8Function(const SafeV8Function& other) - : v8_function_(other.v8_function_), - weak_factory_(this) { - Init(); + : v8_function_(other.v8_function_) { } -v8::Local SafeV8Function::NewHandle() const { - return v8_function_->NewHandle(); +SafeV8Function::~SafeV8Function() { } -void SafeV8Function::Init() { - if (Locker::IsBrowserProcess() && atom::AtomBrowserMainParts::Get()) - atom::AtomBrowserMainParts::Get()->RegisterDestructionCallback( - base::Bind(&SafeV8Function::FreeHandle, weak_factory_.GetWeakPtr())); +bool SafeV8Function::IsAlive() const { + return v8_function_.get() && v8_function_->IsAlive(); } -void SafeV8Function::FreeHandle() { - Locker locker(v8_function_->isolate()); - v8_function_ = nullptr; +v8::Local SafeV8Function::NewHandle(v8::Isolate* isolate) const { + return v8_function_->NewHandle(isolate); } v8::Local CreateFunctionFromTranslater( diff --git a/atom/common/native_mate_converters/callback.h b/atom/common/native_mate_converters/callback.h index 5dd9d3cec9..3cba2b32a8 100644 --- a/atom/common/native_mate_converters/callback.h +++ b/atom/common/native_mate_converters/callback.h @@ -19,23 +19,21 @@ namespace mate { namespace internal { -// Manages the V8 function with RAII, and automatically cleans the handle when -// JavaScript context is destroyed, even when the class is not destroyed. +template +class RefCountedGlobal; + +// Manages the V8 function with RAII. class SafeV8Function { public: SafeV8Function(v8::Isolate* isolate, v8::Local value); SafeV8Function(const SafeV8Function& other); + ~SafeV8Function(); - bool is_alive() const { return v8_function_.get(); } - - v8::Local NewHandle() const; + bool IsAlive() const; + v8::Local NewHandle(v8::Isolate* isolate) const; private: - void Init(); - void FreeHandle(); - - scoped_refptr> v8_function_; - base::WeakPtrFactory weak_factory_; + scoped_refptr> v8_function_; }; // Helper to invoke a V8 function with C++ parameters. @@ -49,12 +47,12 @@ struct V8FunctionInvoker(ArgTypes...)> { ArgTypes... raw) { Locker locker(isolate); v8::EscapableHandleScope handle_scope(isolate); - if (!function.is_alive()) + if (!function.IsAlive()) return v8::Null(isolate); scoped_ptr script_scope( Locker::IsBrowserProcess() ? nullptr : new blink::WebScopedRunV8Script(isolate)); - v8::Local holder = function.NewHandle(); + v8::Local holder = function.NewHandle(isolate); v8::Local context = holder->CreationContext(); v8::Context::Scope context_scope(context); std::vector> args = { ConvertToV8(isolate, raw)... }; @@ -70,12 +68,12 @@ struct V8FunctionInvoker { ArgTypes... raw) { Locker locker(isolate); v8::HandleScope handle_scope(isolate); - if (!function.is_alive()) + if (!function.IsAlive()) return; scoped_ptr script_scope( Locker::IsBrowserProcess() ? nullptr : new blink::WebScopedRunV8Script(isolate)); - v8::Local holder = function.NewHandle(); + v8::Local holder = function.NewHandle(isolate); v8::Local context = holder->CreationContext(); v8::Context::Scope context_scope(context); std::vector> args = { ConvertToV8(isolate, raw)... }; @@ -91,12 +89,12 @@ struct V8FunctionInvoker { Locker locker(isolate); v8::HandleScope handle_scope(isolate); ReturnType ret = ReturnType(); - if (!function.is_alive()) + if (!function.IsAlive()) return ret; scoped_ptr script_scope( Locker::IsBrowserProcess() ? nullptr : new blink::WebScopedRunV8Script(isolate)); - v8::Local holder = function.NewHandle(); + v8::Local holder = function.NewHandle(isolate); v8::Local context = holder->CreationContext(); v8::Context::Scope context_scope(context); std::vector> args = { ConvertToV8(isolate, raw)... };