Preserve the contents of Dart strings with unmatched surrogate halfs by avoiding a UTF16 -> UTF8

Preserve the contents of Dart strings with unmatched surrogate halfs by avoiding a UTF16 -> UTF8 -> UTF16 conversion.

Clean up some confusion between code points and code units.

BUG=http://dartbug.com/20583
BUG=http://dartbug.com/20874
R=turnidge@google.com

Committed: https://code.google.com/p/dart/source/detail?r=40174

[rmacnak, 2014-09-10 18:37:52]

rmacnak@google.com changed reviewers:
+ johnmccutchan@google.com, turnidge@google.com

[turnidge, 2014-09-10 19:55:28]

https://codereview.chromium.org/558853004/diff/80001/runtime/bin/vmservice/ob...
File runtime/bin/vmservice/observatory/lib/src/elements/observatory_element.dart
(right):

https://codereview.chromium.org/558853004/diff/80001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/elements/observatory_element.dart:128:
else if (codeUnit < 10) result.addAll("\\u000$codeUnit".codeUnits);
hex v. dec

https://codereview.chromium.org/558853004/diff/80001/runtime/platform/json.cc
File runtime/platform/json.cc (right):

https://codereview.chromium.org/558853004/diff/80001/runtime/platform/json.cc...
runtime/platform/json.cc:554: void TextBuffer::EscapeAndAddCodeUnit(uint32_t
codeunit) {
Can you take a look at json_test and see if it needs a bit of beefing up?  I'm
seeing the JSON_JSONStream_EscapedString test in there, but it does seem to
exercise the unicode path.

https://codereview.chromium.org/558853004/diff/80001/runtime/vm/json_stream.h
File runtime/vm/json_stream.h (right):

https://codereview.chromium.org/558853004/diff/80001/runtime/vm/json_stream.h...
runtime/vm/json_stream.h:103: bool PrintPropertyS(const char* name, const
String& s, intptr_t limit);
S -> Str?

https://codereview.chromium.org/558853004/diff/80001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/558853004/diff/80001/runtime/vm/object.cc#new...
runtime/vm/object.cc:8528: jsobj.AddPropertyS("name", name, name.Length());
I'm thinking it would be nicer to have the length parameter provide a default
value of, say, -1.  Then we could modify AddDartString to interpret a -1 length
as no limit.  This would make it simpler to use AddPropertyS.

[rmacnak, 2014-09-10 21:38:49]

https://codereview.chromium.org/558853004/diff/80001/runtime/bin/vmservice/ob...
File runtime/bin/vmservice/observatory/lib/src/elements/observatory_element.dart
(right):

https://codereview.chromium.org/558853004/diff/80001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/elements/observatory_element.dart:128:
else if (codeUnit < 10) result.addAll("\\u000$codeUnit".codeUnits);
On 2014/09/10 19:55:28, turnidge wrote:
> hex v. dec

Fixed

https://codereview.chromium.org/558853004/diff/80001/runtime/platform/json.cc
File runtime/platform/json.cc (right):

https://codereview.chromium.org/558853004/diff/80001/runtime/platform/json.cc...
runtime/platform/json.cc:554: void TextBuffer::EscapeAndAddCodeUnit(uint32_t
codeunit) {
On 2014/09/10 19:55:28, turnidge wrote:
> Can you take a look at json_test and see if it needs a bit of beefing up?  I'm
> seeing the JSON_JSONStream_EscapedString test in there, but it does seem to
> exercise the unicode path.

Added.

https://codereview.chromium.org/558853004/diff/80001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/558853004/diff/80001/runtime/vm/object.cc#new...
runtime/vm/object.cc:8528: jsobj.AddPropertyS("name", name, name.Length());
On 2014/09/10 19:55:28, turnidge wrote:
> I'm thinking it would be nicer to have the length parameter provide a default
> value of, say, -1.  Then we could modify AddDartString to interpret a -1
length
> as no limit.  This would make it simpler to use AddPropertyS.

Done.

[koda, 2014-09-10 21:42:06]

koda@google.com changed reviewers:
+ koda@google.com

[koda, 2014-09-10 21:42:07]

https://codereview.chromium.org/558853004/diff/80001/runtime/platform/json.cc
File runtime/platform/json.cc (right):

https://codereview.chromium.org/558853004/diff/80001/runtime/platform/json.cc...
runtime/platform/json.cc:553: // Write a UTF-16 code unit as
Finish this comment.

[rmacnak, 2014-09-10 21:49:59]

https://codereview.chromium.org/558853004/diff/80001/runtime/platform/json.cc
File runtime/platform/json.cc (right):

https://codereview.chromium.org/558853004/diff/80001/runtime/platform/json.cc...
runtime/platform/json.cc:553: // Write a UTF-16 code unit as
On 2014/09/10 21:42:07, koda wrote:
> Finish this comment.

Done.

[turnidge, 2014-09-11 16:35:12]

lgtm w/ a comment.

Thanks for making this change.

https://codereview.chromium.org/558853004/diff/100001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/558853004/diff/100001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:8528: jsobj.AddPropertyStr("name", name, name.Length());
Omit 3rd parameter.

https://codereview.chromium.org/558853004/diff/100001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:8535: jsobj.AddPropertyStr("source", source,
source.Length());
ditto

https://codereview.chromium.org/558853004/diff/100001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:9816: jsobj.AddPropertyStr("url", library_url,
library_url.Length());
ditto

[rmacnak, 2014-09-11 17:57:54]

https://codereview.chromium.org/558853004/diff/100001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/558853004/diff/100001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:8528: jsobj.AddPropertyStr("name", name, name.Length());
On 2014/09/11 16:35:12, turnidge wrote:
> Omit 3rd parameter.

Removed.

[rmacnak, 2014-09-11 18:42:19]

Committed patchset #9 (id:160001) manually as 40174 (presubmit successful).