Bug 1534437 - Make JSFlatString::new_ always take ownership of the |chars| passed to it, and add the same sensible ownership handling to a bunch of callers. r=tcampbell
authorJeff Walden <jwalden@mit.edu>
Fri, 08 Mar 2019 22:28:08 -0800
changeset 524368 b9d87882a36584e2f17331b652cbc8681aee17ce
parent 524367 abcc9ef3e73da68b0fdbe9674364df36b3c25fd4
child 524482 d8b3da69c3a055f6cdc996e7dcd03096c2eb7682
child 524492 c32dcf4e6f92de2e9523312a41fa71b77503f7e2
push id2032
push userffxbld-merge
push dateMon, 13 May 2019 09:36:57 +0000
treeherdermozilla-release@455c1065dcbe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcampbell
bugs1534437
milestone67.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 1534437 - Make JSFlatString::new_ always take ownership of the |chars| passed to it, and add the same sensible ownership handling to a bunch of callers. r=tcampbell Differential Revision: https://phabricator.services.mozilla.com/D23043
js/src/ctypes/CTypes.cpp
js/src/jsapi.cpp
js/src/jsapi.h
js/src/vm/JSAtom.cpp
js/src/vm/StringType-inl.h
js/src/vm/StringType.cpp
js/src/vm/StringType.h
js/xpconnect/src/XPCConvert.cpp
--- a/js/src/ctypes/CTypes.cpp
+++ b/js/src/ctypes/CTypes.cpp
@@ -7748,22 +7748,21 @@ static bool ReadStringCommon(JSContext* 
 
       // Determine the length.
       UniqueTwoByteChars dst(
           inflateUTF8(cx, JS::UTF8Chars(bytes, length), &length).get());
       if (!dst) {
         return false;
       }
 
-      result = JS_NewUCString(cx, dst.get(), length);
+      result = JS_NewUCString(cx, std::move(dst), length);
       if (!result) {
         return false;
       }
 
-      mozilla::Unused << dst.release();
       break;
     }
     case TYPE_int16_t:
     case TYPE_uint16_t:
     case TYPE_short:
     case TYPE_unsigned_short:
     case TYPE_char16_t: {
       char16_t* chars = static_cast<char16_t*>(data);
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -4273,36 +4273,38 @@ JS_PUBLIC_API JSString* JS_AtomizeAndPin
                                                 size_t length) {
   AssertHeapIsIdle();
   CHECK_THREAD(cx);
   JSAtom* atom = Atomize(cx, s, length, PinAtom);
   MOZ_ASSERT_IF(atom, JS_StringHasBeenPinned(cx, atom));
   return atom;
 }
 
