Better support for SpiderMonkey rooted types in the ScriptInterface.

* Adds additional overloads/specializations which are required when
passing JS::Handle<T>/JS::MutableHandle<T> types to different functions.
* Replaces GetPropertyJS with a GetProperty specialization.
* Allows us to avoid the implementation of ToJSVal specializations for
JS::Value and JS::HandleValue. Such conversions should only happen if
there's no way around it and if you are aware of it.
* Adds test to make sure that all potentially required specializations
with custom implementations are instantiated. This should help prevent
introducing bugs in temporarily unused code.

Refs #2415

This was SVN commit r15567.
This commit is contained in:
Yves 2014-07-26 20:31:29 +00:00
parent 30f7837c44
commit e818b08344
6 changed files with 191 additions and 22 deletions

View File

@ -78,11 +78,20 @@ BOOST_PP_REPEAT(SCRIPT_INTERFACE_MAX_ARGS, OVERLOADS, ~)
BOOST_PP_REPEAT(SCRIPT_INTERFACE_MAX_ARGS, OVERLOADS, ~)
#undef OVERLOADS
// The trick is using JS::Rooted<R>* and converting that to a JS::MutableHandle<R> explicitly in the function body.
// Normally the function would take JS::MutableHandle<R> and that conversion would happen implicitly. However, implicit
// conversion does not work with template argument deduction (only exact type matches allowed).
// Implicit conversion from JS::Rooted<R>* to JS::MutableHandle<R> does not work with template argument deduction
// (only exact type matches allowed). We need this overload to allow passing Rooted<R>* using the & operator
// (as people would expect it to work based on the SpiderMonkey rooting guide).
#define OVERLOADS(z, i, data) \
template <typename R TYPENAME_T0_TAIL(z, i)> \
bool CallFunction(jsval val, const char* name, T0_A0_CONST_REF(z,i) JS::Rooted<R>* ret);
BOOST_PP_REPEAT(SCRIPT_INTERFACE_MAX_ARGS, OVERLOADS, ~)
#undef OVERLOADS
// This overload is for the case when a JS::MutableHandle<R> type gets passed into CallFunction directly and
// without requiring implicit conversion.
#define OVERLOADS(z, i, data) \
template <typename R TYPENAME_T0_TAIL(z, i)> \
bool CallFunction(jsval val, const char* name, T0_A0_CONST_REF(z,i) JS::MutableHandle<R> ret);
BOOST_PP_REPEAT(SCRIPT_INTERFACE_MAX_ARGS, OVERLOADS, ~)
#undef OVERLOADS

View File

