Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(354)

Unified Diff: src/value-serializer.cc

Issue 2660093002: ValueSerializer: Distinguish between 'undefined' and an absent property. (Closed)
Patch Set: remove TODO Created 3 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | test/unittests/value-serializer-unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/value-serializer.cc
diff --git a/src/value-serializer.cc b/src/value-serializer.cc
index 9dfaa4a857805abbf42a57856b86248b033a10d9..51fdf2212b4f2a5420fd22b7f98a9a8d4e7401cc 100644
--- a/src/value-serializer.cc
+++ b/src/value-serializer.cc
@@ -25,7 +25,8 @@ namespace internal {
// Version 9: (imported from Blink)
// Version 10: one-byte (Latin-1) strings
-static const uint32_t kLatestVersion = 10;
+// Version 11: properly separate undefined from the hole in arrays
+static const uint32_t kLatestVersion = 11;
static const int kPretenureThreshold = 100 * KB;
@@ -49,6 +50,7 @@ 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',
@@ -538,10 +540,6 @@ 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;
@@ -589,6 +587,13 @@ 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>();
@@ -1320,10 +1325,20 @@ 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>();
- // TODO(jbroman): Distinguish between undefined and a hole.
- if (element->IsUndefined(isolate_)) continue;
+
+ // 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;
+
elements->set(i, *element);
}
« no previous file with comments | « no previous file | test/unittests/value-serializer-unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698