Bug 935696 - Tidy up XPCStringConvert::ReadableToJSVal. r=bz

js/xpconnect/src/XPCConvert.cpp

@@ -194,18 +194,15 @@ XPCConvert::NativeData2JS(MutableHandleValue d, const void* s,
 
                 if (!p->IsVoid()) {
                     nsStringBuffer* buf;
-                    jsval str = XPCStringConvert::ReadableToJSVal(cx, *p, &buf);
-                    if (JSVAL_IS_NULL(str))
+                    if (!XPCStringConvert::ReadableToJSVal(cx, *p, &buf, d))
                         return false;
                     if (buf)
                         buf->AddRef();
-
-                    d.set(str);
                 }
 
                 // *d is defaulted to JSVAL_NULL so no need to set it
                 // again if p is a "void" string
-
+                MOZ_ASSERT_IF(p->IsVoid(), d.isNull());
                 break;
             }
 

js/xpconnect/src/XPCQuickStubs.cpp

@@ -794,10 +794,9 @@ bool
 NonVoidStringToJsval(JSContext *cx, nsAString &str, MutableHandleValue rval)
 {
     nsStringBuffer* sharedBuffer;
-    jsval jsstr = XPCStringConvert::ReadableToJSVal(cx, str, &sharedBuffer);
-    if (JSVAL_IS_NULL(jsstr))
-        return false;
-    rval.set(jsstr);
+    if (!XPCStringConvert::ReadableToJSVal(cx, str, &sharedBuffer, rval))
+      return false;
+
     if (sharedBuffer) {
         // The string was shared but ReadableToJSVal didn't addref it.
         // Move the ownership from str to jsstr.

js/xpconnect/src/XPCString.cpp

@@ -24,6 +24,7 @@
 #include "jsapi.h"
 #include "xpcpublic.h"
 
+using namespace JS;
 
 // static
 void
@@ -59,35 +60,34 @@ const JSStringFinalizer XPCStringConvert::sDOMStringFinalizer =
 
 // convert a readable to a JSString, copying string data
 // static
-jsval
+bool
 XPCStringConvert::ReadableToJSVal(JSContext *cx,
                                   const nsAString &readable,
-                                  nsStringBuffer** sharedBuffer)
+                                  nsStringBuffer** sharedBuffer,
+                                  MutableHandleValue vp)
 {
-    JSString *str;
     *sharedBuffer = nullptr;
 
     uint32_t length = readable.Length();
-
-    if (length == 0)
-        return JS_GetEmptyStringValue(cx);
+    if (length == 0) {
+        vp.set(JS_GetEmptyStringValue(cx));
+        return true;
+    }
 
     nsStringBuffer *buf = nsStringBuffer::FromString(readable);
     if (buf) {
-        JS::RootedValue val(cx);
         bool shared;
-        bool ok = StringBufferToJSVal(cx, buf, length, &val, &shared);
-        if (!ok) {
-            return JS::NullValue();
-        }
-
-        if (shared) {
+        if (!StringBufferToJSVal(cx, buf, length, vp, &shared))
+            return false;
+        if (shared)
             *sharedBuffer = buf;
-        }
-        return val;
+        return true;
     }
 
     // blech, have to copy.
-    str = JS_NewUCStringCopyN(cx, readable.BeginReading(), length);
-    return str ? JS::StringValue(str) : JS::NullValue();
+    JSString *str = JS_NewUCStringCopyN(cx, readable.BeginReading(), length);
+    if (!str)
+        return false;
+    vp.setString(str);
+    return true;
 }

js/xpconnect/src/XPCWrappedJSClass.cpp

@@ -326,9 +326,6 @@ nsXPCWrappedJSClass::GetNamedPropertyAsVariant(XPCCallContext& ccx,
 {
     JSContext* cx = ccx.GetJSContext();
     RootedObject aJSObj(cx, aJSObjArg);
-    bool ok;
-    RootedId id(cx);
-    nsresult rv = NS_ERROR_FAILURE;
 
     AutoScriptEvaluate scriptEval(cx);
     if (!scriptEval.StartEvaluating(aJSObj))
@@ -337,16 +334,21 @@ nsXPCWrappedJSClass::GetNamedPropertyAsVariant(XPCCallContext& ccx,
     // Wrap the string in a jsval after the AutoScriptEvaluate, so that the
     // resulting value ends up in the correct compartment.
     nsStringBuffer* buf;
-    jsval jsstr = XPCStringConvert::ReadableToJSVal(ccx, aName, &buf);
-    if (JSVAL_IS_NULL(jsstr))
+    RootedValue value(cx);
+    if (!XPCStringConvert::ReadableToJSVal(ccx, aName, &buf, &value))
         return NS_ERROR_OUT_OF_MEMORY;
     if (buf)
         buf->AddRef();
 
-    ok = JS_ValueToId(cx, jsstr, id.address()) &&
-        GetNamedPropertyAsVariantRaw(ccx, aJSObj, id, aResult, &rv);
-
-    return ok ? NS_OK : NS_FAILED(rv) ? rv : NS_ERROR_FAILURE;
+    RootedId id(cx);
+    nsresult rv = NS_OK;
+    if (!JS_ValueToId(cx, value, id.address()) ||
+        !GetNamedPropertyAsVariantRaw(ccx, aJSObj, id, aResult, &rv)) {
+        if (NS_FAILED(rv))
+            return rv;
+        return NS_ERROR_FAILURE;
+    }
+    return NS_OK;
 }
 
 /***************************************************************************/

js/xpconnect/src/xpcpublic.h

@@ -198,8 +198,9 @@ public:
     // If the string shares the readable's buffer, that buffer will
     // get assigned to *sharedBuffer.  Otherwise null will be
     // assigned.
-    static jsval ReadableToJSVal(JSContext *cx, const nsAString &readable,
-                                 nsStringBuffer** sharedBuffer);
+    static bool ReadableToJSVal(JSContext *cx, const nsAString &readable,
+                                nsStringBuffer** sharedBuffer,
+                                JS::MutableHandleValue vp);
 
     // Convert the given stringbuffer/length pair to a jsval
     static MOZ_ALWAYS_INLINE bool