@ -27,7 +27,8 @@ struct ScriptInterface_NativeWrapper {
#define OVERLOADS(z, i, data) \
template<TYPENAME_T0_HEAD(z,i) typename F> \
static void call(JSContext* cx, JS::MutableHandleValue rval, F fptr T0_A0(z,i)) { \
ScriptInterface::ToJSVal<R>(cx, rval, fptr(ScriptInterface::GetScriptInterfaceAndCBData(cx) A0_TAIL(z,i))); \
ScriptInterface* pScriptInterface = ScriptInterface::GetScriptInterfaceAndCBData(cx)->pScriptInterface; \
pScriptInterface->AssignOrToJSVal<R>(rval, fptr(ScriptInterface::GetScriptInterfaceAndCBData(cx) A0_TAIL(z,i))); \
}
BOOST_PP_REPEAT(SCRIPT_INTERFACE_MAX_ARGS, OVERLOADS, ~)
@ -53,7 +54,8 @@ struct ScriptInterface_NativeMethodWrapper {
#define OVERLOADS(z, i, data) \
template<TYPENAME_T0_HEAD(z,i) typename F> \
static void call(JSContext* cx, JS::MutableHandleValue rval, TC* c, F fptr T0_A0(z,i)) { \
ScriptInterface::ToJSVal<R>(cx, rval, (c->*fptr)( A0(z,i) )); \
ScriptInterface* pScriptInterface = ScriptInterface::GetScriptInterfaceAndCBData(cx)->pScriptInterface; \
pScriptInterface->AssignOrToJSVal<R>(rval, (c->*fptr)( A0(z,i) )); \
}
BOOST_PP_REPEAT(SCRIPT_INTERFACE_MAX_ARGS, OVERLOADS, ~)
@ -163,6 +165,24 @@ bool ScriptInterface::CallFunction(jsval val, const char* name, T0_A0_CONST_REF(
BOOST_PP_REPEAT(SCRIPT_INTERFACE_MAX_ARGS, OVERLOADS, ~)
#undef OVERLOADS
#define OVERLOADS(z, i, data) \
template<typename R TYPENAME_T0_TAIL(z, i)> \
bool ScriptInterface::CallFunction(jsval val, const char* name, T0_A0_CONST_REF(z,i) JS::MutableHandle<R> ret) \
{ \
JSContext* cx = GetContext(); \
JSAutoRequest rq(cx); \
JS::RootedValue val1(cx, val); \
JS::AutoValueVector argv(cx); \
argv.resize(i); \
BOOST_PP_REPEAT_##z (i, TO_JS_VAL, ~) \
bool ok = CallFunction_(val1, name, argv.length(), argv.begin(), ret); \
if (!ok) \
return false; \
return true; \
}
BOOST_PP_REPEAT(SCRIPT_INTERFACE_MAX_ARGS, OVERLOADS, ~)
#undef OVERLOADS
// Clean up our mess
#undef SCRIPT_PROFILE
#undef NUMBERED_LIST_HEAD

View File

@ -1021,15 +1021,6 @@ bool ScriptInterface::SetGlobal_(const char* name, jsval value, bool replace)
return ok;
}
bool ScriptInterface::GetPropertyJS(jsval obj, const char* name, JS::MutableHandleValue out)
{
JSContext* cx = GetContext();
JSAutoRequest rq(cx);
if (! GetProperty_(obj, name, out))
return false;
return true;
}
bool ScriptInterface::SetProperty_(jsval obj, const char* name, jsval value, bool constant, bool enumerate)
{
JSAutoRequest rq(m->m_cx);

View File

@ -211,14 +211,24 @@ public:
*/
template<typename T>
bool GetProperty(jsval obj, const char* name, T& out);
/**
* Get the named property of the given object.
* This overload takes JS::Rooted<T>* and converts it to JS::MutableHandle<T> in the function body because implicit
* conversion is not supported for templates.
* It's used in the case where a JS::Rooted<T> gets created inside the same function and then passed to GetProperty as
* |out| using the & operator.
*/
template<typename T>
bool GetProperty(jsval obj, const char* name, JS::Rooted<T>* out);
/**
* This function overload is used for JS::MutableHandleValue type.
* If we use JS::RootedValue with the GetProperty function template, it will generate an overload for the type
* JS::RootedValue*, but JS::MutableHandleValue needs to be used when passing JS::RootedValue& to a function.
* Check the SpiderMonkey rooting guide for details.
* Get the named property of the given object.
* This overload gets used in the case when you don't need conversion from a JS::Rooted<T>* and pass JS::MutableHandle<T>
* to GetProperty as |out| parameter directly (usually when you get it as a function parameter).
*/
bool GetPropertyJS(jsval obj, const char* name, JS::MutableHandleValue out);
template<typename T>
bool GetProperty(jsval obj, const char* name, JS::MutableHandle<T> out);
/**
* Get the integer-named property on the given object.
@ -226,6 +236,22 @@ public:
template<typename T>
bool GetPropertyInt(jsval obj, int name, T& out);
/**
* Get the integer-named property on the given object.
* This version is for taking JS::MutableHandle<T> out parameters. Check the comment for GetProperty for
* background information
*/
template<typename T>
bool GetPropertyInt(jsval obj, int name, JS::Rooted<T>* out);
/**
* Get the named property of the given object.
* This overload gets used in the case when you don't need conversion from a JS::Rooted<T>* and pass JS::MutableHandle<T>
* to GetPropertyInt as |out| parameter directly (usually when you get it as a function parameter).
*/
template<typename T>
bool GetPropertyInt(jsval obj, int name, JS::MutableHandle<T> out);
/**
* Check the named property has been defined on the given object.
*/
@ -239,6 +265,7 @@ public:
bool Eval(const char* code);
template<typename CHAR> bool Eval(const CHAR* code, JS::MutableHandleValue out);
template<typename T, typename CHAR> bool Eval(const CHAR* code, T& out);
std::wstring ToString(jsval obj, bool pretty = false);
@ -366,8 +393,6 @@ public:
shared_ptr<StructuredClone> WriteStructuredClone(jsval v);
jsval ReadStructuredClone(const shared_ptr<StructuredClone>& ptr);
private:
/**
* Converts |a| if needed and assigns it to |handle|.
* This is meant for use in other templates where we want to use the same code for JS::RootedValue&/JS::HandleValue and
@ -378,6 +403,8 @@ private:
*/
template <typename T>
void AssignOrToJSVal(JS::MutableHandleValue handle, const T& a);
private:
bool CallFunction_(JS::HandleValue val, const char* name, uint argc, jsval* argv, JS::MutableHandleValue ret);
bool Eval_(const char* code, JS::MutableHandleValue ret);
@ -438,6 +465,18 @@ inline void ScriptInterface::AssignOrToJSVal<JS::RootedValue>(JS::MutableHandleV
handle.set(a);
}
template <>
inline void ScriptInterface::AssignOrToJSVal<JS::HandleValue>(JS::MutableHandleValue handle, const JS::HandleValue& a)
{
handle.set(a);
}
template <>
inline void ScriptInterface::AssignOrToJSVal<JS::Value>(JS::MutableHandleValue handle, const JS::Value& a)
{
handle.set(a);
}
template<typename T0>
bool ScriptInterface::CallFunctionVoid(jsval val, const char* name, const T0& a0)
{
@ -527,6 +566,23 @@ bool ScriptInterface::GetProperty(jsval obj, const char* name, T& out)
return FromJSVal(cx, val, out);
}
template<typename T>
bool ScriptInterface::GetProperty(jsval obj, const char* name, JS::Rooted<T>* out)
{
JS::MutableHandle<T> handleOut(out);
if (! GetProperty_(obj, name, handleOut))
return false;
return true;
}
template<typename T>
bool ScriptInterface::GetProperty(jsval obj, const char* name, JS::MutableHandle<T> out)
{
if (! GetProperty_(obj, name, out))
return false;
return true;
}
template<typename T>
bool ScriptInterface::GetPropertyInt(jsval obj, int name, T& out)
{
@ -537,6 +593,31 @@ bool ScriptInterface::GetPropertyInt(jsval obj, int name, T& out)
return FromJSVal(GetContext(), val, out);
}
template<typename T>
bool ScriptInterface::GetPropertyInt(jsval obj, int name, JS::Rooted<T>* out)
{
JS::MutableHandle<T> handleOut(out);
if (! GetPropertyInt_(obj, name, handleOut))
return false;
return true;
}
template<typename T>
bool ScriptInterface::GetPropertyInt(jsval obj, int name, JS::MutableHandle<T> out)
{
if (! GetPropertyInt_(obj, name, out))
return false;
return true;
}
template<typename CHAR>
bool ScriptInterface::Eval(const CHAR* code, JS::MutableHandleValue ret)
{
if (! Eval_(code, ret))
return false;
return true;
}
template<typename T, typename CHAR>
bool ScriptInterface::Eval(const CHAR* code, T& ret)
{

View File

@ -115,6 +115,74 @@ public:
TS_ASSERT_EQUALS(prop_x1.get(), prop_a.get());
TS_ASSERT_EQUALS(prop_x1.get(), prop_b.get());
}
/**
* This test is mainly to make sure that all required template overloads get instantiated at least once so that compiler errors
* in these functions are revealed instantly (but it also tests the basic functionality of these functions).
*/
void test_rooted_templates()
{
ScriptInterface script("Test", "Test", g_ScriptRuntime);
JSContext* cx = script.GetContext();
JSAutoRequest rq(cx);
JS::RootedValue val(cx);
JS::RootedValue out(cx);
TS_ASSERT(script.Eval("({ '0':0, inc:function() { this[0]++; return this[0]; }, setTo:function(nbr) { this[0] = nbr; } })", &val));
JS::RootedValue nbrVal(cx, JS::NumberValue(3));
int nbr = 0;
// CallFunctionVoid JS::RootedValue& overload
script.CallFunctionVoid(val, "setTo", nbrVal);
// CallFunction JS::RootedValue* overload
script.CallFunction(val, "inc", &out);
ScriptInterface::FromJSVal(cx, out, nbr);
TS_ASSERT_EQUALS(4, nbr);
// GetProperty tests JS::RootedValue* overload
nbr = 0;
script.GetProperty(val, "0", &out); // JS::RootedValue* overload
ScriptInterface::FromJSVal(cx, out, nbr);
TS_ASSERT_EQUALS(nbr, 4);
// GetPropertyInt tests JS::RootedValue* overload
nbr = 0;
script.GetPropertyInt(val, 0, &out);
ScriptInterface::FromJSVal(cx, out, nbr);
TS_ASSERT_EQUALS(nbr, 4);
handle_templates_test(script, val, &out, nbrVal);
}
void handle_templates_test(ScriptInterface& script, JS::HandleValue val, JS::MutableHandleValue out, JS::HandleValue nbrVal)
{
int nbr = 0;
// CallFunctionVoid JS::HandleValue overload
script.CallFunctionVoid(val, "setTo", nbrVal);
// CallFunction JS::MutableHandleValue overload
script.CallFunction(val, "inc", out);
ScriptInterface::FromJSVal(script.GetContext(), out, nbr);
TS_ASSERT_EQUALS(4, nbr);
// GetProperty JS::MutableHandleValue overload
nbr = 0;
script.GetProperty(val, "0", out);
ScriptInterface::FromJSVal(script.GetContext(), out, nbr);
TS_ASSERT_EQUALS(nbr, 4);
// GetPropertyInt JS::MutableHandleValue overload
nbr = 0;
script.GetPropertyInt(val, 0, out);
ScriptInterface::FromJSVal(script.GetContext(), out, nbr);
TS_ASSERT_EQUALS(nbr, 4);
}
void test_random()
{

View File

@ -256,7 +256,7 @@ void CComponentManager::Script_RegisterComponentType_Common(ScriptInterface::CxP
// Find all the ctor prototype's On* methods, and subscribe to the appropriate messages:
JS::RootedValue protoVal(cx);
if (!componentManager->m_ScriptInterface.GetPropertyJS(ctor.get(), "prototype", &protoVal))
if (!componentManager->m_ScriptInterface.GetProperty(ctor.get(), "prototype", &protoVal))
return; // error
std::vector<std::string> methods;