Commit e9db984c authored by Devlin Cronin's avatar Devlin Cronin Committed by Commit Bot

[Gin] Allow ObjectTemplateBuilder and Wrappable to provide a typename

ObjectTemplateBuilder (and Wrappable) allow setting functions that are
exposed in JS and then forwarded to the type in C++. If these are C++
member functions, the context object is retrieved by looking at the
this-object in JavaScript, and trying to convert that to the underlying
C++ type (which will succeed if it was called on the V8 object created
by the Wrappable).

However, these member functions will fail if the JS function is called
on an improper this-object (including null and undefined). This means
that doing things like:

var obj = getObj();  // Some object from a Wrappable
var doFoo = obj.doFoo;
doFoo();

will fail because it's applied on an invalid this-object (undefined),
and the conversion will fail. This makes sense, but unfortunatley the
error gin throws in this case is unhelpful:
"Uncaught TypeError: Error processing argument at index -1, conversion
failure from undefined"

Instead, allow Wrappables and ObjectTemplateBuilders to specify a type
name, which will be surfaced in these errors to provide guidance to
developers. Update the error message to either include the type name or
to match the error message used in similar circumstances in blink
("Illegal invocation").

The new error messages will only be shown for failures in converting to
the context object, not for failure to convert subsequent arguments.

Bug: <File One>

