Bug 1471931 - Part 4: Add NewString and NewStringDontDeflate functions which accept UniquePtr. r=sfink
authorAndré Bargull <andre.bargull@gmail.com>
Thu, 28 Jun 2018 09:30:46 -0700
changeset 424495 4e320fc1cbb8c73e868b4bbbe6dee924eed384bc
parent 424494 d2861f3dcc589d186308e9a3339d7344773fdb3d
child 424496 266767b68e8c4552a2f7a602ca103883d45e91de
push id34211
push userapavel@mozilla.com
push dateSat, 30 Jun 2018 09:49:56 +0000
treeherdermozilla-central@19766d4c54e3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1471931
milestone63.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 1471931 - Part 4: Add NewString and NewStringDontDeflate functions which accept UniquePtr. r=sfink
js/src/builtin/String.cpp
js/src/builtin/TestingFunctions.cpp
js/src/jsapi.cpp
js/src/util/StringBuffer.cpp
js/src/vm/Compartment.cpp
js/src/vm/StringType.cpp
js/src/vm/StringType.h
js/src/vm/StructuredClone.cpp
--- a/js/src/builtin/String.cpp
+++ b/js/src/builtin/String.cpp
@@ -187,44 +187,34 @@ class MOZ_NON_PARAM InlineCharBuffer
                        "expected only inline storage when length fits in inline string");
 
             return NewStringCopyNDontDeflate<CanGC>(cx, inlineStorage, length);
         }
 
         MOZ_ASSERT(heapStorage, "heap storage was not allocated for non-inline string");
 
         heapStorage.get()[length] = '\0'; // Null-terminate
-        JSString* res = NewStringDontDeflate<CanGC>(cx, heapStorage.get(), length);
-        if (!res)
-            return nullptr;
-
-        mozilla::Unused << heapStorage.release();
-        return res;
+        return NewStringDontDeflate<CanGC>(cx, std::move(heapStorage), length);
     }
 
     JSString* toString(JSContext* cx, size_t length)
     {
         MOZ_ASSERT(length == lastRequestedLength);
 
         if (JSInlineString::lengthFits<CharT>(length)) {
             MOZ_ASSERT(!heapStorage,
                        "expected only inline storage when length fits in inline string");
 
             return NewStringCopyN<CanGC>(cx, inlineStorage, length);
         }
 
         MOZ_ASSERT(heapStorage, "heap storage was not allocated for non-inline string");
 
         heapStorage.get()[length] = '\0'; // Null-terminate
-        JSString* res = NewString<CanGC>(cx, heapStorage.get(), length);
-        if (!res)
-            return nullptr;
-
-        mozilla::Unused << heapStorage.release();
-        return res;
+        return NewString<CanGC>(cx, std::move(heapStorage), length);
     }
 };
 
 /*
  * Forward declarations for URI encode/decode and helper routines
  */
 static bool
 str_decodeURI(JSContext* cx, unsigned argc, Value* vp);
@@ -3571,22 +3561,20 @@ js::str_fromCodePoint(JSContext* cx, uns
             return false;
 
         // Step 5.e.
         unicode::UTF16Encode(codePoint, elements.get(), &length);
     }
     elements[length] = 0;
 
     // Step 6.
-    JSString* str = NewString<CanGC>(cx, elements.get(), length);
+    JSString* str = NewString<CanGC>(cx, std::move(elements), length);
     if (!str)
         return false;
 
