Bug 603201 - Make HasPropertyOnPrototype fallible. r=peterv
authorJeff Walden <jwalden@mit.edu>
Fri, 02 Jan 2015 14:02:36 -0600
changeset 222154 1298539468f2d22f36603d78e8e8f94e9c821d61
parent 222153 d9557e125ac38a0701f75037514466c54e5849ba
child 222155 2639ebee70ab2b0e4c3301491e800fe04a6ab839
push id28059
push userryanvm@gmail.com
push dateTue, 06 Jan 2015 15:53:01 +0000
treeherdermozilla-central@4d91c33b351c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv
bugs603201
milestone37.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 603201 - Make HasPropertyOnPrototype fallible. r=peterv
dom/base/WindowNamedPropertiesHandler.cpp
dom/bindings/BindingUtils.cpp
dom/bindings/BindingUtils.h
dom/bindings/Codegen.py
--- a/dom/base/WindowNamedPropertiesHandler.cpp
+++ b/dom/base/WindowNamedPropertiesHandler.cpp
@@ -78,27 +78,31 @@ WindowNamedPropertiesHandler::getOwnProp
                                                    JS::MutableHandle<JSPropertyDescriptor> aDesc)
                                                    const
 {
   if (!JSID_IS_STRING(aId)) {
     // Nothing to do if we're resolving a non-string property.
     return true;
   }
 
-  JS::Rooted<JSObject*> global(aCx, JS_GetGlobalForObject(aCx, aProxy));
-  if (HasPropertyOnPrototype(aCx, aProxy, aId)) {
+  bool hasOnPrototype;
+  if (!HasPropertyOnPrototype(aCx, aProxy, aId, &hasOnPrototype)) {
+    return false;
+  }
+  if (hasOnPrototype) {
     return true;
   }
 
   nsAutoJSString str;
   if (!str.init(aCx, JSID_TO_STRING(aId))) {
     return false;
   }
 
   // Grab the DOM window.
+  JS::Rooted<JSObject*> global(aCx, JS_GetGlobalForObject(aCx, aProxy));
   nsGlobalWindow* win = xpc::WindowOrNull(global);
   if (win->Length() > 0) {
     nsCOMPtr<nsIDOMWindow> childWin = win->GetChildWindow(str);
     if (childWin && ShouldExposeChildWindow(str, childWin)) {
       // We found a subframe of the right name. Shadowing via |var foo| in
       // global scope is still allowed, since |var| only looks up |own|
       // properties. But unqualified shadowing will fail, per-spec.
       JS::Rooted<JS::Value> v(aCx);
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -1608,54 +1608,52 @@ NativePropertyHooks sWorkerNativePropert
   prototypes::id::_ID_Count,
   constructors::id::_ID_Count,
   nullptr
 };
 
 bool
 GetPropertyOnPrototype(JSContext* cx, JS::Handle<JSObject*> proxy,
                        JS::Handle<jsid> id, bool* found,
-                       JS::Value* vp)
+                       JS::MutableHandle<JS::Value> vp)
 {
   JS::Rooted<JSObject*> proto(cx);
   if (!js::GetObjectProto(cx, proxy, &proto)) {
     return false;
   }
   if (!proto) {
     *found = false;
     return true;
   }
 
-  bool hasProp;
-  if (!JS_HasPropertyById(cx, proto, id, &hasProp)) {
+  if (!JS_HasPropertyById(cx, proto, id, found)) {
     return false;
   }
 
-  *found = hasProp;
-  if (!hasProp || !vp) {
+  if (!*found) {
     return true;
   }
 
-  JS::Rooted<JS::Value> value(cx);
-  if (!JS_ForwardGetPropertyTo(cx, proto, id, proxy, &value)) {
-    return false;
-  }
-
-  *vp = value;
-  return true;
+  return JS_ForwardGetPropertyTo(cx, proto, id, proxy, vp);
 }
 
 bool
 HasPropertyOnPrototype(JSContext* cx, JS::Handle<JSObject*> proxy,
-                       JS::Handle<jsid> id)
+                       JS::Handle<jsid> id, bool* has)
 {
-  bool found;
-  // We ignore an error from GetPropertyOnPrototype.  We pass nullptr
-  // for vp so that GetPropertyOnPrototype won't actually do a get.
-  return !GetPropertyOnPrototype(cx, proxy, id, &found, nullptr) || found;
+  JS::Rooted<JSObject*> proto(cx);
+  if (!js::GetObjectProto(cx, proxy, &proto)) {
+    return false;
+  }
+  if (!proto) {
+    *has = false;
+    return true;
+  }
+
+  return JS_HasPropertyById(cx, proto, id, has);
 }
 
 bool
 AppendNamedPropertyIds(JSContext* cx, JS::Handle<JSObject*> proxy,
                        nsTArray<nsString>& names,
                        bool shadowPrototypeProperties,
                        JS::AutoIdVector& props)
 {
@@ -1665,17 +1663,26 @@ AppendNamedPropertyIds(JSContext* cx, JS
       return false;
     }
 
     JS::Rooted<jsid> id(cx);
     if (!JS_ValueToId(cx, v, &id)) {
       return false;
     }
 
-    if (shadowPrototypeProperties || !HasPropertyOnPrototype(cx, proxy, id)) {
+    bool shouldAppend = shadowPrototypeProperties;
+    if (!shouldAppend) {
+      bool has;
+      if (!HasPropertyOnPrototype(cx, proxy, id, &has)) {
+        return false;
+      }
+      shouldAppend = !has;
+    }
+
+    if (shouldAppend) {
       if (!props.append(id)) {
         return false;
       }
     }
   }
 
   return true;
 }
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -1805,26 +1805,25 @@ bool
 UnforgeableValueOf(JSContext* cx, unsigned argc, JS::Value* vp);
 
 bool
 ThrowingConstructor(JSContext* cx, unsigned argc, JS::Value* vp);
 
 bool
 ThrowConstructorWithoutNew(JSContext* cx, const char* name);
 
-// vp is allowed to be null; in that case no get will be attempted,
-// and *found will simply indicate whether the property exists.
 bool
 GetPropertyOnPrototype(JSContext* cx, JS::Handle<JSObject*> proxy,
                        JS::Handle<jsid> id, bool* found,
-                       JS::Value* vp);
-
+                       JS::MutableHandle<JS::Value> vp);
+
+//
 bool
 HasPropertyOnPrototype(JSContext* cx, JS::Handle<JSObject*> proxy,
-                       JS::Handle<jsid> id);
+                       JS::Handle<jsid> id, bool* has);
 
 
 // Append the property names in "names" to "props". If
 // shadowPrototypeProperties is false then skip properties that are also
 // present on the proto chain of proxy.  If shadowPrototypeProperties is true,
 // then the "proxy" argument is ignored.
 bool
 AppendNamedPropertyIds(JSContext* cx, JS::Handle<JSObject*> proxy,
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -10099,25 +10099,51 @@ class CGDOMJSProxyHandler_getOwnPropDesc
                 # "arguments" as opposed to return type, [0] means first (and
                 # only) argument.
                 operations['NamedGetter'].signatures()[0][1][0].identifier.name)
             fillDescriptor = (
                 "FillPropertyDescriptor(desc, proxy, %s, %s);\n"
                 "return true;\n" % (readonly, enumerable))
             templateValues = {'jsvalRef': 'desc.value()', 'jsvalHandle': 'desc.value()',
                               'obj': 'proxy', 'successCode': fillDescriptor}
-            condition = "!HasPropertyOnPrototype(cx, proxy, id)"
+
+            computeCondition = dedent("""
+                bool hasOnProto;
+                if (!HasPropertyOnPrototype(cx, proxy, id, &hasOnProto)) {
+                  return false;
+                }
+                callNamedGetter = !hasOnProto;
+                """)
             if self.descriptor.interface.getExtendedAttribute('OverrideBuiltins'):
-                condition = "(!isXray || %s)" % condition
-            condition = "!ignoreNamedProps && " + condition
+                computeCondition = fill("""
+                    if (!isXray) {
+                      callNamedGetter = true;
+                    } else {
+                      $*{hasOnProto}
+                    }
+                    """,
+                    hasOnProto=computeCondition)
+
+            outerCondition = "!ignoreNamedProps"
             if self.descriptor.supportsIndexedProperties():
-                condition = "!IsArrayIndex(index) && " + condition
-            namedGet = (CGIfWrapper(CGProxyNamedGetter(self.descriptor, templateValues),
-                                    condition).define() +
-                        "\n")
+                outerCondition = "!IsArrayIndex(index) && " + outerCondition
+
+            namedGet = fill("""
+                bool callNamedGetter = false;
+                if (${outerCondition}) {
+                  $*{computeCondition}
+                }
+                if (callNamedGetter) {
+                  $*{namedGetCode}
+                }
+                """,
+                outerCondition=outerCondition,
+                computeCondition=computeCondition,
+                namedGetCode=CGProxyNamedGetter(self.descriptor, templateValues).define())
+            namedGet += "\n"
         else:
             namedGet = ""
 
         return fill(
             """
             bool isXray = xpc::WrapperFactory::IsXrayWrapper(proxy);
             $*{getIndexed}
             $*{getUnforgeable}
@@ -10340,18 +10366,26 @@ class CGDOMJSProxyHandler_delete(ClassMe
                 bool found = false;
                 $*{namedBody}
                 if (found) {
                   return true;
                 }
                 """,
                 namedBody=namedBody)
             if not self.descriptor.interface.getExtendedAttribute('OverrideBuiltins'):
-                delete = CGIfWrapper(CGGeneric(delete),
-                                     "!HasPropertyOnPrototype(cx, proxy, id)").define()
+                delete = fill("""
+                    bool hasOnProto;
+                    if (!HasPropertyOnPrototype(cx, proxy, id, &hasOnProto)) {
+                      return false;
+                    }
+                    if (!hasOnProto) {
+                      $*{delete}
+                    }
+                    """,
+                    delete=delete)
 
         delete += dedent("""
 
             return dom::DOMProxyHandler::delete_(cx, proxy, id, bp);
             """)
 
         return delete
 
@@ -10480,18 +10514,27 @@ class CGDOMJSProxyHandler_hasOwn(ClassMe
                 """
                 bool found = false;
                 $*{presenceChecker}
 
                 *bp = found;
                 """,
                 presenceChecker=CGProxyNamedPresenceChecker(self.descriptor, foundVar="found").define())
             if not self.descriptor.interface.getExtendedAttribute('OverrideBuiltins'):
-                named = CGIfWrapper(CGGeneric(named + "return true;\n"),
-                                    "!HasPropertyOnPrototype(cx, proxy, id)").define()
+                named = fill("""
+                    bool hasOnProto;
+                    if (!HasPropertyOnPrototype(cx, proxy, id, &hasOnProto)) {
+                      return false;
+                    }
+                    if (!hasOnProto) {
+                      $*{protoLacksProperty}
+                      return true;
+                    }
+                    """,
+                    protoLacksProperty=named)
                 named += "*bp = false;\n"
             else:
                 named += "\n"
         else:
             named = "*bp = false;\n"
 
         return fill(
             """
@@ -10588,17 +10631,17 @@ class CGDOMJSProxyHandler_get(ClassMetho
             if self.descriptor.supportsIndexedProperties():
                 getNamed = CGIfWrapper(getNamed, "!IsArrayIndex(index)")
             getNamed = getNamed.define() + "\n"
         else:
             getNamed = ""
 
         getOnPrototype = dedent("""
             bool foundOnPrototype;
-            if (!GetPropertyOnPrototype(cx, proxy, id, &foundOnPrototype, vp.address())) {
+            if (!GetPropertyOnPrototype(cx, proxy, id, &foundOnPrototype, vp)) {
               return false;
             }
 
             if (foundOnPrototype) {
               return true;
             }
 
             """)