Skip to content

Commit

Permalink
[gin] Introduce Wrappable::GetObjectTemplate
Browse files Browse the repository at this point in the history
Instead of explicitly registering object templates for all wrapper
infos, add a method on wrappable that returns the template when
needed.

BUG=none
R=aa@chromium.org,abarth@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@241370 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
jochen@chromium.org committed Dec 17, 2013
1 parent 22c17bc commit cb6c2bb
Show file tree
Hide file tree
Showing 19 changed files with 87 additions and 140 deletions.
10 changes: 0 additions & 10 deletions content/renderer/gin_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,6 @@ class TestGinObject : public gin::Wrappable<TestGinObject> {

gin::WrapperInfo TestGinObject::kWrapperInfo = { gin::kEmbedderNativeGin };

void RegisterTemplates(v8::Isolate* isolate) {
gin::PerIsolateData* data = gin::PerIsolateData::From(isolate);

v8::Handle<v8::ObjectTemplate> templ = v8::ObjectTemplate::New(isolate);
templ->SetInternalFieldCount(gin::kNumberOfInternalFields);
data->SetObjectTemplate(&TestGinObject::kWrapperInfo, templ);
}

class GinBrowserTest : public RenderViewTest {
public:
GinBrowserTest() {}
Expand Down Expand Up @@ -75,8 +67,6 @@ TEST_F(GinBrowserTest, GinAndGarbageCollection) {
v8::Context::Scope context_scope(
view_->GetWebView()->mainFrame()->mainWorldScriptContext());

RegisterTemplates(isolate);

// We create the object inside a scope so it's not kept alive by a handle
// on the stack.
TestGinObject::Create(isolate, &alive);
Expand Down
27 changes: 11 additions & 16 deletions content/renderer/stats_collection_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
#include "content/renderer/render_view_impl.h"
#include "gin/handle.h"
#include "gin/object_template_builder.h"
#include "gin/per_isolate_data.h"
#include "third_party/WebKit/public/web/WebFrame.h"
#include "third_party/WebKit/public/web/WebKit.h"
#include "third_party/WebKit/public/web/WebView.h"
Expand Down Expand Up @@ -75,6 +74,17 @@ gin::WrapperInfo StatsCollectionController::kWrapperInfo = {
gin::kEmbedderNativeGin
};

// static
v8::Local<v8::ObjectTemplate> StatsCollectionController::GetObjectTemplate(
v8::Isolate* isolate) {
return gin::ObjectTemplateBuilder(isolate)
.SetMethod("getHistogram", &StatsCollectionController::GetHistogram)
.SetMethod("getBrowserHistogram",
&StatsCollectionController::GetBrowserHistogram)
.SetMethod("tabLoadTiming", &StatsCollectionController::GetTabLoadTiming)
.Build();
}

// static
void StatsCollectionController::Install(blink::WebFrame* frame) {
v8::Isolate* isolate = blink::mainThreadIsolate();
Expand All @@ -85,21 +95,6 @@ void StatsCollectionController::Install(blink::WebFrame* frame) {

v8::Context::Scope context_scope(context);

gin::PerIsolateData* data = gin::PerIsolateData::From(isolate);
if (data->GetObjectTemplate(&StatsCollectionController::kWrapperInfo)
.IsEmpty()) {
v8::Handle<v8::ObjectTemplate> templ =
gin::ObjectTemplateBuilder(isolate)
.SetMethod("getHistogram", &StatsCollectionController::GetHistogram)
.SetMethod("getBrowserHistogram",
&StatsCollectionController::GetBrowserHistogram)
.SetMethod("tabLoadTiming",
&StatsCollectionController::GetTabLoadTiming)
.Build();
templ->SetInternalFieldCount(gin::kNumberOfInternalFields);
data->SetObjectTemplate(&StatsCollectionController::kWrapperInfo, templ);
}

gin::Handle<StatsCollectionController> controller =
gin::CreateHandle(isolate, new StatsCollectionController());
v8::Handle<v8::Object> global = context->Global();
Expand Down
1 change: 1 addition & 0 deletions content/renderer/stats_collection_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ class StatsCollectionController
: public gin::Wrappable<StatsCollectionController> {
public:
static gin::WrapperInfo kWrapperInfo;
static v8::Local<v8::ObjectTemplate> GetObjectTemplate(v8::Isolate* isolate);

static void Install(blink::WebFrame* frame);

Expand Down
17 changes: 3 additions & 14 deletions gin/function_template.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,12 @@

#include "gin/function_template.h"

#include "gin/per_isolate_data.h"

namespace gin {

WrapperInfo internal::CallbackHolderBase::kWrapperInfo = { kEmbedderNativeGin };
namespace internal {

void InitFunctionTemplates(PerIsolateData* isolate_data) {
if (!isolate_data->GetObjectTemplate(
&internal::CallbackHolderBase::kWrapperInfo).IsEmpty()) {
return;
}
WrapperInfo CallbackHolderBase::kWrapperInfo = { kEmbedderNativeGin };

v8::Handle<v8::ObjectTemplate> templ(
v8::ObjectTemplate::New(isolate_data->isolate()));
templ->SetInternalFieldCount(kNumberOfInternalFields);
isolate_data->SetObjectTemplate(&internal::CallbackHolderBase::kWrapperInfo,
templ);
}
} // namespace internal

} // namespace gin
5 changes: 0 additions & 5 deletions gin/function_template.h
Original file line number Diff line number Diff line change
Expand Up @@ -331,11 +331,6 @@ struct Dispatcher<R(P1, P2, P3, P4)> {
} // namespace internal


// This should be called once per-isolate to initialize the function template
// system.
GIN_EXPORT void InitFunctionTemplates(PerIsolateData* isolate_data);


// CreateFunctionTemplate creates a v8::FunctionTemplate that will create
// JavaScript functions that execute a provided C++ function or base::Callback.
// JavaScript arguments are automatically converted via gin::Converter, as is
Expand Down
6 changes: 0 additions & 6 deletions gin/function_template.h.pump
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ struct CallbackParamTraits<const T*> {
class GIN_EXPORT CallbackHolderBase : public Wrappable<CallbackHolderBase> {
public:
static WrapperInfo kWrapperInfo;

protected:
virtual ~CallbackHolderBase() {}
};
Expand Down Expand Up @@ -187,11 +186,6 @@ $for ARG [[ typename CallbackParamTraits<P$(ARG)>::LocalType a$(ARG);
} // namespace internal


// This should be called once per-isolate to initialize the function template
// system.
GIN_EXPORT void InitFunctionTemplates(PerIsolateData* isolate_data);


// CreateFunctionTemplate creates a v8::FunctionTemplate that will create
// JavaScript functions that execute a provided C++ function or base::Callback.
// JavaScript arguments are automatically converted via gin::Converter, as is
Expand Down
1 change: 0 additions & 1 deletion gin/isolate_holder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ void IsolateHolder::Init() {
v8::Isolate::Scope isolate_scope(isolate_);
v8::HandleScope handle_scope(isolate_);
isolate_data_.reset(new PerIsolateData(isolate_));
InitFunctionTemplates(isolate_data_.get());
}

} // namespace gin
2 changes: 2 additions & 0 deletions gin/object_template_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@
// found in the LICENSE file.

#include "gin/object_template_builder.h"
#include "gin/public/wrapper_info.h"

namespace gin {

ObjectTemplateBuilder::ObjectTemplateBuilder(v8::Isolate* isolate)
: isolate_(isolate), template_(v8::ObjectTemplate::New(isolate)) {
template_->SetInternalFieldCount(kNumberOfInternalFields);
}

ObjectTemplateBuilder::~ObjectTemplateBuilder() {
Expand Down
24 changes: 19 additions & 5 deletions gin/wrappable.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "gin/wrappable.h"

#include "base/logging.h"
#include "gin/object_template_builder.h"
#include "gin/per_isolate_data.h"

namespace gin {
Expand All @@ -17,24 +18,37 @@ WrappableBase::~WrappableBase() {
}

v8::Handle<v8::Object> WrappableBase::GetWrapperImpl(
v8::Isolate* isolate, WrapperInfo* wrapper_info) {
v8::Isolate* isolate,
WrapperInfo* wrapper_info,
GetObjectTemplateFunction template_getter) {
if (wrapper_.IsEmpty())
CreateWrapper(isolate, wrapper_info);
CreateWrapper(isolate, wrapper_info, template_getter);
return v8::Local<v8::Object>::New(isolate, wrapper_);
}

v8::Local<v8::ObjectTemplate> WrappableBase::GetObjectTemplate(
v8::Isolate* isolate) {
return ObjectTemplateBuilder(isolate).Build();
}

void WrappableBase::WeakCallback(
const v8::WeakCallbackData<v8::Object, WrappableBase>& data) {
WrappableBase* wrappable = data.GetParameter();
wrappable->wrapper_.Reset();
delete wrappable;
}

v8::Handle<v8::Object> WrappableBase::CreateWrapper(v8::Isolate* isolate,
WrapperInfo* info) {
v8::Handle<v8::Object> WrappableBase::CreateWrapper(
v8::Isolate* isolate,
WrapperInfo* info,
GetObjectTemplateFunction template_getter) {
PerIsolateData* data = PerIsolateData::From(isolate);
v8::Local<v8::ObjectTemplate> templ = data->GetObjectTemplate(info);
CHECK(!templ.IsEmpty()); // Don't forget to register an object template.
if (templ.IsEmpty()) {
templ = template_getter(isolate);
CHECK(!templ.IsEmpty());
data->SetObjectTemplate(info, templ);
}
CHECK_EQ(kNumberOfInternalFields, templ->InternalFieldCount());
v8::Handle<v8::Object> wrapper = templ->NewInstance();
wrapper->SetAlignedPointerInInternalField(kWrapperInfoIndex, info);
Expand Down
37 changes: 30 additions & 7 deletions gin/wrappable.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,22 @@ GIN_EXPORT void* FromV8Impl(v8::Isolate* isolate,
// USAGE:
// // my_class.h
// class MyClass : Wrappable<MyClass> {
// public:
// static WrapperInfo kWrapperInfo;
//
// // Optional, only required if non-empty template should be used.
// static v8::Local<v8::ObjectTemplate> GetObjectTemplate(
// v8::Isolate* isolate);
// ...
// };
//
// // my_class.cc
// INIT_WRAPABLE(MyClass);
// WrapperInfo MyClass::kWrapperInfo = { kEmbedderNativeGin };
//
// v8::Local<v8::ObjectTemplate> MyClass::GetObjectTemplate(
// v8::Isolate* isolate) {
// return ObjectTemplateBuilder(isolate).SetValue("foobar", 42).Build();
// }
//
// Subclasses should also typically have private constructors and expose a
// static Create function that returns a gin::Handle. Forcing creators through
Expand All @@ -46,18 +57,30 @@ class Wrappable;
// Non-template base class to share code between templates instances.
class GIN_EXPORT WrappableBase {
protected:
typedef v8::Local<v8::ObjectTemplate>(*GetObjectTemplateFunction)(
v8::Isolate*);

WrappableBase();
virtual ~WrappableBase();
v8::Handle<v8::Object> GetWrapperImpl(v8::Isolate* isolate,
WrapperInfo* wrapper_info);
v8::Handle<v8::Object> CreateWrapper(v8::Isolate* isolate,
WrapperInfo* wrapper_info);
v8::Persistent<v8::Object> wrapper_; // Weak

v8::Handle<v8::Object> GetWrapperImpl(
v8::Isolate* isolate,
WrapperInfo* wrapper_info,
GetObjectTemplateFunction template_getter);

static v8::Local<v8::ObjectTemplate> GetObjectTemplate(v8::Isolate* isolate);

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

v8::Handle<v8::Object> CreateWrapper(
v8::Isolate* isolate,
WrapperInfo* wrapper_info,
GetObjectTemplateFunction template_getter);

v8::Persistent<v8::Object> wrapper_; // Weak

DISALLOW_COPY_AND_ASSIGN(WrappableBase);
};

Expand All @@ -69,7 +92,7 @@ class Wrappable : public WrappableBase {
// To customize the wrapper created for a subclass, override GetWrapperInfo()
// instead of overriding this function.
v8::Handle<v8::Object> GetWrapper(v8::Isolate* isolate) {
return GetWrapperImpl(isolate, &T::kWrapperInfo);
return GetWrapperImpl(isolate, &T::kWrapperInfo, &T::GetObjectTemplate);
}

protected:
Expand Down
30 changes: 8 additions & 22 deletions gin/wrappable_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ class MyObject : public Wrappable<MyObject> {
public:
static WrapperInfo kWrapperInfo;

static v8::Local<v8::ObjectTemplate> GetObjectTemplate(v8::Isolate* isolate);

static gin::Handle<MyObject> Create(v8::Isolate* isolate) {
return CreateHandle(isolate, new MyObject());
}
Expand All @@ -44,35 +46,22 @@ class MyObjectBlink : public Wrappable<MyObjectBlink> {
};

WrapperInfo MyObject::kWrapperInfo = { kEmbedderNativeGin };
WrapperInfo MyObject2::kWrapperInfo = { kEmbedderNativeGin };
WrapperInfo MyObjectBlink::kWrapperInfo = { kEmbedderNativeGin };

void RegisterTemplates(v8::Isolate* isolate) {
PerIsolateData* data = PerIsolateData::From(isolate);
DCHECK(data->GetObjectTemplate(&MyObject::kWrapperInfo).IsEmpty());

v8::Handle<v8::ObjectTemplate> templ = ObjectTemplateBuilder(isolate)
v8::Local<v8::ObjectTemplate> MyObject::GetObjectTemplate(
v8::Isolate* isolate) {
return ObjectTemplateBuilder(isolate)
.SetProperty("value", &MyObject::value, &MyObject::set_value)
.Build();
templ->SetInternalFieldCount(kNumberOfInternalFields);
data->SetObjectTemplate(&MyObject::kWrapperInfo, templ);

templ = v8::ObjectTemplate::New(isolate);
templ->SetInternalFieldCount(kNumberOfInternalFields);
data->SetObjectTemplate(&MyObject2::kWrapperInfo, templ);

templ = v8::ObjectTemplate::New(isolate);
templ->SetInternalFieldCount(kNumberOfInternalFields);
data->SetObjectTemplate(&MyObjectBlink::kWrapperInfo, templ);
}

WrapperInfo MyObject2::kWrapperInfo = { kEmbedderNativeGin };
WrapperInfo MyObjectBlink::kWrapperInfo = { kEmbedderNativeGin };

typedef V8Test WrappableTest;

TEST_F(WrappableTest, WrapAndUnwrap) {
v8::Isolate* isolate = instance_->isolate();
v8::HandleScope handle_scope(isolate);

RegisterTemplates(isolate);
Handle<MyObject> obj = MyObject::Create(isolate);

v8::Handle<v8::Value> wrapper = ConvertToV8(isolate, obj.get());
Expand All @@ -87,8 +76,6 @@ TEST_F(WrappableTest, UnwrapFailures) {
v8::Isolate* isolate = instance_->isolate();
v8::HandleScope handle_scope(isolate);

RegisterTemplates(isolate);

// Something that isn't an object.
v8::Handle<v8::Value> thing = v8::Number::New(42);
MyObject* unwrapped = NULL;
Expand Down Expand Up @@ -117,7 +104,6 @@ TEST_F(WrappableTest, GetAndSetProperty) {
v8::Isolate* isolate = instance_->isolate();
v8::HandleScope handle_scope(isolate);

RegisterTemplates(isolate);
gin::Handle<MyObject> obj = MyObject::Create(isolate);

obj->set_value(42);
Expand Down
22 changes: 7 additions & 15 deletions mojo/apps/js/bindings/gl/context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

#include "gin/arguments.h"
#include "gin/object_template_builder.h"
#include "gin/per_isolate_data.h"
#include "mojo/public/gles2/gles2.h"

namespace mojo {
Expand All @@ -22,21 +21,14 @@ gin::Handle<Context> Context::Create(v8::Isolate* isolate, uint64_t encoded,
return gin::CreateHandle(isolate, new Context(encoded, width, height));
}

v8::Handle<v8::ObjectTemplate> Context::GetObjectTemplate(
v8::Local<v8::ObjectTemplate> Context::GetObjectTemplate(
v8::Isolate* isolate) {
gin::PerIsolateData* data = gin::PerIsolateData::From(isolate);
v8::Local<v8::ObjectTemplate> templ = data->GetObjectTemplate(&kWrapperInfo);
if (templ.IsEmpty()) {
templ = gin::ObjectTemplateBuilder(isolate)
.SetValue("VERTEX_SHADER", GL_VERTEX_SHADER)
.SetMethod("createShader", CreateShader)
.SetMethod("shaderSource", ShaderSource)
.SetMethod("compileShader", CompileShader)
.Build();
templ->SetInternalFieldCount(gin::kNumberOfInternalFields);
data->SetObjectTemplate(&kWrapperInfo, templ);
}
return templ;
return gin::ObjectTemplateBuilder(isolate)
.SetValue("VERTEX_SHADER", GL_VERTEX_SHADER)
.SetMethod("createShader", CreateShader)
.SetMethod("shaderSource", ShaderSource)
.SetMethod("compileShader", CompileShader)
.Build();
}

gin::Handle<Shader> Context::CreateShader(const gin::Arguments& args,
Expand Down
4 changes: 2 additions & 2 deletions mojo/apps/js/bindings/gl/context.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ class Context : public gin::Wrappable<Context> {
public:
static gin::WrapperInfo kWrapperInfo;

static v8::Local<v8::ObjectTemplate> GetObjectTemplate(v8::Isolate* isolate);

static gin::Handle<Context> Create(v8::Isolate* isolate, uint64_t encoded,
int width, int height);
static v8::Handle<v8::ObjectTemplate> GetObjectTemplate(v8::Isolate* isolate);

static gin::Handle<Shader> CreateShader(const gin::Arguments& arguments,
GLenum type);
static void ShaderSource(gin::Handle<Shader> shader,
Expand Down
Loading

0 comments on commit cb6c2bb

Please sign in to comment.