-    mozilla::Unused << elements.release();
-
     args.rval().setString(str);
     return true;
 }
 
 static const JSFunctionSpec string_static_methods[] = {
     JS_INLINABLE_FN("fromCharCode", js::str_fromCharCode, 1, 0, StringFromCharCode),
     JS_INLINABLE_FN("fromCodePoint", js::str_fromCodePoint, 1, 0, StringFromCodePoint),
 
--- a/js/src/builtin/TestingFunctions.cpp
+++ b/js/src/builtin/TestingFunctions.cpp
@@ -2430,23 +2430,19 @@ ReadGeckoProfilingStack(JSContext* cx, u
 
             frameKind = NewStringCopyZ<CanGC>(cx, inlineFrame.kind);
             if (!frameKind)
                 return false;
 
             if (!JS_DefineProperty(cx, inlineFrameInfo, "kind", frameKind, propAttrs))
                 return false;
 
-            size_t length = strlen(inlineFrame.label.get());
-            auto* label = reinterpret_cast<Latin1Char*>(inlineFrame.label.release());
-            frameLabel = NewString<CanGC>(cx, label, length);
-            if (!frameLabel) {
-                js_free(label);
+            frameLabel = NewLatin1StringZ(cx, std::move(inlineFrame.label));
+            if (!frameLabel)
                 return false;
-            }
 
             if (!JS_DefineProperty(cx, inlineFrameInfo, "label", frameLabel, propAttrs))
                 return false;
 
             idx = INT_TO_JSID(inlineFrameNo);
             if (!JS_DefinePropertyById(cx, inlineStack, idx, inlineFrameInfo, 0))
                 return false;
 
@@ -3640,20 +3636,20 @@ FindPath(JSContext* cx, unsigned argc, V
         if (!cx->compartment()->wrap(cx, &wrapped))
             return false;
 
         if (!JS_DefineProperty(cx, obj, "node", wrapped, JSPROP_ENUMERATE))
             return false;
 
         heaptools::EdgeName edgeName = std::move(edges[i]);
 
-        RootedString edgeStr(cx, NewString<CanGC>(cx, edgeName.get(), js_strlen(edgeName.get())));
+        size_t edgeNameLength = js_strlen(edgeName.get());
+        RootedString edgeStr(cx, NewString<CanGC>(cx, std::move(edgeName), edgeNameLength));
         if (!edgeStr)
             return false;
-        mozilla::Unused << edgeName.release(); // edgeStr acquired ownership
 
         if (!JS_DefineProperty(cx, obj, "edge", edgeStr, JSPROP_ENUMERATE))
             return false;
 
         result->setDenseElement(length - i - 1, ObjectValue(*obj));
     }
 
     args.rval().setObject(*result);
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -5807,33 +5807,33 @@ JS_AtomizeAndPinStringN(JSContext* cx, c
     return atom;
 }
 
 JS_PUBLIC_API(JSString*)
 JS_NewLatin1String(JSContext* cx, JS::Latin1Char* chars, size_t length)
 {
     AssertHeapIsIdle();
     CHECK_REQUEST(cx);
-    return NewString<CanGC>(cx, chars, length);
+    return NewString(cx, chars, length);
 }
 
 JS_PUBLIC_API(JSString*)
 JS_NewUCString(JSContext* cx, char16_t* chars, size_t length)
 {
     AssertHeapIsIdle();
     CHECK_REQUEST(cx);
-    return NewString<CanGC>(cx, chars, length);
+    return NewString(cx, chars, length);
 }
 
 JS_PUBLIC_API(JSString*)
 JS_NewUCStringDontDeflate(JSContext* cx, char16_t* chars, size_t length)
 {
     AssertHeapIsIdle();
     CHECK_REQUEST(cx);
-    return NewStringDontDeflate<CanGC>(cx, chars, length);
+    return NewStringDontDeflate(cx, chars, length);
 }
 
 JS_PUBLIC_API(JSString*)
 JS_NewUCStringCopyN(JSContext* cx, const char16_t* s, size_t n)
 {
     AssertHeapIsIdle();
     CHECK_REQUEST(cx);
     if (!n)
--- a/js/src/util/StringBuffer.cpp
+++ b/js/src/util/StringBuffer.cpp
@@ -79,27 +79,26 @@ FinishStringFlat(JSContext* cx, StringBu
     size_t len = sb.length();
     if (!sb.append('\0'))
         return nullptr;
 
     UniquePtr<CharT[], JS::FreePolicy> buf(ExtractWellSized<CharT>(cx, cb));
     if (!buf)
         return nullptr;
 
-    JSFlatString* str = NewStringDontDeflate<CanGC>(cx, buf.get(), len);
+    JSFlatString* str = NewStringDontDeflate<CanGC>(cx, std::move(buf), len);
     if (!str)
         return nullptr;
 
     /*
      * The allocation was made on a TempAllocPolicy, so account for the string
      * data on the string's zone.
      */
     cx->updateMallocCounter(sizeof(CharT) * len);
 
-    mozilla::Unused << buf.release();
     return str;
 }
 
 JSFlatString*
 StringBuffer::finishString()
 {
     size_t len = length();
     if (len == 0)
--- a/js/src/vm/Compartment.cpp
+++ b/js/src/vm/Compartment.cpp
@@ -126,32 +126,24 @@ CopyStringPure(JSContext* cx, JSString* 
                : NewStringCopyNDontDeflate<CanGC>(cx, chars.twoByteRange().begin().get(), len);
     }
 
     if (str->hasLatin1Chars()) {
         UniquePtr<Latin1Char[], JS::FreePolicy> copiedChars = str->asRope().copyLatin1CharsZ(cx);
         if (!copiedChars)
             return nullptr;
 
-        auto* rawCopiedChars = copiedChars.release();
-        auto* result = NewString<CanGC>(cx, rawCopiedChars, len);
-        if (!result)
-            js_free(rawCopiedChars);
-        return result;
+        return NewString<CanGC>(cx, std::move(copiedChars), len);
     }
 
     UniqueTwoByteChars copiedChars = str->asRope().copyTwoByteCharsZ(cx);
     if (!copiedChars)
         return nullptr;
 
-    auto* rawCopiedChars = copiedChars.release();
-    auto* result = NewStringDontDeflate<CanGC>(cx, rawCopiedChars, len);
-    if (!result)
-        js_free(rawCopiedChars);
-    return result;
+    return NewStringDontDeflate<CanGC>(cx, std::move(copiedChars), len);
 }
 
 bool
 Compartment::wrap(JSContext* cx, MutableHandleString strp)
 {
     MOZ_ASSERT(cx->compartment() == this);
 
     /* If the string is already in this compartment, we are done. */
--- a/js/src/vm/StringType.cpp
+++ b/js/src/vm/StringType.cpp
@@ -36,16 +36,18 @@ using mozilla::IsSame;
 using mozilla::PodCopy;
 using mozilla::PodEqual;
 using mozilla::RangedPtr;
 using mozilla::RoundUpPow2;
 using mozilla::Unused;
 
 using JS::AutoCheckCannotGC;
 
+using UniqueLatin1Chars = UniquePtr<Latin1Char[], JS::FreePolicy>;
+
 size_t
 JSString::sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf)
 {
     // JSRope: do nothing, we'll count all children chars when we hit the leaf strings.
     if (isRope())
         return 0;
 
     MOZ_ASSERT(isLinear());
@@ -274,29 +276,29 @@ AllocChars(JSString* str, size_t length,
     /* Like length, capacity does not include the null char, so take it out. */
     *capacity = numChars - 1;
 
     JS_STATIC_ASSERT(JSString::MAX_LENGTH * sizeof(CharT) < UINT32_MAX);
     *chars = str->zone()->pod_malloc<CharT>(numChars);
     return *chars != nullptr;
 }
 
-UniquePtr<Latin1Char[], JS::FreePolicy>
+UniqueLatin1Chars
 JSRope::copyLatin1CharsZ(JSContext* maybecx) const
 {
     return copyCharsInternal<Latin1Char>(maybecx, true);
 }
 
 UniqueTwoByteChars
 JSRope::copyTwoByteCharsZ(JSContext* maybecx) const
 {
     return copyCharsInternal<char16_t>(maybecx, true);
 }
 
-UniquePtr<Latin1Char[], JS::FreePolicy>
+UniqueLatin1Chars
 JSRope::copyLatin1Chars(JSContext* maybecx) const
 {
     return copyCharsInternal<Latin1Char>(maybecx, false);
 }
 
 UniqueTwoByteChars
 JSRope::copyTwoByteChars(JSContext* maybecx) const
 {
@@ -1524,80 +1526,120 @@ NewStringDeflated(JSContext* cx, const c
 
 template <AllowGC allowGC>
 static JSFlatString*
 NewStringDeflated(JSContext* cx, const Latin1Char* s, size_t n)
 {
     MOZ_CRASH("Shouldn't be called for Latin1 chars");
 }
 
-template <AllowGC allowGC, typename CharT>
+template <typename CharT>
 JSFlatString*
 js::NewStringDontDeflate(JSContext* cx, CharT* 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);
         return str;
     }
 
     if (JSInlineString::lengthFits<CharT>(length)) {
         JSInlineString* str =
-            NewInlineString<allowGC>(cx, mozilla::Range<const CharT>(chars, length));
+            NewInlineString<CanGC>(cx, mozilla::Range<const CharT>(chars, length));
         if (!str)
             return nullptr;
 
         js_free(chars);
         return str;
     }
 
-    return JSFlatString::new_<allowGC>(cx, chars, length);
+    return JSFlatString::new_<CanGC>(cx, chars, length);
 }
 
 template JSFlatString*
-js::NewStringDontDeflate<CanGC>(JSContext* cx, char16_t* chars, size_t length);
-
-template JSFlatString*
-js::NewStringDontDeflate<NoGC>(JSContext* cx, char16_t* chars, size_t length);
+js::NewStringDontDeflate(JSContext* cx, char16_t* chars, size_t length);
 
 template JSFlatString*
-js::NewStringDontDeflate<CanGC>(JSContext* cx, Latin1Char* chars, size_t length);
+js::NewStringDontDeflate(JSContext* cx, Latin1Char* chars, size_t length);
 
-template JSFlatString*
-js::NewStringDontDeflate<NoGC>(JSContext* cx, Latin1Char* chars, size_t length);
-
-template <AllowGC allowGC, typename CharT>
+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<allowGC>(cx, 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;
     }
 
-    return NewStringDontDeflate<allowGC>(cx, chars, length);
+    return NewStringDontDeflate(cx, chars, length);
+}
+
+template JSFlatString*
+js::NewString(JSContext* cx, char16_t* chars, size_t length);
+
+template JSFlatString*
+js::NewString(JSContext* cx, Latin1Char* 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;
 }
 
 template JSFlatString*
-js::NewString<CanGC>(JSContext* cx, char16_t* chars, size_t length);
+js::NewStringDontDeflate<CanGC>(JSContext* cx, UniqueTwoByteChars chars, size_t length);
+
+template JSFlatString*
+js::NewStringDontDeflate<NoGC>(JSContext* cx, UniqueTwoByteChars chars, size_t length);
+
+template JSFlatString*
+js::NewStringDontDeflate<CanGC>(JSContext* cx, UniqueLatin1Chars chars, size_t length);
 
 template JSFlatString*
-js::NewString<NoGC>(JSContext* cx, char16_t* chars, size_t length);
+js::NewStringDontDeflate<NoGC>(JSContext* cx, UniqueLatin1Chars chars, size_t length);
+
+template <AllowGC allowGC, typename CharT>
+JSFlatString*
+js::NewString(JSContext* cx, UniquePtr<CharT[], JS::FreePolicy> chars, size_t length)
+{
+    if (IsSame<CharT, char16_t>::value && CanStoreCharsAsLatin1(chars.get(), length))
+        return NewStringDeflated<allowGC>(cx, chars.get(), length);
+
+    return NewStringDontDeflate<allowGC>(cx, std::move(chars), length);
+}
 
 template JSFlatString*
-js::NewString<CanGC>(JSContext* cx, Latin1Char* chars, size_t length);
+js::NewString<CanGC>(JSContext* cx, UniqueTwoByteChars chars, size_t length);
 
 template JSFlatString*
-js::NewString<NoGC>(JSContext* cx, Latin1Char* chars, size_t length);
+js::NewString<NoGC>(JSContext* cx, UniqueTwoByteChars chars, size_t length);
+
+template JSFlatString*
+js::NewString<CanGC>(JSContext* cx, UniqueLatin1Chars chars, size_t length);
+
+template JSFlatString*
+js::NewString<NoGC>(JSContext* cx, UniqueLatin1Chars chars, size_t length);
 
 namespace js {
 
 template <AllowGC allowGC, typename CharT>
 JSFlatString*
 NewStringCopyNDontDeflate(JSContext* cx, const CharT* s, size_t n)
 {
     if (JSFlatString* str = TryEmptyOrStaticString(cx, s, n))
@@ -1634,22 +1676,19 @@ template JSFlatString*
 NewStringCopyNDontDeflate<CanGC>(JSContext* cx, const Latin1Char* s, size_t n);
 
 template JSFlatString*
 NewStringCopyNDontDeflate<NoGC>(JSContext* cx, const Latin1Char* s, size_t n);
 
 JSFlatString*
 NewLatin1StringZ(JSContext* cx, UniqueChars chars)
 {
-    JSFlatString* str = NewString<CanGC>(cx, (Latin1Char*)chars.get(), strlen(chars.get()));
-    if (!str)
-        return nullptr;
-
-    mozilla::Unused << chars.release();
-    return str;
+    size_t length = strlen(chars.get());
+    UniqueLatin1Chars latin1(reinterpret_cast<Latin1Char*>(chars.release()));
+    return NewString<CanGC>(cx, std::move(latin1), length);
 }
 
 template <AllowGC allowGC, typename CharT>
 JSFlatString*
 NewStringCopyN(JSContext* cx, const CharT* s, size_t n)
 {
     if (IsSame<CharT, char16_t>::value && CanStoreCharsAsLatin1(s, n))
         return NewStringDeflated<allowGC>(cx, s, n);
@@ -1674,36 +1713,30 @@ JSFlatString*
 NewStringCopyUTF8N(JSContext* cx, const JS::UTF8Chars utf8)
 {
     JS::SmallestEncoding encoding = JS::FindSmallestEncoding(utf8);
     if (encoding == JS::SmallestEncoding::ASCII)
         return NewStringCopyN<allowGC>(cx, utf8.begin().get(), utf8.length());
 
     size_t length;
     if (encoding == JS::SmallestEncoding::Latin1) {
-        Latin1Char* latin1 = UTF8CharsToNewLatin1CharsZ(cx, utf8, &length).get();
+        UniqueLatin1Chars latin1(UTF8CharsToNewLatin1CharsZ(cx, utf8, &length).get());
         if (!latin1)
             return nullptr;
 
-        JSFlatString* result = NewString<allowGC>(cx, latin1, length);
-        if (!result)
-            js_free((void*)latin1);
-        return result;
+        return NewString<allowGC>(cx, std::move(latin1), length);
     }
 
     MOZ_ASSERT(encoding == JS::SmallestEncoding::UTF16);
 
-    char16_t* utf16 = UTF8CharsToNewTwoByteCharsZ(cx, utf8, &length).get();
+    UniqueTwoByteChars utf16(UTF8CharsToNewTwoByteCharsZ(cx, utf8, &length).get());
     if (!utf16)
         return nullptr;
 
-    JSFlatString* result = NewString<allowGC>(cx, utf16, length);
-    if (!result)
-        js_free((void*)utf16);
-    return result;
+    return NewString<allowGC>(cx, std::move(utf16), length);
 }
 
 template JSFlatString*
 NewStringCopyUTF8N<CanGC>(JSContext* cx, const JS::UTF8Chars utf8);
 
 MOZ_ALWAYS_INLINE JSString*
 ExternalStringCache::lookup(const char16_t* chars, size_t len) const
 {
--- a/js/src/vm/StringType.h
+++ b/js/src/vm/StringType.h
@@ -1439,24 +1439,34 @@ StringToNewUTF8CharsZ(JSContext* maybecx
         return nullptr;
 
     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. */
-template <js::AllowGC allowGC, typename CharT>
+template <typename CharT>
 extern JSFlatString*
 NewString(JSContext* cx, CharT* chars, size_t length);
 
 /* Like NewString, but doesn't try to deflate to Latin1. */
+template <typename CharT>
+extern JSFlatString*
+NewStringDontDeflate(JSContext* cx, CharT* chars, size_t length);
+
+/* GC-allocate a string descriptor for the given malloc-allocated chars. */
 template <js::AllowGC allowGC, typename CharT>
 extern JSFlatString*
-NewStringDontDeflate(JSContext* cx, CharT* chars, size_t length);
+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(JSContext* cx, UniquePtr<CharT[], JS::FreePolicy> chars, size_t length);
 
 extern JSLinearString*
 NewDependentString(JSContext* cx, JSString* base, size_t start, size_t length);
 
 /* Take ownership of an array of Latin1Chars. */
 extern JSFlatString*
 NewLatin1StringZ(JSContext* cx, UniqueChars chars);
 
--- a/js/src/vm/StructuredClone.cpp
+++ b/js/src/vm/StructuredClone.cpp
@@ -1924,20 +1924,17 @@ JSStructuredCloneReader::readStringImpl(
     if (nchars > JSString::MAX_LENGTH) {
         JS_ReportErrorNumberASCII(context(), GetErrorMessage, nullptr, JSMSG_SC_BAD_SERIALIZED_DATA,
                                   "string length");
         return nullptr;
     }
     UniquePtr<CharT[], JS::FreePolicy> chars = AllocateChars<CharT>(context(), nchars);
     if (!chars || !in.readChars(chars.get(), nchars))
         return nullptr;
-    JSString* str = NewString<CanGC>(context(), chars.get(), nchars);
-    if (str)
-        mozilla::Unused << chars.release();
-    return str;
+    return NewString<CanGC>(context(), std::move(chars), nchars);
 }
 
 JSString*
 JSStructuredCloneReader::readString(uint32_t data)
 {
     uint32_t nchars = data & JS_BITMASK(31);
     bool latin1 = data & (1 << 31);
     return latin1 ? readStringImpl<Latin1Char>(nchars) : readStringImpl<char16_t>(nchars);