Bug 603201 - Make HasPropertyOnPrototype fallible. r=peterv

dom/base/WindowNamedPropertiesHandler.cpp

@@ -83,8 +83,11 @@ WindowNamedPropertiesHandler::getOwnPropDescriptor(JSContext* aCx,
     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;
   }
 
@@ -94,6 +97,7 @@ WindowNamedPropertiesHandler::getOwnPropDescriptor(JSContext* aCx,
   }
 
   // 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);

dom/bindings/BindingUtils.cpp

@@ -1613,7 +1613,7 @@ NativePropertyHooks sWorkerNativePropertyHooks = {
 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)) {
@@ -1624,33 +1624,31 @@ GetPropertyOnPrototype(JSContext* cx, JS::Handle<JSObject*> proxy,
     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
@@ -1670,7 +1668,16 @@ AppendNamedPropertyIds(JSContext* cx, JS::Handle<JSObject*> proxy,
       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;
       }

dom/bindings/BindingUtils.h

@@ -1810,16 +1810,15 @@ 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

dom/bindings/Codegen.py

@@ -10104,15 +10104,41 @@ class CGDOMJSProxyHandler_getOwnPropDescriptor(ClassMethod):
                 "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 = ""
 
@@ -10345,8 +10371,16 @@ class CGDOMJSProxyHandler_delete(ClassMethod):
                 """,
                 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("""
 
@@ -10485,8 +10519,17 @@ class CGDOMJSProxyHandler_hasOwn(ClassMethod):
                 """,
                 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"
@@ -10593,7 +10636,7 @@ class CGDOMJSProxyHandler_get(ClassMethod):
 
         getOnPrototype = dedent("""
             bool foundOnPrototype;
-            if (!GetPropertyOnPrototype(cx, proxy, id, &foundOnPrototype, vp.address())) {
+            if (!GetPropertyOnPrototype(cx, proxy, id, &foundOnPrototype, vp)) {
               return false;
             }