Blink-compatible serialization of dictionary-like objects.

As part of this CL, object reference tracking is implemented (and tested with a
self-referential object). This sort of reference tracking will be shared with
other receivers (array, date, regexp and host objects).

Not included in this CL is compatibility with version-0 objects (which don't
support a non-tree object graph, and require a little stack to correctly
deserialize).

BUG=chromium:148757

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

src/value-serializer.cc

@@ -54,9 +54,18 @@ enum class SerializationTag : uint8_t {
   // byteLength:uint32_t, then raw data
   kUtf8String = 'S',
   kTwoByteString = 'c',
+  // Reference to a serialized object. objectID:uint32_t
+  kObjectReference = '^',
+  // Beginning of a JS object.
+  kBeginJSObject = 'o',
+  // End of a JS object. numProperties:uint32_t
+  kEndJSObject = '{',
 };
 
-ValueSerializer::ValueSerializer() {}
+ValueSerializer::ValueSerializer(Isolate* isolate)
+    : isolate_(isolate),
+      zone_(isolate->allocator()),
+      id_map_(isolate->heap(), &zone_) {}
 
 ValueSerializer::~ValueSerializer() {}
 
@@ -144,6 +153,8 @@ Maybe<bool> ValueSerializer::WriteObject(Handle<Object> object) {
       if (object->IsString()) {
         WriteString(Handle<String>::cast(object));
         return Just(true);
+      } else if (object->IsJSReceiver()) {
+        return WriteJSReceiver(Handle<JSReceiver>::cast(object));
       }
       UNIMPLEMENTED();
       return Nothing<bool>();
@@ -218,13 +229,95 @@ void ValueSerializer::WriteString(Handle<String> string) {
   }
 }
 
+Maybe<bool> ValueSerializer::WriteJSReceiver(Handle<JSReceiver> receiver) {
+  // If the object has already been serialized, just write its ID.
+  uint32_t* id_map_entry = id_map_.Get(receiver);
+  if (uint32_t id = *id_map_entry) {
+    WriteTag(SerializationTag::kObjectReference);
+    WriteVarint(id - 1);
+    return Just(true);
+  }
+
+  // Otherwise, allocate an ID for it.
+  uint32_t id = next_id_++;
+  *id_map_entry = id + 1;
+
+  // Eliminate callable and exotic objects, which should not be serialized.
+  InstanceType instance_type = receiver->map()->instance_type();
+  if (receiver->IsCallable() || instance_type <= LAST_SPECIAL_RECEIVER_TYPE) {
+    return Nothing<bool>();
+  }
+
+  // If we are at the end of the stack, abort. This function may recurse.
+  if (StackLimitCheck(isolate_).HasOverflowed()) return Nothing<bool>();
+
+  HandleScope scope(isolate_);
+  switch (instance_type) {
+    case JS_OBJECT_TYPE:
+    case JS_API_OBJECT_TYPE:
+      return WriteJSObject(Handle<JSObject>::cast(receiver));
+    default:
+      UNIMPLEMENTED();
+      break;
+  }
+  return Nothing<bool>();
+}
+
+Maybe<bool> ValueSerializer::WriteJSObject(Handle<JSObject> object) {
+  WriteTag(SerializationTag::kBeginJSObject);
+  Handle<FixedArray> keys;
+  uint32_t properties_written;
+  if (!KeyAccumulator::GetKeys(object, KeyCollectionMode::kOwnOnly,
+                               ENUMERABLE_STRINGS)
+           .ToHandle(&keys) ||
+      !WriteJSObjectProperties(object, keys).To(&properties_written)) {
+    return Nothing<bool>();
+  }
+  WriteTag(SerializationTag::kEndJSObject);
+  WriteVarint<uint32_t>(properties_written);
+  return Just(true);
+}
+
+Maybe<uint32_t> ValueSerializer::WriteJSObjectProperties(
+    Handle<JSObject> object, Handle<FixedArray> keys) {
+  uint32_t properties_written = 0;
+  int length = keys->length();
+  for (int i = 0; i < length; i++) {
+    Handle<Object> key(keys->get(i), isolate_);
+
+    bool success;
+    LookupIterator it = LookupIterator::PropertyOrElement(
+        isolate_, object, key, &success, LookupIterator::OWN);
+    DCHECK(success);
+    Handle<Object> value;
+    if (!Object::GetProperty(&it).ToHandle(&value)) return Nothing<uint32_t>();
+
+    // If the property is no longer found, do not serialize it.
+    // This could happen if a getter deleted the property.
+    if (!it.IsFound()) continue;
+
+    if (!WriteObject(key).FromMaybe(false) ||
+        !WriteObject(value).FromMaybe(false)) {
+      return Nothing<uint32_t>();
+    }
+
+    properties_written++;
+  }
+  return Just(properties_written);
+}
+
 ValueDeserializer::ValueDeserializer(Isolate* isolate,
                                      Vector<const uint8_t> data)
     : isolate_(isolate),
       position_(data.start()),
