Bug 1256688 - Change BPH::has to follow [[HasProperty]] for ordinary objects. r=jorendorff
authorTom Schuster <evilpies@gmail.com>
Sat, 19 Mar 2016 01:30:03 +0100
changeset 289430 7db58032977aaafc2bd70e035c1b6ae37aab207a
parent 289429 0ecd210e6b1e5ba43acd03416356178273b66fd5
child 289431 3f5ac8a1b9561e5a00b1efe80484d9c776a8dd11
push id73840
push userevilpies@gmail.com
push dateSat, 19 Mar 2016 00:30:35 +0000
treeherdermozilla-inbound@94f67b8f0583 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs1256688
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1256688 - Change BPH::has to follow [[HasProperty]] for ordinary objects. r=jorendorff
dom/bindings/DOMJSProxyHandler.cpp
dom/bindings/DOMJSProxyHandler.h
js/src/proxy/BaseProxyHandler.cpp
--- a/dom/bindings/DOMJSProxyHandler.cpp
+++ b/dom/bindings/DOMJSProxyHandler.cpp
@@ -255,45 +255,16 @@ bool
 BaseDOMProxyHandler::getOwnEnumerablePropertyKeys(JSContext* cx,
                                                   JS::Handle<JSObject*> proxy,
                                                   JS::AutoIdVector& props) const
 {
   return ownPropNames(cx, proxy, JSITER_OWNONLY, props);
 }
 
 bool
-DOMProxyHandler::has(JSContext* cx, JS::Handle<JSObject*> proxy, JS::Handle<jsid> id, bool* bp) const
-{
-  if (!hasOwn(cx, proxy, id, bp)) {
-    return false;
-  }
-
-  if (*bp) {
-    // We have the property ourselves; no need to worry about our prototype
-    // chain.
-    return true;
-  }
-
-  // OK, now we have to look at the proto
-  JS::Rooted<JSObject*> proto(cx);
-  if (!js::GetObjectProto(cx, proxy, &proto)) {
-    return false;
-  }
-  if (!proto) {
-    return true;
-  }
-  bool protoHasProp;
-  bool ok = JS_HasPropertyById(cx, proto, id, &protoHasProp);
-  if (ok) {
-    *bp = protoHasProp;
-  }
-  return ok;
-}
-
-bool
 DOMProxyHandler::setCustom(JSContext* cx, JS::Handle<JSObject*> proxy, JS::Handle<jsid> id,
                            JS::Handle<JS::Value> v, bool *done) const
 {
   *done = false;
   return true;
 }
 
 //static
--- a/dom/bindings/DOMJSProxyHandler.h
+++ b/dom/bindings/DOMJSProxyHandler.h
@@ -109,18 +109,16 @@ public:
                               JS::Handle<JS::PropertyDescriptor> desc,
                               JS::ObjectOpResult &result, bool *defined) const;
   bool delete_(JSContext* cx, JS::Handle<JSObject*> proxy, JS::Handle<jsid> id,
                JS::ObjectOpResult &result) const override;
   bool preventExtensions(JSContext* cx, JS::Handle<JSObject*> proxy,
                          JS::ObjectOpResult& result) const override;
   bool isExtensible(JSContext *cx, JS::Handle<JSObject*> proxy, bool *extensible)
                     const override;
-  bool has(JSContext* cx, JS::Handle<JSObject*> proxy, JS::Handle<jsid> id,
-           bool* bp) const override;
   bool set(JSContext *cx, JS::Handle<JSObject*> proxy, JS::Handle<jsid> id,
            JS::Handle<JS::Value> v, JS::Handle<JS::Value> receiver, JS::ObjectOpResult &result)
            const override;
 
   /*
    * If assigning to proxy[id] hits a named setter with OverrideBuiltins or
    * an indexed setter, call it and set *done to true on success. Otherwise, set
    * *done to false.
--- a/js/src/proxy/BaseProxyHandler.cpp
+++ b/js/src/proxy/BaseProxyHandler.cpp
@@ -21,20 +21,42 @@ BaseProxyHandler::enter(JSContext* cx, H
     *bp = true;
     return true;
 }
 
 bool
 BaseProxyHandler::has(JSContext* cx, HandleObject proxy, HandleId id, bool* bp) const
 {
     assertEnteredPolicy(cx, proxy, id, GET);
-    Rooted<PropertyDescriptor> desc(cx);
-    if (!getPropertyDescriptor(cx, proxy, id, &desc))
+
+    // This method is not covered by any spec, but we follow ES 2016
+    // (February 11, 2016) 9.1.7.1 fairly closely.
+
+    // Step 2. (Step 1 is a superfluous assertion.)
+    // Non-standard: Use our faster hasOwn trap.
+    if (!hasOwn(cx, proxy, id, bp))
         return false;
-    *bp = !!desc.object();
+
+    // Step 3.
+    if (*bp)
+        return true;
+
+    // The spec calls this variable "parent", but that word has weird
+    // connotations in SpiderMonkey, so let's go with "proto".
+    // Step 4.
+    RootedObject proto(cx);
+    if (!GetPrototype(cx, proxy, &proto))
+        return false;
+
+    // Step 5.,5.a.
+    if (proto)
+        return HasProperty(cx, proto, id, bp);
+
+    // Step 6.
+    *bp = false;
     return true;
 }
 
 bool
 BaseProxyHandler::getPropertyDescriptor(JSContext* cx, HandleObject proxy, HandleId id,
                                         MutableHandle<PropertyDescriptor> desc) const
 {
     assertEnteredPolicy(cx, proxy, id, GET | SET | GET_PROPERTY_DESCRIPTOR);