Bug 1034627 part 6 - Fix XPCVariant to work with Latin1 strings and nursery strings. r=bholley
authorJan de Mooij <jdemooij@mozilla.com>
Fri, 11 Jul 2014 09:38:55 +0200
changeset 215503 2db09518dc66b9e9cf57f7aca32b706534a7db45
parent 215502 59467641b84df7f5a2c432a7e2ba2a9f33b66a87
child 215504 e6843f1ad69745cc5707e465470d9f86e96b4eda
push id515
push userraliiev@mozilla.com
push dateMon, 06 Oct 2014 12:51:51 +0000
treeherdermozilla-release@267c7a481bef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
bugs1034627
milestone33.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 1034627 part 6 - Fix XPCVariant to work with Latin1 strings and nursery strings. r=bholley
js/xpconnect/src/XPCVariant.cpp
xpcom/ds/nsVariant.cpp
xpcom/ds/nsVariant.h
--- a/js/xpconnect/src/XPCVariant.cpp
+++ b/js/xpconnect/src/XPCVariant.cpp
@@ -1,16 +1,18 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
 /* vim: set ts=8 sts=4 et sw=4 tw=99: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /* nsIVariant implementation for xpconnect. */
 
+#include "mozilla/Range.h"
+
 #include "xpcprivate.h"
 #include "nsCxPusher.h"
 
 #include "jsfriendapi.h"
 #include "jsprf.h"
 #include "jswrapper.h"
 
 using namespace JS;
@@ -53,20 +55,17 @@ XPCVariant::XPCVariant(JSContext* cx, js
 }
 
 XPCTraceableVariant::~XPCTraceableVariant()
 {
     jsval val = GetJSValPreserveColor();
 
     MOZ_ASSERT(val.isGCThing(), "Must be traceable or unlinked");
 
-    // If val is JSVAL_STRING, we don't need to clean anything up; simply
-    // removing the string from the root set is good.
-    if (!val.isString())
-        nsVariant::Cleanup(&mData);
+    nsVariant::Cleanup(&mData);
 
     if (!val.isNull())
         RemoveFromRootSet();
 }
 
 void XPCTraceableVariant::TraceJS(JSTracer* trc)
 {
     MOZ_ASSERT(mJSVal.isMarkable());
@@ -91,20 +90,16 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(
     }
 
     nsVariant::Traverse(tmp->mData, cb);
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(XPCVariant)
     JS::Value val = tmp->GetJSValPreserveColor();
 
-    // We're sharing val's buffer, clear the pointer to it so Cleanup() won't
-    // try to delete it
-    if (val.isString())
-        tmp->mData.u.wstr.mWStringValue = nullptr;
     nsVariant::Cleanup(&tmp->mData);
 
     if (val.isMarkable()) {
         XPCTraceableVariant *v = static_cast<XPCTraceableVariant*>(tmp);
         v->RemoveFromRootSet();
     }
     tmp->mJSVal = JS::NullValue();
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
@@ -283,35 +278,28 @@ bool XPCVariant::InitializeData(JSContex
         return NS_SUCCEEDED(nsVariant::SetToVoid(&mData));
     if (val.isNull())
         return NS_SUCCEEDED(nsVariant::SetToEmpty(&mData));
     if (val.isString()) {
         JSString* str = val.toString();
         if (!str)
             return false;
 
-        // Don't use nsVariant::SetFromWStringWithSize, because that will copy
-        // the data.  Just handle this ourselves.  Note that it's ok to not
-        // copy because we added mJSVal as a GC root.
         MOZ_ASSERT(mData.mType == nsIDataType::VTYPE_EMPTY,
                    "Why do we already have data?");
 
-        // Despite the fact that the variant holds the length, there are
-        // implicit assumptions that mWStringValue[mWStringLength] == 0
-        size_t length;
-        const jschar *chars = JS_GetStringCharsZAndLength(cx, str, &length);
-        if (!chars)
+        size_t length = JS_GetStringLength(str);
+        if (!NS_SUCCEEDED(nsVariant::AllocateWStringWithSize(&mData, length)))
             return false;
 
-        mData.u.wstr.mWStringValue = const_cast<jschar *>(chars);
-        // Use C-style cast, because reinterpret cast from size_t to
-        // uint32_t is not valid on some platforms.
-        mData.u.wstr.mWStringLength = (uint32_t)length;
-        mData.mType = nsIDataType::VTYPE_WSTRING_SIZE_IS;
+        mozilla::Range<jschar> destChars(mData.u.wstr.mWStringValue, length);
+        if (!JS_CopyStringChars(cx, destChars, str))
+            return false;
 
+        MOZ_ASSERT(mData.u.wstr.mWStringValue[length] == '\0');
         return true;
     }
 
     // leaving only JSObject...
     MOZ_ASSERT(val.isObject(), "invalid type of jsval!");
 
     RootedObject jsobj(cx, &val.toObject());
 
--- a/xpcom/ds/nsVariant.cpp
+++ b/xpcom/ds/nsVariant.cpp
@@ -1552,16 +1552,28 @@ nsVariant::SetFromWStringWithSize(nsDisc
   if (!(aData->u.wstr.mWStringValue =
           (char16_t*)nsMemory::Clone(aValue, (aSize + 1) * sizeof(char16_t)))) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
   aData->u.wstr.mWStringLength = aSize;
   DATA_SETTER_EPILOGUE(aData, VTYPE_WSTRING_SIZE_IS);
 }
 /* static */ nsresult
+nsVariant::AllocateWStringWithSize(nsDiscriminatedUnion* aData, uint32_t aSize)
+{
+  DATA_SETTER_PROLOGUE(aData);
+  if (!(aData->u.wstr.mWStringValue =
+          (char16_t*)NS_Alloc((aSize + 1) * sizeof(char16_t)))) {
+    return NS_ERROR_OUT_OF_MEMORY;
+  }
+  aData->u.wstr.mWStringValue[aSize] = '\0';
+  aData->u.wstr.mWStringLength = aSize;
+  DATA_SETTER_EPILOGUE(aData, VTYPE_WSTRING_SIZE_IS);
+}
+/* static */ nsresult
 nsVariant::SetToVoid(nsDiscriminatedUnion* aData)
 {
   DATA_SETTER_PROLOGUE(aData);
   DATA_SETTER_EPILOGUE(aData, VTYPE_VOID);
 }
 /* static */ nsresult
 nsVariant::SetToEmpty(nsDiscriminatedUnion* aData)
 {
--- a/xpcom/ds/nsVariant.h
+++ b/xpcom/ds/nsVariant.h
@@ -185,16 +185,21 @@ public:
                                const nsIID* aIID, uint32_t aCount,
                                void* aValue);
   static nsresult SetFromStringWithSize(nsDiscriminatedUnion* aData,
                                         uint32_t aSize, const char* aValue);
   static nsresult SetFromWStringWithSize(nsDiscriminatedUnion* aData,
                                          uint32_t aSize,
                                          const char16_t* aValue);
 
+  // Like SetFromWStringWithSize, but leaves the string uninitialized. It does
+  // does write the null-terminator.
+  static nsresult AllocateWStringWithSize(nsDiscriminatedUnion* aData,
+                                          uint32_t aSize);
+
   static nsresult SetToVoid(nsDiscriminatedUnion* aData);
   static nsresult SetToEmpty(nsDiscriminatedUnion* aData);
   static nsresult SetToEmptyArray(nsDiscriminatedUnion* aData);
 
   static void Traverse(const nsDiscriminatedUnion& aData,
                        nsCycleCollectionTraversalCallback& aCb);
 
 private: