ValueSerializer: Share string encoding code with String and RegExp objects.

This avoids the need to pull in the UTF-8 encoding code from the public API,
and allows it to take advantage of any supported way that i::String can be
encoded (one- or two-byte).

Backward compatibility is maintained, but this is the behavior beginning
with this version.

BUG=chromium:686159

Review-Url: https://codereview.chromium.org/2665653004
Cr-Commit-Position: refs/heads/master@{#42872}

src/value-serializer.cc

@@ -26,7 +26,8 @@ namespace internal {
 // Version 9: (imported from Blink)
 // Version 10: one-byte (Latin-1) strings
 // Version 11: properly separate undefined from the hole in arrays
-static const uint32_t kLatestVersion = 11;
+// Version 12: regexp and string objects share normal string encoding
+static const uint32_t kLatestVersion = 12;
 
 static const int kPretenureThreshold = 100 * KB;
 
@@ -650,18 +651,8 @@ Maybe<bool> ValueSerializer::WriteJSValue(Handle<JSValue> value) {
     WriteTag(SerializationTag::kNumberObject);
     WriteDouble(inner_value->Number());
   } else if (inner_value->IsString()) {
-    // TODO(jbroman): Replace UTF-8 encoding with the same options available for
-    // ordinary strings.
     WriteTag(SerializationTag::kStringObject);
-    v8::Local<v8::String> api_string =
-        Utils::ToLocal(handle(String::cast(inner_value), isolate_));
-    uint32_t utf8_length = api_string->Utf8Length();
-    WriteVarint(utf8_length);
-    uint8_t* dest;
-    if (ReserveRawBytes(utf8_length).To(&dest)) {
-      api_string->WriteUtf8(reinterpret_cast<char*>(dest), utf8_length, nullptr,
-                            v8::String::NO_NULL_TERMINATION);
-    }
+    WriteString(handle(String::cast(inner_value), isolate_));
   } else {
     DCHECK(inner_value->IsSymbol());
     ThrowDataCloneError(MessageTemplate::kDataCloneError, value);
@@ -672,15 +663,7 @@ Maybe<bool> ValueSerializer::WriteJSValue(Handle<JSValue> value) {
 
 void ValueSerializer::WriteJSRegExp(JSRegExp* regexp) {
   WriteTag(SerializationTag::kRegExp);
-  v8::Local<v8::String> api_string =
-      Utils::ToLocal(handle(regexp->Pattern(), isolate_));
-  uint32_t utf8_length = api_string->Utf8Length();
-  WriteVarint(utf8_length);
-  uint8_t* dest;
-  if (ReserveRawBytes(utf8_length).To(&dest)) {
-    api_string->WriteUtf8(reinterpret_cast<char*>(dest), utf8_length, nullptr,
-                          v8::String::NO_NULL_TERMINATION);
-  }
+  WriteString(handle(regexp->Pattern(), isolate_));
   WriteVarint(static_cast<uint32_t>(regexp->GetFlags()));
 }
 
@@ -1161,6 +1144,15 @@ MaybeHandle<Object> ValueDeserializer::ReadObjectInternal() {
   }
 }
 
+MaybeHandle<String> ValueDeserializer::ReadString() {
+  if (version_ < 12) return ReadUtf8String();
+  Handle<Object> object;
+  if (!ReadObject().ToHandle(&object) || !object->IsString()) {
+    return MaybeHandle<String>();
+  }
+  return Handle<String>::cast(object);
+}
+
 MaybeHandle<String> ValueDeserializer::ReadUtf8String() {
   uint32_t utf8_length;
   Vector<const uint8_t> utf8_bytes;
@@ -1399,7 +1391,7 @@ MaybeHandle<JSValue> ValueDeserializer::ReadJSValue(SerializationTag tag) {
     }
     case SerializationTag::kStringObject: {
       Handle<String> string;
-      if (!ReadUtf8String().ToHandle(&string)) return MaybeHandle<JSValue>();
+      if (!ReadString().ToHandle(&string)) return MaybeHandle<JSValue>();
       value = Handle<JSValue>::cast(isolate_->factory()->NewJSObject(
           isolate_->string_function(), pretenure_));
       value->set_value(*string);
@@ -1418,7 +1410,7 @@ MaybeHandle<JSRegExp> ValueDeserializer::ReadJSRegExp() {
   Handle<String> pattern;
   uint32_t raw_flags;
   Handle<JSRegExp> regexp;
-  if (!ReadUtf8String().ToHandle(&pattern) ||
+  if (!ReadString().ToHandle(&pattern) ||
       !ReadVarint<uint32_t>().To(&raw_flags) ||
       !JSRegExp::New(pattern, static_cast<JSRegExp::Flags>(raw_flags))
            .ToHandle(&regexp)) {

src/value-serializer.h

@@ -229,6 +229,11 @@ class ValueDeserializer {
   // "stack machine".
   MaybeHandle<Object> ReadObjectInternal() WARN_UNUSED_RESULT;
 
+  // Reads a string intended to be part of a more complicated object.
+  // Before v12, these are UTF-8 strings. After, they can be any encoding
+  // permissible for a string (with the relevant tag).
+  MaybeHandle<String> ReadString() WARN_UNUSED_RESULT;
+
   // Reading V8 objects of specific kinds.
   // The tag is assumed to have already been read.
   MaybeHandle<String> ReadUtf8String() WARN_UNUSED_RESULT;

test/unittests/value-serializer-unittest.cc

@@ -1501,6 +1501,16 @@ TEST_F(ValueSerializerTest, DecodeValueObjects) {
         EXPECT_TRUE(EvaluateScriptForResultBool("result.a instanceof String"));
         EXPECT_TRUE(EvaluateScriptForResultBool("result.a === result.b"));
       });
+
+  // String object containing a Latin-1 string.
+  DecodeTest({0xff, 0x0c, 0x73, 0x22, 0x06, 'Q', 'u', 0xe9, 'b', 'e', 'c'},
+             [this](Local<Value> value) {
+               EXPECT_TRUE(EvaluateScriptForResultBool(
+                   "Object.getPrototypeOf(result) === String.prototype"));
+               EXPECT_TRUE(EvaluateScriptForResultBool(
+                   "result.valueOf() === 'Qu\\xe9bec'"));
+               EXPECT_TRUE(EvaluateScriptForResultBool("result.length === 6"));
+             });
 }
 
 TEST_F(ValueSerializerTest, RoundTripRegExp) {
@@ -1560,6 +1570,15 @@ TEST_F(ValueSerializerTest, DecodeRegExp) {
         EXPECT_TRUE(EvaluateScriptForResultBool("result.a instanceof RegExp"));
         EXPECT_TRUE(EvaluateScriptForResultBool("result.a === result.b"));
       });
+
+  // RegExp containing a Latin-1 string.
+  DecodeTest(
+      {0xff, 0x0c, 0x52, 0x22, 0x06, 'Q', 'u', 0xe9, 'b', 'e', 'c', 0x02},
+      [this](Local<Value> value) {
+        ASSERT_TRUE(value->IsRegExp());
+        EXPECT_TRUE(EvaluateScriptForResultBool(
+            "result.toString() === '/Qu\\xe9bec/i'"));
+      });
 }
 
 TEST_F(ValueSerializerTest, RoundTripMap) {