Bug 935696 - Tidy up XPCStringConvert::ReadableToJSVal. r=bz
authorTom Schuster <evilpies@gmail.com>
Sat, 16 Nov 2013 13:31:36 +0100
changeset 155062 9f77a15c224cdb85323e5032ca477035849835c3
parent 155061 bb541c67920f7a1043544f25a68612691f148a3b
child 155063 cf9ea7edd6e210f0925816712e821d26b90ec72c
push id25662
push useremorley@mozilla.com
push dateMon, 18 Nov 2013 10:53:03 +0000
treeherdermozilla-central@59f6274ce8f1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs935696
milestone28.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 935696 - Tidy up XPCStringConvert::ReadableToJSVal. r=bz
js/xpconnect/src/XPCConvert.cpp
js/xpconnect/src/XPCQuickStubs.cpp
js/xpconnect/src/XPCString.cpp
js/xpconnect/src/XPCWrappedJSClass.cpp
js/xpconnect/src/xpcpublic.h
--- a/js/xpconnect/src/XPCConvert.cpp
+++ b/js/xpconnect/src/XPCConvert.cpp
@@ -189,28 +189,25 @@ XPCConvert::NativeData2JS(MutableHandleV
         case nsXPTType::T_DOMSTRING:
             {
                 const nsAString* p = *((const nsAString**)s);
                 if (!p)
                     break;
 
                 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;
             }
 
         case nsXPTType::T_CHAR_STR:
             {
                 char* p = *((char**)s);
                 if (!p)
                     break;
--- a/js/xpconnect/src/XPCQuickStubs.cpp
+++ b/js/xpconnect/src/XPCQuickStubs.cpp
@@ -789,20 +789,19 @@ xpc_qsJsvalToWcharStr(JSContext *cx, jsv
 }
 
 namespace xpc {
 
 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.
         str.ForgetSharedBuffer();
     }
     return true;
 }
 
--- a/js/xpconnect/src/XPCString.cpp
+++ b/js/xpconnect/src/XPCString.cpp
@@ -19,16 +19,17 @@
  */
 
 #include "nscore.h"
 #include "nsString.h"
 #include "nsStringBuffer.h"
 #include "jsapi.h"
 #include "xpcpublic.h"
 
+using namespace JS;
 
 // static
 void
 XPCStringConvert::FreeZoneCache(JS::Zone *zone)
 {
     // Put the zone user data into an AutoPtr (which will do the cleanup for us),
     // and null out the user data (which may already be null).
     nsAutoPtr<ZoneStringCache> cache(static_cast<ZoneStringCache*>(JS_GetZoneUserData(zone)));
@@ -54,40 +55,39 @@ XPCStringConvert::FinalizeDOMString(cons
     buf->Release();
 }
 
 const JSStringFinalizer XPCStringConvert::sDOMStringFinalizer =
     { XPCStringConvert::FinalizeDOMString };
 
 // 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;
 }
--- a/js/xpconnect/src/XPCWrappedJSClass.cpp
+++ b/js/xpconnect/src/XPCWrappedJSClass.cpp
@@ -321,37 +321,39 @@ GetNamedPropertyAsVariantRaw(XPCCallCont
 nsresult
 nsXPCWrappedJSClass::GetNamedPropertyAsVariant(XPCCallContext& ccx,
                                                JSObject* aJSObjArg,
                                                const nsAString& aName,
                                                nsIVariant** aResult)
 {
     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))
         return NS_ERROR_FAILURE;
 
     // 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;
 }
 
 /***************************************************************************/
 
 // static
 nsresult
 nsXPCWrappedJSClass::BuildPropertyEnumerator(XPCCallContext& ccx,
                                              JSObject* aJSObjArg,
--- a/js/xpconnect/src/xpcpublic.h
+++ b/js/xpconnect/src/xpcpublic.h
@@ -193,18 +193,19 @@ class XPCStringConvert
         JSString* mString;
     };
 
 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
     StringBufferToJSVal(JSContext* cx, nsStringBuffer* buf, uint32_t length,
                         JS::MutableHandleValue rval, bool* sharedBuffer)
     {
         JS::Zone *zone = js::GetContextZone(cx);
         ZoneStringCache *cache = static_cast<ZoneStringCache*>(JS_GetZoneUserData(zone));