Object.observe: include oldValue in change records,

plus more accurate distinction of different change types.

Required handlifying more code.

Also fixed a handlification bug in JSProxy::GetElementAttributeWithHandler.

R=verwaest@chromium.org
BUG=

Review URL: https://codereview.chromium.org/11362115

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@12888 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
This commit is contained in:
rossberg@chromium.org 2012-11-07 14:14:50 +00:00
parent 101d64c1a6
commit e059e64c98
5 changed files with 155 additions and 90 deletions

View File

@ -5088,6 +5088,7 @@ PropertyAttributes JSReceiver::GetPropertyAttribute(String* key) {
return GetPropertyAttributeWithReceiver(this, key);
}
// TODO(504): this may be useful in other places too where JSGlobalProxy
// is used.
Object* JSObject::BypassGlobalProxy() {

View File

@ -2761,12 +2761,14 @@ MUST_USE_RESULT PropertyAttributes JSProxy::GetPropertyAttributeWithHandler(
MUST_USE_RESULT PropertyAttributes JSProxy::GetElementAttributeWithHandler(
JSReceiver* receiver,
JSReceiver* receiver_raw,
uint32_t index) {
Isolate* isolate = GetIsolate();
HandleScope scope(isolate);
Handle<JSProxy> proxy(this);
Handle<JSReceiver> receiver(receiver_raw);
Handle<String> name = isolate->factory()->Uint32ToString(index);
return GetPropertyAttributeWithHandler(receiver, *name);
return proxy->GetPropertyAttributeWithHandler(*receiver, *name);
}
@ -2901,7 +2903,7 @@ MaybeObject* JSObject::SetPropertyForResult(LookupResult* lookup,
Handle<Object> old_value(heap->the_hole_value());
if (FLAG_harmony_observation && map()->is_observed()) {
// TODO(observe): save oldValue
old_value = handle(lookup->GetLazyValue());
}
// This is a real property that is not read-only, or it is a
@ -2953,7 +2955,7 @@ MaybeObject* JSObject::SetPropertyForResult(LookupResult* lookup,
result = self->ConvertDescriptorToField(*name, *value, attributes);
}
} else if (details.type() == CALLBACKS) {
result = ConvertDescriptorToField(*name, *value, attributes);
result = self->ConvertDescriptorToField(*name, *value, attributes);
} else {
ASSERT(details.type() == CONSTANT_FUNCTION);
@ -2966,7 +2968,7 @@ MaybeObject* JSObject::SetPropertyForResult(LookupResult* lookup,
} else {
// Otherwise, replace with a map transition to a new map with a FIELD,
// even if the value is a constant function.
result = ConvertTransitionToMapTransition(
result = self->ConvertTransitionToMapTransition(
lookup->GetTransitionIndex(), *name, *value, attributes);
}
}
@ -2981,7 +2983,16 @@ MaybeObject* JSObject::SetPropertyForResult(LookupResult* lookup,
if (!result->ToHandle(&hresult)) return result;
if (FLAG_harmony_observation && map()->is_observed()) {
this->EnqueueChangeRecord("updated", name, old_value);
if (lookup->IsTransition()) {
self->EnqueueChangeRecord("new", name, old_value);
} else {
LookupResult new_lookup(self->GetIsolate());
self->LocalLookup(*name, &new_lookup);
ASSERT(!new_lookup.GetLazyValue()->IsTheHole());
if (!new_lookup.GetLazyValue()->SameValue(*old_value)) {
self->EnqueueChangeRecord("updated", name, old_value);
}
}
}
return *hresult;
@ -3010,22 +3021,22 @@ Handle<Object> JSObject::SetLocalPropertyIgnoreAttributes(
MaybeObject* JSObject::SetLocalPropertyIgnoreAttributes(
String* name,
Object* value,
String* name_raw,
Object* value_raw,
PropertyAttributes attributes) {
// Make sure that the top context does not change when doing callbacks or
// interceptor calls.
AssertNoContextChange ncc;
Isolate* isolate = GetIsolate();
LookupResult lookup(isolate);
LocalLookup(name, &lookup);
if (!lookup.IsFound()) map()->LookupTransition(this, name, &lookup);
LocalLookup(name_raw, &lookup);
if (!lookup.IsFound()) map()->LookupTransition(this, name_raw, &lookup);
// Check access rights if needed.
if (IsAccessCheckNeeded()) {
if (!isolate->MayNamedAccess(this, name, v8::ACCESS_SET)) {
if (!isolate->MayNamedAccess(this, name_raw, v8::ACCESS_SET)) {
return SetPropertyWithFailedAccessCheck(&lookup,
name,
value,
name_raw,
value_raw,
false,
kNonStrictMode);
}
@ -3033,47 +3044,56 @@ MaybeObject* JSObject::SetLocalPropertyIgnoreAttributes(
if (IsJSGlobalProxy()) {
Object* proto = GetPrototype();
if (proto->IsNull()) return value;
if (proto->IsNull()) return value_raw;
ASSERT(proto->IsJSGlobalObject());
return JSObject::cast(proto)->SetLocalPropertyIgnoreAttributes(
name,
value,
name_raw,
value_raw,
attributes);
}
// Check for accessor in prototype chain removed here in clone.
if (!lookup.IsFound()) {
// Neither properties nor transitions found.
return AddProperty(name, value, attributes, kNonStrictMode);
return AddProperty(name_raw, value_raw, attributes, kNonStrictMode);
}
// From this point on everything needs to be handlified.
HandleScope scope(GetIsolate());
Handle<JSObject> self(this);
Handle<String> name(name_raw);
Handle<Object> value(value_raw);
Handle<Object> old_value(isolate->heap()->the_hole_value());
PropertyAttributes old_attributes = ABSENT;
if (FLAG_harmony_observation && map()->is_observed()) {
// TODO(observe): save oldValue
old_value = handle(lookup.GetLazyValue());
old_attributes = lookup.GetAttributes();
}
// Check of IsReadOnly removed from here in clone.
MaybeObject* result = value;
MaybeObject* result = *value;
switch (lookup.type()) {
case NORMAL: {
PropertyDetails details = PropertyDetails(attributes, NORMAL);
result = SetNormalizedProperty(name, value, details);
result = self->SetNormalizedProperty(*name, *value, details);
break;
}
case FIELD:
result = FastPropertyAtPut(lookup.GetFieldIndex(), value);
result = self->FastPropertyAtPut(lookup.GetFieldIndex(), *value);
break;
case CONSTANT_FUNCTION:
// Only replace the function if necessary.
if (value == lookup.GetConstantFunction()) return value;
// Preserve the attributes of this existing property.
attributes = lookup.GetAttributes();
result = ConvertDescriptorToField(name, value, attributes);
if (*value != lookup.GetConstantFunction()) {
// Preserve the attributes of this existing property.
attributes = lookup.GetAttributes();
result = self->ConvertDescriptorToField(*name, *value, attributes);
}
break;
case CALLBACKS:
case INTERCEPTOR:
// Override callback in clone
result = ConvertDescriptorToField(name, value, attributes);
result = self->ConvertDescriptorToField(*name, *value, attributes);
break;
case TRANSITION: {
Map* transition_map = lookup.GetTransitionTarget();
@ -3085,22 +3105,20 @@ MaybeObject* JSObject::SetLocalPropertyIgnoreAttributes(
if (details.type() == FIELD) {
if (attributes == details.attributes()) {
int field_index = descriptors->GetFieldIndex(descriptor);
result = AddFastPropertyUsingMap(transition_map,
name,
value,
field_index);
result = self->AddFastPropertyUsingMap(
transition_map, *name, *value, field_index);
} else {
result = ConvertDescriptorToField(name, value, attributes);
result = self->ConvertDescriptorToField(*name, *value, attributes);
}
} else if (details.type() == CALLBACKS) {
result = ConvertDescriptorToField(name, value, attributes);
result = self->ConvertDescriptorToField(*name, *value, attributes);
} else {
ASSERT(details.type() == CONSTANT_FUNCTION);
// Replace transition to CONSTANT FUNCTION with a map transition to a
// new map with a FIELD, even if the value is a function.
result = ConvertTransitionToMapTransition(
lookup.GetTransitionIndex(), name, value, attributes);
result = self->ConvertTransitionToMapTransition(
lookup.GetTransitionIndex(), *name, *value, attributes);
}
break;
}
@ -3113,9 +3131,19 @@ MaybeObject* JSObject::SetLocalPropertyIgnoreAttributes(
if (!result->ToHandle(&hresult)) return result;
if (FLAG_harmony_observation && map()->is_observed()) {
const char* type =
attributes == lookup.GetAttributes() ? "updated" : "reconfigured";
this->EnqueueChangeRecord(type, handle(name), old_value);
if (lookup.IsTransition()) {
self->EnqueueChangeRecord("new", name, old_value);
} else {
LookupResult new_lookup(isolate);
self->LocalLookup(*name, &new_lookup);
ASSERT(!new_lookup.GetLazyValue()->IsTheHole());
if (old_value->IsTheHole() ||
new_lookup.GetAttributes() != old_attributes) {
self->EnqueueChangeRecord("reconfigured", name, old_value);
} else if (!new_lookup.GetLazyValue()->SameValue(*old_value)) {
self->EnqueueChangeRecord("updated", name, old_value);
}
}
}
return *hresult;
@ -3203,38 +3231,39 @@ PropertyAttributes JSReceiver::GetPropertyAttributeWithReceiver(
? NONE : ABSENT;
}
// Named property.
LookupResult result(GetIsolate());
Lookup(key, &result);
return GetPropertyAttribute(receiver, &result, key, true);
LookupResult lookup(GetIsolate());
Lookup(key, &lookup);
return GetPropertyAttributeForResult(receiver, &lookup, key, true);
}
PropertyAttributes JSReceiver::GetPropertyAttribute(JSReceiver* receiver,
LookupResult* result,
String* name,
bool continue_search) {
PropertyAttributes JSReceiver::GetPropertyAttributeForResult(
JSReceiver* receiver,
LookupResult* lookup,
String* name,
bool continue_search) {
// Check access rights if needed.
if (IsAccessCheckNeeded()) {
JSObject* this_obj = JSObject::cast(this);
Heap* heap = GetHeap();
if (!heap->isolate()->MayNamedAccess(this_obj, name, v8::ACCESS_HAS)) {
return this_obj->GetPropertyAttributeWithFailedAccessCheck(
receiver, result, name, continue_search);
receiver, lookup, name, continue_search);
}
}
if (result->IsFound()) {
switch (result->type()) {
if (lookup->IsFound()) {
switch (lookup->type()) {
case NORMAL: // fall through
case FIELD:
case CONSTANT_FUNCTION:
case CALLBACKS:
return result->GetAttributes();
return lookup->GetAttributes();
case HANDLER: {
return JSProxy::cast(result->proxy())->GetPropertyAttributeWithHandler(
return JSProxy::cast(lookup->proxy())->GetPropertyAttributeWithHandler(
receiver, name);
}
case INTERCEPTOR:
return result->holder()->GetPropertyAttributeWithInterceptor(
return lookup->holder()->GetPropertyAttributeWithInterceptor(
JSObject::cast(receiver), name, continue_search);
case TRANSITION:
case NONEXISTENT:
@ -3253,9 +3282,9 @@ PropertyAttributes JSReceiver::GetLocalPropertyAttribute(String* name) {
return ABSENT;
}
// Named property.
LookupResult result(GetIsolate());
LocalLookup(name, &result);
return GetPropertyAttribute(this, &result, name, false);
LookupResult lookup(GetIsolate());
LocalLookup(name, &lookup);
return GetPropertyAttributeForResult(this, &lookup, name, false);
}
@ -4039,10 +4068,14 @@ MaybeObject* JSObject::DeleteProperty(String* name, DeleteMode mode) {
return isolate->heap()->false_value();
}
// From this point on everything needs to be handlified.
HandleScope scope(isolate);
Handle<JSObject> self(this);
Handle<String> hname(name);
Handle<Object> old_value(isolate->heap()->the_hole_value());
if (FLAG_harmony_observation && map()->is_observed()) {
// TODO(observe): save oldValue
old_value = handle(lookup.GetLazyValue());
}
MaybeObject* result;
@ -4050,24 +4083,25 @@ MaybeObject* JSObject::DeleteProperty(String* name, DeleteMode mode) {
if (lookup.IsInterceptor()) {
// Skip interceptor if forcing a deletion.
if (mode == FORCE_DELETION) {
result = DeletePropertyPostInterceptor(name, mode);
result = self->DeletePropertyPostInterceptor(*hname, mode);
} else {
result = DeletePropertyWithInterceptor(name);
result = self->DeletePropertyWithInterceptor(*hname);
}
} else {
// Normalize object if needed.
Object* obj;
result = NormalizeProperties(CLEAR_INOBJECT_PROPERTIES, 0);
if (!result->ToObject(&obj)) return result;
result = self->NormalizeProperties(CLEAR_INOBJECT_PROPERTIES, 0);
if (!result->To(&obj)) return result;
// Make sure the properties are normalized before removing the entry.
result = DeleteNormalizedProperty(name, mode);
result = self->DeleteNormalizedProperty(*hname, mode);
}
Handle<Object> hresult;
if (!result->ToHandle(&hresult)) return result;
if (FLAG_harmony_observation && map()->is_observed()) {
this->EnqueueChangeRecord("deleted", handle(name), old_value);
if (!self->HasLocalProperty(*hname))
self->EnqueueChangeRecord("deleted", hname, old_value);
}
return *hresult;
@ -4669,14 +4703,14 @@ void JSObject::DefineAccessor(Handle<JSObject> object,
object->DefineAccessor(*name, *getter, *setter, attributes));
}
MaybeObject* JSObject::DefineAccessor(String* name,
Object* getter,
Object* setter,
MaybeObject* JSObject::DefineAccessor(String* name_raw,
Object* getter_raw,
Object* setter_raw,
PropertyAttributes attributes) {
Isolate* isolate = GetIsolate();
// Check access rights if needed.
if (IsAccessCheckNeeded() &&
!isolate->MayNamedAccess(this, name, v8::ACCESS_SET)) {
!isolate->MayNamedAccess(this, name_raw, v8::ACCESS_SET)) {
isolate->ReportFailedAccessCheck(this, v8::ACCESS_SET);
return isolate->heap()->undefined_value();
}
@ -4686,7 +4720,7 @@ MaybeObject* JSObject::DefineAccessor(String* name,
if (proto->IsNull()) return this;
ASSERT(proto->IsJSGlobalObject());
return JSObject::cast(proto)->DefineAccessor(
name, getter, setter, attributes);
name_raw, getter_raw, setter_raw, attributes);
}
// Make sure that the top context does not change when doing callbacks or
@ -4694,30 +4728,47 @@ MaybeObject* JSObject::DefineAccessor(String* name,
AssertNoContextChange ncc;
// Try to flatten before operating on the string.
name->TryFlatten();
name_raw->TryFlatten();
if (!CanSetCallback(name)) return isolate->heap()->undefined_value();
if (!CanSetCallback(name_raw)) return isolate->heap()->undefined_value();
// From this point on everything needs to be handlified.
HandleScope scope(GetIsolate());
Handle<JSObject> self(this);
Handle<String> name(name_raw);
Handle<Object> getter(getter_raw);
Handle<Object> setter(setter_raw);
uint32_t index = 0;
bool is_element = name->AsArrayIndex(&index);
Handle<Object> old_value(isolate->heap()->the_hole_value());
bool preexists = false;
if (FLAG_harmony_observation && map()->is_observed()) {
LookupResult result(isolate);
LocalLookup(name, &result);
preexists = result.IsFound();
// TODO(observe): save oldValue
if (is_element) {
preexists = HasLocalElement(index);
if (preexists) {
// TODO(observe): distinguish the case where it's an accessor
old_value = Object::GetElement(self, index);
}
} else {
LookupResult lookup(isolate);
LocalLookup(*name, &lookup);
preexists = lookup.IsProperty();
if (preexists) old_value = handle(lookup.GetLazyValue());
}
}
uint32_t index = 0;
MaybeObject* result = name->AsArrayIndex(&index)
? DefineElementAccessor(index, getter, setter, attributes)
: DefinePropertyAccessor(name, getter, setter, attributes);
MaybeObject* result = is_element ?
self->DefineElementAccessor(index, *getter, *setter, attributes) :
self->DefinePropertyAccessor(*name, *getter, *setter, attributes);
Handle<Object> hresult;
if (!result->ToHandle(&hresult)) return result;
if (FLAG_harmony_observation && map()->is_observed()) {
const char* type = preexists ? "reconfigured" : "new";
this->EnqueueChangeRecord(type, handle(name), old_value);
self->EnqueueChangeRecord(type, name, old_value);
}
return *hresult;

View File

@ -1499,10 +1499,10 @@ class JSReceiver: public HeapObject {
Smi* GenerateIdentityHash();
private:
PropertyAttributes GetPropertyAttribute(JSReceiver* receiver,
LookupResult* result,
String* name,
bool continue_search);
PropertyAttributes GetPropertyAttributeForResult(JSReceiver* receiver,
LookupResult* result,
String* name,
bool continue_search);
DISALLOW_IMPLICIT_CONSTRUCTORS(JSReceiver);
};

View File

@ -290,7 +290,7 @@ class LookupResult BASE_EMBEDDED {
case CONSTANT_FUNCTION:
return GetConstantFunction();
default:
return Smi::FromInt(0);
return Isolate::Current()->heap()->the_hole_value();
}
}

View File

@ -250,26 +250,39 @@ obj.a = 2;
obj["a"] = 3;
delete obj.a;
obj.a = 4;
obj.a = 4; // ignored
obj.a = 5;
Object.defineProperty(obj, "a", {value: 6});
Object.defineProperty(obj, "a", {writable: false});
obj.a = 7; // ignored
Object.defineProperty(obj, "a", {value: 8});
Object.defineProperty(obj, "a", {value: 7, writable: true});
Object.defineProperty(obj, "a", {get: function() {}});
Object.defineProperty(obj, "a", {get: function() {}});
delete obj.a;
Object.defineProperty(obj, "a", {get: function() {}});
delete obj.a;
Object.defineProperty(obj, "a", {get: function() {}, configurable: true});
Object.defineProperty(obj, "a", {value: 9, writable: true});
obj.a = 10;
delete obj.a;
Object.defineProperty(obj, "a", {value: 11, configurable: true});
Object.deliverChangeRecords(observer.callback);
// TODO(observe): oldValue not included yet.
observer.assertCallbackRecords([
{ object: obj, name: "a", type: "updated" },
{ object: obj, name: "a", type: "updated" },
{ object: obj, name: "a", type: "updated", oldValue: 1 },
{ object: obj, name: "a", type: "updated", oldValue: 2 },
{ object: obj, name: "a", type: "deleted", oldValue: 3 },
{ 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: "updated", oldValue: 6 },
{ object: obj, name: "a", type: "reconfigured", oldValue: 8 },
{ object: obj, name: "a", type: "reconfigured", oldValue: 7 },
{ object: obj, name: "a", type: "reconfigured" },
{ object: obj, name: "a", type: "deleted" },
{ object: obj, name: "a", type: "new" },
{ object: obj, name: "a", type: "updated" },
{ object: obj, name: "a", type: "updated" },
{ object: obj, name: "a", type: "reconfigured" },
{ object: obj, name: "a", type: "updated" },
{ object: obj, name: "a", type: "reconfigured" },
{ object: obj, name: "a", type: "deleted" },
{ object: obj, name: "a", type: "updated", oldValue: 9 },
{ object: obj, name: "a", type: "deleted", oldValue: 10 },
{ object: obj, name: "a", type: "new" },
]);