Bug 737251 - Reuse the throwing code in jsobj.cpp for jsproxy.cpp. r=luke

js/src/jit-test/tests/basic/bug737251.js

@@ -0,0 +1,10 @@
+a = {}
+a.getOwnPropertyDescriptor = XML;
+b = Proxy.create(a)
+for (x in this)
+try {
+  (function() {
+    "use strict";
+    b[2] = x // don't assert
+  })()
+} catch (e) {}

js/src/jsobj.cpp

@@ -1830,38 +1830,54 @@ PropDesc::initialize(JSContext *cx, const Value &origval, bool checkAccessors)
     return true;
 }
 
+namespace js {
+
+bool
+Throw(JSContext *cx, jsid id, unsigned errorNumber)
+{
+    JS_ASSERT(js_ErrorFormatString[errorNumber].argCount == 1);
+
+    JSString *idstr = IdToString(cx, id);
+    if (!idstr)
+       return false;
+    JSAutoByteString bytes(cx, idstr);
+    if (!bytes)
+        return false;
+    JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, errorNumber, bytes.ptr());
+    return false;
+}
+
+bool
+Throw(JSContext *cx, JSObject *obj, unsigned errorNumber)
+{
+    if (js_ErrorFormatString[errorNumber].argCount == 1) {
+        js_ReportValueErrorFlags(cx, JSREPORT_ERROR, errorNumber,
+                                 JSDVG_IGNORE_STACK, ObjectValue(*obj),
+                                 NULL, NULL, NULL);
+    } else {
+        JS_ASSERT(js_ErrorFormatString[errorNumber].argCount == 0);
+        JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, errorNumber);
+    }
+    return false;
+}
+
+} /* namespace js */
+
 static JSBool
 Reject(JSContext *cx, unsigned errorNumber, bool throwError, jsid id, bool *rval)
 {
-    if (throwError) {
-        jsid idstr;
-        if (!js_ValueToStringId(cx, IdToValue(id), &idstr))
-           return JS_FALSE;
-        JSAutoByteString bytes(cx, JSID_TO_STRING(idstr));
-        if (!bytes)
-            return JS_FALSE;
-        JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, errorNumber, bytes.ptr());
-        return JS_FALSE;
-    }
+    if (throwError)
+        return Throw(cx, id, errorNumber);
 
     *rval = false;
-    return JS_TRUE;
+    return true;
 }
 
 static JSBool
 Reject(JSContext *cx, JSObject *obj, unsigned errorNumber, bool throwError, bool *rval)
 {
-    if (throwError) {
-        if (js_ErrorFormatString[errorNumber].argCount == 1) {
-            js_ReportValueErrorFlags(cx, JSREPORT_ERROR, errorNumber,
-                                     JSDVG_IGNORE_STACK, ObjectValue(*obj),
-                                     NULL, NULL, NULL);
-        } else {
-            JS_ASSERT(js_ErrorFormatString[errorNumber].argCount == 0);
-            JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, errorNumber);
-        }
-        return JS_FALSE;
-    }
+    if (throwError)
+        return Throw(cx, obj, errorNumber);
 
     *rval = false;
     return JS_TRUE;

js/src/jsobj.h

@@ -1626,6 +1626,13 @@ InformalValueTypeName(const Value &v);
 inline void
 DestroyIdArray(FreeOp *fop, JSIdArray *ida);
 
+/* Helpers for throwing. These always return false. */
+extern bool
+Throw(JSContext *cx, jsid id, unsigned errorNumber);
+
+extern bool
+Throw(JSContext *cx, JSObject *obj, unsigned errorNumber);
+
 }  /* namespace js */
 
 #endif /* jsobj_h___ */

js/src/jsproxy.cpp

@@ -183,17 +183,8 @@ ProxyHandler::set(JSContext *cx, JSObject *proxy, JSObject *receiver, jsid id, b
     /* The control-flow here differs from ::get() because of the fall-through case below. */
     if (desc.obj) {
         // Check for read-only properties.
-        if (desc.attrs & JSPROP_READONLY) {
-            if (strict) {
-                JSAutoByteString bytes(cx, JSID_TO_STRING(id));
-                if (!bytes)
-                    return false;
-                JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
-                                     JSMSG_CANT_REDEFINE_PROP, bytes.ptr());
-                return false;
-            }
-            return true;
-        }
+        if (desc.attrs & JSPROP_READONLY)
+            return strict ? Throw(cx, id, JSMSG_CANT_REDEFINE_PROP) : true;
         if (!desc.setter) {
             // Be wary of the odd explicit undefined setter case possible through
             // Object.defineProperty.
@@ -219,17 +210,8 @@ ProxyHandler::set(JSContext *cx, JSObject *proxy, JSObject *receiver, jsid id, b
         return false;
     if (desc.obj) {
         // Check for read-only properties.
-        if (desc.attrs & JSPROP_READONLY) {
-            if (strict) {
-                JSAutoByteString bytes(cx, JSID_TO_STRING(id));
-                if (!bytes)
-                    return false;
-                JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
-                                     JSMSG_CANT_REDEFINE_PROP, bytes.ptr());
-                return false;
-            }
-            return true;
-        }
+        if (desc.attrs & JSPROP_READONLY)
+            return strict ? Throw(cx, id, JSMSG_CANT_REDEFINE_PROP) : true;
         if (!desc.setter) {
             // Be wary of the odd explicit undefined setter case possible through
             // Object.defineProperty.