-JS_PUBLIC_API JSString* JS_NewLatin1String(JSContext* cx, JS::Latin1Char* chars,
-                                           size_t length) {
+JS_PUBLIC_API JSString* JS_NewLatin1String(
+    JSContext* cx, js::UniquePtr<JS::Latin1Char[], JS::FreePolicy> chars,
+    size_t length) {
   AssertHeapIsIdle();
   CHECK_THREAD(cx);
-  return NewString(cx, chars, length);
-}
-
-JS_PUBLIC_API JSString* JS_NewUCString(JSContext* cx, char16_t* chars,
+  return NewString(cx, std::move(chars), length);
+}
+
+JS_PUBLIC_API JSString* JS_NewUCString(JSContext* cx,
+                                       JS::UniqueTwoByteChars chars,
                                        size_t length) {
   AssertHeapIsIdle();
   CHECK_THREAD(cx);
-  return NewString(cx, chars, length);
+  return NewString(cx, std::move(chars), length);
 }
 
 JS_PUBLIC_API JSString* JS_NewUCStringDontDeflate(JSContext* cx,
-                                                  char16_t* chars,
+                                                  JS::UniqueTwoByteChars chars,
                                                   size_t length) {
   AssertHeapIsIdle();
   CHECK_THREAD(cx);
-  return NewStringDontDeflate(cx, chars, length);
+  return NewStringDontDeflate(cx, std::move(chars), length);
 }
 
 JS_PUBLIC_API JSString* JS_NewUCStringCopyN(JSContext* cx, const char16_t* s,
                                             size_t n) {
   AssertHeapIsIdle();
   CHECK_THREAD(cx);
   if (!n) {
     return cx->names().empty;
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -2599,26 +2599,26 @@ extern JS_PUBLIC_API JSString* JS_Atomiz
 
 extern JS_PUBLIC_API JSString* JS_AtomizeAndPinStringN(JSContext* cx,
                                                        const char* s,
                                                        size_t length);
 
 extern JS_PUBLIC_API JSString* JS_AtomizeAndPinString(JSContext* cx,
                                                       const char* s);
 
-extern JS_PUBLIC_API JSString* JS_NewLatin1String(JSContext* cx,
-                                                  JS::Latin1Char* chars,
-                                                  size_t length);
-
-extern JS_PUBLIC_API JSString* JS_NewUCString(JSContext* cx, char16_t* chars,
+extern JS_PUBLIC_API JSString* JS_NewLatin1String(
+    JSContext* cx, js::UniquePtr<JS::Latin1Char[], JS::FreePolicy> chars,
+    size_t length);
+
+extern JS_PUBLIC_API JSString* JS_NewUCString(JSContext* cx,
+                                              JS::UniqueTwoByteChars chars,
                                               size_t length);
 
-extern JS_PUBLIC_API JSString* JS_NewUCStringDontDeflate(JSContext* cx,
-                                                         char16_t* chars,
-                                                         size_t length);
+extern JS_PUBLIC_API JSString* JS_NewUCStringDontDeflate(
+    JSContext* cx, JS::UniqueTwoByteChars chars, size_t length);
 
 extern JS_PUBLIC_API JSString* JS_NewUCStringCopyN(JSContext* cx,
                                                    const char16_t* s, size_t n);
 
 extern JS_PUBLIC_API JSString* JS_NewUCStringCopyZ(JSContext* cx,
                                                    const char16_t* s);
 
 extern JS_PUBLIC_API JSString* JS_AtomizeUCStringN(JSContext* cx,
--- a/js/src/vm/JSAtom.cpp
+++ b/js/src/vm/JSAtom.cpp
@@ -897,23 +897,17 @@ static MOZ_ALWAYS_INLINE JSFlatString* M
   UniquePtr<CharT[], JS::FreePolicy> newStr(js_pod_malloc<CharT>(length + 1));
   if (!newStr) {
     return nullptr;
   }
 
   InflateUTF8CharsToBufferAndTerminate(chars->utf8, newStr.get(), length,
                                        chars->encoding);
 
-  JSFlatString* str = JSFlatString::new_<NoGC>(cx, newStr.get(), length);
-  if (!str) {
-    return nullptr;
-  }
-
-  mozilla::Unused << newStr.release();
-  return str;
+  return JSFlatString::new_<NoGC>(cx, std::move(newStr), length);
 }
 
 // Another 2 variants of MakeFlatStringForAtomization.
 // This is used by AtomizeUTF8OrWTF8CharsWrapper with UTF8Chars or WTF8Chars.
 template <typename InputCharsT>
 /* static */ MOZ_ALWAYS_INLINE JSFlatString* MakeFlatStringForAtomization(
     JSContext* cx, const AtomizeUTF8OrWTF8CharsWrapper<InputCharsT>* chars,
     size_t length) {
--- a/js/src/vm/StringType-inl.h
+++ b/js/src/vm/StringType-inl.h
@@ -11,16 +11,17 @@
 
 #include "mozilla/PodOperations.h"
 #include "mozilla/Range.h"
 
 #include "gc/Allocator.h"
 #include "gc/FreeOp.h"
 #include "gc/Marking.h"
 #include "gc/StoreBuffer.h"
+#include "js/UniquePtr.h"
 #include "vm/JSContext.h"
 #include "vm/Realm.h"
 
 #include "gc/StoreBuffer-inl.h"
 
 namespace js {
 
 // Allocate a thin inline string if possible, and a fat inline string if not.
@@ -253,19 +254,19 @@ MOZ_ALWAYS_INLINE void JSFlatString::ini
 
 MOZ_ALWAYS_INLINE void JSFlatString::init(const JS::Latin1Char* chars,
                                           size_t length) {
   setLengthAndFlags(length, INIT_FLAT_FLAGS | LATIN1_CHARS_BIT);
   d.s.u2.nonInlineCharsLatin1 = chars;
 }
 
 template <js::AllowGC allowGC, typename CharT>
-MOZ_ALWAYS_INLINE JSFlatString* JSFlatString::new_(JSContext* cx,
-                                                   const CharT* chars,
-                                                   size_t length) {
+MOZ_ALWAYS_INLINE JSFlatString* JSFlatString::new_(
+    JSContext* cx, js::UniquePtr<CharT[], JS::FreePolicy> chars,
+    size_t length) {
   MOZ_ASSERT(chars[length] == CharT(0));
 
   if (!validateLength(cx, length)) {
     return nullptr;
   }
 
   JSFlatString* str;
   if (cx->zone()->isAtomsZone()) {
@@ -273,31 +274,29 @@ MOZ_ALWAYS_INLINE JSFlatString* JSFlatSt
   } else {
     str = js::AllocateString<JSFlatString, allowGC>(cx, js::gc::DefaultHeap);
   }
   if (!str) {
     return nullptr;
   }
 
   if (!str->isTenured()) {
-    // The chars pointer is only considered to be handed over to this
-    // function on a successful return. If the following registration
-    // fails, the string is partially initialized and must be made valid,
-    // or its finalizer may attempt to free uninitialized memory.
-    void* ptr = const_cast<void*>(static_cast<const void*>(chars));
-    if (!cx->runtime()->gc.nursery().registerMallocedBuffer(ptr)) {
-      str->init((JS::Latin1Char*)nullptr, 0);
+    // If the following registration fails, the string is partially initialized
+    // and must be made valid, or its finalizer may attempt to free
+    // uninitialized memory.
+    if (!cx->runtime()->gc.nursery().registerMallocedBuffer(chars.get())) {
+      str->init(static_cast<JS::Latin1Char*>(nullptr), 0);
       if (allowGC) {
         ReportOutOfMemory(cx);
       }
       return nullptr;
     }
   }
 
-  str->init(chars, length);
+  str->init(chars.release(), length);
   return str;
 }
 
 inline js::PropertyName* JSFlatString::toPropertyName(JSContext* cx) {
 #ifdef DEBUG
   uint32_t dummy;
   MOZ_ASSERT(!isIndex(&dummy));
 #endif
--- a/js/src/vm/StringType.cpp
+++ b/js/src/vm/StringType.cpp
@@ -1579,23 +1579,17 @@ static JSFlatString* NewStringDeflated(J
   auto news = cx->make_pod_array<Latin1Char>(n + 1);
   if (!news) {
     return nullptr;
   }
 
   MOZ_ASSERT(CanStoreCharsAsLatin1(s, n));
   FillFromCompatibleAndTerminate(news.get(), s, n);
 
-  JSFlatString* str = JSFlatString::new_<allowGC>(cx, news.get(), n);
-  if (!str) {
-    return nullptr;
-  }
-
-  mozilla::Unused << news.release();
-  return str;
+  return JSFlatString::new_<allowGC>(cx, std::move(news), n);
 }
 
 template <AllowGC allowGC>
 static JSFlatString* NewStringDeflated(JSContext* cx, const Latin1Char* s,
                                        size_t n) {
   MOZ_CRASH("Shouldn't be called for Latin1 chars");
 }
 
@@ -1617,98 +1611,78 @@ static JSFlatString* NewStringDeflatedFr
   auto news = cx->make_pod_array<Latin1Char>(length + 1);
   if (!news) {
     cx->recoverFromOutOfMemory();
     return nullptr;
   }
 
   FillFromCompatibleAndTerminate(news.get(), chars, length);
 
-  JSFlatString* str = JSFlatString::new_<NoGC>(cx, news.get(), length);
-  if (!str) {
-    return nullptr;
-  }
-
-  mozilla::Unused << news.release();
-  return str;
+  return JSFlatString::new_<NoGC>(cx, std::move(news), length);
 }
 
 template <typename CharT>
-JSFlatString* js::NewStringDontDeflate(JSContext* cx, CharT* chars,
+JSFlatString* js::NewStringDontDeflate(JSContext* cx,
+                                       UniquePtr<CharT[], JS::FreePolicy> chars,
                                        size_t length) {
-  if (JSFlatString* str = TryEmptyOrStaticString(cx, chars, length)) {
-    // Free |chars| because we're taking possession of it, but it's no
-    // longer needed because we use the static string instead.
-    js_free(chars);
+  if (JSFlatString* str = TryEmptyOrStaticString(cx, chars.get(), length)) {
     return str;
   }
 
   if (JSInlineString::lengthFits<CharT>(length)) {
-    JSInlineString* str =
-        NewInlineString<CanGC>(cx, mozilla::Range<const CharT>(chars, length));
-    if (!str) {
-      return nullptr;
-    }
-
-    js_free(chars);
-    return str;
+    // |chars.get()| is safe because 1) |NewInlineString| necessarily *copies*,
+    // and 2) |chars| frees its contents only when this function returns.
+    return NewInlineString<CanGC>(
+        cx, mozilla::Range<const CharT>(chars.get(), length));
   }
 
-  return JSFlatString::new_<CanGC>(cx, chars, length);
+  return JSFlatString::new_<CanGC>(cx, std::move(chars), length);
 }
 
-template JSFlatString* js::NewStringDontDeflate(JSContext* cx, char16_t* chars,
+template JSFlatString* js::NewStringDontDeflate(JSContext* cx,
+                                                UniqueTwoByteChars chars,
                                                 size_t length);
 
 template JSFlatString* js::NewStringDontDeflate(JSContext* cx,
-                                                Latin1Char* chars,
+                                                UniqueLatin1Chars chars,
                                                 size_t length);
 
 template <typename CharT>
-JSFlatString* js::NewString(JSContext* cx, CharT* chars, size_t length) {
-  if (IsSame<CharT, char16_t>::value && CanStoreCharsAsLatin1(chars, length)) {
-    JSFlatString* s = NewStringDeflated<CanGC>(cx, chars, length);
-    if (!s) {
-      return nullptr;
-    }
-
-    // Free |chars| because we're taking possession of it but not using it.
-    js_free(chars);
-    return s;
+JSFlatString* js::NewString(JSContext* cx,
+                            UniquePtr<CharT[], JS::FreePolicy> chars,
+                            size_t length) {
+  if (IsSame<CharT, char16_t>::value &&
+      CanStoreCharsAsLatin1(chars.get(), length)) {
+    // Deflating copies from |chars.get()| and lets |chars| be freed on return.
+    return NewStringDeflated<CanGC>(cx, chars.get(), length);
   }
 
-  return NewStringDontDeflate(cx, chars, length);
+  return NewStringDontDeflate(cx, std::move(chars), length);
 }
 
-template JSFlatString* js::NewString(JSContext* cx, char16_t* chars,
+template JSFlatString* js::NewString(JSContext* cx, UniqueTwoByteChars chars,
                                      size_t length);
 
-template JSFlatString* js::NewString(JSContext* cx, Latin1Char* chars,
+template JSFlatString* js::NewString(JSContext* cx, UniqueLatin1Chars chars,
                                      size_t length);
 
 template <AllowGC allowGC, typename CharT>
 JSFlatString* js::NewStringDontDeflate(JSContext* cx,
                                        UniquePtr<CharT[], JS::FreePolicy> chars,
                                        size_t length) {
   if (JSFlatString* str = TryEmptyOrStaticString(cx, chars.get(), length)) {
     return str;
   }
 
   if (JSInlineString::lengthFits<CharT>(length)) {
     return NewInlineString<allowGC>(
         cx, mozilla::Range<const CharT>(chars.get(), length));
   }
 
-  JSFlatString* str = JSFlatString::new_<allowGC>(cx, chars.get(), length);
-  if (!str) {
-    return nullptr;
-  }
-
-  mozilla::Unused << chars.release();
-  return str;
+  return JSFlatString::new_<allowGC>(cx, std::move(chars), length);
 }
 
 template JSFlatString* js::NewStringDontDeflate<CanGC>(JSContext* cx,
                                                        UniqueTwoByteChars chars,
                                                        size_t length);
 
 template JSFlatString* js::NewStringDontDeflate<NoGC>(JSContext* cx,
                                                       UniqueTwoByteChars chars,
@@ -1768,23 +1742,17 @@ JSFlatString* NewStringCopyNDontDeflate(
     if (!allowGC) {
       cx->recoverFromOutOfMemory();
     }
     return nullptr;
   }
 
   FillAndTerminate(news.get(), s, n);
 
-  JSFlatString* str = JSFlatString::new_<allowGC>(cx, news.get(), n);
-  if (!str) {
-    return nullptr;
-  }
-
-  mozilla::Unused << news.release();
-  return str;
+  return JSFlatString::new_<allowGC>(cx, std::move(news), n);
 }
 
 template JSFlatString* NewStringCopyNDontDeflate<CanGC>(JSContext* cx,
                                                         const char16_t* s,
                                                         size_t n);
 
 template JSFlatString* NewStringCopyNDontDeflate<NoGC>(JSContext* cx,
                                                        const char16_t* s,
@@ -1814,23 +1782,17 @@ static JSFlatString* NewUndeflatedString
   auto news = cx->make_pod_array<char16_t>(length + 1);
   if (!news) {
     cx->recoverFromOutOfMemory();
     return nullptr;
   }
 
   FillAndTerminate(news.get(), chars, length);
 
-  if (JSFlatString* str = JSFlatString::new_<NoGC>(cx, news.get(), length)) {
-    // Successful JSFlatString::new_ took ownership of |news.release()|.
-    mozilla::Unused << news.release();
-    return str;
-  }
-
-  return nullptr;
+  return JSFlatString::new_<NoGC>(cx, std::move(news), length);
 }
 
 JSFlatString* NewLatin1StringZ(JSContext* cx, UniqueChars chars) {
   size_t length = strlen(chars.get());
   UniqueLatin1Chars latin1(reinterpret_cast<Latin1Char*>(chars.release()));
   return NewString<CanGC>(cx, std::move(latin1), length);
 }
 
--- a/js/src/vm/StringType.h
+++ b/js/src/vm/StringType.h
@@ -965,17 +965,18 @@ class JSFlatString : public JSLinearStri
   template <typename CharT>
   static bool isIndexSlow(const CharT* s, size_t length, uint32_t* indexp);
 
   void init(const char16_t* chars, size_t length);
   void init(const JS::Latin1Char* chars, size_t length);
 
  public:
   template <js::AllowGC allowGC, typename CharT>
-  static inline JSFlatString* new_(JSContext* cx, const CharT* chars,
+  static inline JSFlatString* new_(JSContext* cx,
+                                   js::UniquePtr<CharT[], JS::FreePolicy> chars,
                                    size_t length);
 
   inline bool isIndexSlow(uint32_t* indexp) const {
     MOZ_ASSERT(JSString::isFlat());
     JS::AutoCheckCannotGC nogc;
     if (hasLatin1Chars()) {
       const JS::Latin1Char* s = latin1Chars(nogc);
       return mozilla::IsAsciiDigit(*s) && isIndexSlow(s, length(), indexp);
@@ -1528,26 +1529,33 @@ static inline UniqueChars StringToNewUTF
 
   return UniqueChars(
       linear->hasLatin1Chars()
           ? JS::CharsToNewUTF8CharsZ(maybecx, linear->latin1Range(nogc)).c_str()
           : JS::CharsToNewUTF8CharsZ(maybecx, linear->twoByteRange(nogc))
                 .c_str());
 }
 
-/* GC-allocate a string descriptor for the given malloc-allocated chars. */
+/**
+ * Allocate a string with the given contents, potentially GCing in the process.
+ */
 template <typename CharT>
-extern JSFlatString* NewString(JSContext* cx, CharT* chars, size_t length);
+extern JSFlatString* NewString(JSContext* cx,
+                               UniquePtr<CharT[], JS::FreePolicy> chars,
+                               size_t length);
 
-/* Like NewString, but doesn't try to deflate to Latin1. */
+/* Like NewString, but doesn't attempt to deflate to Latin1. */
 template <typename CharT>
-extern JSFlatString* NewStringDontDeflate(JSContext* cx, CharT* chars,
-                                          size_t length);
+extern JSFlatString* NewStringDontDeflate(
+    JSContext* cx, UniquePtr<CharT[], JS::FreePolicy> chars, size_t length);
 
-/* GC-allocate a string descriptor for the given malloc-allocated chars. */
+/**
+ * Allocate a string with the given contents.  If |allowGC == CanGC|, this may
+ * trigger a GC.
+ */
 template <js::AllowGC allowGC, typename CharT>
 extern JSFlatString* NewString(JSContext* cx,
                                UniquePtr<CharT[], JS::FreePolicy> chars,
                                size_t length);
 
 /* Like NewString, but doesn't try to deflate to Latin1. */
 template <js::AllowGC allowGC, typename CharT>
 extern JSFlatString* NewStringDontDeflate(
--- a/js/xpconnect/src/XPCConvert.cpp
+++ b/js/xpconnect/src/XPCConvert.cpp
@@ -276,34 +276,35 @@ bool XPCConvert::NativeData2JS(MutableHa
         return false;
       }
 
       // Usage of UTF-8 in XPConnect is mostly for things that are
       // almost always ASCII, so the inexact allocations below
       // should be fine.
 
       if (IsUTF8Latin1(*utf8String)) {
-        char* buffer = static_cast<char*>(JS_malloc(cx, allocLen.value()));
+        using UniqueLatin1Chars =
+            js::UniquePtr<JS::Latin1Char[], JS::FreePolicy>;
+
+        UniqueLatin1Chars buffer(
+            static_cast<JS::Latin1Char*>(JS_malloc(cx, allocLen.value())));
         if (!buffer) {
           return false;
         }
-        size_t written =
-            LossyConvertUTF8toLatin1(*utf8String, MakeSpan(buffer, len));
+
+        size_t written = LossyConvertUTF8toLatin1(
+            *utf8String, MakeSpan(reinterpret_cast<char*>(buffer.get()), len));
         buffer[written] = 0;
 
-        // JS_NewLatin1String takes ownership on success, i.e. a
-        // successful call will make it the responsiblity of the JS VM
-        // to free the buffer.
         // written can never exceed len, so the truncation is OK.
-        JSString* str = JS_NewLatin1String(
-            cx, reinterpret_cast<JS::Latin1Char*>(buffer), written);
+        JSString* str = JS_NewLatin1String(cx, std::move(buffer), written);
         if (!str) {
-          JS_free(cx, buffer);
           return false;
         }
+
         d.setString(str);
         return true;
       }
 
       // 1-byte sequences decode to 1 UTF-16 code unit
       // 2-byte sequences decode to 1 UTF-16 code unit
       // 3-byte sequences decode to 1 UTF-16 code unit
       // 4-byte sequences decode to 2 UTF-16 code units
@@ -311,41 +312,38 @@ bool XPCConvert::NativeData2JS(MutableHa
       // the number of input code units (but see the comment
       // below). allocLen already takes the zero terminator
       // into account.
       allocLen *= sizeof(char16_t);
       if (!allocLen.isValid()) {
         return false;
       }
 
-      char16_t* buffer =
-          static_cast<char16_t*>(JS_malloc(cx, allocLen.value()));
+      JS::UniqueTwoByteChars buffer(
+          static_cast<char16_t*>(JS_malloc(cx, allocLen.value())));
       if (!buffer) {
         return false;
       }
 
       // For its internal simplicity, ConvertUTF8toUTF16 requires the
       // destination to be one code unit longer than the source, but
       // it never actually writes more code units than the number of
       // code units in the source. That's why it's OK to claim the
       // output buffer has len + 1 space but then still expect to
       // have space for the zero terminator.
-      size_t written =
-          ConvertUTF8toUTF16(*utf8String, MakeSpan(buffer, allocLen.value()));
+      size_t written = ConvertUTF8toUTF16(
+          *utf8String, MakeSpan(buffer.get(), allocLen.value()));
       MOZ_RELEASE_ASSERT(written <= len);
       buffer[written] = 0;
 
-      // JS_NewUCStringDontDeflate takes ownership on success, i.e. a
-      // successful call will make it the responsiblity of the JS VM
-      // to free the buffer.
-      JSString* str = JS_NewUCStringDontDeflate(cx, buffer, written);
+      JSString* str = JS_NewUCStringDontDeflate(cx, std::move(buffer), written);
       if (!str) {
-        JS_free(cx, buffer);
         return false;
       }
+
       d.setString(str);
       return true;
     }
     case nsXPTType::T_CSTRING: {
       const nsACString* cString = static_cast<const nsACString*>(s);
 
       if (!cString || cString->IsVoid()) {
         d.setNull();