Bug 1352323 - Add JS_NewMaybeExternalString function that creates either an external string or returns a static string. r=jandem
authorTooru Fujisawa <arai_a@mac.com>
Tue, 04 Apr 2017 15:45:49 +0900
changeset 351105 4cda9336503172c5bad871155c1abf1c44f45fa9
parent 351104 ebc1182ae0a58e1e6d4641edd41913e2c879da64
child 351106 7cafcea59af957a1b73e6b49a56b1c283c987ddc
push id31599
push usercbook@mozilla.com
push dateTue, 04 Apr 2017 10:35:26 +0000
treeherdermozilla-central@891981e67948 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1352323
milestone55.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 1352323 - Add JS_NewMaybeExternalString function that creates either an external string or returns a static string. r=jandem
js/src/jsapi.cpp
js/src/jsapi.h
js/src/vm/String.cpp
js/src/vm/String.h
js/xpconnect/src/XPCString.cpp
js/xpconnect/src/xpcpublic.h
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -1494,16 +1494,25 @@ JS_NewExternalString(JSContext* cx, cons
                      const JSStringFinalizer* fin)
 {
     AssertHeapIsIdle();
     CHECK_REQUEST(cx);
     JSString* s = JSExternalString::new_(cx, chars, length, fin);
     return s;
 }
 
+JS_PUBLIC_API(JSString*)
+JS_NewMaybeExternalString(JSContext* cx, const char16_t* chars, size_t length,
+                          const JSStringFinalizer* fin, bool* isExternal)
+{
+    AssertHeapIsIdle();
+    CHECK_REQUEST(cx);
+    return NewMaybeExternalString(cx, chars, length, fin, isExternal);
+}
+
 extern JS_PUBLIC_API(bool)
 JS_IsExternalString(JSString* str)
 {
     return str->isExternal();
 }
 
 extern JS_PUBLIC_API(const JSStringFinalizer*)
 JS_GetExternalStringFinalizer(JSString* str)
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -1850,16 +1850,27 @@ JS_SetGCParametersBasedOnAvailableMemory
  * Create a new JSString whose chars member refers to external memory, i.e.,
  * memory requiring application-specific finalization.
  */
 extern JS_PUBLIC_API(JSString*)
 JS_NewExternalString(JSContext* cx, const char16_t* chars, size_t length,
                      const JSStringFinalizer* fin);
 
 /**
+ * Create a new JSString whose chars member may refer to external memory.
+ * If the returned string refers to the external memory, |*isExternal| is set
+ * to true.  Otherwise the returned string is not an external string and
+ * |*isExternal| is set to false.  If |*isExternal| is false, |fin| won't be
+ * called.
+ */
+extern JS_PUBLIC_API(JSString*)
+JS_NewMaybeExternalString(JSContext* cx, const char16_t* chars, size_t length,
+                          const JSStringFinalizer* fin, bool* isExternal);
+
+/**
  * Return whether 'str' was created with JS_NewExternalString or
  * JS_NewExternalStringWithClosure.
  */
 extern JS_PUBLIC_API(bool)
 JS_IsExternalString(JSString* str);
 
 /**
  * Return the 'fin' arg passed to JS_NewExternalString.
--- a/js/src/vm/String.cpp
+++ b/js/src/vm/String.cpp
@@ -1408,16 +1408,29 @@ NewStringCopyUTF8N(JSContext* cx, const 
     if (!result)
         js_free((void*)utf16);
     return result;
 }
 
 template JSFlatString*
 NewStringCopyUTF8N<CanGC>(JSContext* cx, const JS::UTF8Chars utf8);
 
+JSString*
+NewMaybeExternalString(JSContext* cx, const char16_t* s, size_t n, const JSStringFinalizer* fin,
+                       bool* isExternal)
+{
+    if (JSString* str = TryEmptyOrStaticString(cx, s, n)) {
+        *isExternal = false;
+        return str;
+    }
+
+    *isExternal = true;
+    return JSExternalString::new_(cx, s, n, fin);
+}
+
 } /* namespace js */
 
 #ifdef DEBUG
 void
 JSExtensibleString::dumpRepresentation(FILE* fp, int indent) const
 {
     dumpRepresentationHeader(fp, indent, "JSExtensibleString");
     indent += 2;
--- a/js/src/vm/String.h
+++ b/js/src/vm/String.h
@@ -1308,16 +1308,20 @@ NewStringCopyUTF8N(JSContext* cx, const 
 
 template <js::AllowGC allowGC>
 inline JSFlatString*
 NewStringCopyUTF8Z(JSContext* cx, const JS::ConstUTF8CharsZ utf8)
 {
     return NewStringCopyUTF8N<allowGC>(cx, JS::UTF8Chars(utf8.c_str(), strlen(utf8.c_str())));
 }
 
+JSString*
+NewMaybeExternalString(JSContext* cx, const char16_t* s, size_t n, const JSStringFinalizer* fin,
+                       bool* isExternal);
+
 JS_STATIC_ASSERT(sizeof(HashNumber) == 4);
 
 } /* namespace js */
 
 // Addon IDs are interned atoms which are never destroyed. This detail is
 // not exposed outside the API.
 class JSAddonId : public JSAtom
 {};
--- a/js/xpconnect/src/XPCString.cpp
+++ b/js/xpconnect/src/XPCString.cpp
@@ -91,19 +91,20 @@ XPCStringConvert::ReadableToJSVal(JSCont
                                   nsStringBuffer** sharedBuffer,
                                   MutableHandleValue vp)
 {
     *sharedBuffer = nullptr;
 
     uint32_t length = readable.Length();
 
     if (readable.IsLiteral()) {
-        JSString* str = JS_NewExternalString(cx,
-                                             static_cast<const char16_t*>(readable.BeginReading()),
-                                             length, &sLiteralFinalizer);
+        bool ignored;
+        JSString* str = JS_NewMaybeExternalString(cx,
+                                                  static_cast<const char16_t*>(readable.BeginReading()),
+                                                  length, &sLiteralFinalizer, &ignored);
         if (!str)
             return false;
         vp.setString(str);
         return true;
     }
 
     nsStringBuffer* buf = nsStringBuffer::FromString(readable);
     if (buf) {
--- a/js/xpconnect/src/xpcpublic.h
+++ b/js/xpconnect/src/xpcpublic.h
@@ -262,23 +262,32 @@ public:
         if (cache && buf == cache->mBuffer && length == cache->mLength) {
             MOZ_ASSERT(JS::GetStringZone(cache->mString) == zone);
             JS::StringReadBarrier(cache->mString);
             rval.setString(cache->mString);
             *sharedBuffer = false;
             return true;
         }
 
-        JSString* str = JS_NewExternalString(cx,
-                                             static_cast<char16_t*>(buf->Data()),
-                                             length, &sDOMStringFinalizer);
+        bool isExternal;
+        JSString* str = JS_NewMaybeExternalString(cx,
+                                                  static_cast<char16_t*>(buf->Data()),
+                                                  length, &sDOMStringFinalizer, &isExternal);
         if (!str) {
             return false;
         }
         rval.setString(str);
+
+        // If JS_NewMaybeExternalString returns non-external string, finalizer
+        // won't be called.  Do not store it to cache.
+        if (!isExternal) {
+            *sharedBuffer = false;
+            return true;
+        }
+
         if (!cache) {
             cache = new ZoneStringCache();
             JS_SetZoneUserData(zone, cache);
         }
         cache->mBuffer = buf;
         cache->mLength = length;
         cache->mString = str;
         *sharedBuffer = true;