From 6fe561067c842ec9d884f1b3356b4218af9a0dfa Mon Sep 17 00:00:00 2001 From: "aa@chromium.org" Date: Sun, 8 Dec 2013 07:10:58 +0000 Subject: Gin: Make it easier to implement Wrappable. BUG= Review URL: https://codereview.chromium.org/105743007 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@239387 0039d316-1c4b-4281-b951-d872f2087c98 --- gin/function_template.cc | 6 +- gin/function_template.h | 13 ++-- gin/function_template.h.pump | 12 ++-- gin/object_template_builder.h | 2 +- gin/wrappable.cc | 48 +++++++++++---- gin/wrappable.h | 134 +++++++++++++++++++++--------------------- gin/wrappable_unittest.cc | 54 ++++------------- 7 files changed, 129 insertions(+), 140 deletions(-) (limited to 'gin') diff --git a/gin/function_template.cc b/gin/function_template.cc index 07882e8..3043087 100644 --- a/gin/function_template.cc +++ b/gin/function_template.cc @@ -8,11 +8,7 @@ namespace gin { -WrapperInfo internal::CallbackHolderBase::kWrapperInfo = { kEmbedderNativeGin }; - -WrapperInfo* internal::CallbackHolderBase::GetWrapperInfo() { - return &kWrapperInfo; -} +INIT_WRAPPABLE(internal::CallbackHolderBase); void InitFunctionTemplates(PerIsolateData* isolate_data) { if (!isolate_data->GetObjectTemplate( diff --git a/gin/function_template.h b/gin/function_template.h index 67df90e9..86ca1a62 100644 --- a/gin/function_template.h +++ b/gin/function_template.h @@ -32,7 +32,6 @@ enum CreateFunctionTemplateFlags { namespace internal { -// TODO(aa): Move this to base/template_util.h as remove_const. template struct CallbackParamTraits { typedef T LocalType; @@ -56,12 +55,12 @@ struct CallbackParamTraits { // It inherits from Wrappable, which delete itself when both (a) the refcount // via base::RefCounted has dropped to zero, and (b) there are no more // JavaScript references in V8. -class CallbackHolderBase : public Wrappable { - public: - virtual WrapperInfo* GetWrapperInfo() OVERRIDE; - static WrapperInfo kWrapperInfo; + +// This simple base class is used so that we can share a single object template +// among every CallbackHolder instance. +class CallbackHolderBase : public Wrappable { protected: - virtual ~CallbackHolderBase() {} + ~CallbackHolderBase() {} }; template @@ -72,7 +71,7 @@ class CallbackHolder : public CallbackHolderBase { base::Callback callback; int flags; private: - virtual ~CallbackHolder() {} + ~CallbackHolder() {} }; diff --git a/gin/function_template.h.pump b/gin/function_template.h.pump index 23941c0..66c2263 100644 --- a/gin/function_template.h.pump +++ b/gin/function_template.h.pump @@ -58,12 +58,12 @@ struct CallbackParamTraits { // It inherits from Wrappable, which delete itself when both (a) the refcount // via base::RefCounted has dropped to zero, and (b) there are no more // JavaScript references in V8. -class CallbackHolderBase : public Wrappable { - public: - virtual WrapperInfo* GetWrapperInfo() OVERRIDE; - static WrapperInfo kWrapperInfo; + +// This simple base class is used so that we can share a single object template +// among every CallbackHolder instance. +class CallbackHolderBase : public Wrappable { protected: - virtual ~CallbackHolderBase() {} + ~CallbackHolderBase() {} }; template @@ -74,7 +74,7 @@ class CallbackHolder : public CallbackHolderBase { base::Callback callback; int flags; private: - virtual ~CallbackHolder() {} + ~CallbackHolder() {} }; diff --git a/gin/object_template_builder.h b/gin/object_template_builder.h index 9af5e4f..3103678 100644 --- a/gin/object_template_builder.h +++ b/gin/object_template_builder.h @@ -43,7 +43,7 @@ struct CallbackTraits > { // from the first normal parameter. template struct CallbackTraits::value >::type> { + base::is_member_function_pointer::value>::type> { static v8::Handle CreateTemplate(v8::Isolate* isolate, T callback) { return CreateFunctionTemplate(isolate, base::Bind(callback), diff --git a/gin/wrappable.cc b/gin/wrappable.cc index 0338211..4a9ef0e 100644 --- a/gin/wrappable.cc +++ b/gin/wrappable.cc @@ -9,28 +9,29 @@ namespace gin { -Wrappable::Wrappable() { +WrappableBase::WrappableBase() { } -Wrappable::~Wrappable() { +WrappableBase::~WrappableBase() { wrapper_.Reset(); } -v8::Handle Wrappable::GetWrapper(v8::Isolate* isolate) { - if (wrapper_.IsEmpty()) - CreateWrapper(isolate); - return v8::Local::New(isolate, wrapper_); +v8::Handle WrappableBase::GetWrapperImpl( + v8::Isolate* isolate, WrapperInfo* wrapper_info) { + if (wrapper_.IsEmpty()) + CreateWrapper(isolate, wrapper_info); + return v8::Local::New(isolate, wrapper_); } -void Wrappable::WeakCallback( - const v8::WeakCallbackData& data) { - Wrappable* wrappable = data.GetParameter(); +void WrappableBase::WeakCallback( + const v8::WeakCallbackData& data) { + WrappableBase* wrappable = data.GetParameter(); wrappable->wrapper_.Reset(); delete wrappable; } -v8::Handle Wrappable::CreateWrapper(v8::Isolate* isolate) { - WrapperInfo* info = GetWrapperInfo(); +v8::Handle WrappableBase::CreateWrapper(v8::Isolate* isolate, + WrapperInfo* info) { PerIsolateData* data = PerIsolateData::From(isolate); v8::Local templ = data->GetObjectTemplate(info); CHECK(!templ.IsEmpty()); // Don't forget to register an object template. @@ -43,4 +44,29 @@ v8::Handle Wrappable::CreateWrapper(v8::Isolate* isolate) { return wrapper; } +namespace internal { + +void* FromV8Impl(v8::Isolate* isolate, v8::Handle val, + WrapperInfo* wrapper_info) { + if (!val->IsObject()) + return NULL; + v8::Handle obj = v8::Handle::Cast(val); + WrapperInfo* info = WrapperInfo::From(obj); + + // If this fails, the object is not managed by Gin. It is either a normal JS + // object that's not wrapping any external C++ object, or it is wrapping some + // C++ object, but that object isn't managed by Gin (maybe Blink). + if (!info) + return NULL; + + // If this fails, the object is managed by Gin, but it's not wrapping an + // instance of the C++ class associated with wrapper_info. + if (info != wrapper_info) + return NULL; + + return obj->GetAlignedPointerFromInternalField(kEncodedValueIndex); +} + +} // namespace internal + } // namespace gin diff --git a/gin/wrappable.h b/gin/wrappable.h index 741bf8f..3f9ebe3 100644 --- a/gin/wrappable.h +++ b/gin/wrappable.h @@ -11,91 +11,93 @@ namespace gin { -// Wrappable is an abstract base class for C++ objects that have cooresponding -// v8 wrapper objects. To retain a Wrappable object on the stack, use a -// gin::Handle. -class Wrappable { - public: - // Subclasses must return the WrapperInfo object associated with the - // v8::ObjectTemplate for their subclass. When creating a v8 wrapper for - // this object, we'll look up the appropriate v8::ObjectTemplate in the - // PerIsolateData using this WrapperInfo pointer. - virtual WrapperInfo* GetWrapperInfo() = 0; - - // Subclasses much also contain a static member variable named |kWrapperInfo| - // of type WrapperInfo: - // - // static WrapperInfo kWrapperInfo; - // - // If |obj| is a concrete instance of the subclass, then obj->GetWrapperInfo() - // must return &kWrapperInfo. - // - // We use both the dynamic |GetWrapperInfo| function and the static - // |kWrapperInfo| member variable during wrapping and the unwrapping. During - // wrapping, we use GetWrapperInfo() to make sure we use the correct - // v8::ObjectTemplate for the object regardless of the declared C++ type. - // During unwrapping, we use the static member variable to prevent type errors - // during the downcast from Wrappable to the subclass. - - // Retrieve (or create) the v8 wrapper object cooresponding to this object. - // To customize the wrapper created for a subclass, override GetWrapperInfo() - // instead of overriding this function. - v8::Handle GetWrapper(v8::Isolate* isolate); +namespace internal { + +void* FromV8Impl(v8::Isolate* isolate, v8::Handle val, + WrapperInfo* info); + +} // namespace internal + + +// Wrappable is a base class for C++ objects that have corresponding v8 wrapper +// objects. To retain a Wrappable object on the stack, use a gin::Handle. +// +// USAGE: +// // my_class.h +// class MyClass : Wrappable { +// ... +// }; +// +// // my_class.cc +// INIT_WRAPABLE(MyClass); +// +// Subclasses should also typically have private constructors and expose a +// static Create function that returns a gin::Handle. Forcing creators through +// this static Create function will enforce that clients actually create a +// wrapper for the object. If clients fail to create a wrapper for a wrappable +// object, the object will leak because we use the weak callback from the +// wrapper as the signal to delete the wrapped object. +template +class Wrappable; - // Subclasses should have private constructors and expose a static Create - // function that returns a gin::Handle. Forcing creators through this static - // Create function will enforce that clients actually create a wrapper for - // the object. If clients fail to create a wrapper for a wrappable object, - // the object will leak because we use the weak callback from the wrapper - // as the signal to delete the wrapped object. +// Non-template base class to share code between templates instances. +class WrappableBase { protected: - Wrappable(); - virtual ~Wrappable(); + WrappableBase(); + ~WrappableBase(); + v8::Handle GetWrapperImpl(v8::Isolate* isolate, + WrapperInfo* wrapper_info); + v8::Handle CreateWrapper(v8::Isolate* isolate, + WrapperInfo* wrapper_info); + v8::Persistent wrapper_; // Weak private: static void WeakCallback( - const v8::WeakCallbackData& data); - v8::Handle CreateWrapper(v8::Isolate* isolate); + const v8::WeakCallbackData& data); - v8::Persistent wrapper_; // Weak + DISALLOW_COPY_AND_ASSIGN(WrappableBase); +}; + + +template +class Wrappable : public WrappableBase { + public: + static WrapperInfo kWrapperInfo; + // Retrieve (or create) the v8 wrapper object cooresponding to this object. + // To customize the wrapper created for a subclass, override GetWrapperInfo() + // instead of overriding this function. + v8::Handle GetWrapper(v8::Isolate* isolate) { + return GetWrapperImpl(isolate, &kWrapperInfo); + } + + protected: + Wrappable() {} + ~Wrappable() {} DISALLOW_COPY_AND_ASSIGN(Wrappable); }; + +// Subclasses of Wrappable must call this within a cc file to initialize their +// WrapperInfo. +#define INIT_WRAPPABLE(TYPE) \ +template<> \ +gin::WrapperInfo gin::Wrappable::kWrapperInfo = { kEmbedderNativeGin }; + + // This converter handles any subclass of Wrappable. template struct Converter::value>::type> { + base::is_convertible*>::value>::type> { static v8::Handle ToV8(v8::Isolate* isolate, T* val) { return val->GetWrapper(isolate); } static bool FromV8(v8::Isolate* isolate, v8::Handle val, T** out) { - if (!val->IsObject()) - return false; - v8::Handle obj = v8::Handle::Cast(val); - WrapperInfo* info = WrapperInfo::From(obj); - - // If this fails, the object is not managed by Gin. It is either a normal JS - // object that's not wrapping any external C++ object, or it is wrapping - // some C++ object, but that object isn't managed by Gin (maybe Blink). - if (!info) - return false; - - // If this fails, the object is managed by Gin, but it's not wrapping an - // instance of T. - if (info != &T::kWrapperInfo) - return false; - - void* pointer = obj->GetAlignedPointerFromInternalField(kEncodedValueIndex); - T* result = static_cast(pointer); - - // If this fails, something fishy is going on. |info| should have come from - // T::GetWrapperInfo(), but somehow is now different than it. So panic. - CHECK(result->GetWrapperInfo() == info); - *out = result; - return true; + *out = static_cast(internal::FromV8Impl(isolate, val, + &T::kWrapperInfo)); + return *out != NULL; } }; diff --git a/gin/wrappable_unittest.cc b/gin/wrappable_unittest.cc index 8886a79..b2b859f 100644 --- a/gin/wrappable_unittest.cc +++ b/gin/wrappable_unittest.cc @@ -14,65 +14,32 @@ #include "testing/gtest/include/gtest/gtest.h" namespace gin { -namespace { -class MyObject : public Wrappable { +class MyObject : public Wrappable { public: - static gin::Handle Create(v8::Isolate* isolate); + static gin::Handle Create(v8::Isolate* isolate) { + return CreateHandle(isolate, new MyObject()); + } int value() const { return value_; } void set_value(int value) { value_ = value; } - static WrapperInfo kWrapperInfo; - virtual WrapperInfo* GetWrapperInfo() OVERRIDE; - private: MyObject() : value_(0) {} - virtual ~MyObject() {} + ~MyObject() {} int value_; }; -WrapperInfo MyObject::kWrapperInfo = { kEmbedderNativeGin }; - -gin::Handle MyObject::Create(v8::Isolate* isolate) { - return CreateHandle(isolate, new MyObject()); -} - -WrapperInfo* MyObject::GetWrapperInfo() { - return &kWrapperInfo; -} - - -class MyObject2 : public Wrappable { - public: - MyObject2() { - } - static WrapperInfo kWrapperInfo; - virtual WrapperInfo* GetWrapperInfo() OVERRIDE; +class MyObject2 : public Wrappable { }; -WrapperInfo MyObject2::kWrapperInfo = { kEmbedderNativeGin }; - -WrapperInfo* MyObject2::GetWrapperInfo() { - return &kWrapperInfo; -} - - -class MyObjectBlink : public Wrappable { - public: - MyObjectBlink() { - } - static WrapperInfo kWrapperInfo; - virtual WrapperInfo* GetWrapperInfo() OVERRIDE; +class MyObjectBlink : public Wrappable { }; -WrapperInfo MyObjectBlink::kWrapperInfo = { kEmbedderBlink }; - -WrapperInfo* MyObjectBlink::GetWrapperInfo() { - return &kWrapperInfo; -} - +INIT_WRAPPABLE(gin::MyObject); +INIT_WRAPPABLE(gin::MyObject2); +INIT_WRAPPABLE(gin::MyObjectBlink); void RegisterTemplates(v8::Isolate* isolate) { PerIsolateData* data = PerIsolateData::From(isolate); @@ -173,5 +140,4 @@ TEST_F(WrappableTest, GetAndSetProperty) { EXPECT_EQ(191, obj->value()); } -} // namespace } // namespace gin -- cgit v1.1