From 811d8614fd9266b1694be8462541d645a939b13c Mon Sep 17 00:00:00 2001 From: Tom Schuster Date: Fri, 6 Feb 2015 15:43:20 +0100 Subject: [PATCH] Bug 1124935 - Remove LookupProperty from JS_GetPropertyDescriptor. r=efaust,bz --- dom/html/test/test_bug1081037.html | 11 +---- js/src/asmjs/AsmJSLink.cpp | 4 +- js/src/jsapi-tests/moz.build | 1 + .../jsapi-tests/testGetPropertyDescriptor.cpp | 32 ++++++++++++++ js/src/jsapi.cpp | 42 +------------------ js/src/jsiter.cpp | 4 +- js/src/jsobj.cpp | 24 +++++++++++ js/src/jsobj.h | 4 ++ js/src/proxy/DirectProxyHandler.cpp | 2 +- js/src/proxy/Proxy.cpp | 2 +- js/src/proxy/ScriptedDirectProxyHandler.cpp | 2 +- 11 files changed, 70 insertions(+), 58 deletions(-) create mode 100644 js/src/jsapi-tests/testGetPropertyDescriptor.cpp diff --git a/dom/html/test/test_bug1081037.html b/dom/html/test/test_bug1081037.html index 783e8557c561..9d878258023d 100644 --- a/dom/html/test/test_bug1081037.html +++ b/dom/html/test/test_bug1081037.html @@ -96,7 +96,6 @@ shouldThrow(function() { var getOwn = 0; var defineProp = 0; -var _has = 0; var handler2 = { getOwnPropertyDescriptor: function(target, name) { if (name == "constructor") { @@ -109,12 +108,6 @@ var handler2 = { defineProp++; } return Object.defineProperty(target, name, propertyDescriptor); - }, - has: function(target, name) { - if (name == "constructor") { - _has++; - } - return name in target; } }; var proxy2 = new Proxy({}, handler2); @@ -125,8 +118,6 @@ document.registerElement('x-proxymagic2', { is(getOwn, 1, "number of getOwnPropertyDescriptor calls from registerElement: " + getOwn); is(defineProp, 1, "number of defineProperty calls from registerElement: " + defineProp); -is(_has, 1, "number of 'has' calls from registerElement: " + _has); - @@ -139,4 +130,4 @@ is(_has, 1, "number of 'has' calls from registerElement: " + _has);
- \ No newline at end of file + diff --git a/js/src/asmjs/AsmJSLink.cpp b/js/src/asmjs/AsmJSLink.cpp index 42457b9a65d0..94a4cedc337a 100644 --- a/js/src/asmjs/AsmJSLink.cpp +++ b/js/src/asmjs/AsmJSLink.cpp @@ -88,9 +88,9 @@ GetDataProperty(JSContext *cx, HandleValue objVal, HandlePropertyName field, Mut if (IsScriptedProxy(obj)) return LinkFail(cx, "accessing property of a Proxy"); - Rooted desc(cx); + Rooted desc(cx); RootedId id(cx, NameToId(field)); - if (!JS_GetPropertyDescriptorById(cx, obj, id, &desc)) + if (!GetPropertyDescriptor(cx, obj, id, &desc)) return false; if (!desc.object()) diff --git a/js/src/jsapi-tests/moz.build b/js/src/jsapi-tests/moz.build index 9babe2350778..0f2c4d65ab49 100644 --- a/js/src/jsapi-tests/moz.build +++ b/js/src/jsapi-tests/moz.build @@ -43,6 +43,7 @@ UNIFIED_SOURCES += [ 'testGCNursery.cpp', 'testGCOutOfMemory.cpp', 'testGCStoreBufferRemoval.cpp', + 'testGetPropertyDescriptor.cpp', 'testHashTable.cpp', 'testHashTableInit.cpp', 'testIndexToString.cpp', diff --git a/js/src/jsapi-tests/testGetPropertyDescriptor.cpp b/js/src/jsapi-tests/testGetPropertyDescriptor.cpp new file mode 100644 index 000000000000..3cc4def91a87 --- /dev/null +++ b/js/src/jsapi-tests/testGetPropertyDescriptor.cpp @@ -0,0 +1,32 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "jsapi-tests/tests.h" + +BEGIN_TEST(test_GetPropertyDescriptor) +{ + JS::RootedValue v(cx); + EVAL("({ somename : 123 })", &v); + CHECK(v.isObject()); + + JS::RootedObject obj(cx, &v.toObject()); + JS::Rooted desc(cx); + + CHECK(JS_GetPropertyDescriptor(cx, obj, "somename", &desc)); + CHECK_EQUAL(desc.object(), obj); + CHECK_SAME(desc.value(), JS::Int32Value(123)); + + CHECK(JS_GetPropertyDescriptor(cx, obj, "not-here", &desc)); + CHECK_EQUAL(desc.object(), nullptr); + + CHECK(JS_GetPropertyDescriptor(cx, obj, "toString", &desc)); + JS::RootedObject objectProto(cx, JS_GetObjectPrototype(cx, obj)); + CHECK(objectProto); + CHECK_EQUAL(desc.object(), objectProto); + CHECK(desc.value().isObject()); + CHECK(JS::IsCallable(&desc.value().toObject())); + + return true; +} +END_TEST(test_GetPropertyDescriptor) diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index 8c003eb47dbd..d8978e6ca012 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -2624,46 +2624,6 @@ JS::ParsePropertyDescriptorObject(JSContext *cx, return true; } -static bool -GetPropertyDescriptorById(JSContext *cx, HandleObject obj, HandleId id, - MutableHandle desc) -{ - RootedObject obj2(cx); - RootedShape shape(cx); - - if (!LookupProperty(cx, obj, id, &obj2, &shape)) - return false; - - desc.clear(); - if (!shape) - return true; - - desc.object().set(obj2); - if (obj2->isNative()) { - if (IsImplicitDenseOrTypedArrayElement(shape)) { - desc.setEnumerable(); - desc.value().set(obj2->as().getDenseOrTypedArrayElement(JSID_TO_INT(id))); - } else { - desc.setAttributes(shape->attributes()); - desc.setGetter(shape->getter()); - desc.setSetter(shape->setter()); - MOZ_ASSERT(desc.value().isUndefined()); - if (shape->hasSlot()) - desc.value().set(obj2->as().getSlot(shape->slot())); - } - - return true; - } - - // When we hit a proxy during lookup, the property might be - // on the prototype of the proxy, thus use getPropertyDescriptor. - if (obj2->is()) - return Proxy::getPropertyDescriptor(cx, obj2, id, desc); - - // Assume other non-natives (i.e. TypedObjects) behave in a sane way. - return GetOwnPropertyDescriptor(cx, obj2, id, desc); -} - JS_PUBLIC_API(bool) JS_GetOwnPropertyDescriptorById(JSContext *cx, HandleObject obj, HandleId id, MutableHandle desc) @@ -2689,7 +2649,7 @@ JS_PUBLIC_API(bool) JS_GetPropertyDescriptorById(JSContext *cx, HandleObject obj, HandleId id, MutableHandle desc) { - return GetPropertyDescriptorById(cx, obj, id, desc); + return GetPropertyDescriptor(cx, obj, id, desc); } JS_PUBLIC_API(bool) diff --git a/js/src/jsiter.cpp b/js/src/jsiter.cpp index 4ebc885c2844..6e8bdc3f5dd9 100644 --- a/js/src/jsiter.cpp +++ b/js/src/jsiter.cpp @@ -1173,7 +1173,7 @@ SuppressDeletedPropertyHelper(JSContext *cx, HandleObject obj, StringPredicate p return false; Rooted desc(cx); - if (!JS_GetPropertyDescriptorById(cx, proto, id, &desc)) + if (!GetPropertyDescriptor(cx, proto, id, &desc)) return false; if (desc.object()) { @@ -1183,7 +1183,7 @@ SuppressDeletedPropertyHelper(JSContext *cx, HandleObject obj, StringPredicate p } /* - * If JS_GetPropertyDescriptorById above removed a property from + * If GetPropertyDescriptorById above removed a property from * ni, start over. */ if (props_end != ni->props_end || props_cursor != ni->props_cursor) diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index 651cb9948d91..c38f81f28086 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -3338,6 +3338,30 @@ js::SetImmutablePrototype(ExclusiveContext *cx, HandleObject obj, bool *succeede return true; } +bool +js::GetPropertyDescriptor(JSContext *cx, HandleObject obj, HandleId id, + MutableHandle desc) +{ + RootedObject pobj(cx); + + for (pobj = obj; pobj;) { + if (pobj->is()) + return Proxy::getPropertyDescriptor(cx, pobj, id, desc); + + if (!GetOwnPropertyDescriptor(cx, pobj, id, desc)) + return false; + + if (desc.object()) + return true; + + if (!GetPrototype(cx, pobj, &pobj)) + return false; + } + + MOZ_ASSERT(!desc.object()); + return true; +} + bool js::ToPrimitive(JSContext *cx, HandleObject obj, JSType hint, MutableHandleValue vp) { diff --git a/js/src/jsobj.h b/js/src/jsobj.h index 90b93bb1a7c7..3ddbee62564e 100644 --- a/js/src/jsobj.h +++ b/js/src/jsobj.h @@ -925,6 +925,10 @@ DeleteElement(JSContext *cx, js::HandleObject obj, uint32_t index, bool *succeed extern bool SetImmutablePrototype(js::ExclusiveContext *cx, JS::HandleObject obj, bool *succeeded); +extern bool +GetPropertyDescriptor(JSContext *cx, HandleObject obj, HandleId id, + MutableHandle desc); + /* * Deprecated. A version of HasProperty that also returns the object on which * the property was found (but that information is unreliable for proxies), and diff --git a/js/src/proxy/DirectProxyHandler.cpp b/js/src/proxy/DirectProxyHandler.cpp index 29bd91fc9e1b..533f21a4c066 100644 --- a/js/src/proxy/DirectProxyHandler.cpp +++ b/js/src/proxy/DirectProxyHandler.cpp @@ -20,7 +20,7 @@ DirectProxyHandler::getPropertyDescriptor(JSContext *cx, HandleObject proxy, Han assertEnteredPolicy(cx, proxy, id, GET | SET | GET_PROPERTY_DESCRIPTOR); MOZ_ASSERT(!hasPrototype()); // Should never be called if there's a prototype. RootedObject target(cx, proxy->as().target()); - return JS_GetPropertyDescriptorById(cx, target, id, desc); + return GetPropertyDescriptor(cx, target, id, desc); } bool diff --git a/js/src/proxy/Proxy.cpp b/js/src/proxy/Proxy.cpp index 0093f513a23f..7ccdead5398a 100644 --- a/js/src/proxy/Proxy.cpp +++ b/js/src/proxy/Proxy.cpp @@ -111,7 +111,7 @@ Proxy::getPropertyDescriptor(JSContext *cx, HandleObject proxy, HandleId id, return false; if (desc.object()) return true; - INVOKE_ON_PROTOTYPE(cx, handler, proxy, JS_GetPropertyDescriptorById(cx, proto, id, desc)); + INVOKE_ON_PROTOTYPE(cx, handler, proxy, GetPropertyDescriptor(cx, proto, id, desc)); } bool diff --git a/js/src/proxy/ScriptedDirectProxyHandler.cpp b/js/src/proxy/ScriptedDirectProxyHandler.cpp index 6b9e17d25b5a..aaa43913704c 100644 --- a/js/src/proxy/ScriptedDirectProxyHandler.cpp +++ b/js/src/proxy/ScriptedDirectProxyHandler.cpp @@ -429,7 +429,7 @@ ScriptedDirectProxyHandler::getPropertyDescriptor(JSContext *cx, HandleObject pr MOZ_ASSERT(!desc.object()); return true; } - return JS_GetPropertyDescriptorById(cx, proto, id, desc); + return GetPropertyDescriptor(cx, proto, id, desc); } // ES6 (5 April 2014) 9.5.5 Proxy.[[GetOwnProperty]](P)