Backed out changeset 110e44b996f8 (bug 1034627) for Valgrind Testfailures on a CLOSED TREE
authorCarsten "Tomcat" Book <cbook@mozilla.com>
Fri, 11 Jul 2014 11:00:21 +0200
changeset 193570 384a5aede186996ac11a0462ea49a507462b27e3
parent 193569 35870757328d3964876124c87874579b9456180c
child 193571 71d8b5c5357b80cf7ccbb8b51fa3f8dc7845b2ff
push id27123
push userryanvm@gmail.com
push dateFri, 11 Jul 2014 20:35:05 +0000
treeherdermozilla-central@84bd8d9f4256 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1034627
milestone33.0a1
backs out110e44b996f87dd7dca441a19a0d2ee114d27d8d
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
Backed out changeset 110e44b996f8 (bug 1034627) for Valgrind Testfailures on a CLOSED TREE
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,18 +1,16 @@
 /* -*- 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;
@@ -93,16 +91,20 @@ 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
@@ -281,28 +283,35 @@ 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?");
 
-        size_t length = JS_GetStringLength(str);
-        if (!NS_SUCCEEDED(nsVariant::AllocateWStringWithSize(&mData, length)))
+        // 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)
             return false;
 
-        mozilla::Range<jschar> destChars(mData.u.wstr.mWStringValue, length);
-        if (!JS_CopyStringChars(cx, destChars, str))
-            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;
 
-        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,28 +1552,16 @@ 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,21 +185,16 @@ 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: