Skip to content

Commit

Permalink
gin: Only use primitive types for creating function templates
Browse files Browse the repository at this point in the history 
Otherwise, the function template will keep the context alive it was
created in.

BUG=342272
R=dcarney@chromium.org,aa@chromium.org,haraken@chromium.org

Review URL: https://codereview.chromium.org/141143012

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@250412 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
jochen@chromium.org committed Feb 11, 2014
1 parent 58e576f commit bf01429
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 56 deletions.
20 changes: 19 additions & 1 deletion gin/function_template.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,25 @@ namespace gin {

namespace internal {

WrapperInfo CallbackHolderBase::kWrapperInfo = { kEmbedderNativeGin };
CallbackHolderBase::CallbackHolderBase(v8::Isolate* isolate)
: v8_ref_(isolate, v8::External::New(isolate, this)) {
v8_ref_.SetWeak(this, &CallbackHolderBase::WeakCallback);
}

CallbackHolderBase::~CallbackHolderBase() {
DCHECK(v8_ref_.IsEmpty());
}

v8::Handle<v8::External> CallbackHolderBase::GetHandle(v8::Isolate* isolate) {
return v8::Local<v8::External>::New(isolate, v8_ref_);
}

// static
void CallbackHolderBase::WeakCallback(
const v8::WeakCallbackData<v8::External, CallbackHolderBase>& data) {
data.GetParameter()->v8_ref_.Reset();
delete data.GetParameter();
}

} // namespace internal

Expand Down
84 changes: 51 additions & 33 deletions gin/function_template.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,6 @@
#include "gin/arguments.h"
#include "gin/converter.h"
#include "gin/gin_export.h"
#include "gin/handle.h"
#include "gin/public/gin_embedders.h"
#include "gin/public/wrapper_info.h"
#include "gin/wrappable.h"

#include "v8/include/v8.h"

namespace gin {
Expand Down Expand Up @@ -50,31 +45,39 @@ struct CallbackParamTraits<const T*> {
// CallbackHolder and CallbackHolderBase are used to pass a base::Callback from
// CreateFunctionTemplate through v8 (via v8::FunctionTemplate) to
// DispatchToCallback, where it is invoked.
//
// v8::FunctionTemplate only supports passing void* as data so how do we know
// when to delete the base::Callback? That's where CallbackHolderBase comes in.
// 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.

// This simple base class is used so that we can share a single object template
// among every CallbackHolder instance.
class GIN_EXPORT CallbackHolderBase : public Wrappable<CallbackHolderBase> {
class GIN_EXPORT CallbackHolderBase {
public:
static WrapperInfo kWrapperInfo;
v8::Handle<v8::External> GetHandle(v8::Isolate* isolate);

protected:
virtual ~CallbackHolderBase() {}
explicit CallbackHolderBase(v8::Isolate* isolate);
virtual ~CallbackHolderBase();

private:
static void WeakCallback(
const v8::WeakCallbackData<v8::External, CallbackHolderBase>& data);

v8::Persistent<v8::External> v8_ref_;

DISALLOW_COPY_AND_ASSIGN(CallbackHolderBase);
};

template<typename Sig>
class CallbackHolder : public CallbackHolderBase {
public:
CallbackHolder(const base::Callback<Sig>& callback, int flags)
: callback(callback), flags(flags) {}
CallbackHolder(v8::Isolate* isolate,
const base::Callback<Sig>& callback,
int flags)
: CallbackHolderBase(isolate), callback(callback), flags(flags) {}
base::Callback<Sig> callback;
int flags;
private:
virtual ~CallbackHolder() {}

DISALLOW_COPY_AND_ASSIGN(CallbackHolder);
};


Expand Down Expand Up @@ -293,8 +296,10 @@ struct Dispatcher<R()> {
static void DispatchToCallback(
const v8::FunctionCallbackInfo<v8::Value>& info) {
Arguments args(info);
CallbackHolderBase* holder_base = NULL;
CHECK(args.GetData(&holder_base));
v8::Handle<v8::External> v8_holder;
CHECK(args.GetData(&v8_holder));
CallbackHolderBase* holder_base = reinterpret_cast<CallbackHolderBase*>(
v8_holder->Value());

typedef CallbackHolder<R()> HolderT;
HolderT* holder = static_cast<HolderT*>(holder_base);
Expand All @@ -308,8 +313,10 @@ struct Dispatcher<R(P1)> {
static void DispatchToCallback(
const v8::FunctionCallbackInfo<v8::Value>& info) {
Arguments args(info);
CallbackHolderBase* holder_base = NULL;
CHECK(args.GetData(&holder_base));
v8::Handle<v8::External> v8_holder;
CHECK(args.GetData(&v8_holder));
CallbackHolderBase* holder_base = reinterpret_cast<CallbackHolderBase*>(
v8_holder->Value());

typedef CallbackHolder<R(P1)> HolderT;
HolderT* holder = static_cast<HolderT*>(holder_base);
Expand All @@ -329,8 +336,10 @@ struct Dispatcher<R(P1, P2)> {
static void DispatchToCallback(
const v8::FunctionCallbackInfo<v8::Value>& info) {
Arguments args(info);
CallbackHolderBase* holder_base = NULL;
CHECK(args.GetData(&holder_base));
v8::Handle<v8::External> v8_holder;
CHECK(args.GetData(&v8_holder));
CallbackHolderBase* holder_base = reinterpret_cast<CallbackHolderBase*>(
v8_holder->Value());

typedef CallbackHolder<R(P1, P2)> HolderT;
HolderT* holder = static_cast<HolderT*>(holder_base);
Expand All @@ -352,8 +361,10 @@ struct Dispatcher<R(P1, P2, P3)> {
static void DispatchToCallback(
const v8::FunctionCallbackInfo<v8::Value>& info) {
Arguments args(info);
CallbackHolderBase* holder_base = NULL;
CHECK(args.GetData(&holder_base));
v8::Handle<v8::External> v8_holder;
CHECK(args.GetData(&v8_holder));
CallbackHolderBase* holder_base = reinterpret_cast<CallbackHolderBase*>(
v8_holder->Value());

typedef CallbackHolder<R(P1, P2, P3)> HolderT;
HolderT* holder = static_cast<HolderT*>(holder_base);
Expand All @@ -377,8 +388,10 @@ struct Dispatcher<R(P1, P2, P3, P4)> {
static void DispatchToCallback(
const v8::FunctionCallbackInfo<v8::Value>& info) {
Arguments args(info);
CallbackHolderBase* holder_base = NULL;
CHECK(args.GetData(&holder_base));
v8::Handle<v8::External> v8_holder;
CHECK(args.GetData(&v8_holder));
CallbackHolderBase* holder_base = reinterpret_cast<CallbackHolderBase*>(
v8_holder->Value());

typedef CallbackHolder<R(P1, P2, P3, P4)> HolderT;
HolderT* holder = static_cast<HolderT*>(holder_base);
Expand All @@ -405,8 +418,10 @@ struct Dispatcher<R(P1, P2, P3, P4, P5)> {
static void DispatchToCallback(
const v8::FunctionCallbackInfo<v8::Value>& info) {
Arguments args(info);
CallbackHolderBase* holder_base = NULL;
CHECK(args.GetData(&holder_base));
v8::Handle<v8::External> v8_holder;
CHECK(args.GetData(&v8_holder));
CallbackHolderBase* holder_base = reinterpret_cast<CallbackHolderBase*>(
v8_holder->Value());

typedef CallbackHolder<R(P1, P2, P3, P4, P5)> HolderT;
HolderT* holder = static_cast<HolderT*>(holder_base);
Expand Down Expand Up @@ -436,8 +451,10 @@ struct Dispatcher<R(P1, P2, P3, P4, P5, P6)> {
static void DispatchToCallback(
const v8::FunctionCallbackInfo<v8::Value>& info) {
Arguments args(info);
CallbackHolderBase* holder_base = NULL;
CHECK(args.GetData(&holder_base));
v8::Handle<v8::External> v8_holder;
CHECK(args.GetData(&v8_holder));
CallbackHolderBase* holder_base = reinterpret_cast<CallbackHolderBase*>(
v8_holder->Value());

typedef CallbackHolder<R(P1, P2, P3, P4, P5, P6)> HolderT;
HolderT* holder = static_cast<HolderT*>(holder_base);
Expand Down Expand Up @@ -475,12 +492,13 @@ v8::Local<v8::FunctionTemplate> CreateFunctionTemplate(
v8::Isolate* isolate, const base::Callback<Sig> callback,
int callback_flags = 0) {
typedef internal::CallbackHolder<Sig> HolderT;
gin::Handle<HolderT> holder = CreateHandle(
isolate, new HolderT(callback, callback_flags));
HolderT* holder = new HolderT(isolate, callback, callback_flags);

return v8::FunctionTemplate::New(
isolate,
&internal::Dispatcher<Sig>::DispatchToCallback,
ConvertToV8<internal::CallbackHolderBase*>(isolate, holder.get()));
ConvertToV8<v8::Handle<v8::External> >(isolate,
holder->GetHandle(isolate)));
}

} // namespace gin
Expand Down
50 changes: 28 additions & 22 deletions gin/function_template.h.pump
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,6 @@ $var MAX_ARITY = 6
#include "gin/arguments.h"
#include "gin/converter.h"
#include "gin/gin_export.h"
#include "gin/handle.h"
#include "gin/public/gin_embedders.h"
#include "gin/public/wrapper_info.h"
#include "gin/wrappable.h"

#include "v8/include/v8.h"

namespace gin {
Expand Down Expand Up @@ -51,33 +46,41 @@ struct CallbackParamTraits<const T*> {


// CallbackHolder and CallbackHolderBase are used to pass a base::Callback from
// CreateFunctionTemplate through v8 (via v8::FunctionTemplate) to
// CreateFunctionTemplate through v8 (via v8::FunctionTemplate) to
// DispatchToCallback, where it is invoked.
//
// v8::FunctionTemplate only supports passing void* as data so how do we know
// when to delete the base::Callback? That's where CallbackHolderBase comes in.
// 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.

// This simple base class is used so that we can share a single object template
// among every CallbackHolder instance.
class GIN_EXPORT CallbackHolderBase : public Wrappable<CallbackHolderBase> {
class GIN_EXPORT CallbackHolderBase {
public:
static WrapperInfo kWrapperInfo;
v8::Handle<v8::External> GetHandle(v8::Isolate* isolate);

protected:
virtual ~CallbackHolderBase() {}
explicit CallbackHolderBase(v8::Isolate* isolate);
virtual ~CallbackHolderBase();

private:
static void WeakCallback(
const v8::WeakCallbackData<v8::External, CallbackHolderBase>& data);

v8::Persistent<v8::External> v8_ref_;

DISALLOW_COPY_AND_ASSIGN(CallbackHolderBase);
};

template<typename Sig>
class CallbackHolder : public CallbackHolderBase {
public:
CallbackHolder(const base::Callback<Sig>& callback, int flags)
: callback(callback), flags(flags) {}
CallbackHolder(v8::Isolate* isolate,
const base::Callback<Sig>& callback,
int flags)
: CallbackHolderBase(isolate), callback(callback), flags(flags) {}
base::Callback<Sig> callback;
int flags;
private:
virtual ~CallbackHolder() {}

DISALLOW_COPY_AND_ASSIGN(CallbackHolder);
};


Expand Down Expand Up @@ -169,8 +172,10 @@ struct Dispatcher<R($for ARG , [[P$(ARG)]])> {
static void DispatchToCallback(
const v8::FunctionCallbackInfo<v8::Value>& info) {
Arguments args(info);
CallbackHolderBase* holder_base = NULL;
CHECK(args.GetData(&holder_base));
v8::Handle<v8::External> v8_holder;
CHECK(args.GetData(&v8_holder));
CallbackHolderBase* holder_base = reinterpret_cast<CallbackHolderBase*>(
v8_holder->Value());

typedef CallbackHolder<R($for ARG , [[P$(ARG)]])> HolderT;
HolderT* holder = static_cast<HolderT*>(holder_base);
Expand Down Expand Up @@ -207,12 +212,13 @@ v8::Local<v8::FunctionTemplate> CreateFunctionTemplate(
v8::Isolate* isolate, const base::Callback<Sig> callback,
int callback_flags = 0) {
typedef internal::CallbackHolder<Sig> HolderT;
gin::Handle<HolderT> holder = CreateHandle(
isolate, new HolderT(callback, callback_flags));
HolderT* holder = new HolderT(isolate, callback, callback_flags);

return v8::FunctionTemplate::New(
isolate,
&internal::Dispatcher<Sig>::DispatchToCallback,
ConvertToV8<internal::CallbackHolderBase*>(isolate, holder.get()));
ConvertToV8<v8::Handle<v8::External> >(isolate,
holder->GetHandle(isolate)));
}

} // namespace gin
Expand Down

0 comments on commit bf01429

Please sign in to comment.