Bug 645468 - Remove js_TryMethod: its semantics aren't what most of its users want, and its utility is limited. r=luke

js/src/jsarray.cpp

@@ -1280,16 +1280,14 @@ array_toString_sub(JSContext *cx, JSObject *obj, JSBool locale,
         }
 
         /* Get element's character string. */
-        if (!(hole || rval->isNullOrUndefined())) {
+        if (!hole && !rval->isNullOrUndefined()) {
             if (locale) {
                 /* Work on obj.toLocalString() instead. */
-                JSObject *robj;
+                JSObject *robj = ToObject(cx, rval);
-
+                if (!robj)
-                if (!js_ValueToObjectOrNull(cx, *rval, &robj))
                     goto out;
-                rval->setObjectOrNull(robj);
+                jsid id = ATOM_TO_JSID(cx->runtime->atomState.toLocaleStringAtom);
-                JSAtom *atom = cx->runtime->atomState.toLocaleStringAtom;
+                if (!robj->callMethod(cx, id, 0, NULL, rval))
-                if (!js_TryMethod(cx, robj, atom, 0, NULL, rval))
                     goto out;
             }
 

js/src/jsobj.cpp

@@ -5480,6 +5480,14 @@ JSObject::reportNotExtensible(JSContext *cx, uintN report)
                                     NULL, NULL, NULL);
 }
 
+bool
+JSObject::callMethod(JSContext *cx, jsid id, uintN argc, Value *argv, Value *vp)
+{
+    Value fval;
+    return js_GetMethod(cx, this, id, JSGET_NO_METHOD_BARRIER, &fval) &&
+           ExternalInvoke(cx, ObjectValue(*this), fval, argc, argv, vp);
+}
+
 JSBool
 js_SetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, uintN defineHow,
                      Value *vp, JSBool strict)
@@ -5899,12 +5907,21 @@ DefaultValue(JSContext *cx, JSObject *obj, JSType hint, Value *vp)
             return true;
         }
 
-        if (!js_TryMethod(cx, obj, cx->runtime->atomState.toStringAtom, 0, NULL, &v))
+        Value fval;
+        jsid id = ATOM_TO_JSID(cx->runtime->atomState.toStringAtom);
+        if (!js_GetMethod(cx, obj, id, JSGET_NO_METHOD_BARRIER, &fval))
             return false;
