Revert of ValueSerializer: Distinguish between 'undefined' and an absent...

Revert of ValueSerializer: Distinguish between 'undefined' and an absent property. (patchset #2 id:20001 of https://codereview.chromium.org/2660093002/ )

Reason for revert:
Seems to break layout tests:
https://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Linux%2064/builds/13146

https://github.com/v8/v8/wiki/Blink-layout-tests

Original issue's description:
> ValueSerializer: Distinguish between 'undefined' and an absent property.
>
> Dealing with this case requires a wire format change. It is possible that an
> element can be absent even in an array where the dense format was chosen
> (because the array initially had no holes), if the elements are modified while
> they are being serialized. In this case, a new tag for the "hole" is emitted.
>
> The logic to treat undefined in dense arrays as an absent property is restricted
> to versions of the wire format that this tag did not exist.
>
> BUG=chromium:686159,chromium:665820
>
> Review-Url: https://codereview.chromium.org/2660093002
> Cr-Commit-Position: refs/heads/master@{#42784}
> Committed: https://chromium.googlesource.com/v8/v8/+/dc85f4c8338c1c824af4f7ee3274dc9f95d14e49

TBR=jkummerow@chromium.org,jbroman@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:686159,chromium:665820

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

src/value-serializer.cc

@@ -25,8 +25,7 @@ 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;
+static const uint32_t kLatestVersion = 10;
 
 static const int kPretenureThreshold = 100 * KB;
 
@@ -50,7 +49,6 @@ enum class SerializationTag : uint8_t {
   // refTableSize:uint32_t (previously used for sanity checks; safe to ignore)
   kVerifyObjectCount = '?',
   // Oddballs (no data).
-  kTheHole = '-',
   kUndefined = '_',
   kNull = '0',
   kTrue = 'T',
@@ -540,6 +538,10 @@ Maybe<bool> ValueSerializer::WriteJSArray(Handle<JSArray> array) {
 
   if (should_serialize_densely) {
     DCHECK_LE(length, static_cast<uint32_t>(FixedArray::kMaxLength));
+
+    // TODO(jbroman): Distinguish between undefined and a hole (this can happen
+    // if serializing one of the elements deletes another). This requires wire
+    // format changes.
     WriteTag(SerializationTag::kBeginDenseJSArray);
     WriteVarint<uint32_t>(length);
     uint32_t i = 0;
@@ -587,13 +589,6 @@ Maybe<bool> ValueSerializer::WriteJSArray(Handle<JSArray> array) {
       // with.
       Handle<Object> element;
       LookupIterator it(isolate_, array, i, array, LookupIterator::OWN);
-      if (!it.IsFound()) {
-        // This can happen in the case where an array that was originally dense
-        // became sparse during serialization. It's too late to switch to the
-        // sparse format, but we can mark the elements as absent.
-        WriteTag(SerializationTag::kTheHole);
-        continue;
-      }
       if (!Object::GetProperty(&it).ToHandle(&element) ||
           !WriteObject(element).FromMaybe(false)) {
         return Nothing<bool>();
@@ -1325,20 +1320,10 @@ MaybeHandle<JSArray> ValueDeserializer::ReadDenseJSArray() {
 
   Handle<FixedArray> elements(FixedArray::cast(array->elements()), isolate_);
   for (uint32_t i = 0; i < length; i++) {
-    SerializationTag tag;
-    if (PeekTag().To(&tag) && tag == SerializationTag::kTheHole) {
-      ConsumeTag(SerializationTag::kTheHole);
-      continue;
-    }
-
     Handle<Object> element;
     if (!ReadObject().ToHandle(&element)) return MaybeHandle<JSArray>();
-
-    // Serialization versions less than 11 encode the hole the same as
-    // undefined. For consistency with previous behavior, store these as the
-    // hole. Past version 11, undefined means undefined.
-    if (version_ < 11 && element->IsUndefined(isolate_)) continue;
-
+    // TODO(jbroman): Distinguish between undefined and a hole.
+    if (element->IsUndefined(isolate_)) continue;
     elements->set(i, *element);
   }
 

test/unittests/value-serializer-unittest.cc

@@ -1245,36 +1245,6 @@ TEST_F(ValueSerializerTest, DecodeSparseArrayVersion0) {
       });
 }
 
-TEST_F(ValueSerializerTest, RoundTripDenseArrayContainingUndefined) {
-  // In previous serialization versions, this would be interpreted as an absent
-  // property.
-  RoundTripTest("[undefined]", [this](Local<Value> value) {
-    ASSERT_TRUE(value->IsArray());
-    EXPECT_EQ(1u, Array::Cast(*value)->Length());
-    EXPECT_TRUE(EvaluateScriptForResultBool("result.hasOwnProperty(0)"));
-    EXPECT_TRUE(EvaluateScriptForResultBool("result[0] === undefined"));
-  });
-}
-
-TEST_F(ValueSerializerTest, DecodeDenseArrayContainingUndefined) {
-  // In previous versions, "undefined" in a dense array signified absence of the
-  // element (for compatibility). In new versions, it has a separate encoding.
-  DecodeTest({0xff, 0x09, 0x41, 0x01, 0x5f, 0x24, 0x00, 0x01},
-             [this](Local<Value> value) {
-               EXPECT_TRUE(EvaluateScriptForResultBool("!(0 in result)"));
-             });
-  DecodeTest(
-      {0xff, 0x0b, 0x41, 0x01, 0x5f, 0x24, 0x00, 0x01},
-      [this](Local<Value> value) {
-        EXPECT_TRUE(EvaluateScriptForResultBool("0 in result"));
-        EXPECT_TRUE(EvaluateScriptForResultBool("result[0] === undefined"));
-      });
-  DecodeTest({0xff, 0x0b, 0x41, 0x01, 0x2d, 0x24, 0x00, 0x01},
-             [this](Local<Value> value) {
-               EXPECT_TRUE(EvaluateScriptForResultBool("!(0 in result)"));
-             });
-}
-
 TEST_F(ValueSerializerTest, RoundTripDate) {
   RoundTripTest("new Date(1e6)", [](Local<Value> value) {
     ASSERT_TRUE(value->IsDate());