Bug 1034191, bug 1032726 and bug 1034627 did most of the work, but there are still a few API calls left.

This patch adds a JS_ParseJSON overload that takes a HandleString instead of jschar* + length. This API could be useful elsewhere, and I want to keep the changes to AndroidBridge as small as possible because I can't compile it locally :)

This patch adds AssignJSFlatString, which is just like AssignJSString but it takes a JSFlatString and it's infallible. It also doesn't need a JSContext so we can use it in some places where we can't easily use AssignJSString.

I think this should be it. If not we'll find out when we remove or change the APIs :)

Comment on attachment 8451151 [details] [diff] [review] Part 1 - AndroidBridge Review of attachment 8451151 [details] [diff] [review]: ----------------------------------------------------------------- Please update the JSAPI reference for the new overloads: https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/JSAPI_reference/JS_ParseJSON Also add a note to https://developer.mozilla.org/en-US/docs/SpiderMonkey/38 about this. ::: js/src/jsapi.cpp @@ +5731,5 @@ > return ParseJSONWithReviver(cx, mozilla::Range<const jschar>(chars, len), reviver, vp); > } > > JS_PUBLIC_API(bool) > +JS_ParseJSON(JSContext *cx, HandleString str, MutableHandleValue vp) If you're going to add this overload, could you add the same one for JS_ParseJSONWithReviver as well?

Comment on attachment 8451154 [details] [diff] [review] Part 2 - AssignJSFlatString >+ MOZ_ASSERT(mExpr.Length() == 0); MOZ_ASSERT(mExpr.IsEmpty()); r=me on the non-js/src bits with that.

Comment on attachment 8451159 [details] [diff] [review] Part 3 - The rest r=me

Comment on attachment 8451154 [details] [diff] [review] Part 2 - AssignJSFlatString Review from Terrence on the js/src changes (bz reviewed the rest).

Comment on attachment 8451154 [details] [diff] [review] Part 2 - AssignJSFlatString Review of attachment 8451154 [details] [diff] [review]: ----------------------------------------------------------------- r=me

Parts 2 and 3: https://hg.mozilla.org/integration/mozilla-inbound/rev/046c577511ef https://hg.mozilla.org/integration/mozilla-inbound/rev/06fc258b16f8

Comment on attachment 8451154 [details] [diff] [review] Part 2 - AssignJSFlatString >+ dest.SetCapacity(len + 1); >+ js::CopyFlatStringChars(dest.BeginWriting(), s, len); >+ dest.BeginWriting()[len] = '\0'; >+ dest.SetLength(len); 1. SetLength calls SetCapacity 2. SetCapacity (and therefore SetLength) writes the null terminator 3. It follows that SetCapacity adds 1 for the null terminator

(In reply to neil@parkwaycc.co.uk from comment #10) > 1. SetLength calls SetCapacity > 2. SetCapacity (and therefore SetLength) writes the null terminator > 3. It follows that SetCapacity adds 1 for the null terminator Thanks. You also mentioned this in the AssignJSString bug and I needinfo'ed myself on it; I'll fix them both soonish.

https://hg.mozilla.org/mozilla-central/rev/046c577511ef https://hg.mozilla.org/mozilla-central/rev/06fc258b16f8 https://hg.mozilla.org/mozilla-central/rev/247a5733d2ba

And part 1: https://hg.mozilla.org/integration/mozilla-inbound/rev/8b9e1310637c (In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #4) > Please update the JSAPI reference for the new overloads: > > https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/ > JSAPI_reference/JS_ParseJSON > > Also add a note to https://developer.mozilla.org/en-US/docs/SpiderMonkey/38 > about this. needinfo'ing myself to do this once the patch is on m-c. > If you're going to add this overload, could you add the same one for > JS_ParseJSONWithReviver as well? Done.