Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-11-24 13:21:05 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1483869 part 4 - Make wrappers work correctly with bound functions. r=iain,peterv

Browse Source 
Now that bound functions use a `JSClass` that's not `JSFunction`, we need to give
them a `JSProtoKey` to make (xray) wrapper calls work. This fixes a lot of xpcshell
and browser test failures.

The old implementation used `JSProto_Function`. This patch adds `JSProto_BoundFunction`
and (as much as possible) tries to use the same code paths for it.

Differential Revision: https://phabricator.services.mozilla.com/D170568
This commit is contained in:
Jan de Mooij 2023-02-27 13:05:43 +00:00
parent 7fb0cc95b9
commit 456e577517
8 changed files with 78 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
js/public/ProtoKey.h
View File

@ -50,6 +50,7 @@
IMAGINARY(Null, dummy) \
REAL(Object, OCLASP(Plain)) \
REAL(Function, &FunctionClass) \
IMAGINARY(BoundFunction, OCLASP(BoundFunction)) \
REAL(Array, OCLASP(Array)) \
REAL(Boolean, OCLASP(Boolean)) \
REAL(JSON, CLASP(JSON)) \

8
js/src/jsapi.cpp
View File

@ -1097,6 +1097,14 @@ JS_PUBLIC_API bool JS_GetClassPrototype(JSContext* cx, JSProtoKey key,
MutableHandleObject objp) {
AssertHeapIsIdle();
CHECK_THREAD(cx);
// Bound functions don't have their own prototype object: they reuse the
// prototype of the target object. This is typically Function.prototype so we
// use that here.
if (key == JSProto_BoundFunction) {
key = JSProto_Function;
}
JSObject* proto = GlobalObject::getOrCreatePrototype(cx, key);
if (!proto) {
return false;

6
js/src/vm/BoundFunctionObject.cpp
View File

@ -306,7 +306,11 @@ static const ObjectOps objOps = {
const JSClass BoundFunctionObject::class_ = {
"BoundFunctionObject",
JSCLASS_HAS_RESERVED_SLOTS(BoundFunctionObject::SlotCount),
// Note: bound functions don't have their own constructor or prototype (they
// use the prototype of the target object), but we give them a JSProtoKey
// because that's what Xray wrappers use to identify builtin objects.
JSCLASS_HAS_CACHED_PROTO(JSProto_BoundFunction) |
JSCLASS_HAS_RESERVED_SLOTS(BoundFunctionObject::SlotCount),
&classOps,
JS_NULL_CLASS_SPEC,
JS_NULL_CLASS_EXT,

6
js/src/vm/GlobalObject.cpp
View File

@ -94,6 +94,7 @@ bool GlobalObject::skipDeselectedConstructor(JSContext* cx, JSProtoKey key) {
case JSProto_Null:
case JSProto_Object:
case JSProto_Function:
case JSProto_BoundFunction:
case JSProto_Array:
case JSProto_Boolean:
case JSProto_JSON:
@ -224,6 +225,8 @@ bool GlobalObject::resolveConstructor(JSContext* cx,
Handle<GlobalObject*> global,
JSProtoKey key, IfClassIsDisabled mode) {
MOZ_ASSERT(key != JSProto_Null);
MOZ_ASSERT(key != JSProto_BoundFunction,
"bound functions don't have their own proto object");
MOZ_ASSERT(!global->isStandardClassResolved(key));
MOZ_ASSERT(cx->compartment() == global->compartment());
@ -650,7 +653,8 @@ bool GlobalObject::initStandardClasses(JSContext* cx,
for (size_t k = 0; k < JSProto_LIMIT; ++k) {
JSProtoKey key = static_cast<JSProtoKey>(k);
if (key != JSProto_Null && !global->isStandardClassResolved(key)) {
if (key != JSProto_Null && key != JSProto_BoundFunction &&
!global->isStandardClassResolved(key)) {
if (!resolveConstructor(cx, global, static_cast<JSProtoKey>(k),
IfClassIsDisabled::DoNothing)) {
return false;

19
js/xpconnect/tests/chrome/test_xrayToJS.xhtml
View File

@ -130,6 +130,25 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=933681
ok(!Object.getOwnPropertyNames(foopyFunction).includes('prototype'), "Should not list prototype");
ok(Object.getOwnPropertyNames(iwin.Array).includes('prototype'), "Should list prototype for standard constructor");
// Test BoundFunction.
iwin.eval('var boundFoopyFunction = foopyFunction.bind(null, 1)');
var boundFoopyFunction = Cu.unwaiveXrays(Cu.waiveXrays(iwin).boundFoopyFunction);
is(boundFoopyFunction.name, "bound namedFoopyFunction", "bound .name works over Xrays");
is(boundFoopyFunction.length, 2, "bound .length works over Xrays");
is(JSON.stringify(Object.getOwnPropertyNames(boundFoopyFunction).sort()), '["length","name"]', "Should list length and name");
// Mutate .name, it's just a data property.
iwin.eval('Object.defineProperty(boundFoopyFunction, "name", {value: "foobar", configurable: true, writable: true});');
is(boundFoopyFunction.name, "foobar", "mutated .name works over Xrays");
iwin.eval('boundFoopyFunction.name = 123;');
is(boundFoopyFunction.name, undefined, "only support string for .name");
iwin.eval('delete boundFoopyFunction.name');
is(boundFoopyFunction.name, undefined, "deleted .name works over Xrays");
// Mutate .length.
iwin.eval('Object.defineProperty(boundFoopyFunction, "length", {value: 456, configurable: true, writable: true});');
is(boundFoopyFunction.length, 456, "mutated .length works over Xrays");
iwin.eval('boundFoopyFunction.length = "bar";');
is(boundFoopyFunction.length, undefined, "only support number for .length");
// Test proxies.
var targetObject = new iwin.Object();
targetObject.foo = 9;

5
js/xpconnect/wrappers/WrapperFactory.cpp
View File

@ -485,7 +485,8 @@ JSObject* WrapperFactory::Rewrap(JSContext* cx, HandleObject existing,
if (originIsChrome && !targetIsChrome) {
// If this is a chrome function being exposed to content, we need to allow
// call (but nothing else).
if ((IdentifyStandardInstance(obj) == JSProto_Function)) {
JSProtoKey key = IdentifyStandardInstance(obj);
if (key == JSProto_Function || key == JSProto_BoundFunction) {
wrapper = &FilteringWrapper<CrossCompartmentSecurityWrapper,
OpaqueWithCall>::singleton;
}
@ -493,7 +494,7 @@ JSObject* WrapperFactory::Rewrap(JSContext* cx, HandleObject existing,
// For vanilla JSObjects exposed from chrome to content, we use a wrapper
// that fails silently in a few cases. We'd like to get rid of this
// eventually, but in their current form they don't cause much trouble.
else if (IdentifyStandardInstance(obj) == JSProto_Object) {
else if (key == JSProto_Object) {
wrapper = &ChromeObjectWrapper::singleton;
}

36
js/xpconnect/wrappers/XrayWrapper.cpp
View File

@ -91,6 +91,7 @@ static bool IsJSXraySupported(JSProtoKey key) {
case JSProto_Object:
case JSProto_Array:
case JSProto_Function:
case JSProto_BoundFunction:
case JSProto_TypedArray:
case JSProto_SavedFrame:
case JSProto_RegExp:
@ -673,6 +674,30 @@ bool JSXrayTraits::resolveOwnProperty(
if (id == GetJSIDByIndex(cx, XPCJSContext::IDX_LASTINDEX)) {
return getOwnPropertyFromWrapperIfSafe(cx, wrapper, id, desc);
}
} else if (key == JSProto_BoundFunction) {
// Bound functions have configurable .name and .length own data
// properties. Only support string values for .name and number values for
// .length.
if (id == GetJSIDByIndex(cx, XPCJSContext::IDX_NAME)) {
if (!getOwnPropertyFromWrapperIfSafe(cx, wrapper, id, desc)) {
return false;
}
if (desc.isSome() &&
(!desc->isDataDescriptor() || !desc->value().isString())) {
desc.reset();
}
return true;
}
if (id == GetJSIDByIndex(cx, XPCJSContext::IDX_LENGTH)) {
if (!getOwnPropertyFromWrapperIfSafe(cx, wrapper, id, desc)) {
return false;
}
if (desc.isSome() &&
(!desc->isDataDescriptor() || !desc->value().isNumber())) {
desc.reset();
}
return true;
}
}
// The rest of this function applies only to prototypes.
@ -979,6 +1004,11 @@ bool JSXrayTraits::enumerateNames(JSContext* cx, HandleObject wrapper,
if (!props.append(GetJSIDByIndex(cx, XPCJSContext::IDX_LASTINDEX))) {
return false;
}
} else if (key == JSProto_BoundFunction) {
if (!props.append(GetJSIDByIndex(cx, XPCJSContext::IDX_LENGTH)) ||
!props.append(GetJSIDByIndex(cx, XPCJSContext::IDX_NAME))) {
return false;
}
}
// The rest of this function applies only to prototypes.
@ -1008,7 +1038,8 @@ bool JSXrayTraits::construct(JSContext* cx, HandleObject wrapper,
return false;
}
if (xpc::JSXrayTraits::getProtoKey(holder) == JSProto_Function) {
const JSProtoKey key = xpc::JSXrayTraits::getProtoKey(holder);
if (key == JSProto_Function) {
JSProtoKey standardConstructor = constructorFor(holder);
if (standardConstructor == JSProto_Null) {
return baseInstance.construct(cx, wrapper, args);
@ -1042,6 +1073,9 @@ bool JSXrayTraits::construct(JSContext* cx, HandleObject wrapper,
args.rval().setObject(*result);
return true;
}
if (key == JSProto_BoundFunction) {
return baseInstance.construct(cx, wrapper, args);
}
JS::RootedValue v(cx, JS::ObjectValue(*wrapper));
js::ReportIsNotFunction(cx, v);

3
js/xpconnect/wrappers/XrayWrapper.h
View File

@ -211,7 +211,8 @@ class JSXrayTraits : public XrayTraits {
if (!holder) {
return false;
}
if (xpc::JSXrayTraits::getProtoKey(holder) == JSProto_Function) {
JSProtoKey key = xpc::JSXrayTraits::getProtoKey(holder);
if (key == JSProto_Function || key == JSProto_BoundFunction) {
return baseInstance.call(cx, wrapper, args);
}