-      end_(data.start() + data.length()) {}
+      end_(data.start() + data.length()),
+      id_map_(Handle<SeededNumberDictionary>::cast(
+          isolate->global_handles()->Create(
+              *SeededNumberDictionary::New(isolate, 0)))) {}
 
-ValueDeserializer::~ValueDeserializer() {}
+ValueDeserializer::~ValueDeserializer() {
+  GlobalHandles::Destroy(Handle<Object>::cast(id_map_).location());
+}
 
 Maybe<bool> ValueDeserializer::ReadHeader() {
   if (position_ < end_ &&
@@ -236,6 +329,17 @@ Maybe<bool> ValueDeserializer::ReadHeader() {
   return Just(true);
 }
 
+Maybe<SerializationTag> ValueDeserializer::PeekTag() const {
+  const uint8_t* peek_position = position_;
+  SerializationTag tag;
+  do {
+    if (peek_position >= end_) return Nothing<SerializationTag>();
+    tag = static_cast<SerializationTag>(*peek_position);
+    peek_position++;
+  } while (tag == SerializationTag::kPadding);
+  return Just(tag);
+}
+
 Maybe<SerializationTag> ValueDeserializer::ReadTag() {
   SerializationTag tag;
   do {
@@ -337,6 +441,13 @@ MaybeHandle<Object> ValueDeserializer::ReadObject() {
       return ReadUtf8String();
     case SerializationTag::kTwoByteString:
       return ReadTwoByteString();
+    case SerializationTag::kObjectReference: {
+      uint32_t id;
+      if (!ReadVarint<uint32_t>().To(&id)) return MaybeHandle<Object>();
+      return GetObjectWithID(id);
+    }
+    case SerializationTag::kBeginJSObject:
+      return ReadJSObject();
     default:
       return MaybeHandle<Object>();
   }
@@ -377,5 +488,86 @@ MaybeHandle<String> ValueDeserializer::ReadTwoByteString() {
   return string;
 }
 
+MaybeHandle<JSObject> ValueDeserializer::ReadJSObject() {
+  // If we are at the end of the stack, abort. This function may recurse.
+  if (StackLimitCheck(isolate_).HasOverflowed()) return MaybeHandle<JSObject>();
+
+  uint32_t id = next_id_++;
+  HandleScope scope(isolate_);
+  Handle<JSObject> object =
+      isolate_->factory()->NewJSObject(isolate_->object_function());
+  AddObjectWithID(id, object);
+
+  uint32_t num_properties;
+  uint32_t expected_num_properties;
+  if (!ReadJSObjectProperties(object, SerializationTag::kEndJSObject)
+           .To(&num_properties) ||
+      !ReadVarint<uint32_t>().To(&expected_num_properties) ||
+      num_properties != expected_num_properties) {
+    return MaybeHandle<JSObject>();
+  }
+
+  DCHECK(HasObjectWithID(id));
+  return scope.CloseAndEscape(object);
+}
+
+Maybe<uint32_t> ValueDeserializer::ReadJSObjectProperties(
+    Handle<JSObject> object, SerializationTag end_tag) {
+  for (uint32_t num_properties = 0;; num_properties++) {
+    SerializationTag tag;
+    if (!PeekTag().To(&tag)) return Nothing<uint32_t>();
+    if (tag == end_tag) {
+      SerializationTag consumed_tag = ReadTag().ToChecked();
+      USE(consumed_tag);
+      DCHECK(tag == consumed_tag);
+      return Just(num_properties);
+    }
+
+    Handle<Object> key;
+    if (!ReadObject().ToHandle(&key)) return Nothing<uint32_t>();
+    Handle<Object> value;
+    if (!ReadObject().ToHandle(&value)) return Nothing<uint32_t>();
+
+    bool success;
+    LookupIterator it = LookupIterator::PropertyOrElement(
+        isolate_, object, key, &success, LookupIterator::OWN);
+    if (!success ||
+        JSObject::DefineOwnPropertyIgnoreAttributes(&it, value, NONE)
+            .is_null()) {
+      return Nothing<uint32_t>();
+    }
+  }
+}
+
+bool ValueDeserializer::HasObjectWithID(uint32_t id) {
+  return id_map_->Has(isolate_, id);
+}
+
+MaybeHandle<JSReceiver> ValueDeserializer::GetObjectWithID(uint32_t id) {
+  int index = id_map_->FindEntry(isolate_, id);
+  if (index == SeededNumberDictionary::kNotFound) {
+    return MaybeHandle<JSReceiver>();
+  }
+  Object* value = id_map_->ValueAt(index);
+  DCHECK(value->IsJSReceiver());
+  return Handle<JSReceiver>(JSReceiver::cast(value), isolate_);
+}
+
+void ValueDeserializer::AddObjectWithID(uint32_t id,
+                                        Handle<JSReceiver> object) {
+  DCHECK(!HasObjectWithID(id));
+  const bool used_as_prototype = false;
+  Handle<SeededNumberDictionary> new_dictionary =
+      SeededNumberDictionary::AtNumberPut(id_map_, id, object,
+                                          used_as_prototype);
+
+  // If the dictionary was reallocated, update the global handle.
+  if (!new_dictionary.is_identical_to(id_map_)) {
+    GlobalHandles::Destroy(Handle<Object>::cast(id_map_).location());
+    id_map_ = Handle<SeededNumberDictionary>::cast(
+        isolate_->global_handles()->Create(*new_dictionary));
+  }
+}
+
 }  // namespace internal
 }  // namespace v8

src/value-serializer.h

@@ -11,7 +11,9 @@
 #include "include/v8.h"
 #include "src/base/compiler-specific.h"
 #include "src/base/macros.h"
+#include "src/identity-map.h"
 #include "src/vector.h"
+#include "src/zone.h"
 
 namespace v8 {
 namespace internal {
@@ -32,7 +34,7 @@ enum class SerializationTag : uint8_t;
  */
 class ValueSerializer {
  public:
-  ValueSerializer();
+  explicit ValueSerializer(Isolate* isolate);
   ~ValueSerializer();
 
   /*
@@ -68,8 +70,26 @@ class ValueSerializer {
   void WriteSmi(Smi* smi);
   void WriteHeapNumber(HeapNumber* number);
   void WriteString(Handle<String> string);
+  Maybe<bool> WriteJSReceiver(Handle<JSReceiver> receiver) WARN_UNUSED_RESULT;
+  Maybe<bool> WriteJSObject(Handle<JSObject> object) WARN_UNUSED_RESULT;
 
+  /*
+   * Reads the specified keys from the object and writes key-value pairs to the
+   * buffer. Returns the number of keys actually written, which may be smaller
+   * if some keys are not own properties when accessed.
+   */
+  Maybe<uint32_t> WriteJSObjectProperties(
+      Handle<JSObject> object, Handle<FixedArray> keys) WARN_UNUSED_RESULT;
+
+  Isolate* const isolate_;
   std::vector<uint8_t> buffer_;
+  Zone zone_;
+
+  // To avoid extra lookups in the identity map, ID+1 is actually stored in the
+  // map (checking if the used identity is zero is the fast way of checking if
+  // the entry is new).
+  IdentityMap<uint32_t> id_map_;
+  uint32_t next_id_ = 0;
 
   DISALLOW_COPY_AND_ASSIGN(ValueSerializer);
 };
@@ -95,6 +115,7 @@ class ValueDeserializer {
 
  private:
   // Reading the wire format.
+  Maybe<SerializationTag> PeekTag() const WARN_UNUSED_RESULT;
   Maybe<SerializationTag> ReadTag() WARN_UNUSED_RESULT;
   template <typename T>
   Maybe<T> ReadVarint() WARN_UNUSED_RESULT;
@@ -107,11 +128,26 @@ class ValueDeserializer {
   // The tag is assumed to have already been read.
   MaybeHandle<String> ReadUtf8String() WARN_UNUSED_RESULT;
   MaybeHandle<String> ReadTwoByteString() WARN_UNUSED_RESULT;
+  MaybeHandle<JSObject> ReadJSObject() WARN_UNUSED_RESULT;
+
+  /*
+   * Reads key-value pairs into the object until the specified end tag is
+   * encountered. If successful, returns the number of properties read.
+   */
+  Maybe<uint32_t> ReadJSObjectProperties(Handle<JSObject> object,
+                                         SerializationTag end_tag);
+
+  // Manipulating the map from IDs to reified objects.
+  bool HasObjectWithID(uint32_t id);
+  MaybeHandle<JSReceiver> GetObjectWithID(uint32_t id);
+  void AddObjectWithID(uint32_t id, Handle<JSReceiver> object);
 
   Isolate* const isolate_;
   const uint8_t* position_;
   const uint8_t* const end_;
   uint32_t version_ = 0;
+  Handle<SeededNumberDictionary> id_map_;  // Always a global handle.
+  uint32_t next_id_ = 0;
 
   DISALLOW_COPY_AND_ASSIGN(ValueDeserializer);
 };

test/unittests/value-serializer-unittest.cc

@@ -38,21 +38,49 @@ class ValueSerializerTest : public TestWithIsolate {
                });
   }
 
+  // Variant for the common case where a script is used to build the original
+  // value.
+  template <typename OutputFunctor>
+  void RoundTripTest(const char* source, const OutputFunctor& output_functor) {
+    RoundTripTest([this, source]() { return EvaluateScriptForInput(source); },
+                  output_functor);
+  }
+
+  Maybe<std::vector<uint8_t>> DoEncode(Local<Value> value) {
+    // This approximates what the API implementation would do.
+    // TODO(jbroman): Use the public API once it exists.
+    i::Isolate* internal_isolate = reinterpret_cast<i::Isolate*>(isolate());
+    i::HandleScope handle_scope(internal_isolate);
+    i::ValueSerializer serializer(internal_isolate);
+    serializer.WriteHeader();
+    if (serializer.WriteObject(Utils::OpenHandle(*value)).FromMaybe(false)) {
+      return Just(serializer.ReleaseBuffer());
+    }
+    if (internal_isolate->has_pending_exception()) {
+      internal_isolate->OptionalRescheduleException(true);
+    }
+    return Nothing<std::vector<uint8_t>>();
+  }
+
   template <typename InputFunctor, typename EncodedDataFunctor>
   void EncodeTest(const InputFunctor& input_functor,
                   const EncodedDataFunctor& encoded_data_functor) {
     Context::Scope scope(serialization_context());
     TryCatch try_catch(isolate());
-    // TODO(jbroman): Use the public API once it exists.
     Local<Value> input_value = input_functor();
-    i::Isolate* internal_isolate = reinterpret_cast<i::Isolate*>(isolate());
-    i::HandleScope handle_scope(internal_isolate);
-    i::ValueSerializer serializer;
-    serializer.WriteHeader();
-    ASSERT_TRUE(serializer.WriteObject(Utils::OpenHandle(*input_value))
-                    .FromMaybe(false));
+    std::vector<uint8_t> buffer;
+    ASSERT_TRUE(DoEncode(input_value).To(&buffer));
     ASSERT_FALSE(try_catch.HasCaught());
-    encoded_data_functor(serializer.ReleaseBuffer());
+    encoded_data_functor(buffer);
+  }
+
+  template <typename MessageFunctor>
+  void InvalidEncodeTest(const char* source, const MessageFunctor& functor) {
+    Context::Scope scope(serialization_context());
+    TryCatch try_catch(isolate());
+    Local<Value> input_value = EvaluateScriptForInput(source);
+    ASSERT_TRUE(DoEncode(input_value).IsNothing());
+    functor(try_catch.Message());
   }
 
   template <typename OutputFunctor>
@@ -388,5 +416,216 @@ TEST_F(ValueSerializerTest, EncodeTwoByteStringUsesPadding) {
       });
 }
 
+TEST_F(ValueSerializerTest, RoundTripDictionaryObject) {
+  // Empty object.
+  RoundTripTest("({})", [this](Local<Value> value) {
+    ASSERT_TRUE(value->IsObject());
+    EXPECT_TRUE(EvaluateScriptForResultBool(
+        "Object.getPrototypeOf(result) === Object.prototype"));
+    EXPECT_TRUE(EvaluateScriptForResultBool(
+        "Object.getOwnPropertyNames(result).length === 0"));
+  });
+  // String key.
+  RoundTripTest("({ a: 42 })", [this](Local<Value> value) {
+    ASSERT_TRUE(value->IsObject());
+    EXPECT_TRUE(EvaluateScriptForResultBool("result.hasOwnProperty('a')"));
+    EXPECT_TRUE(EvaluateScriptForResultBool("result.a === 42"));
+    EXPECT_TRUE(EvaluateScriptForResultBool(
+        "Object.getOwnPropertyNames(result).length === 1"));
+  });
+  // Integer key (treated as a string, but may be encoded differently).
+  RoundTripTest("({ 42: 'a' })", [this](Local<Value> value) {
+    ASSERT_TRUE(value->IsObject());
+    EXPECT_TRUE(EvaluateScriptForResultBool("result.hasOwnProperty('42')"));
+    EXPECT_TRUE(EvaluateScriptForResultBool("result[42] === 'a'"));
+    EXPECT_TRUE(EvaluateScriptForResultBool(
+        "Object.getOwnPropertyNames(result).length === 1"));
+  });
+  // Key order must be preserved.
+  RoundTripTest("({ x: 1, y: 2, a: 3 })", [this](Local<Value> value) {
+    EXPECT_TRUE(EvaluateScriptForResultBool(
+        "Object.getOwnPropertyNames(result).toString() === 'x,y,a'"));
+  });
+  // A harder case of enumeration order.
+  // Indexes first, in order (but not 2^32 - 1, which is not an index), then the
+  // remaining (string) keys, in the order they were defined.
+  RoundTripTest(
+      "({ a: 2, 0xFFFFFFFF: 1, 0xFFFFFFFE: 3, 1: 0 })",
+      [this](Local<Value> value) {
+        EXPECT_TRUE(EvaluateScriptForResultBool(
+            "Object.getOwnPropertyNames(result).toString() === "
+            "'1,4294967294,a,4294967295'"));
+        EXPECT_TRUE(EvaluateScriptForResultBool("result.a === 2"));
+        EXPECT_TRUE(EvaluateScriptForResultBool("result[0xFFFFFFFF] === 1"));
+        EXPECT_TRUE(EvaluateScriptForResultBool("result[0xFFFFFFFE] === 3"));
+        EXPECT_TRUE(EvaluateScriptForResultBool("result[1] === 0"));
+      });
+  // This detects a fairly subtle case: the object itself must be in the map
+  // before its properties are deserialized, so that references to it can be
+  // resolved.
+  RoundTripTest(
+      "(() => { var y = {}; y.self = y; return y; })()",
+      [this](Local<Value> value) {
+        ASSERT_TRUE(value->IsObject());
+        EXPECT_TRUE(EvaluateScriptForResultBool("result === result.self"));
+      });
+}
+
+TEST_F(ValueSerializerTest, DecodeDictionaryObject) {
+  // Empty object.
+  DecodeTest({0xff, 0x09, 0x3f, 0x00, 0x6f, 0x7b, 0x00, 0x00},
+             [this](Local<Value> value) {
+               ASSERT_TRUE(value->IsObject());
+               EXPECT_TRUE(EvaluateScriptForResultBool(
+                   "Object.getPrototypeOf(result) === Object.prototype"));
+               EXPECT_TRUE(EvaluateScriptForResultBool(
+                   "Object.getOwnPropertyNames(result).length === 0"));
+             });
+  // String key.
+  DecodeTest(
+      {0xff, 0x09, 0x3f, 0x00, 0x6f, 0x3f, 0x01, 0x53, 0x01, 0x61, 0x3f, 0x01,
+       0x49, 0x54, 0x7b, 0x01},
+      [this](Local<Value> value) {
+        ASSERT_TRUE(value->IsObject());
+        EXPECT_TRUE(EvaluateScriptForResultBool("result.hasOwnProperty('a')"));
+        EXPECT_TRUE(EvaluateScriptForResultBool("result.a === 42"));
+        EXPECT_TRUE(EvaluateScriptForResultBool(
+            "Object.getOwnPropertyNames(result).length === 1"));
+      });
+  // Integer key (treated as a string, but may be encoded differently).
+  DecodeTest(
+      {0xff, 0x09, 0x3f, 0x00, 0x6f, 0x3f, 0x01, 0x49, 0x54, 0x3f, 0x01, 0x53,
+       0x01, 0x61, 0x7b, 0x01},
+      [this](Local<Value> value) {
+        ASSERT_TRUE(value->IsObject());
+        EXPECT_TRUE(EvaluateScriptForResultBool("result.hasOwnProperty('42')"));
+        EXPECT_TRUE(EvaluateScriptForResultBool("result[42] === 'a'"));
+        EXPECT_TRUE(EvaluateScriptForResultBool(
+            "Object.getOwnPropertyNames(result).length === 1"));
+      });
+  // Key order must be preserved.
+  DecodeTest(
+      {0xff, 0x09, 0x3f, 0x00, 0x6f, 0x3f, 0x01, 0x53, 0x01, 0x78, 0x3f, 0x01,
+       0x49, 0x02, 0x3f, 0x01, 0x53, 0x01, 0x79, 0x3f, 0x01, 0x49, 0x04, 0x3f,
+       0x01, 0x53, 0x01, 0x61, 0x3f, 0x01, 0x49, 0x06, 0x7b, 0x03},
+      [this](Local<Value> value) {
+        EXPECT_TRUE(EvaluateScriptForResultBool(
+            "Object.getOwnPropertyNames(result).toString() === 'x,y,a'"));
+      });
+  // A harder case of enumeration order.
+  DecodeTest(
+      {0xff, 0x09, 0x3f, 0x00, 0x6f, 0x3f, 0x01, 0x49, 0x02, 0x3f, 0x01,
+       0x49, 0x00, 0x3f, 0x01, 0x55, 0xfe, 0xff, 0xff, 0xff, 0x0f, 0x3f,
+       0x01, 0x49, 0x06, 0x3f, 0x01, 0x53, 0x01, 0x61, 0x3f, 0x01, 0x49,
+       0x04, 0x3f, 0x01, 0x53, 0x0a, 0x34, 0x32, 0x39, 0x34, 0x39, 0x36,
+       0x37, 0x32, 0x39, 0x35, 0x3f, 0x01, 0x49, 0x02, 0x7b, 0x04},
+      [this](Local<Value> value) {
+        EXPECT_TRUE(EvaluateScriptForResultBool(
+            "Object.getOwnPropertyNames(result).toString() === "
+            "'1,4294967294,a,4294967295'"));
+        EXPECT_TRUE(EvaluateScriptForResultBool("result.a === 2"));
+        EXPECT_TRUE(EvaluateScriptForResultBool("result[0xFFFFFFFF] === 1"));
+        EXPECT_TRUE(EvaluateScriptForResultBool("result[0xFFFFFFFE] === 3"));
+        EXPECT_TRUE(EvaluateScriptForResultBool("result[1] === 0"));
+      });
+  // This detects a fairly subtle case: the object itself must be in the map
+  // before its properties are deserialized, so that references to it can be
+  // resolved.
+  DecodeTest(
+      {0xff, 0x09, 0x3f, 0x00, 0x6f, 0x3f, 0x01, 0x53, 0x04, 0x73,
+       0x65, 0x6c, 0x66, 0x3f, 0x01, 0x5e, 0x00, 0x7b, 0x01, 0x00},
+      [this](Local<Value> value) {
+        ASSERT_TRUE(value->IsObject());
+        EXPECT_TRUE(EvaluateScriptForResultBool("result === result.self"));
+      });
+}
+
+TEST_F(ValueSerializerTest, RoundTripOnlyOwnEnumerableStringKeys) {
+  // Only "own" properties should be serialized, not ones on the prototype.
+  RoundTripTest("(() => { var x = {}; x.__proto__ = {a: 4}; return x; })()",
+                [this](Local<Value> value) {
+                  EXPECT_TRUE(EvaluateScriptForResultBool("!('a' in result)"));
+                });
+  // Only enumerable properties should be serialized.
+  RoundTripTest(
+      "(() => {"
+      "  var x = {};"
+      "  Object.defineProperty(x, 'a', {value: 1, enumerable: false});"
+      "  return x;"
+      "})()",
+      [this](Local<Value> value) {
+        EXPECT_TRUE(EvaluateScriptForResultBool("!('a' in result)"));
+      });
+  // Symbol keys should not be serialized.
+  RoundTripTest("({ [Symbol()]: 4 })", [this](Local<Value> value) {
+    EXPECT_TRUE(EvaluateScriptForResultBool(
+        "Object.getOwnPropertySymbols(result).length === 0"));
+  });
+}
+
+TEST_F(ValueSerializerTest, RoundTripTrickyGetters) {
+  // Keys are enumerated before any setters are called, but if there is no own
+  // property when the value is to be read, then it should not be serialized.
+  RoundTripTest("({ get a() { delete this.b; return 1; }, b: 2 })",
+                [this](Local<Value> value) {
+                  EXPECT_TRUE(EvaluateScriptForResultBool("!('b' in result)"));
+                });
+  // Keys added after the property enumeration should not be serialized.
+  RoundTripTest("({ get a() { this.b = 3; }})", [this](Local<Value> value) {
+    EXPECT_TRUE(EvaluateScriptForResultBool("!('b' in result)"));
+  });
+  // But if you remove a key and add it back, that's fine. But it will appear in
+  // the original place in enumeration order.
+  RoundTripTest(
+      "({ get a() { delete this.b; this.b = 4; }, b: 2, c: 3 })",
+      [this](Local<Value> value) {
+        EXPECT_TRUE(EvaluateScriptForResultBool(
+            "Object.getOwnPropertyNames(result).toString() === 'a,b,c'"));
+        EXPECT_TRUE(EvaluateScriptForResultBool("result.b === 4"));
+      });
+  // Similarly, it only matters if a property was enumerable when the
+  // enumeration happened.
+  RoundTripTest(
+      "({ get a() {"
+      "    Object.defineProperty(this, 'b', {value: 2, enumerable: false});"
+      "}, b: 1})",
+      [this](Local<Value> value) {
+        EXPECT_TRUE(EvaluateScriptForResultBool("result.b === 2"));
+      });
+  RoundTripTest(
+      "(() => {"
+      "  var x = {"
+      "    get a() {"
+      "      Object.defineProperty(this, 'b', {value: 2, enumerable: true});"
+      "    }"
+      "  };"
+      "  Object.defineProperty(x, 'b',"
+      "      {value: 1, enumerable: false, configurable: true});"
+      "  return x;"
+      "})()",
+      [this](Local<Value> value) {
+        EXPECT_TRUE(EvaluateScriptForResultBool("!('b' in result)"));
+      });
+  // The property also should not be read if it can only be found on the
+  // prototype chain (but not as an own property) after enumeration.
+  RoundTripTest(
+      "(() => {"
+      "  var x = { get a() { delete this.b; }, b: 1 };"
+      "  x.__proto__ = { b: 0 };"
+      "  return x;"
+      "})()",
+      [this](Local<Value> value) {
+        EXPECT_TRUE(EvaluateScriptForResultBool("!('b' in result)"));
+      });
+  // If an exception is thrown by script, encoding must fail and the exception
+  // must be thrown.
+  InvalidEncodeTest("({ get a() { throw new Error('sentinel'); } })",
+                    [](Local<Message> message) {
+                      ASSERT_FALSE(message.IsEmpty());
+                      EXPECT_NE(std::string::npos,
+                                Utf8Value(message->Get()).find("sentinel"));
+                    });
+}
+
 }  // namespace
 }  // namespace v8