From 43324bd36feafc825f5f15580b2f9aab0d63cd6f Mon Sep 17 00:00:00 2001 From: Tom Schuster Date: Wed, 11 Feb 2015 23:40:47 +0100 Subject: [PATCH] Bug 1125437 - Get rid of SetPropertyAttributes and use DefineProperty to follow ES6 specification. r=efaust --- dom/bindings/Codegen.py | 1 - dom/plugins/base/nsJSNPRuntime.cpp | 1 - js/public/Class.h | 7 +--- js/src/builtin/TypedObject.cpp | 17 -------- js/src/builtin/TypedObject.h | 3 -- .../tests/basic/typed-array-sealed-frozen.js | 2 +- js/src/jit-test/tests/proxy/freeze-proxy.js | 22 ++++++++++ js/src/jit-test/tests/proxy/seal-proxy.js | 16 ++++++++ js/src/jsfriendapi.h | 4 -- js/src/jsobj.cpp | 40 +++++++++++++------ js/src/jsobj.h | 7 ---- js/src/jsobjinlines.h | 10 ----- js/src/proxy/Proxy.cpp | 11 ----- js/src/vm/NativeObject.cpp | 34 ---------------- js/src/vm/NativeObject.h | 4 -- js/src/vm/ScopeObject.cpp | 16 -------- js/src/vm/UnboxedObject.cpp | 10 ----- js/src/vm/UnboxedObject.h | 3 -- js/xpconnect/src/XPCWrappedNativeJSOps.cpp | 1 - js/xpconnect/src/xpcprivate.h | 2 - 20 files changed, 69 insertions(+), 142 deletions(-) create mode 100644 js/src/jit-test/tests/proxy/freeze-proxy.js create mode 100644 js/src/jit-test/tests/proxy/seal-proxy.js diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py index e99b9b2ec9e7..18e3022b43da 100644 --- a/dom/bindings/Codegen.py +++ b/dom/bindings/Codegen.py @@ -397,7 +397,6 @@ class CGDOMJSClass(CGThing): nullptr, /* getProperty */ nullptr, /* setProperty */ nullptr, /* getOwnPropertyDescriptor */ - nullptr, /* setPropertyAttributes */ nullptr, /* deleteProperty */ nullptr, /* watch */ nullptr, /* unwatch */ diff --git a/dom/plugins/base/nsJSNPRuntime.cpp b/dom/plugins/base/nsJSNPRuntime.cpp index 874fe228ddce..1fa1321ecf60 100644 --- a/dom/plugins/base/nsJSNPRuntime.cpp +++ b/dom/plugins/base/nsJSNPRuntime.cpp @@ -227,7 +227,6 @@ const static js::Class sNPObjectJSWrapperClass = nullptr, // getProperty nullptr, // setProperty nullptr, // getOwnPropertyDescriptor - nullptr, // setPropertyAttributes nullptr, // deleteProperty nullptr, nullptr, // watch/unwatch nullptr, // getElements diff --git a/js/public/Class.h b/js/public/Class.h index 27991c9df34a..b2326f0fe543 100644 --- a/js/public/Class.h +++ b/js/public/Class.h @@ -177,8 +177,6 @@ typedef bool (* GetOwnPropertyOp)(JSContext *cx, JS::HandleObject obj, JS::HandleId id, JS::MutableHandle desc); typedef bool -(* SetAttributesOp)(JSContext *cx, JS::HandleObject obj, JS::HandleId id, unsigned *attrsp); -typedef bool (* DeletePropertyOp)(JSContext *cx, JS::HandleObject obj, JS::HandleId id, bool *succeeded); typedef bool @@ -346,7 +344,6 @@ struct ObjectOps GetPropertyOp getProperty; SetPropertyOp setProperty; GetOwnPropertyOp getOwnPropertyDescriptor; - SetAttributesOp setAttributes; DeletePropertyOp deleteProperty; WatchOp watch; UnwatchOp unwatch; @@ -357,7 +354,7 @@ struct ObjectOps #define JS_NULL_OBJECT_OPS \ {nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, \ - nullptr, nullptr, nullptr, nullptr} + nullptr, nullptr, nullptr} } // namespace js @@ -368,7 +365,7 @@ typedef void (*JSClassInternal)(); struct JSClass { JS_CLASS_MEMBERS(JSFinalizeOp); - void *reserved[24]; + void *reserved[23]; }; #define JSCLASS_HAS_PRIVATE (1<<0) // objects have private slot diff --git a/js/src/builtin/TypedObject.cpp b/js/src/builtin/TypedObject.cpp index 68bf7c3aa5d4..8f9d20d2099f 100644 --- a/js/src/builtin/TypedObject.cpp +++ b/js/src/builtin/TypedObject.cpp @@ -2042,22 +2042,6 @@ IsOwnId(JSContext *cx, HandleObject obj, HandleId id) return false; } -bool -TypedObject::obj_setPropertyAttributes(JSContext *cx, HandleObject obj, HandleId id, - unsigned *attrsp) -{ - if (IsOwnId(cx, obj, id)) - return ReportPropertyError(cx, JSMSG_CANT_REDEFINE_PROP, id); - - RootedObject proto(cx, obj->getProto()); - if (!proto) { - *attrsp = 0; - return true; - } - - return SetPropertyAttributes(cx, proto, id, attrsp); -} - bool TypedObject::obj_deleteProperty(JSContext *cx, HandleObject obj, HandleId id, bool *succeeded) { @@ -2333,7 +2317,6 @@ LazyArrayBufferTable::sizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) TypedObject::obj_getProperty, \ TypedObject::obj_setProperty, \ TypedObject::obj_getOwnPropertyDescriptor, \ - TypedObject::obj_setPropertyAttributes, \ TypedObject::obj_deleteProperty, \ nullptr, nullptr, /* watch/unwatch */ \ nullptr, /* getElements */ \ diff --git a/js/src/builtin/TypedObject.h b/js/src/builtin/TypedObject.h index b14a19e95ca8..9874654167e0 100644 --- a/js/src/builtin/TypedObject.h +++ b/js/src/builtin/TypedObject.h @@ -550,9 +550,6 @@ class TypedObject : public JSObject static bool obj_getOwnPropertyDescriptor(JSContext *cx, HandleObject obj, HandleId id, MutableHandle desc); - static bool obj_setPropertyAttributes(JSContext *cx, HandleObject obj, - HandleId id, unsigned *attrsp); - static bool obj_deleteProperty(JSContext *cx, HandleObject obj, HandleId id, bool *succeeded); static bool obj_enumerate(JSContext *cx, HandleObject obj, AutoIdVector &properties); diff --git a/js/src/jit-test/tests/basic/typed-array-sealed-frozen.js b/js/src/jit-test/tests/basic/typed-array-sealed-frozen.js index 5084d4f7e87a..90c0b9a3536f 100644 --- a/js/src/jit-test/tests/basic/typed-array-sealed-frozen.js +++ b/js/src/jit-test/tests/basic/typed-array-sealed-frozen.js @@ -26,7 +26,7 @@ for (constructor of constructors) { Object.seal(a); // Should complain that it can't change attributes of indexed typed array properties. - assertThrowsInstanceOf(() => Object.freeze(a), InternalError); + assertThrowsInstanceOf(() => Object.freeze(a), TypeError); } print(); diff --git a/js/src/jit-test/tests/proxy/freeze-proxy.js b/js/src/jit-test/tests/proxy/freeze-proxy.js new file mode 100644 index 000000000000..ff82c50fd300 --- /dev/null +++ b/js/src/jit-test/tests/proxy/freeze-proxy.js @@ -0,0 +1,22 @@ +var called = []; +var proxy = new Proxy({a: 1, get b() {}}, { + getOwnPropertyDescriptor(target, P) { + called.push("getOwnPropertyDescriptor"); + return Object.getOwnPropertyDescriptor(target, P); + }, + defineProperty(target, P, desc) { + called.push("defineProperty"); + if (P == "a") { + assertEq(Object.getOwnPropertyNames(desc).length, 2); + assertEq(desc.configurable, false); + assertEq(desc.writable, false); + } else { + assertEq(Object.getOwnPropertyNames(desc).length, 1); + assertEq(desc.configurable, false); + } + return Object.defineProperty(target, P, desc); + } +}); + +Object.freeze(proxy); +assertEq(called.toString(), "getOwnPropertyDescriptor,defineProperty,getOwnPropertyDescriptor,defineProperty"); diff --git a/js/src/jit-test/tests/proxy/seal-proxy.js b/js/src/jit-test/tests/proxy/seal-proxy.js new file mode 100644 index 000000000000..74923cde01d2 --- /dev/null +++ b/js/src/jit-test/tests/proxy/seal-proxy.js @@ -0,0 +1,16 @@ +var called = []; +var proxy = new Proxy({a: 1}, { + getOwnPropertyDescriptor(target, P) { + called.push("getOwnPropertyDescriptor"); + return Object.getOwnPropertyDescriptor(target, P); + }, + defineProperty(target, P, desc) { + called.push("defineProperty"); + assertEq(Object.getOwnPropertyNames(desc).length, 1); + assertEq(desc.configurable, false); + return Object.defineProperty(target, P, desc); + } +}); + +Object.seal(proxy); +assertEq(called.toString(), "defineProperty"); diff --git a/js/src/jsfriendapi.h b/js/src/jsfriendapi.h index 3b8da6f759d4..096f4242f604 100644 --- a/js/src/jsfriendapi.h +++ b/js/src/jsfriendapi.h @@ -302,7 +302,6 @@ namespace js { js::proxy_GetProperty, \ js::proxy_SetProperty, \ js::proxy_GetOwnPropertyDescriptor, \ - js::proxy_SetPropertyAttributes, \ js::proxy_DeleteProperty, \ js::proxy_Watch, js::proxy_Unwatch, \ js::proxy_GetElements, \ @@ -342,9 +341,6 @@ extern JS_FRIEND_API(bool) proxy_GetOwnPropertyDescriptor(JSContext *cx, JS::HandleObject obj, JS::HandleId id, JS::MutableHandle desc); extern JS_FRIEND_API(bool) -proxy_SetPropertyAttributes(JSContext *cx, JS::HandleObject obj, JS::HandleId id, - unsigned *attrsp); -extern JS_FRIEND_API(bool) proxy_DeleteProperty(JSContext *cx, JS::HandleObject obj, JS::HandleId id, bool *succeeded); extern JS_FRIEND_API(void) diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index f8cf0938b61b..c0f7fc2c4574 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -1038,31 +1038,47 @@ js::SetIntegrityLevel(JSContext *cx, HandleObject obj, IntegrityLevel level) } else { RootedId id(cx); Rooted desc(cx); + + const unsigned AllowConfigure = JSPROP_IGNORE_ENUMERATE | JSPROP_IGNORE_READONLY | + JSPROP_IGNORE_VALUE; + const unsigned AllowConfigureAndWritable = AllowConfigure & ~JSPROP_IGNORE_READONLY; + + // 8.a/9.a. The two different loops are merged here. for (size_t i = 0; i < keys.length(); i++) { id = keys[i]; - if (!GetOwnPropertyDescriptor(cx, obj, id, &desc)) - return false; + if (level == IntegrityLevel::Sealed) { + // 8.a.i. + desc.setAttributes(AllowConfigure | JSPROP_PERMANENT); + } else { + // 9.a.i-ii. + Rooted currentDesc(cx); + if (!GetOwnPropertyDescriptor(cx, obj, id, ¤tDesc)) + return false; - if (!desc.object()) - continue; + // 9.a.iii. + if (!currentDesc.object()) + continue; - unsigned attrs = desc.attributes(); - unsigned new_attrs = GetSealedOrFrozenAttributes(attrs, level); + // 9.a.iii.1-2 + if (currentDesc.isAccessorDescriptor()) + desc.setAttributes(AllowConfigure | JSPROP_PERMANENT); + else + desc.setAttributes(AllowConfigureAndWritable | JSPROP_PERMANENT | JSPROP_READONLY); + } - // If we already have the attributes we need, skip the setAttributes call. - if ((attrs | new_attrs) == attrs) - continue; + desc.object().set(obj); - attrs |= new_attrs; - if (!SetPropertyAttributes(cx, obj, id, &attrs)) + // 8.a.i-ii. / 9.a.iii.3-4 + bool result; + if (!StandardDefineProperty(cx, obj, id, desc, &result)) return false; } } // Ordinarily ArraySetLength handles this, but we're going behind its back // right now, so we must do this manually. Neither the custom property - // tree mutations nor the setPropertyAttributes call in the above code will + // tree mutations nor the StandardDefineProperty call in the above code will // do this for us. // // ArraySetLength also implements the capacity <= length invariant for diff --git a/js/src/jsobj.h b/js/src/jsobj.h index e470496a0fe2..27ca10ebcc7a 100644 --- a/js/src/jsobj.h +++ b/js/src/jsobj.h @@ -940,13 +940,6 @@ LookupProperty(JSContext *cx, HandleObject obj, PropertyName *name, extern bool HasOwnProperty(JSContext *cx, HandleObject obj, HandleId id, bool *result); -/* - * Deprecated. Search the prototype chain for `obj[id]` and redefine it to have - * the given property attributes. - */ -inline bool -SetPropertyAttributes(JSContext *cx, HandleObject obj, HandleId id, unsigned *attrsp); - /* * Set a watchpoint: a synchronous callback when the given property of the * given object is set. diff --git a/js/src/jsobjinlines.h b/js/src/jsobjinlines.h index 02daa602b029..379fd76c0b0b 100644 --- a/js/src/jsobjinlines.h +++ b/js/src/jsobjinlines.h @@ -196,16 +196,6 @@ js::DeleteElement(JSContext *cx, HandleObject obj, uint32_t index, bool *succeed /* * */ -inline bool -js::SetPropertyAttributes(JSContext *cx, HandleObject obj, HandleId id, unsigned *attrsp) -{ - MarkTypePropertyNonData(cx, obj, id); - SetAttributesOp op = obj->getOps()->setAttributes; - if (op) - return op(cx, obj, id, attrsp); - return NativeSetPropertyAttributes(cx, obj.as(), id, attrsp); -} - inline bool JSObject::isQualifiedVarObj() { diff --git a/js/src/proxy/Proxy.cpp b/js/src/proxy/Proxy.cpp index 21bb65a060ed..bd300650fbb0 100644 --- a/js/src/proxy/Proxy.cpp +++ b/js/src/proxy/Proxy.cpp @@ -579,17 +579,6 @@ js::proxy_GetOwnPropertyDescriptor(JSContext *cx, HandleObject obj, HandleId id, return Proxy::getOwnPropertyDescriptor(cx, obj, id, desc); } -bool -js::proxy_SetPropertyAttributes(JSContext *cx, HandleObject obj, HandleId id, unsigned *attrsp) -{ - /* Lookup the current property descriptor so we have setter/getter/value. */ - Rooted desc(cx); - if (!Proxy::getOwnPropertyDescriptor(cx, obj, id, &desc)) - return false; - desc.setAttributes(*attrsp); - return Proxy::defineProperty(cx, obj, id, &desc); -} - bool js::proxy_DeleteProperty(JSContext *cx, HandleObject obj, HandleId id, bool *succeeded) { diff --git a/js/src/vm/NativeObject.cpp b/js/src/vm/NativeObject.cpp index 34c21d8151cc..431677d2dcb6 100644 --- a/js/src/vm/NativeObject.cpp +++ b/js/src/vm/NativeObject.cpp @@ -2266,37 +2266,3 @@ js::NativeDeleteProperty(JSContext *cx, HandleNativeObject obj, HandleId id, boo return SuppressDeletedProperty(cx, obj, id); } - -/* * */ - -bool -js::NativeSetPropertyAttributes(JSContext *cx, HandleNativeObject obj, HandleId id, - unsigned *attrsp) -{ - RootedObject nobj(cx); - RootedShape shape(cx); - if (!NativeLookupProperty(cx, obj, id, &nobj, &shape)) - return false; - if (!shape) - return true; - if (nobj->isNative() && IsImplicitDenseOrTypedArrayElement(shape)) { - if (IsAnyTypedArray(nobj.get())) { - if (*attrsp == (JSPROP_ENUMERATE | JSPROP_PERMANENT)) - return true; - JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_SET_ARRAY_ATTRS); - return false; - } - if (!NativeObject::sparsifyDenseElement(cx, nobj.as(), JSID_TO_INT(id))) - return false; - shape = nobj->as().lookup(cx, id); - } - if (nobj->isNative()) { - if (!NativeObject::changePropertyAttributes(cx, nobj.as(), shape, *attrsp)) - return false; - if (*attrsp & JSPROP_READONLY) - MarkTypePropertyNonWritable(cx, nobj, id); - return true; - } else { - return SetPropertyAttributes(cx, nobj, id, attrsp); - } -} diff --git a/js/src/vm/NativeObject.h b/js/src/vm/NativeObject.h index 1c2f0629ae30..4529b5cb6883 100644 --- a/js/src/vm/NativeObject.h +++ b/js/src/vm/NativeObject.h @@ -1344,10 +1344,6 @@ extern bool NativeGetExistingProperty(JSContext *cx, HandleObject receiver, HandleNativeObject obj, HandleShape shape, MutableHandle vp); -extern bool -NativeSetPropertyAttributes(JSContext *cx, HandleNativeObject obj, HandleId id, unsigned *attrsp); - - /* * */ /* diff --git a/js/src/vm/ScopeObject.cpp b/js/src/vm/ScopeObject.cpp index 4b560f53eebb..2c4ce25795dd 100644 --- a/js/src/vm/ScopeObject.cpp +++ b/js/src/vm/ScopeObject.cpp @@ -500,13 +500,6 @@ with_GetOwnPropertyDescriptor(JSContext *cx, HandleObject obj, HandleId id, return GetOwnPropertyDescriptor(cx, actual, id, desc); } -static bool -with_SetPropertyAttributes(JSContext *cx, HandleObject obj, HandleId id, unsigned *attrsp) -{ - RootedObject actual(cx, &obj->as().object()); - return SetPropertyAttributes(cx, actual, id, attrsp); -} - static bool with_DeleteProperty(JSContext *cx, HandleObject obj, HandleId id, bool *succeeded) { @@ -551,7 +544,6 @@ const Class DynamicWithObject::class_ = { with_GetProperty, with_SetProperty, with_GetOwnPropertyDescriptor, - with_SetPropertyAttributes, with_DeleteProperty, nullptr, nullptr, /* watch/unwatch */ nullptr, /* getElements */ @@ -943,13 +935,6 @@ uninitialized_GetOwnPropertyDescriptor(JSContext *cx, HandleObject obj, HandleId return false; } -static bool -uninitialized_SetPropertyAttributes(JSContext *cx, HandleObject obj, HandleId id, unsigned *attrsp) -{ - ReportUninitializedLexicalId(cx, id); - return false; -} - static bool uninitialized_DeleteProperty(JSContext *cx, HandleObject obj, HandleId id, bool *succeeded) { @@ -981,7 +966,6 @@ const Class UninitializedLexicalObject::class_ = { uninitialized_GetProperty, uninitialized_SetProperty, uninitialized_GetOwnPropertyDescriptor, - uninitialized_SetPropertyAttributes, uninitialized_DeleteProperty, nullptr, nullptr, /* watch/unwatch */ nullptr, /* getElements */ diff --git a/js/src/vm/UnboxedObject.cpp b/js/src/vm/UnboxedObject.cpp index a0d27c437dd2..f32cf45d0693 100644 --- a/js/src/vm/UnboxedObject.cpp +++ b/js/src/vm/UnboxedObject.cpp @@ -333,15 +333,6 @@ UnboxedPlainObject::obj_getOwnPropertyDescriptor(JSContext *cx, HandleObject obj return true; } -/* static */ bool -UnboxedPlainObject::obj_setPropertyAttributes(JSContext *cx, HandleObject obj, - HandleId id, unsigned *attrsp) -{ - if (!obj->as().convertToNative(cx)) - return false; - return SetPropertyAttributes(cx, obj, id, attrsp); -} - /* static */ bool UnboxedPlainObject::obj_deleteProperty(JSContext *cx, HandleObject obj, HandleId id, bool *succeeded) @@ -393,7 +384,6 @@ const Class UnboxedPlainObject::class_ = { UnboxedPlainObject::obj_getProperty, UnboxedPlainObject::obj_setProperty, UnboxedPlainObject::obj_getOwnPropertyDescriptor, - UnboxedPlainObject::obj_setPropertyAttributes, UnboxedPlainObject::obj_deleteProperty, UnboxedPlainObject::obj_watch, nullptr, /* No unwatch needed, as watch() converts the object to native */ diff --git a/js/src/vm/UnboxedObject.h b/js/src/vm/UnboxedObject.h index 2a54d2ef107e..2bfd488f3f64 100644 --- a/js/src/vm/UnboxedObject.h +++ b/js/src/vm/UnboxedObject.h @@ -148,9 +148,6 @@ class UnboxedPlainObject : public JSObject static bool obj_getOwnPropertyDescriptor(JSContext *cx, HandleObject obj, HandleId id, MutableHandle desc); - static bool obj_setPropertyAttributes(JSContext *cx, HandleObject obj, - HandleId id, unsigned *attrsp); - static bool obj_deleteProperty(JSContext *cx, HandleObject obj, HandleId id, bool *succeeded); static bool obj_enumerate(JSContext *cx, HandleObject obj, AutoIdVector &properties); diff --git a/js/xpconnect/src/XPCWrappedNativeJSOps.cpp b/js/xpconnect/src/XPCWrappedNativeJSOps.cpp index 979aec86e685..950801e80864 100644 --- a/js/xpconnect/src/XPCWrappedNativeJSOps.cpp +++ b/js/xpconnect/src/XPCWrappedNativeJSOps.cpp @@ -686,7 +686,6 @@ const XPCWrappedNativeJSClass XPC_WN_NoHelper_JSClass = { nullptr, // getProperty nullptr, // setProperty nullptr, // getOwnPropertyDescriptor - nullptr, // setPropertyAttributes nullptr, // deleteProperty nullptr, nullptr, // watch/unwatch nullptr, // getElements diff --git a/js/xpconnect/src/xpcprivate.h b/js/xpconnect/src/xpcprivate.h index 7420f0a94b89..273c826cb8a6 100644 --- a/js/xpconnect/src/xpcprivate.h +++ b/js/xpconnect/src/xpcprivate.h @@ -978,7 +978,6 @@ XPC_WN_JSOp_ThisObject(JSContext *cx, JS::HandleObject obj); nullptr, /* getProperty */ \ nullptr, /* setProperty */ \ nullptr, /* getOwnPropertyDescriptor */ \ - nullptr, /* setPropertyAttributes */ \ nullptr, /* deleteProperty */ \ nullptr, nullptr, /* watch/unwatch */ \ nullptr, /* getElements */ \ @@ -993,7 +992,6 @@ XPC_WN_JSOp_ThisObject(JSContext *cx, JS::HandleObject obj); nullptr, /* getProperty */ \ nullptr, /* setProperty */ \ nullptr, /* getOwnPropertyDescriptor */ \ - nullptr, /* setPropertyAttributes */ \ nullptr, /* deleteProperty */ \ nullptr, nullptr, /* watch/unwatch */ \ nullptr, /* getElements */ \