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

Unified Diff: src/runtime.cc

Issue 262053011: Confusion on changing data property callback attributes (Closed) Base URL: https://v8.googlecode.com/svn/branches/bleeding_edge
Patch Set: Rebase. Created 6 years, 7 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 | « src/objects-inl.h ('k') | test/cctest/test-api.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index 68a730bb6643bb426f0576fbbaf15911b50a3360..c5b60e91d2aec12f1ac3ba354bb9982938967aec 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -5175,26 +5175,6 @@ RUNTIME_FUNCTION(Runtime_DefineOrRedefineDataProperty) {
LookupResult lookup(isolate);
js_object->LookupOwnRealNamedProperty(name, &lookup);
- // Special case for callback properties.
- if (lookup.IsPropertyCallbacks()) {
- Handle<Object> callback(lookup.GetCallbackObject(), isolate);
- // Avoid redefining callback as data property, just use the stored
- // setter to update the value instead.
- // TODO(mstarzinger): So far this only works if property attributes don't
- // change, this should be fixed once we cleanup the underlying code.
- ASSERT(!callback->IsForeign());
- if (callback->IsAccessorInfo() &&
- lookup.GetAttributes() == attr) {
- Handle<Object> result_object;
- ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
- isolate, result_object,
- JSObject::SetPropertyWithCallback(js_object, name, obj_value,
- handle(lookup.holder()),
- callback, STRICT));
- return *result_object;
- }
- }
-
// Take special care when attributes are different and there is already
// a property. For simplicity we normalize the property which enables us
// to not worry about changing the instance_descriptor and creating a new
@@ -5209,14 +5189,25 @@ RUNTIME_FUNCTION(Runtime_DefineOrRedefineDataProperty) {
// we don't have to check for null.
js_object = Handle<JSObject>(JSObject::cast(js_object->GetPrototype()));
}
- JSObject::NormalizeProperties(js_object, CLEAR_INOBJECT_PROPERTIES, 0);
+
+ if (attr != lookup.GetAttributes() ||
+ (lookup.IsPropertyCallbacks() &&
+ !lookup.GetCallbackObject()->IsAccessorInfo())) {
+ JSObject::NormalizeProperties(js_object, CLEAR_INOBJECT_PROPERTIES, 0);
+ }
+
// Use IgnoreAttributes version since a readonly property may be
// overridden and SetProperty does not allow this.
Handle<Object> result;
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
isolate, result,
JSObject::SetOwnPropertyIgnoreAttributes(
- js_object, name, obj_value, attr));
+ js_object, name, obj_value, attr,
+ Object::OPTIMAL_REPRESENTATION,
+ ALLOW_AS_CONSTANT,
+ JSReceiver::PERFORM_EXTENSIBILITY_CHECK,
+ JSReceiver::MAY_BE_STORE_FROM_KEYED,
+ JSObject::DONT_FORCE_FIELD));
return *result;
}
« no previous file with comments | « src/objects-inl.h ('k') | test/cctest/test-api.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698