From 57ebeb1ec71860f823def10d8dd1d8152fcdde86 Mon Sep 17 00:00:00 2001 From: Tom Schuster Date: Wed, 11 Feb 2015 23:40:47 +0100 Subject: [PATCH] Bug 1125437 - Remove CheckDefineProperty and use StandardDefineProperty instead. r=efaust --- js/ipc/WrapperAnswer.cpp | 21 +------ .../tests/proxy/defineProperty-fallback.js | 8 +++ js/src/jsfriendapi.h | 18 ------ js/src/jsobj.cpp | 63 ------------------- js/src/proxy/DirectProxyHandler.cpp | 9 +-- 5 files changed, 13 insertions(+), 106 deletions(-) create mode 100644 js/src/jit-test/tests/proxy/defineProperty-fallback.js diff --git a/js/ipc/WrapperAnswer.cpp b/js/ipc/WrapperAnswer.cpp index 883ea710582..80018aaa11d 100644 --- a/js/ipc/WrapperAnswer.cpp +++ b/js/ipc/WrapperAnswer.cpp @@ -179,26 +179,9 @@ WrapperAnswer::RecvDefineProperty(const ObjectId &objId, const JSIDVariant &idVa if (!toDescriptor(cx, descriptor, &desc)) return fail(cx, rs); - if (!js::CheckDefineProperty(cx, obj, id, desc.value(), desc.attributes(), - desc.getter(), desc.setter())) - { + bool ignored; + if (!js_DefineOwnProperty(cx, obj, id, desc, &ignored)) return fail(cx, rs); - } - - if (!JS_DefinePropertyById(cx, obj, id, desc.value(), - // Descrriptors never store JSNatives for - // accessors: they have either JSFunctions or - // JSPropertyOps. - desc.attributes() | JSPROP_PROPOP_ACCESSORS, - JS_PROPERTYOP_GETTER(desc.getter() - ? desc.getter() - : JS_PropertyStub), - JS_PROPERTYOP_SETTER(desc.setter() - ? desc.setter() - : JS_StrictPropertyStub))) - { - return fail(cx, rs); - } return ok(rs); } diff --git a/js/src/jit-test/tests/proxy/defineProperty-fallback.js b/js/src/jit-test/tests/proxy/defineProperty-fallback.js new file mode 100644 index 00000000000..e8f52dc3827 --- /dev/null +++ b/js/src/jit-test/tests/proxy/defineProperty-fallback.js @@ -0,0 +1,8 @@ +"use strict"; +var obj = {}; +Object.defineProperty(obj, "test", {configurable: false, writable: false, value: "hey"}); +Object.defineProperty(obj, "test", {configurable: false, writable: false}); + +var wrapper = new Proxy(obj, {}); +Object.defineProperty(wrapper, "test", {configurable: false, writable: false}); +assertEq(wrapper.test, "hey"); diff --git a/js/src/jsfriendapi.h b/js/src/jsfriendapi.h index c4373ce6e37..3b8da6f759d 100644 --- a/js/src/jsfriendapi.h +++ b/js/src/jsfriendapi.h @@ -2546,24 +2546,6 @@ GetElementsWithAdder(JSContext *cx, JS::HandleObject obj, JS::HandleObject recei JS_FRIEND_API(bool) ForwardToNative(JSContext *cx, JSNative native, const JS::CallArgs &args); -/* - * Helper function. To approximate a call to the [[DefineOwnProperty]] internal - * method described in ES5, first call this, then call JS_DefinePropertyById. - * - * JS_DefinePropertyById by itself does not enforce the invariants on - * non-configurable properties when obj->isNative(). This function performs the - * relevant checks (specified in ES5 8.12.9 [[DefineOwnProperty]] steps 1-11), - * but only if obj is native. - * - * The reason for the messiness here is that ES5 uses [[DefineOwnProperty]] as - * a sort of extension point, but there is no hook in js::Class, - * js::ProxyHandler, or the JSAPI with precisely the right semantics for it. - */ -extern JS_FRIEND_API(bool) -CheckDefineProperty(JSContext *cx, JS::HandleObject obj, JS::HandleId id, JS::HandleValue value, - unsigned attrs, - JSPropertyOp getter = nullptr, JSStrictPropertyOp setter = nullptr); - /* * Helper function for HTMLDocument and HTMLFormElement. * diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index f3766a47ece..f8cf0938b61 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -466,69 +466,6 @@ Reject(JSContext *cx, HandleId id, unsigned errorNumber, bool throwError, bool * return true; } -static unsigned -ApplyOrDefaultAttributes(unsigned attrs, Handle desc) -{ - bool present = !!desc.object(); - bool enumerable = present ? desc.isEnumerable() : false; - bool writable = present ? !desc.isReadonly() : false; - bool configurable = present ? !desc.isPermanent() : false; - return ApplyAttributes(attrs, enumerable, writable, configurable); -} - -// See comments on CheckDefineProperty in jsfriendapi.h. -// -// DefinePropertyOnObject has its own implementation of these checks. -// -JS_FRIEND_API(bool) -js::CheckDefineProperty(JSContext *cx, HandleObject obj, HandleId id, HandleValue value, - unsigned attrs, PropertyOp getter, StrictPropertyOp setter) -{ - MOZ_ASSERT(getter != JS_PropertyStub); - MOZ_ASSERT(setter != JS_StrictPropertyStub); - - if (!obj->isNative()) - return true; - - // ES5 8.12.9 Step 1. Even though we know obj is native, we use generic - // APIs for shorter, more readable code. - Rooted desc(cx); - if (!GetOwnPropertyDescriptor(cx, obj, id, &desc)) - return false; - - // Appropriately handle the potential for ignored attributes. Since the proxy code calls us - // directly, these might flow in legitimately. Ensure that we compare against the values that - // are intended. - attrs = ApplyOrDefaultAttributes(attrs, desc) & ~JSPROP_IGNORE_VALUE; - - // This does not have to check obj's extensibility when !desc.obj (steps - // 2-3) because the low-level methods JSObject::{add,put}Property check - // for that. - if (desc.object() && desc.isPermanent()) { - // Steps 6-11, skipping step 10.a.ii. Prohibit redefining a permanent - // property with different metadata, except to make a writable property - // non-writable. - if (getter != desc.getter() || - setter != desc.setter() || - (attrs != desc.attributes() && attrs != (desc.attributes() | JSPROP_READONLY))) - { - return Throw(cx, id, JSMSG_CANT_REDEFINE_PROP); - } - - // Step 10.a.ii. Prohibit changing the value of a non-configurable, - // non-writable data property. - if ((desc.attributes() & (JSPROP_GETTER | JSPROP_SETTER | JSPROP_READONLY)) == JSPROP_READONLY) { - bool same; - if (!SameValue(cx, value, desc.value(), &same)) - return false; - if (!same) - return JSObject::reportReadOnly(cx, id); - } - } - return true; -} - - /*** Standard-compliant property definition (used by Object.defineProperty) **********************/ static bool diff --git a/js/src/proxy/DirectProxyHandler.cpp b/js/src/proxy/DirectProxyHandler.cpp index bcdf5e07e6e..da3044d6c4a 100644 --- a/js/src/proxy/DirectProxyHandler.cpp +++ b/js/src/proxy/DirectProxyHandler.cpp @@ -29,7 +29,7 @@ DirectProxyHandler::getOwnPropertyDescriptor(JSContext *cx, HandleObject proxy, { assertEnteredPolicy(cx, proxy, id, GET | SET | GET_PROPERTY_DESCRIPTOR); RootedObject target(cx, proxy->as().target()); - return js::GetOwnPropertyDescriptor(cx, target, id, desc); + return GetOwnPropertyDescriptor(cx, target, id, desc); } bool @@ -38,11 +38,8 @@ DirectProxyHandler::defineProperty(JSContext *cx, HandleObject proxy, HandleId i { assertEnteredPolicy(cx, proxy, id, SET); RootedObject target(cx, proxy->as().target()); - RootedValue v(cx, desc.value()); - return CheckDefineProperty(cx, target, id, v, desc.attributes(), - desc.getter(), desc.setter()) && - DefineProperty(cx, target, id, v, desc.getter(), desc.setter(), - desc.attributes()); + bool ignored; + return StandardDefineProperty(cx, target, id, desc, &ignored); } bool