Change-Id: I515a17e92992bfcb709b97455b9167b350e931f2
Reviewed-on: https://chromium-review.googlesource.com/987304
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: default avatarJeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549219}
parent 92154689
...@@ -390,14 +390,10 @@ TEST_F(APIEventHandlerTest, DifferentCallingMethods) { ...@@ -390,14 +390,10 @@ TEST_F(APIEventHandlerTest, DifferentCallingMethods) {
"})"; "})";
{ {
v8::Local<v8::Value> args[] = {event}; v8::Local<v8::Value> args[] = {event};
// TODO(devlin): This is the generic type error that gin throws. It's not
// very descriptive, nor does it match the web (which would just say e.g.
// "Illegal invocation"). Might be worth updating later.
RunFunctionAndExpectError( RunFunctionAndExpectError(
FunctionFromString(context, kAddListenerOnNull), FunctionFromString(context, kAddListenerOnNull), context, 1, args,
context, 1, args, "Uncaught TypeError: Illegal invocation: Function must be called on "
"Uncaught TypeError: Error processing argument at index -1," "an object of type Event");
" conversion failure from undefined");
} }
EXPECT_EQ(0u, handler()->GetNumEventListenersForTesting(kEventName, context)); EXPECT_EQ(0u, handler()->GetNumEventListenersForTesting(kEventName, context));
......
...@@ -20,6 +20,7 @@ namespace { ...@@ -20,6 +20,7 @@ namespace {
constexpr const char kEmitterKey[] = "emitter"; constexpr const char kEmitterKey[] = "emitter";
constexpr const char kArgumentsKey[] = "arguments"; constexpr const char kArgumentsKey[] = "arguments";
constexpr const char kFilterKey[] = "filter"; constexpr const char kFilterKey[] = "filter";
constexpr const char kEventEmitterTypeName[] = "Event";
} // namespace } // namespace
...@@ -48,6 +49,10 @@ gin::ObjectTemplateBuilder EventEmitter::GetObjectTemplateBuilder( ...@@ -48,6 +49,10 @@ gin::ObjectTemplateBuilder EventEmitter::GetObjectTemplateBuilder(
.SetMethod("dispatch", &EventEmitter::Dispatch); .SetMethod("dispatch", &EventEmitter::Dispatch);
} }
const char* EventEmitter::GetTypeName() {
return kEventEmitterTypeName;
}
void EventEmitter::Fire(v8::Local<v8::Context> context, void EventEmitter::Fire(v8::Local<v8::Context> context,
std::vector<v8::Local<v8::Value>>* args, std::vector<v8::Local<v8::Value>>* args,
const EventFilteringInfo* filter, const EventFilteringInfo* filter,
......
...@@ -36,6 +36,7 @@ class EventEmitter final : public gin::Wrappable<EventEmitter> { ...@@ -36,6 +36,7 @@ class EventEmitter final : public gin::Wrappable<EventEmitter> {
// gin::Wrappable: // gin::Wrappable:
gin::ObjectTemplateBuilder GetObjectTemplateBuilder( gin::ObjectTemplateBuilder GetObjectTemplateBuilder(
v8::Isolate* isolate) final; v8::Isolate* isolate) final;
const char* GetTypeName() final;
// Fires the event to any listeners. // Fires the event to any listeners.
// Warning: This can run arbitrary JS code, so the |context| may be // Warning: This can run arbitrary JS code, so the |context| may be
......
...@@ -4,6 +4,8 @@ ...@@ -4,6 +4,8 @@
#include "gin/function_template.h" #include "gin/function_template.h"
#include "base/strings/strcat.h"
namespace gin { namespace gin {
namespace internal { namespace internal {
...@@ -35,6 +37,30 @@ void CallbackHolderBase::SecondWeakCallback( ...@@ -35,6 +37,30 @@ void CallbackHolderBase::SecondWeakCallback(
delete data.GetParameter(); delete data.GetParameter();
} }
void ThrowConversionError(Arguments* args,
const InvokerOptions& invoker_options,
size_t index) {
if (index == 0 && invoker_options.holder_is_first_argument) {
// Failed to get the appropriate `this` object. This can happen if a
// method is invoked using Function.prototype.[call|apply] and passed an
// invalid (or null) `this` argument.
std::string error =
invoker_options.holder_type
? base::StrCat({"Illegal invocation: Function must be "
"called on an object of type ",
invoker_options.holder_type})
: "Illegal invocation";
args->ThrowTypeError(error);
} else {
// Otherwise, this failed parsing on a different argument.
// Arguments::ThrowError() will try to include appropriate information.
// Ideally we would include the expected c++ type in the error message
// here, too (which we can access via typeid(ArgType).name()), however we
// compile with no-rtti, which disables typeid.
args->ThrowError();
}
}
} // namespace internal } // namespace internal
} // namespace gin } // namespace gin
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "base/callback.h" #include "base/callback.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/strings/strcat.h"
#include "gin/arguments.h" #include "gin/arguments.h"
#include "gin/converter.h" #include "gin/converter.h"
#include "gin/gin_export.h" #include "gin/gin_export.h"
...@@ -17,8 +18,9 @@ ...@@ -17,8 +18,9 @@
namespace gin { namespace gin {
enum CreateFunctionTemplateFlags { struct InvokerOptions {
HolderIsFirstArgument = 1 << 0, bool holder_is_first_argument = false;
const char* holder_type = nullptr; // Null if unknown or not applicable.
}; };
namespace internal { namespace internal {
...@@ -66,22 +68,26 @@ class CallbackHolder : public CallbackHolderBase { ...@@ -66,22 +68,26 @@ class CallbackHolder : public CallbackHolderBase {
public: public:
CallbackHolder(v8::Isolate* isolate, CallbackHolder(v8::Isolate* isolate,
base::RepeatingCallback<Sig> callback, base::RepeatingCallback<Sig> callback,
int flags) InvokerOptions invoker_options)
: CallbackHolderBase(isolate), : CallbackHolderBase(isolate),
callback(std::move(callback)), callback(std::move(callback)),
flags(flags) {} invoker_options(std::move(invoker_options)) {}
base::RepeatingCallback<Sig> callback; base::RepeatingCallback<Sig> callback;
int flags; InvokerOptions invoker_options;
private: private:
virtual ~CallbackHolder() {} virtual ~CallbackHolder() {}
DISALLOW_COPY_AND_ASSIGN(CallbackHolder); DISALLOW_COPY_AND_ASSIGN(CallbackHolder);
}; };
template<typename T> template <typename T>
bool GetNextArgument(Arguments* args, int create_flags, bool is_first, bool GetNextArgument(Arguments* args,
const InvokerOptions& invoker_options,
bool is_first,
T* result) { T* result) {
if (is_first && (create_flags & HolderIsFirstArgument) != 0) { if (is_first && invoker_options.holder_is_first_argument) {
return args->GetHolder(result); return args->GetHolder(result);
} else { } else {
return args->GetNext(result); return args->GetNext(result);
...@@ -90,24 +96,35 @@ bool GetNextArgument(Arguments* args, int create_flags, bool is_first, ...@@ -90,24 +96,35 @@ bool GetNextArgument(Arguments* args, int create_flags, bool is_first,
// For advanced use cases, we allow callers to request the unparsed Arguments // For advanced use cases, we allow callers to request the unparsed Arguments
// object and poke around in it directly. // object and poke around in it directly.
inline bool GetNextArgument(Arguments* args, int create_flags, bool is_first, inline bool GetNextArgument(Arguments* args,
const InvokerOptions& invoker_options,
bool is_first,
Arguments* result) { Arguments* result) {
*result = *args; *result = *args;
return true; return true;
} }
inline bool GetNextArgument(Arguments* args, int create_flags, bool is_first, inline bool GetNextArgument(Arguments* args,
const InvokerOptions& invoker_options,
bool is_first,
Arguments** result) { Arguments** result) {
*result = args; *result = args;
return true; return true;
} }
// It's common for clients to just need the isolate, so we make that easy. // It's common for clients to just need the isolate, so we make that easy.
inline bool GetNextArgument(Arguments* args, int create_flags, inline bool GetNextArgument(Arguments* args,
bool is_first, v8::Isolate** result) { const InvokerOptions& invoker_options,
bool is_first,
v8::Isolate** result) {
*result = args->isolate(); *result = args->isolate();
return true; return true;
} }
// Throws an error indicating conversion failure.
GIN_EXPORT void ThrowConversionError(Arguments* args,
const InvokerOptions& invoker_options,
size_t index);
// Class template for extracting and storing single argument for callback // Class template for extracting and storing single argument for callback
// at position |index|. // at position |index|.
template <size_t index, typename ArgType> template <size_t index, typename ArgType>
...@@ -117,14 +134,10 @@ struct ArgumentHolder { ...@@ -117,14 +134,10 @@ struct ArgumentHolder {
ArgLocalType value; ArgLocalType value;
bool ok; bool ok;
ArgumentHolder(Arguments* args, int create_flags) ArgumentHolder(Arguments* args, const InvokerOptions& invoker_options)
: ok(GetNextArgument(args, create_flags, index == 0, &value)) { : ok(GetNextArgument(args, invoker_options, index == 0, &value)) {
if (!ok) { if (!ok)
// Ideally we would include the expected c++ type in the error ThrowConversionError(args, invoker_options, index);
// message which we can access via typeid(ArgType).name()
// however we compile with no-rtti, which disables typeid.
args->ThrowError();
}
} }
}; };
...@@ -141,9 +154,9 @@ class Invoker<std::index_sequence<indices...>, ArgTypes...> ...@@ -141,9 +154,9 @@ class Invoker<std::index_sequence<indices...>, ArgTypes...>
// C++ has always been strict about the class initialization order, // C++ has always been strict about the class initialization order,
// so it is guaranteed ArgumentHolders will be initialized (and thus, will // so it is guaranteed ArgumentHolders will be initialized (and thus, will
// extract arguments from Arguments) in the right order. // extract arguments from Arguments) in the right order.
Invoker(Arguments* args, int create_flags) Invoker(Arguments* args, const InvokerOptions& invoker_options)
: ArgumentHolder<indices, ArgTypes>(args, create_flags)..., args_(args) { : ArgumentHolder<indices, ArgTypes>(args, invoker_options)...,
} args_(args) {}
bool IsOK() { bool IsOK() {
return And(ArgumentHolder<indices, ArgTypes>::ok...); return And(ArgumentHolder<indices, ArgTypes>::ok...);
...@@ -191,7 +204,7 @@ struct Dispatcher<ReturnType(ArgTypes...)> { ...@@ -191,7 +204,7 @@ struct Dispatcher<ReturnType(ArgTypes...)> {
HolderT* holder = static_cast<HolderT*>(holder_base); HolderT* holder = static_cast<HolderT*>(holder_base);
using Indices = std::index_sequence_for<ArgTypes...>; using Indices = std::index_sequence_for<ArgTypes...>;
Invoker<Indices, ArgTypes...> invoker(&args, holder->flags); Invoker<Indices, ArgTypes...> invoker(&args, holder->invoker_options);
if (invoker.IsOK()) if (invoker.IsOK())
invoker.DispatchToCallback(holder->callback); invoker.DispatchToCallback(holder->callback);
} }
...@@ -199,11 +212,13 @@ struct Dispatcher<ReturnType(ArgTypes...)> { ...@@ -199,11 +212,13 @@ struct Dispatcher<ReturnType(ArgTypes...)> {
} // namespace internal } // namespace internal
// CreateFunctionTemplate creates a v8::FunctionTemplate that will create // CreateFunctionTemplate creates a v8::FunctionTemplate that will create
// JavaScript functions that execute a provided C++ function or base::Callback. // JavaScript functions that execute a provided C++ function or base::Callback.
// JavaScript arguments are automatically converted via gin::Converter, as is // JavaScript arguments are automatically converted via gin::Converter, as is
// the return value of the C++ function, if any. // the return value of the C++ function, if any. |invoker_options| contains
// additional parameters. If it contains a holder_type, it will be used to
// provide a useful conversion error if the holder is the first argument. If not
// provided, a generic invocation error will be used.
// //
// NOTE: V8 caches FunctionTemplates for a lifetime of a web page for its own // NOTE: V8 caches FunctionTemplates for a lifetime of a web page for its own
// internal reasons, thus it is generally a good idea to cache the template // internal reasons, thus it is generally a good idea to cache the template
...@@ -213,9 +228,10 @@ template <typename Sig> ...@@ -213,9 +228,10 @@ template <typename Sig>
v8::Local<v8::FunctionTemplate> CreateFunctionTemplate( v8::Local<v8::FunctionTemplate> CreateFunctionTemplate(
v8::Isolate* isolate, v8::Isolate* isolate,
base::RepeatingCallback<Sig> callback, base::RepeatingCallback<Sig> callback,
int callback_flags = 0) { InvokerOptions invoker_options = {}) {
typedef internal::CallbackHolder<Sig> HolderT; typedef internal::CallbackHolder<Sig> HolderT;
HolderT* holder = new HolderT(isolate, std::move(callback), callback_flags); HolderT* holder =
new HolderT(isolate, std::move(callback), std::move(invoker_options));
v8::Local<v8::FunctionTemplate> tmpl = v8::FunctionTemplate::New( v8::Local<v8::FunctionTemplate> tmpl = v8::FunctionTemplate::New(
isolate, &internal::Dispatcher<Sig>::DispatchToCallback, isolate, &internal::Dispatcher<Sig>::DispatchToCallback,
......
...@@ -115,7 +115,7 @@ class MyInterceptor : public Wrappable<MyInterceptor>, ...@@ -115,7 +115,7 @@ class MyInterceptor : public Wrappable<MyInterceptor>,
return function_template; return function_template;
function_template = CreateFunctionTemplate( function_template = CreateFunctionTemplate(
isolate, base::BindRepeating(&MyInterceptor::Call), isolate, base::BindRepeating(&MyInterceptor::Call),
HolderIsFirstArgument); InvokerOptions{true, nullptr});
template_cache_.Set(name, function_template); template_cache_.Set(name, function_template);
return function_template; return function_template;
} }
......
...@@ -141,7 +141,13 @@ void IndexedPropertyEnumerator( ...@@ -141,7 +141,13 @@ void IndexedPropertyEnumerator(
} // namespace } // namespace
ObjectTemplateBuilder::ObjectTemplateBuilder(v8::Isolate* isolate) ObjectTemplateBuilder::ObjectTemplateBuilder(v8::Isolate* isolate)
: isolate_(isolate), template_(v8::ObjectTemplate::New(isolate)) { : ObjectTemplateBuilder(isolate, nullptr) {}
ObjectTemplateBuilder::ObjectTemplateBuilder(v8::Isolate* isolate,
const char* type_name)
: isolate_(isolate),
type_name_(type_name),
template_(v8::ObjectTemplate::New(isolate)) {
template_->SetInternalFieldCount(kNumberOfInternalFields); template_->SetInternalFieldCount(kNumberOfInternalFields);
} }
......
...@@ -17,21 +17,27 @@ ...@@ -17,21 +17,27 @@
namespace gin { namespace gin {
namespace internal {
template <typename T> template <typename T>
v8::Local<v8::FunctionTemplate> CreateFunctionTemplate(v8::Isolate* isolate, v8::Local<v8::FunctionTemplate> CreateFunctionTemplate(v8::Isolate* isolate,
T callback) { T callback,
const char* type_name) {
// We need to handle member function pointers case specially because the first // We need to handle member function pointers case specially because the first
// parameter for callbacks to MFP should typically come from the the // parameter for callbacks to MFP should typically come from the the
// JavaScript "this" object the function was called on, not from the first // JavaScript "this" object the function was called on, not from the first
// normal parameter. // normal parameter.
int callback_flags = 0; InvokerOptions options;
if (std::is_member_function_pointer<T>::value) if (std::is_member_function_pointer<T>::value) {
callback_flags = HolderIsFirstArgument; options.holder_is_first_argument = true;
options.holder_type = type_name;
return CreateFunctionTemplate( }
isolate, base::BindRepeating(std::move(callback)), callback_flags); return ::gin::CreateFunctionTemplate(
isolate, base::BindRepeating(std::move(callback)), std::move(options));
} }
} // namespace internal
template <typename T> template <typename T>
void SetAsFunctionHandler(v8::Isolate* isolate, void SetAsFunctionHandler(v8::Isolate* isolate,
v8::Local<v8::ObjectTemplate> tmpl, v8::Local<v8::ObjectTemplate> tmpl,
...@@ -40,12 +46,10 @@ void SetAsFunctionHandler(v8::Isolate* isolate, ...@@ -40,12 +46,10 @@ void SetAsFunctionHandler(v8::Isolate* isolate,
// parameter for callbacks to MFP should typically come from the the // parameter for callbacks to MFP should typically come from the the
// JavaScript "this" object the function was called on, not from the first // JavaScript "this" object the function was called on, not from the first
// normal parameter. // normal parameter.
int callback_flags = 0; InvokerOptions options = {std::is_member_function_pointer<T>::value, nullptr};
if (std::is_member_function_pointer<T>::value)
callback_flags = HolderIsFirstArgument;
CreateFunctionHandler(isolate, tmpl, base::BindRepeating(std::move(callback)), CreateFunctionHandler(isolate, tmpl, base::BindRepeating(std::move(callback)),
callback_flags); std::move(options));
} }
// ObjectTemplateBuilder provides a handy interface to creating // ObjectTemplateBuilder provides a handy interface to creating
...@@ -53,6 +57,7 @@ void SetAsFunctionHandler(v8::Isolate* isolate, ...@@ -53,6 +57,7 @@ void SetAsFunctionHandler(v8::Isolate* isolate,
class GIN_EXPORT ObjectTemplateBuilder { class GIN_EXPORT ObjectTemplateBuilder {
public: public:
explicit ObjectTemplateBuilder(v8::Isolate* isolate); explicit ObjectTemplateBuilder(v8::Isolate* isolate);
ObjectTemplateBuilder(v8::Isolate* isolate, const char* type_name);
ObjectTemplateBuilder(const ObjectTemplateBuilder& other); ObjectTemplateBuilder(const ObjectTemplateBuilder& other);
~ObjectTemplateBuilder(); ~ObjectTemplateBuilder();
...@@ -71,19 +76,22 @@ class GIN_EXPORT ObjectTemplateBuilder { ...@@ -71,19 +76,22 @@ class GIN_EXPORT ObjectTemplateBuilder {
template<typename T> template<typename T>
ObjectTemplateBuilder& SetMethod(const base::StringPiece& name, ObjectTemplateBuilder& SetMethod(const base::StringPiece& name,
const T& callback) { const T& callback) {
return SetImpl(name, CreateFunctionTemplate(isolate_, callback)); return SetImpl(
name, internal::CreateFunctionTemplate(isolate_, callback, type_name_));
} }
template<typename T> template<typename T>
ObjectTemplateBuilder& SetProperty(const base::StringPiece& name, ObjectTemplateBuilder& SetProperty(const base::StringPiece& name,
const T& getter) { const T& getter) {
return SetPropertyImpl(name, CreateFunctionTemplate(isolate_, getter), return SetPropertyImpl(
v8::Local<v8::FunctionTemplate>()); name, internal::CreateFunctionTemplate(isolate_, getter, type_name_),
v8::Local<v8::FunctionTemplate>());
} }
template<typename T, typename U> template<typename T, typename U>
ObjectTemplateBuilder& SetProperty(const base::StringPiece& name, ObjectTemplateBuilder& SetProperty(const base::StringPiece& name,
const T& getter, const U& setter) { const T& getter, const U& setter) {
return SetPropertyImpl(name, CreateFunctionTemplate(isolate_, getter), return SetPropertyImpl(
CreateFunctionTemplate(isolate_, setter)); name, internal::CreateFunctionTemplate(isolate_, getter, type_name_),
internal::CreateFunctionTemplate(isolate_, setter, type_name_));
} }
ObjectTemplateBuilder& AddNamedPropertyInterceptor(); ObjectTemplateBuilder& AddNamedPropertyInterceptor();
ObjectTemplateBuilder& AddIndexedPropertyInterceptor(); ObjectTemplateBuilder& AddIndexedPropertyInterceptor();
...@@ -99,6 +107,10 @@ class GIN_EXPORT ObjectTemplateBuilder { ...@@ -99,6 +107,10 @@ class GIN_EXPORT ObjectTemplateBuilder {
v8::Isolate* isolate_; v8::Isolate* isolate_;
// If provided, |type_name_| will be used to give a user-friendly error
// message if a member function is invoked on the wrong type of object.
const char* type_name_ = nullptr;
// ObjectTemplateBuilder should only be used on the stack. // ObjectTemplateBuilder should only be used on the stack.
v8::Local<v8::ObjectTemplate> template_; v8::Local<v8::ObjectTemplate> template_;
}; };
......
...@@ -18,7 +18,11 @@ WrappableBase::~WrappableBase() { ...@@ -18,7 +18,11 @@ WrappableBase::~WrappableBase() {
ObjectTemplateBuilder WrappableBase::GetObjectTemplateBuilder( ObjectTemplateBuilder WrappableBase::GetObjectTemplateBuilder(
v8::Isolate* isolate) { v8::Isolate* isolate) {
return ObjectTemplateBuilder(isolate); return ObjectTemplateBuilder(isolate, GetTypeName());
}
const char* WrappableBase::GetTypeName() {
return nullptr;
} }
void WrappableBase::FirstWeakCallback( void WrappableBase::FirstWeakCallback(
......
...@@ -70,6 +70,10 @@ class GIN_EXPORT WrappableBase { ...@@ -70,6 +70,10 @@ class GIN_EXPORT WrappableBase {
// Overrides of this method should be declared final and not overridden again. // Overrides of this method should be declared final and not overridden again.
virtual ObjectTemplateBuilder GetObjectTemplateBuilder(v8::Isolate* isolate); virtual ObjectTemplateBuilder GetObjectTemplateBuilder(v8::Isolate* isolate);
// Returns a readable type name that will be used in surfacing errors. The
// default implementation returns nullptr, which results in a generic error.
virtual const char* GetTypeName();
v8::MaybeLocal<v8::Object> GetWrapperImpl(v8::Isolate* isolate, v8::MaybeLocal<v8::Object> GetWrapperImpl(v8::Isolate* isolate,
WrapperInfo* wrapper_info); WrapperInfo* wrapper_info);
......
...@@ -16,6 +16,11 @@ ...@@ -16,6 +16,11 @@
namespace gin { namespace gin {
namespace {
// A non-member function to be bound to an ObjectTemplateBuilder.
void NonMemberMethod() {}
// This useless base class ensures that the value of a pointer to a MyObject // This useless base class ensures that the value of a pointer to a MyObject
// (below) is not the same as the value of that pointer cast to the object's // (below) is not the same as the value of that pointer cast to the object's
// WrappableBase base. // WrappableBase base.
...@@ -45,16 +50,22 @@ class MyObject : public BaseClass, ...@@ -45,16 +50,22 @@ class MyObject : public BaseClass,
int value() const { return value_; } int value() const { return value_; }
void set_value(int value) { value_ = value; } void set_value(int value) { value_ = value; }
void Method() {}
protected: protected:
MyObject() : value_(0) {} MyObject() : value_(0) {}
ObjectTemplateBuilder GetObjectTemplateBuilder(v8::Isolate* isolate) final { ObjectTemplateBuilder GetObjectTemplateBuilder(v8::Isolate* isolate) final {
return Wrappable<MyObject>::GetObjectTemplateBuilder(isolate) return Wrappable<MyObject>::GetObjectTemplateBuilder(isolate)
.SetProperty("value", &MyObject::value, &MyObject::set_value); .SetProperty("value", &MyObject::value, &MyObject::set_value)
.SetMethod("memberMethod", &MyObject::Method)
.SetMethod("nonMemberMethod", &NonMemberMethod);
} }
~MyObject() override = default; ~MyObject() override = default;
private: private:
int value_; int value_;
DISALLOW_COPY_AND_ASSIGN(MyObject);
}; };
class MyObject2 : public Wrappable<MyObject2> { class MyObject2 : public Wrappable<MyObject2> {
...@@ -62,8 +73,35 @@ class MyObject2 : public Wrappable<MyObject2> { ...@@ -62,8 +73,35 @@ class MyObject2 : public Wrappable<MyObject2> {
static WrapperInfo kWrapperInfo; static WrapperInfo kWrapperInfo;
}; };
class MyNamedObject : public Wrappable<MyNamedObject> {
public:
static WrapperInfo kWrapperInfo;
static gin::Handle<MyNamedObject> Create(v8::Isolate* isolate) {
return CreateHandle(isolate, new MyNamedObject());
}
void Method() {}
protected:
MyNamedObject() = default;
ObjectTemplateBuilder GetObjectTemplateBuilder(v8::Isolate* isolate) final {
return Wrappable<MyNamedObject>::GetObjectTemplateBuilder(isolate)
.SetMethod("memberMethod", &MyNamedObject::Method)
.SetMethod("nonMemberMethod", &NonMemberMethod);
}
const char* GetTypeName() final { return "MyNamedObject"; }
~MyNamedObject() override = default;
private:
DISALLOW_COPY_AND_ASSIGN(MyNamedObject);
};
WrapperInfo MyObject::kWrapperInfo = { kEmbedderNativeGin }; WrapperInfo MyObject::kWrapperInfo = { kEmbedderNativeGin };
WrapperInfo MyObject2::kWrapperInfo = { kEmbedderNativeGin }; WrapperInfo MyObject2::kWrapperInfo = { kEmbedderNativeGin };
WrapperInfo MyNamedObject::kWrapperInfo = {kEmbedderNativeGin};
} // namespace
typedef V8Test WrappableTest; typedef V8Test WrappableTest;
...@@ -136,4 +174,118 @@ TEST_F(WrappableTest, GetAndSetProperty) { ...@@ -136,4 +174,118 @@ TEST_F(WrappableTest, GetAndSetProperty) {
EXPECT_EQ(191, obj->value()); EXPECT_EQ(191, obj->value());
} }
TEST_F(WrappableTest, MethodInvocationErrorsOnUnnamedObject) {
v8::Isolate* isolate = instance_->isolate();
v8::HandleScope handle_scope(isolate);
v8::Local<v8::Context> context = context_.Get(isolate);
gin::Handle<MyObject> obj = MyObject::Create(isolate);
v8::Local<v8::Object> v8_object =
ConvertToV8(isolate, obj.get()).ToLocalChecked().As<v8::Object>();
v8::Local<v8::Value> member_method =
v8_object->Get(context, StringToV8(isolate, "memberMethod"))
.ToLocalChecked();
ASSERT_TRUE(member_method->IsFunction());
v8::Local<v8::Value> non_member_method =
v8_object->Get(context, StringToV8(isolate, "nonMemberMethod"))
.ToLocalChecked();
ASSERT_TRUE(non_member_method->IsFunction());
auto get_error = [isolate, context](v8::Local<v8::Value> function_to_run,
v8::Local<v8::Value> context_object) {
constexpr char kScript[] =
"(function(toRun, contextObject) { toRun.apply(contextObject, []); })";
v8::Local<v8::String> source = StringToV8(isolate, kScript);
EXPECT_FALSE(source.IsEmpty());
v8::TryCatch try_catch(isolate);
v8::Local<v8::Script> script =
v8::Script::Compile(context, source).ToLocalChecked();
v8::Local<v8::Value> val = script->Run(context).ToLocalChecked();
v8::Local<v8::Function> func;
EXPECT_TRUE(ConvertFromV8(isolate, val, &func));
v8::Local<v8::Value> argv[] = {function_to_run, context_object};
func->Call(v8::Undefined(isolate), arraysize(argv), argv);
if (!try_catch.HasCaught())
return std::string();
return V8ToString(try_catch.Message()->Get());
};
EXPECT_EQ(std::string(), get_error(member_method, v8_object));
EXPECT_EQ(std::string(), get_error(non_member_method, v8_object));
EXPECT_EQ("Uncaught TypeError: Illegal invocation",
get_error(member_method, v8::Null(isolate)));
// A non-member function shouldn't throw errors for being applied on a
// null (or invalid) object.
EXPECT_EQ(std::string(), get_error(non_member_method, v8::Null(isolate)));
v8::Local<v8::Object> wrong_object = v8::Object::New(isolate);
// We should get an error for passing the wrong object.
EXPECT_EQ("Uncaught TypeError: Illegal invocation",
get_error(member_method, wrong_object));
// But again, not for a "static" method.
EXPECT_EQ(std::string(), get_error(non_member_method, v8::Null(isolate)));
}
TEST_F(WrappableTest, MethodInvocationErrorsOnNamedObject) {
v8::Isolate* isolate = instance_->isolate();
v8::HandleScope handle_scope(isolate);
v8::Local<v8::Context> context = context_.Get(isolate);
gin::Handle<MyNamedObject> obj = MyNamedObject::Create(isolate);
v8::Local<v8::Object> v8_object =
ConvertToV8(isolate, obj.get()).ToLocalChecked().As<v8::Object>();
v8::Local<v8::Value> member_method =
v8_object->Get(context, StringToV8(isolate, "memberMethod"))
.ToLocalChecked();
ASSERT_TRUE(member_method->IsFunction());
v8::Local<v8::Value> non_member_method =
v8_object->Get(context, StringToV8(isolate, "nonMemberMethod"))
.ToLocalChecked();
ASSERT_TRUE(non_member_method->IsFunction());
auto get_error = [isolate, context](v8::Local<v8::Value> function_to_run,
v8::Local<v8::Value> context_object) {
constexpr char kScript[] =
"(function(toRun, contextObject) { toRun.apply(contextObject, []); })";
v8::Local<v8::String> source = StringToV8(isolate, kScript);
EXPECT_FALSE(source.IsEmpty());
v8::TryCatch try_catch(isolate);
v8::Local<v8::Script> script =
v8::Script::Compile(context, source).ToLocalChecked();
v8::Local<v8::Value> val = script->Run(context).ToLocalChecked();
v8::Local<v8::Function> func;
EXPECT_TRUE(ConvertFromV8(isolate, val, &func));
v8::Local<v8::Value> argv[] = {function_to_run, context_object};
func->Call(v8::Undefined(isolate), arraysize(argv), argv);
if (!try_catch.HasCaught())
return std::string();
return V8ToString(try_catch.Message()->Get());
};
EXPECT_EQ(std::string(), get_error(member_method, v8_object));
EXPECT_EQ(std::string(), get_error(non_member_method, v8_object));
EXPECT_EQ(
"Uncaught TypeError: Illegal invocation: Function must be called on "
"an object of type MyNamedObject",
get_error(member_method, v8::Null(isolate)));
// A non-member function shouldn't throw errors for being applied on a
// null (or invalid) object.
EXPECT_EQ(std::string(), get_error(non_member_method, v8::Null(isolate)));
v8::Local<v8::Object> wrong_object = v8::Object::New(isolate);
// We should get an error for passing the wrong object.
EXPECT_EQ(
"Uncaught TypeError: Illegal invocation: Function must be called on "
"an object of type MyNamedObject",
get_error(member_method, wrong_object));
// But again, not for a "static" method.
EXPECT_EQ(std::string(), get_error(non_member_method, v8::Null(isolate)));
}
} // namespace gin } // namespace gin
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment