Bug 1330593 part 1 - Allow non-flat external strings. r=jwalden,bz
authorJan de Mooij <jdemooij@mozilla.com>
Thu, 26 Jan 2017 18:40:41 +0100
changeset 331259 eed0b80462a2f573f22b990c003f5f42ebd3c1f1
parent 331258 47828603cd9c74ac3192b92b5a3a9c59fece1a15
child 331260 c11fe7c58837ebf4266357d1f31896c7cc4a49b9
push id31264
push userkwierso@gmail.com
push dateFri, 27 Jan 2017 00:19:08 +0000
treeherdermozilla-central@1e0e193b0812 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwalden, bz
bugs1330593
milestone54.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 1330593 part 1 - Allow non-flat external strings. r=jwalden,bz
js/public/Class.h
js/src/builtin/TestingFunctions.cpp
js/src/jit-test/tests/basic/external-strings.js
js/src/jit/VMFunctions.cpp
js/src/jsapi-tests/testExternalStrings.cpp
js/src/vm/String-inl.h
js/src/vm/String.cpp
js/src/vm/String.h
js/xpconnect/src/XPCString.cpp
js/xpconnect/src/xpcpublic.h
--- a/js/public/Class.h
+++ b/js/public/Class.h
@@ -470,17 +470,17 @@ typedef bool
  * from other live objects or from GC roots.  Obviously, finalizers must never
  * store a reference to obj.
  */
 typedef void
 (* JSFinalizeOp)(JSFreeOp* fop, JSObject* obj);
 
 /** Finalizes external strings created by JS_NewExternalString. */
 struct JSStringFinalizer {
-    void (*finalize)(const JSStringFinalizer* fin, char16_t* chars);
+    void (*finalize)(JS::Zone* zone, const JSStringFinalizer* fin, char16_t* chars);
 };
 
 /**
  * Check whether v is an instance of obj.  Return false on error or exception,
  * true on success with true in *bp if v is an instance of obj, false in
  * *bp otherwise.
  */
 typedef bool
--- a/js/src/builtin/TestingFunctions.cpp
+++ b/js/src/builtin/TestingFunctions.cpp
@@ -1236,16 +1236,73 @@ EnableTrackAllocations(JSContext* cx, un
 
 static bool
 DisableTrackAllocations(JSContext* cx, unsigned argc, Value* vp)
 {
     SetAllocationMetadataBuilder(cx, nullptr);
     return true;
 }
 
+static void
+FinalizeExternalString(Zone* zone, const JSStringFinalizer* fin, char16_t* chars);
+
+static const JSStringFinalizer ExternalStringFinalizer =
+    { FinalizeExternalString };
+
+static void
+FinalizeExternalString(Zone* zone, const JSStringFinalizer* fin, char16_t* chars)
+{
+    MOZ_ASSERT(fin == &ExternalStringFinalizer);
+    js_free(chars);
+}
+
+static bool
+NewExternalString(JSContext* cx, unsigned argc, Value* vp)
+{
+    CallArgs args = CallArgsFromVp(argc, vp);
+
+    if (args.length() != 1 || !args[0].isString()) {
+        JS_ReportErrorASCII(cx, "newExternalString takes exactly one string argument.");
+        return false;
+    }
+
+    RootedString str(cx, args[0].toString());
+    size_t len = str->length();
+
+    UniqueTwoByteChars buf(js_pod_malloc<char16_t>(len));
+    if (!JS_CopyStringChars(cx, mozilla::Range<char16_t>(buf.get(), len), str))
+        return false;
+
+    JSString* res = JS_NewExternalString(cx, buf.get(), len, &ExternalStringFinalizer);
+    if (!res)
+        return false;
+
+    mozilla::Unused << buf.release();
+    args.rval().setString(res);
+    return true;
+}
+
+static bool
+EnsureFlatString(JSContext* cx, unsigned argc, Value* vp)
+{
+    CallArgs args = CallArgsFromVp(argc, vp);
+
+    if (args.length() != 1 || !args[0].isString()) {
+        JS_ReportErrorASCII(cx, "ensureFlatString takes exactly one string argument.");
+        return false;
+    }
+
+    JSFlatString* flat = args[0].toString()->ensureFlat(cx);
+    if (!flat)
+        return false;
+
+    args.rval().setString(flat);
+    return true;
+}
+
 #if defined(DEBUG) || defined(JS_OOM_BREAKPOINT)
 static bool
 OOMThreadTypes(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     args.rval().setInt32(js::oom::THREAD_TYPE_MAX);
     return true;
 }
@@ -4150,16 +4207,24 @@ static const JSFunctionSpecWithHelp Test
 "  Start capturing the JS stack at every allocation. Note that this sets an\n"
 "  object metadata callback that will override any other object metadata\n"
 "  callback that may be set."),
 
     JS_FN_HELP("disableTrackAllocations", DisableTrackAllocations, 0, 0,
 "disableTrackAllocations()",
 "  Stop capturing the JS stack at every allocation."),
 
+    JS_FN_HELP("newExternalString", NewExternalString, 1, 0,
+"newExternalString(str)",
+"  Copies str's chars and returns a new external string."),
+
+    JS_FN_HELP("ensureFlatString", EnsureFlatString, 1, 0,
+"ensureFlatString(str)",
+"  Ensures str is a flat (null-terminated) string and returns it."),
+
 #if defined(DEBUG) || defined(JS_OOM_BREAKPOINT)
     JS_FN_HELP("oomThreadTypes", OOMThreadTypes, 0, 0,
 "oomThreadTypes()",
 "  Get the number of thread types that can be used as an argument for\n"
 "oomAfterAllocations() and oomAtAllocation()."),
 
     JS_FN_HELP("oomAfterAllocations", OOMAfterAllocations, 2, 0,
 "oomAfterAllocations(count [,threadType])",
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/external-strings.js
@@ -0,0 +1,16 @@
+assertEq(newExternalString(""), "");
+assertEq(newExternalString("abc"), "abc");
+assertEq(newExternalString("abc\0def\u1234"), "abc\0def\u1234");
+
+var o = {foo: 2, "foo\0": 4};
+var ext = newExternalString("foo");
+assertEq(o[ext], 2);
+var ext2 = newExternalString("foo\0");
+assertEq(o[ext2], 4);
+
+eval(newExternalString("assertEq(1, 1)"));
+
+// Make sure ensureFlat does the right thing for external strings.
+ext = newExternalString("abc\0defg\0");
+assertEq(ensureFlatString(ext), "abc\0defg\0");
+assertEq(ensureFlatString(ext), "abc\0defg\0");
--- a/js/src/jit/VMFunctions.cpp
+++ b/js/src/jit/VMFunctions.cpp
@@ -1222,17 +1222,19 @@ AssertValidStringPtr(JSContext* cx, JSSt
     if (str->isFatInline()) {
         MOZ_ASSERT(kind == gc::AllocKind::FAT_INLINE_STRING ||
                    kind == gc::AllocKind::FAT_INLINE_ATOM);
     } else if (str->isExternal()) {
         MOZ_ASSERT(kind == gc::AllocKind::EXTERNAL_STRING);
     } else if (str->isAtom()) {
         MOZ_ASSERT(kind == gc::AllocKind::ATOM);
     } else if (str->isFlat()) {
-        MOZ_ASSERT(kind == gc::AllocKind::STRING || kind == gc::AllocKind::FAT_INLINE_STRING);
+        MOZ_ASSERT(kind == gc::AllocKind::STRING ||
+                   kind == gc::AllocKind::FAT_INLINE_STRING ||
+                   kind == gc::AllocKind::EXTERNAL_STRING);
     } else {
         MOZ_ASSERT(kind == gc::AllocKind::STRING);
     }
 #endif
 }
 
 void
 AssertValidSymbolPtr(JSContext* cx, JS::Symbol* sym)
--- a/js/src/jsapi-tests/testExternalStrings.cpp
+++ b/js/src/jsapi-tests/testExternalStrings.cpp
@@ -14,23 +14,23 @@ static const char16_t arr[] = {
     'h', 'i', ',', 'd', 'o', 'n', '\'', 't', ' ', 'd', 'e', 'l', 'e', 't', 'e', ' ', 'm', 'e', '\0'
 };
 static const size_t arrlen = ArrayLength(arr) - 1;
 
 static int finalized1 = 0;
 static int finalized2 = 0;
 
 static void
-finalize_str(const JSStringFinalizer* fin, char16_t* chars);
+finalize_str(JS::Zone* zone, const JSStringFinalizer* fin, char16_t* chars);
 
 static const JSStringFinalizer finalizer1 = { finalize_str };
 static const JSStringFinalizer finalizer2 = { finalize_str };
 
 static void
-finalize_str(const JSStringFinalizer* fin, char16_t* chars)
+finalize_str(JS::Zone* zone, const JSStringFinalizer* fin, char16_t* chars)
 {
     if (chars && PodEqual(const_cast<const char16_t*>(chars), arr, arrlen)) {
         if (fin == &finalizer1) {
             ++finalized1;
         } else if (fin == &finalizer2) {
             ++finalized2;
         }
     }
--- a/js/src/vm/String-inl.h
+++ b/js/src/vm/String-inl.h
@@ -318,18 +318,16 @@ JSExternalString::init(const char16_t* c
     d.s.u2.nonInlineCharsTwoByte = chars;
     d.s.u3.externalFinalizer = fin;
 }
 
 MOZ_ALWAYS_INLINE JSExternalString*
 JSExternalString::new_(JSContext* cx, const char16_t* chars, size_t length,
                        const JSStringFinalizer* fin)
 {
-    MOZ_ASSERT(chars[length] == 0);
-
     if (!validateLength(cx, length))
         return nullptr;
     JSExternalString* str = js::Allocate<JSExternalString>(cx);
     if (!str)
         return nullptr;
     str->init(chars, length, fin);
     cx->updateMallocCounter((length + 1) * sizeof(char16_t));
     return str;
@@ -399,13 +397,21 @@ JSAtom::finalize(js::FreeOp* fop)
 
     if (!isInline())
         fop->free_(nonInlineCharsRaw());
 }
 
 inline void
 JSExternalString::finalize(js::FreeOp* fop)
 {
+    if (!JSString::isExternal()) {
+        // This started out as an external string, but was turned into a
+        // non-external string by JSExternalString::ensureFlat.
+        MOZ_ASSERT(isFlat());
+        fop->free_(nonInlineCharsRaw());
+        return;
+    }
+
     const JSStringFinalizer* fin = externalFinalizer();
-    fin->finalize(fin, const_cast<char16_t*>(rawTwoByteChars()));
+    fin->finalize(zone(), fin, const_cast<char16_t*>(rawTwoByteChars()));
 }
 
 #endif /* vm_String_inl_h */
--- a/js/src/vm/String.cpp
+++ b/js/src/vm/String.cpp
@@ -39,37 +39,37 @@ JSString::sizeOfExcludingThis(mozilla::M
         return 0;
 
     MOZ_ASSERT(isLinear());
 
     // JSDependentString: do nothing, we'll count the chars when we hit the base string.
     if (isDependent())
         return 0;
 
-    MOZ_ASSERT(isFlat());
-
-    // JSExtensibleString: count the full capacity, not just the used space.
-    if (isExtensible()) {
-        JSExtensibleString& extensible = asExtensible();
-        return extensible.hasLatin1Chars()
-               ? mallocSizeOf(extensible.rawLatin1Chars())
-               : mallocSizeOf(extensible.rawTwoByteChars());
-    }
-
     // JSExternalString: Ask the embedding to tell us what's going on.  If it
     // doesn't want to say, don't count, the chars could be stored anywhere.
     if (isExternal()) {
         if (auto* cb = runtimeFromMainThread()->externalStringSizeofCallback) {
             // Our callback isn't supposed to cause GC.
             JS::AutoSuppressGCAnalysis nogc;
             return cb(this, mallocSizeOf);
         }
         return 0;
     }
 
+    MOZ_ASSERT(isFlat());
+
+    // JSExtensibleString: count the full capacity, not just the used space.
+    if (isExtensible()) {
+        JSExtensibleString& extensible = asExtensible();
+        return extensible.hasLatin1Chars()
+               ? mallocSizeOf(extensible.rawLatin1Chars())
+               : mallocSizeOf(extensible.rawTwoByteChars());
+    }
+
     // JSInlineString, JSFatInlineString [JSInlineAtom, JSFatInlineAtom]: the chars are inline.
     if (isInline())
         return 0;
 
     // JSAtom, JSUndependedString: measure the space for the chars.  For
     // JSUndependedString, there is no need to count the base string, for the
     // same reason as JSDependentString above.
     JSFlatString& flat = asFlat();
@@ -671,17 +671,17 @@ js::ConcatStrings(ExclusiveContext* cx,
 template JSString*
 js::ConcatStrings<CanGC>(ExclusiveContext* cx, HandleString left, HandleString right);
 
 template JSString*
 js::ConcatStrings<NoGC>(ExclusiveContext* cx, JSString* const& left, JSString* const& right);
 
 template <typename CharT>
 JSFlatString*
-JSDependentString::undependInternal(ExclusiveContext* cx)
+JSDependentString::undependInternal(JSContext* cx)
 {
     size_t n = length();
     CharT* s = cx->pod_malloc<CharT>(n + 1);
     if (!s)
         return nullptr;
 
     AutoCheckCannotGC nogc;
     PodCopy(s, nonInlineChars<CharT>(nogc), n);
@@ -696,17 +696,17 @@ JSDependentString::undependInternal(Excl
         d.u1.flags = UNDEPENDED_FLAGS | LATIN1_CHARS_BIT;
     else
         d.u1.flags = UNDEPENDED_FLAGS;
 
     return &this->asFlat();
 }
 
 JSFlatString*
-JSDependentString::undepend(ExclusiveContext* cx)
+JSDependentString::undepend(JSContext* cx)
 {
     MOZ_ASSERT(JSString::isDependent());
     return hasLatin1Chars()
            ? undependInternal<Latin1Char>(cx)
            : undependInternal<char16_t>(cx);
 }
 
 #ifdef DEBUG
@@ -1047,16 +1047,55 @@ AutoStableStringChars::copyTwoByteChars(
     chars[length] = 0;
 
     state_ = TwoByte;
     twoByteChars_ = chars;
     s_ = linearString;
     return true;
 }
 
+JSFlatString*
+JSString::ensureFlat(JSContext* cx)
+{
+    if (isFlat())
+        return &asFlat();
+    if (isDependent())
+        return asDependent().undepend(cx);
+    if (isRope())
+        return asRope().flatten(cx);
+    return asExternal().ensureFlat(cx);
+}
+
+JSFlatString*
+JSExternalString::ensureFlat(JSContext* cx)
+{
+    MOZ_ASSERT(hasTwoByteChars());
+
+    size_t n = length();
+    char16_t* s = cx->pod_malloc<char16_t>(n + 1);
+    if (!s)
+        return nullptr;
+
+    // Copy the chars before finalizing the string.
+    {
+        AutoCheckCannotGC nogc;
+        PodCopy(s, nonInlineChars<char16_t>(nogc), n);
+        s[n] = '\0';
+    }
+
+    // Release the external chars.
+    finalize(cx->runtime()->defaultFreeOp());
+
+    // Transform the string into a non-external, flat string.
+    setNonInlineChars<char16_t>(s);
+    d.u1.flags = FLAT_BIT;
+
+    return &this->asFlat();
+}
+
 #ifdef DEBUG
 void
 JSAtom::dump(FILE* fp)
 {
     fprintf(fp, "JSAtom* (%p) = ", (void*) this);
     this->JSString::dump(fp);
 }
 
--- a/js/src/vm/String.h
+++ b/js/src/vm/String.h
@@ -101,23 +101,23 @@ static const size_t UINT32_CHAR_BUFFER_L
  *
  * C++ type                     operations+fields / invariants+properties
  * ==========================   =========================================
  * JSString (abstract)          get(Latin1|TwoByte)CharsZ, get(Latin1|TwoByte)Chars, length / -
  *  | \
  *  | JSRope                    leftChild, rightChild / -
  *  |
  * JSLinearString (abstract)    latin1Chars, twoByteChars / might be null-terminated
- *  | \
- *  | JSDependentString         base / -
+ *  |  |
+ *  |  +-- JSDependentString    base / -
+ *  |  |
+ *  |  +-- JSExternalString     - / char array memory managed by embedding
  *  |
  * JSFlatString                 - / null terminated
  *  |  |
- *  |  +-- JSExternalString     - / char array memory managed by embedding
- *  |  |
  *  |  +-- JSExtensibleString   tracks total buffer capacity (including current text)
  *  |  |
  *  |  +-- JSUndependedString   original dependent base / -
  *  |  |
  *  |  +-- JSInlineString (abstract)    - / chars stored in header
  *  |      |
  *  |      +-- JSThinInlineString       - / header is normal
  *  |      |
@@ -216,22 +216,22 @@ class JSString : public js::gc::TenuredC
      *
      *   String        Instance     Subtype
      *   type          encoding     predicate
      *   ------------------------------------
      *   Rope          000000       000000
      *   Linear        -           !000000
      *   HasBase       -            xxxx1x
      *   Dependent     000010       000010
+     *   External      100000       100000
      *   Flat          -            xxxxx1
      *   Undepended    000011       000011
      *   Extensible    010001       010001
      *   Inline        000101       xxx1xx
      *   FatInline     010101       x1x1xx
-     *   External      100001       100001
      *   Atom          001001       xx1xxx
      *   PermanentAtom 101001       1x1xxx
      *   InlineAtom    -            xx11xx
      *   FatInlineAtom -            x111xx
      *
      * Note that the first 4 flag bits (from right to left in the previous table)
      * have the following meaning and can be used for some hot queries:
      *
@@ -252,17 +252,17 @@ class JSString : public js::gc::TenuredC
     static const uint32_t HAS_BASE_BIT           = JS_BIT(1);
     static const uint32_t INLINE_CHARS_BIT       = JS_BIT(2);
     static const uint32_t ATOM_BIT               = JS_BIT(3);
 
     static const uint32_t ROPE_FLAGS             = 0;
     static const uint32_t DEPENDENT_FLAGS        = HAS_BASE_BIT;
     static const uint32_t UNDEPENDED_FLAGS       = FLAT_BIT | HAS_BASE_BIT;
     static const uint32_t EXTENSIBLE_FLAGS       = FLAT_BIT | JS_BIT(4);
-    static const uint32_t EXTERNAL_FLAGS         = FLAT_BIT | JS_BIT(5);
+    static const uint32_t EXTERNAL_FLAGS         = JS_BIT(5);
 
     static const uint32_t FAT_INLINE_MASK        = INLINE_CHARS_BIT | JS_BIT(4);
     static const uint32_t PERMANENT_ATOM_MASK    = ATOM_BIT | JS_BIT(5);
 
     /* Initial flags for thin inline and fat inline strings. */
     static const uint32_t INIT_THIN_INLINE_FLAGS = FLAT_BIT | INLINE_CHARS_BIT;
     static const uint32_t INIT_FAT_INLINE_FLAGS  = FLAT_BIT | FAT_INLINE_MASK;
 
@@ -347,17 +347,17 @@ class JSString : public js::gc::TenuredC
     }
     bool hasTwoByteChars() const {
         return !(d.u1.flags & LATIN1_CHARS_BIT);
     }
 
     /* Fallible conversions to more-derived string types. */
 
     inline JSLinearString* ensureLinear(js::ExclusiveContext* cx);
-    inline JSFlatString* ensureFlat(js::ExclusiveContext* cx);
+    JSFlatString* ensureFlat(JSContext* cx);
 
     static bool ensureLinear(js::ExclusiveContext* cx, JSString* str) {
         return str->ensureLinear(cx) != nullptr;
     }
 
     /* Type query and debug-checked casts */
 
     MOZ_ALWAYS_INLINE
@@ -600,17 +600,17 @@ static_assert(sizeof(JSRope) == sizeof(J
               "string subclasses must be binary-compatible with JSString");
 
 class JSLinearString : public JSString
 {
     friend class JSString;
     friend class js::AutoStableStringChars;
 
     /* Vacuous and therefore unimplemented. */
-    JSLinearString* ensureLinear(JSContext* cx) = delete;
+    JSLinearString* ensureLinear(js::ExclusiveContext* cx) = delete;
     bool isLinear() const = delete;
     JSLinearString& asLinear() const = delete;
 
   protected:
     /* Returns void pointer to latin1/twoByte chars, for finalizers. */
     MOZ_ALWAYS_INLINE
     void* nonInlineCharsRaw() const {
         MOZ_ASSERT(!isInline());
@@ -680,20 +680,20 @@ class JSLinearString : public JSString
 };
 
 static_assert(sizeof(JSLinearString) == sizeof(JSString),
               "string subclasses must be binary-compatible with JSString");
 
 class JSDependentString : public JSLinearString
 {
     friend class JSString;
-    JSFlatString* undepend(js::ExclusiveContext* cx);
+    JSFlatString* undepend(JSContext* cx);
 
     template <typename CharT>
-    JSFlatString* undependInternal(js::ExclusiveContext* cx);
+    JSFlatString* undependInternal(JSContext* cx);
 
     void init(js::ExclusiveContext* cx, JSLinearString* base, size_t start,
               size_t length);
 
     /* Vacuous and therefore unimplemented. */
     bool isDependent() const = delete;
     JSDependentString& asDependent() const = delete;
 
@@ -908,17 +908,17 @@ class JSFatInlineString : public JSInlin
 
     MOZ_ALWAYS_INLINE void finalize(js::FreeOp* fop);
 };
 
 static_assert(sizeof(JSFatInlineString) % js::gc::CellSize == 0,
               "fat inline strings shouldn't waste space up to the next cell "
               "boundary");
 
-class JSExternalString : public JSFlatString
+class JSExternalString : public JSLinearString
 {
     void init(const char16_t* chars, size_t length, const JSStringFinalizer* fin);
 
     /* Vacuous and therefore unimplemented. */
     bool isExternal() const = delete;
     JSExternalString& asExternal() const = delete;
 
   public:
@@ -937,16 +937,18 @@ class JSExternalString : public JSFlatSt
     const char16_t* twoByteChars() const {
         return rawTwoByteChars();
     }
 
     /* Only called by the GC for strings with the AllocKind::EXTERNAL_STRING kind. */
 
     inline void finalize(js::FreeOp* fop);
 
+    JSFlatString* ensureFlat(JSContext* cx);
+
 #ifdef DEBUG
     void dumpRepresentation(FILE* fp, int indent) const;
 #endif
 };
 
 static_assert(sizeof(JSExternalString) == sizeof(JSString),
               "string subclasses must be binary-compatible with JSString");
 
@@ -1344,26 +1346,16 @@ JSString::getChar(js::ExclusiveContext* 
 MOZ_ALWAYS_INLINE JSLinearString*
 JSString::ensureLinear(js::ExclusiveContext* cx)
 {
     return isLinear()
            ? &asLinear()
            : asRope().flatten(cx);
 }
 
-MOZ_ALWAYS_INLINE JSFlatString*
-JSString::ensureFlat(js::ExclusiveContext* cx)
-{
-    return isFlat()
-           ? &asFlat()
-           : isDependent()
-             ? asDependent().undepend(cx)
-             : asRope().flatten(cx);
-}
-
 inline JSLinearString*
 JSString::base() const
 {
     MOZ_ASSERT(hasBase());
     MOZ_ASSERT(!d.s.u3.base->isInline());
     return d.s.u3.base;
 }
 
--- a/js/xpconnect/src/XPCString.cpp
+++ b/js/xpconnect/src/XPCString.cpp
@@ -36,37 +36,50 @@ XPCStringConvert::FreeZoneCache(JS::Zone
     nsAutoPtr<ZoneStringCache> cache(static_cast<ZoneStringCache*>(JS_GetZoneUserData(zone)));
     JS_SetZoneUserData(zone, nullptr);
 }
 
 // static
 void
 XPCStringConvert::ClearZoneCache(JS::Zone* zone)
 {
+    // Although we clear the cache in FinalizeDOMString if needed, we also clear
+    // the cache here to avoid a dangling JSString* pointer when compacting GC
+    // moves the external string in memory.
+
     ZoneStringCache* cache = static_cast<ZoneStringCache*>(JS_GetZoneUserData(zone));
     if (cache) {
         cache->mBuffer = nullptr;
         cache->mString = nullptr;
     }
 }
 
 // static
 void
-XPCStringConvert::FinalizeLiteral(const JSStringFinalizer* fin, char16_t* chars)
+XPCStringConvert::FinalizeLiteral(JS::Zone* zone, const JSStringFinalizer* fin, char16_t* chars)
 {
 }
 
 const JSStringFinalizer XPCStringConvert::sLiteralFinalizer =
     { XPCStringConvert::FinalizeLiteral };
 
 // static
 void
-XPCStringConvert::FinalizeDOMString(const JSStringFinalizer* fin, char16_t* chars)
+XPCStringConvert::FinalizeDOMString(JS::Zone* zone, const JSStringFinalizer* fin, char16_t* chars)
 {
     nsStringBuffer* buf = nsStringBuffer::FromData(chars);
+
+    // Clear the ZoneStringCache if needed, as this can be called outside GC
+    // when flattening an external string.
+    ZoneStringCache* cache = static_cast<ZoneStringCache*>(JS_GetZoneUserData(zone));
+    if (cache && cache->mBuffer == buf) {
+        cache->mBuffer = nullptr;
+        cache->mString = nullptr;
+    }
+
     buf->Release();
 }
 
 const JSStringFinalizer XPCStringConvert::sDOMStringFinalizer =
     { XPCStringConvert::FinalizeDOMString };
 
 // convert a readable to a JSString, copying string data
 // static
--- a/js/xpconnect/src/xpcpublic.h
+++ b/js/xpconnect/src/xpcpublic.h
@@ -273,19 +273,19 @@ public:
     {
         return JS_IsExternalString(str) &&
                JS_GetExternalStringFinalizer(str) == &sDOMStringFinalizer;
     }
 
 private:
     static const JSStringFinalizer sLiteralFinalizer, sDOMStringFinalizer;
 
-    static void FinalizeLiteral(const JSStringFinalizer* fin, char16_t* chars);
+    static void FinalizeLiteral(JS::Zone* zone, const JSStringFinalizer* fin, char16_t* chars);
 
-    static void FinalizeDOMString(const JSStringFinalizer* fin, char16_t* chars);
+    static void FinalizeDOMString(JS::Zone* zone, const JSStringFinalizer* fin, char16_t* chars);
 
     XPCStringConvert() = delete;
 };
 
 class nsIAddonInterposition;
 
 namespace xpc {