Bug 1041140 Fix design flaws in FakeString r=bz
authorNeil Rashbrook <neil@parkwaycc.co.uk>
Sat, 19 Jul 2014 14:23:19 +0100
changeset 215923 9350909a34017d71dc947d4ac118ad3527203227
parent 215922 a9c483baa100c28835c7603d3d379c4c982ce842
child 215924 a870613a0e5afd9e11ae88d8e3bda38882d3c2ad
push id3857
push userraliiev@mozilla.com
push dateTue, 02 Sep 2014 16:39:23 +0000
treeherdermozilla-beta@5638b907b505 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1041140
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 1041140 Fix design flaws in FakeString r=bz
dom/base/nsJSUtils.h
dom/bindings/BindingUtils.cpp
dom/bindings/BindingUtils.h
--- a/dom/base/nsJSUtils.h
+++ b/dom/base/nsJSUtils.h
@@ -140,38 +140,31 @@ public:
 
 template<typename T>
 inline bool
 AssignJSString(JSContext *cx, T &dest, JSString *s)
 {
   size_t len = js::GetStringLength(s);
   static_assert(js::MaxStringLength < (1 << 28),
                 "Shouldn't overflow here or in SetCapacity");
-  if (MOZ_UNLIKELY(!dest.SetCapacity(len + 1, mozilla::fallible_t()))) {
+  if (MOZ_UNLIKELY(!dest.SetLength(len, mozilla::fallible_t()))) {
     JS_ReportOutOfMemory(cx);
     return false;
   }
-  if (MOZ_UNLIKELY(!js::CopyStringChars(cx, dest.BeginWriting(), s, len))) {
-    return false;
-  }
-  dest.BeginWriting()[len] = '\0';
-  dest.SetLength(len);
-  return true;
+  return js::CopyStringChars(cx, dest.BeginWriting(), s, len);
 }
 
 inline void
 AssignJSFlatString(nsAString &dest, JSFlatString *s)
 {
   size_t len = js::GetFlatStringLength(s);
   static_assert(js::MaxStringLength < (1 << 28),
                 "Shouldn't overflow here or in SetCapacity");
-  dest.SetCapacity(len + 1);
+  dest.SetLength(len);
   js::CopyFlatStringChars(dest.BeginWriting(), s, len);
-  dest.BeginWriting()[len] = '\0';
-  dest.SetLength(len);
 }
 
 class nsAutoJSString : public nsAutoString
 {
 public:
 
   /**
    * nsAutoJSString should be default constructed, which leaves it empty
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -2228,20 +2228,18 @@ ConvertJSValueToByteString(JSContext* cx
     }
   } else {
     length = js::GetStringLength(s);
   }
 
   static_assert(js::MaxStringLength < UINT32_MAX,
                 "length+1 shouldn't overflow");
 
-  result.SetCapacity(length+1);
+  result.SetLength(length);
   JS_EncodeStringToBuffer(cx, s, result.BeginWriting(), length);
-  result.BeginWriting()[length] = '\0';
-  result.SetLength(length);
 
   return true;
 }
 
 bool
 IsInPrivilegedApp(JSContext* aCx, JSObject* aObj)
 {
   using mozilla::dom::workers::GetWorkerPrivateFromContext;
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -1776,42 +1776,35 @@ struct FakeString {
     return mData;
   }
 
   nsString::size_type Length() const
   {
     return mLength;
   }
 
-  // Reserve space to write aCapacity chars, including null-terminator.
-  // Caller is responsible for calling SetLength and writing the null-
-  // terminator.
-  bool SetCapacity(nsString::size_type aCapacity, mozilla::fallible_t const&) {
-    MOZ_ASSERT(aCapacity > 0, "Capacity must include null-terminator");
-
+  // Reserve space to write aLength chars, not including null-terminator.
+  bool SetLength(nsString::size_type aLength, mozilla::fallible_t const&) {
     // Use mInlineStorage for small strings.
-    if (aCapacity <= sInlineCapacity) {
+    if (aLength < sInlineCapacity) {
       SetData(mInlineStorage);
-      return true;
+    } else {
+      nsStringBuffer *buf = nsStringBuffer::Alloc((aLength + 1) * sizeof(nsString::char_type)).take();
+      if (MOZ_UNLIKELY(!buf)) {
+        return false;
+      }
+
+      SetData(static_cast<nsString::char_type*>(buf->Data()));
+      mFlags = nsString::F_SHARED | nsString::F_TERMINATED;
     }
-
-    nsRefPtr<nsStringBuffer> buf = nsStringBuffer::Alloc(aCapacity * sizeof(nsString::char_type));
-    if (MOZ_UNLIKELY(!buf)) {
-      return false;
-    }
-
-    SetData(static_cast<nsString::char_type*>(buf.forget().take()->Data()));
-    mFlags = nsString::F_SHARED | nsString::F_TERMINATED;
+    mLength = aLength;
+    mData[mLength] = char16_t(0);
     return true;
   }
 
-  void SetLength(nsString::size_type aLength) {
-    mLength = aLength;
-  }
-
   // If this ever changes, change the corresponding code in the
   // Optional<nsAString> specialization as well.
   const nsAString* ToAStringPtr() const {
     return reinterpret_cast<const nsString*>(this);
   }
 
   nsAString* ToAStringPtr() {
     return reinterpret_cast<nsString*>(this);