-        if (!v.isPrimitive()) {
+        if (js_IsCallable(fval)) {
+            if (!ExternalInvoke(cx, ObjectValue(*obj), fval, 0, NULL, &v))
+                return false;
+            if (v.isPrimitive()) {
+                *vp = v;
+                return true;
+            }
+        }
+
         if (!obj->getClass()->convert(cx, obj, hint, &v))
             return false;
-        }
     } else {
         /* Optimize (new String(...)).valueOf(). */
         Class *clasp = obj->getClass();
@@ -5924,8 +5941,18 @@ DefaultValue(JSContext *cx, JSObject *obj, JSType hint, Value *vp)
             return false;
         if (v.isObject()) {
             JS_ASSERT(hint != TypeOfValue(cx, v));
-            if (!js_TryMethod(cx, obj, cx->runtime->atomState.toStringAtom, 0, NULL, &v))
+            Value fval;
+            jsid id = ATOM_TO_JSID(cx->runtime->atomState.toStringAtom);
+            if (!js_GetMethod(cx, obj, id, JSGET_NO_METHOD_BARRIER, &fval))
                 return false;
+            if (js_IsCallable(fval)) {
+                if (!ExternalInvoke(cx, ObjectValue(*obj), fval, 0, NULL, &v))
+                    return false;
+                if (v.isPrimitive()) {
+                    *vp = v;
+                    return true;
+                }
+            }
         }
     }
     if (v.isObject()) {
@@ -6243,35 +6270,21 @@ js_ValueToNonNullObject(JSContext *cx, const Value &v)
 JSBool
 js_TryValueOf(JSContext *cx, JSObject *obj, JSType type, Value *rval)
 {
-    Value argv[1];
-
-    argv[0].setString(cx->runtime->atomState.typeAtoms[type]);
-    return js_TryMethod(cx, obj, cx->runtime->atomState.valueOfAtom,
-                        1, argv, rval);
-}
-
-JSBool
-js_TryMethod(JSContext *cx, JSObject *obj, JSAtom *atom,
-             uintN argc, Value *argv, Value *rval)
-{
-    JS_CHECK_RECURSION(cx, return JS_FALSE);
-
-    /*
-     * Report failure only if an appropriate method was found, and calling it
-     * returned failure.  We propagate failure in this case to make exceptions
-     * behave properly.
-     */
-    JSErrorReporter older = JS_SetErrorReporter(cx, NULL);
-    jsid id = ATOM_TO_JSID(atom);
     Value fval;
-    JSBool ok = js_GetMethod(cx, obj, id, JSGET_NO_METHOD_BARRIER, &fval);
+    jsid id = ATOM_TO_JSID(cx->runtime->atomState.valueOfAtom);
-    JS_SetErrorReporter(cx, older);
+    if (!js_GetMethod(cx, obj, id, JSGET_NO_METHOD_BARRIER, &fval))
-    if (!ok)
         return false;
-
+    if (js_IsCallable(fval)) {
-    if (fval.isPrimitive())
+        Value v;
-        return JS_TRUE;
+        Value argv[] = { StringValue(cx->runtime->atomState.typeAtoms[type]) };
-    return ExternalInvoke(cx, ObjectValue(*obj), fval, argc, argv, rval);
+        if (!ExternalInvoke(cx, ObjectValue(*obj), fval, JS_ARRAY_LENGTH(argv), argv, &v))
+            return false;
+        if (v.isPrimitive()) {
+            *rval = v;
+            return true;
+        }
+    }
+    return true;
 }
 
 #if JS_HAS_XDR

js/src/jsobj.h

@@ -1186,6 +1186,14 @@ struct JSObject : js::gc::Cell {
     bool reportNotConfigurable(JSContext* cx, jsid id, uintN report = JSREPORT_ERROR);
     bool reportNotExtensible(JSContext *cx, uintN report = JSREPORT_ERROR);
 
+    /*
+     * Get the property with the given id, then call it as a function with the
+     * given arguments, providing this object as |this|. If the property isn't
+     * callable a TypeError will be thrown. On success the value returned by
+     * the call is stored in *vp.
+     */
+    bool callMethod(JSContext *cx, jsid id, uintN argc, js::Value *argv, js::Value *vp);
+
   private:
     js::Shape *getChildProperty(JSContext *cx, js::Shape *parent, js::Shape &child);
 
@@ -1884,10 +1892,6 @@ js_ValueToNonNullObject(JSContext *cx, const js::Value &v);
 extern JSBool
 js_TryValueOf(JSContext *cx, JSObject *obj, JSType type, js::Value *rval);
 
-extern JSBool
-js_TryMethod(JSContext *cx, JSObject *obj, JSAtom *atom,
-             uintN argc, js::Value *argv, js::Value *rval);
-
 extern JSBool
 js_XDRObject(JSXDRState *xdr, JSObject **objp);
 

js/src/json.cpp

@@ -170,15 +170,19 @@ js_json_stringify(JSContext *cx, uintN argc, Value *vp)
 JSBool
 js_TryJSON(JSContext *cx, Value *vp)
 {
-    // Checks whether the return value implements toJSON()
+    if (!vp->isObject())
-    JSBool ok = JS_TRUE;
+        return true;
 
-    if (vp->isObject()) {
     JSObject *obj = &vp->toObject();
-        ok = js_TryMethod(cx, obj, cx->runtime->atomState.toJSONAtom, 0, NULL, vp);
+    Value fval;
+    jsid id = ATOM_TO_JSID(cx->runtime->atomState.toJSONAtom);
+    if (!js_GetMethod(cx, obj, id, JSGET_NO_METHOD_BARRIER, &fval))
+        return false;
+    if (js_IsCallable(fval)) {
+        if (!ExternalInvoke(cx, ObjectValue(*obj), fval, 0, NULL, vp))
+            return false;
     }
-
+    return true;
-    return ok;
 }
 
 

js/src/jsstr.cpp

@@ -3875,11 +3875,17 @@ js_ValueToSource(JSContext *cx, const Value &v)
         return js_ValueToString(cx, v);
     }
 
-    JSAtom *atom = cx->runtime->atomState.toSourceAtom;
+    Value rval = NullValue();
-    AutoValueRooter tvr(cx);
+    Value fval;
-    if (!js_TryMethod(cx, &v.toObject(), atom, 0, NULL, tvr.addr()))
+    jsid id = ATOM_TO_JSID(cx->runtime->atomState.toSourceAtom);
-        return NULL;
+    if (!js_GetMethod(cx, &v.toObject(), id, JSGET_NO_METHOD_BARRIER, &fval))
-    return js_ValueToString(cx, tvr.value());
+        return false;
+    if (js_IsCallable(fval)) {
+        if (!ExternalInvoke(cx, v, fval, 0, NULL, &rval))
+            return false;
+    }
+
+    return js_ValueToString(cx, rval);
 }
 
 namespace js {

js/src/jsxml.cpp

@@ -4867,7 +4867,11 @@ xml_deleteProperty(JSContext *cx, JSObject *obj, jsid id, Value *rval, JSBool st
 JSBool
 xml_convert(JSContext *cx, JSObject *obj, JSType type, Value *rval)
 {
-    return js_TryMethod(cx, obj, cx->runtime->atomState.toStringAtom, 0, NULL, rval);
+    JSString *str = js_ValueToString(cx, ObjectValue(*obj));
+    if (!str)
+        return false;
+    *rval = StringValue(str);
+    return true;
 }
 
 static JSBool

js/src/tests/js1_7/regress/regress-351503-01.js

@@ -80,7 +80,7 @@ catch(ex)
 }
 reportCompare(expect, actual, summary + ': 3');
 
-expect = 'TypeError: ({}) is not a function';
+expect = "TypeError: can't convert ({toString:{}}) to primitive type";
 try
 {
   3 + ({toString:({}) }) ;

js/src/tests/js1_7/regress/regress-351503-02.js

@@ -84,7 +84,7 @@ function test()
   }
   reportCompare(expect, actual, summary + ': 3');
 
-  expect = 'TypeError: ({}) is not a function';
+  expect = "TypeError: can't convert ({toString:{}}) to primitive type";
   try
   {
     3 + ({toString:({}) }) ;