Revision: 13565
Author: [email protected]
Date: Wed Jan 30 13:07:28 2013
Log: Object.observe: don't unnecessarily emit oldValue for
reconfigurations of data properties
When a data property has its attributes changed but its value remains the
same,
don't emit an oldValue. This makes the API more consistent by only emitting
oldValue when the value of a property has actually changed (or been removed,
in the case of a reconfiguration as an accessor property or a deletion).
Review URL: https://codereview.chromium.org/11820004
http://code.google.com/p/v8/source/detail?r=13565
Modified:
/branches/bleeding_edge/src/objects.cc
/branches/bleeding_edge/test/mjsunit/harmony/object-observe.js
=======================================
--- /branches/bleeding_edge/src/objects.cc Wed Jan 30 07:07:58 2013
+++ /branches/bleeding_edge/src/objects.cc Wed Jan 30 13:07:28 2013
@@ -3177,14 +3177,17 @@
if (is_observed) {
if (lookup.IsTransition()) {
EnqueueChangeRecord(self, "new", name, old_value);
+ } else if (old_value->IsTheHole()) {
+ EnqueueChangeRecord(self, "reconfigured", name, old_value);
} else {
LookupResult new_lookup(isolate);
self->LocalLookup(*name, &new_lookup, true);
- if (old_value->IsTheHole() ||
- new_lookup.GetAttributes() != old_attributes) {
+ bool value_changed = new_lookup.IsDataProperty() &&
+ !old_value->SameValue(*Object::GetProperty(self, name));
+ if (new_lookup.GetAttributes() != old_attributes) {
+ if (!value_changed) old_value =
isolate->factory()->the_hole_value();
EnqueueChangeRecord(self, "reconfigured", name, old_value);
- } else if (new_lookup.IsDataProperty() &&
- !Object::GetProperty(self, name)->SameValue(*old_value)) {
+ } else if (value_changed) {
EnqueueChangeRecord(self, "updated", name, old_value);
}
}
@@ -10337,10 +10340,17 @@
EnqueueChangeRecord(
self, "updated", isolate->factory()->length_symbol(),
old_length);
}
- } else if (old_attributes != new_attributes || old_value->IsTheHole()) {
+ } else if (old_value->IsTheHole()) {
EnqueueChangeRecord(self, "reconfigured", name, old_value);
- } else if (!old_value->SameValue(*Object::GetElement(self, index))) {
- EnqueueChangeRecord(self, "updated", name, old_value);
+ } else {
+ bool value_changed =
+ !old_value->SameValue(*Object::GetElement(self, index));
+ if (old_attributes != new_attributes) {
+ if (!value_changed) old_value = isolate->factory()->the_hole_value();
+ EnqueueChangeRecord(self, "reconfigured", name, old_value);
+ } else if (value_changed) {
+ EnqueueChangeRecord(self, "updated", name, old_value);
+ }
}
return *hresult;
=======================================
--- /branches/bleeding_edge/test/mjsunit/harmony/object-observe.js Thu Dec
20 07:03:30 2012
+++ /branches/bleeding_edge/test/mjsunit/harmony/object-observe.js Wed Jan
30 13:07:28 2013
@@ -371,7 +371,7 @@
{ object: obj, name: "a", type: "new" },
{ object: obj, name: "a", type: "updated", oldValue: 4 },
{ object: obj, name: "a", type: "updated", oldValue: 5 },
- { object: obj, name: "a", type: "reconfigured", oldValue: 6 },
+ { object: obj, name: "a", type: "reconfigured" },
{ object: obj, name: "a", type: "updated", oldValue: 6 },
{ object: obj, name: "a", type: "reconfigured", oldValue: 8 },
{ object: obj, name: "a", type: "reconfigured", oldValue: 7 },
@@ -429,7 +429,7 @@
{ object: obj, name: "1", type: "new" },
{ object: obj, name: "1", type: "updated", oldValue: 4 },
{ object: obj, name: "1", type: "updated", oldValue: 5 },
- { object: obj, name: "1", type: "reconfigured", oldValue: 6 },
+ { object: obj, name: "1", type: "reconfigured" },
{ object: obj, name: "1", type: "updated", oldValue: 6 },
{ object: obj, name: "1", type: "reconfigured", oldValue: 8 },
{ object: obj, name: "1", type: "reconfigured", oldValue: 7 },
@@ -493,7 +493,7 @@
{ object: obj, name: prop, type: "new" },
{ object: obj, name: prop, type: "updated", oldValue: 4 },
{ object: obj, name: prop, type: "updated", oldValue: 5 },
- { object: obj, name: prop, type: "reconfigured", oldValue: 6 },
+ { object: obj, name: prop, type: "reconfigured" },
{ object: obj, name: prop, type: "updated", oldValue: 6 },
{ object: obj, name: prop, type: "reconfigured", oldValue: 8 },
{ object: obj, name: prop, type: "reconfigured", oldValue: 7 },
@@ -537,7 +537,7 @@
{ object: obj, name: prop, type: "updated", oldValue: 4 },
{ object: obj, name: prop, type: "updated", oldValue: 5 },
{ object: obj, name: prop, type: "updated", oldValue: 6 },
- { object: obj, name: prop, type: "reconfigured", oldValue: 7 },
+ { object: obj, name: prop, type: "reconfigured" },
]);
Object.unobserve(obj, observer.callback);
}
@@ -657,7 +657,7 @@
{ object: arr, name: '1', type: 'deleted', oldValue: 'b' },
{ object: arr, name: 'length', type: 'updated', oldValue: 2 },
{ object: arr, name: 'length', type: 'updated', oldValue: 1 },
- { object: arr, name: 'length', type: 'reconfigured', oldValue: 10 },
+ { object: arr, name: 'length', type: 'reconfigured' },
{ object: arr2, name: '1', type: 'deleted', oldValue: 'beta' },
{ object: arr2, name: 'length', type: 'updated', oldValue: 2 },
{ object: arr2, name: 'length', type: 'reconfigured', oldValue: 1 },
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
