From c1ac963e534c2409522edcc92a61e977279506de Mon Sep 17 00:00:00 2001 From: whessev8 Date: Thu, 2 Oct 2008 13:45:21 +0000 Subject: [PATCH] Replaces two non-private uses of AddProperty with IgnoreAttributesAndSetLocalProperty. Adds attributes parameter to IgnoreAtt..Property(). Makes IgnoreAtt..Property() an exact clone of SetProperty(), with explicit changes. Review URL: http://codereview.chromium.org/5665 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@411 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/accessors.cc | 5 ++- src/handles.cc | 19 ++++----- src/handles.h | 10 ++--- src/objects.cc | 119 +++++++++++++++++++++++++++++++++++++------------------ src/objects.h | 3 +- src/runtime.cc | 4 +- 6 files changed, 103 insertions(+), 57 deletions(-) diff --git a/src/accessors.cc b/src/accessors.cc index f02894b..cfaf44c 100644 --- a/src/accessors.cc +++ b/src/accessors.cc @@ -122,10 +122,11 @@ Object* Accessors::ArraySetLength(JSObject* object, Object* value, void*) { } else { // This means one of the object's prototypes is a JSArray and // the object does not have a 'length' property. - return object->AddProperty(Heap::length_symbol(), value, NONE); + // Calling SetProperty causes an infinite loop. + return object->IgnoreAttributesAndSetLocalProperty(Heap::length_symbol(), + value, NONE); } } - return Top::Throw(*Factory::NewRangeError("invalid_array_length", HandleVector(NULL, 0))); } diff --git a/src/handles.cc b/src/handles.cc index 8c1b957..858ef93 100644 --- a/src/handles.cc +++ b/src/handles.cc @@ -137,13 +137,6 @@ Handle SetPrototype(Handle function, } -void AddProperty(Handle object, - Handle key, - Handle value, - PropertyAttributes attributes) { - CALL_HEAP_FUNCTION_VOID(object->AddProperty(*key, *value, attributes)); -} - Handle SetProperty(Handle object, Handle key, Handle value, @@ -156,11 +149,19 @@ Handle SetProperty(Handle object, Handle key, Handle value, PropertyAttributes attributes) { - CALL_HEAP_FUNCTION(Runtime::SetObjectProperty(object, key, value, attributes), - Object); + CALL_HEAP_FUNCTION( + Runtime::SetObjectProperty(object, key, value, attributes), Object); } +Handle IgnoreAttributesAndSetLocalProperty(Handle object, + Handle key, + Handle value, + PropertyAttributes attributes) { + CALL_HEAP_FUNCTION(object-> + IgnoreAttributesAndSetLocalProperty(*key, *value, attributes), Object); +} + Handle SetPropertyWithInterceptor(Handle object, Handle key, Handle value, diff --git a/src/handles.h b/src/handles.h index 229b6a4..75f219d 100644 --- a/src/handles.h +++ b/src/handles.h @@ -102,11 +102,6 @@ void TransformToFastProperties(Handle object, int unused_property_fields); void FlattenString(Handle str); -void AddProperty(Handle object, - Handle key, - Handle value, - PropertyAttributes attributes); - Handle SetProperty(Handle object, Handle key, Handle value, @@ -117,6 +112,11 @@ Handle SetProperty(Handle object, Handle value, PropertyAttributes attributes); +Handle IgnoreAttributesAndSetLocalProperty(Handle object, + Handle key, + Handle value, + PropertyAttributes attributes); + Handle SetPropertyWithInterceptor(Handle object, Handle key, Handle value, diff --git a/src/objects.cc b/src/objects.cc index a5e8771..dc64337 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -1569,51 +1569,94 @@ Object* JSObject::SetProperty(LookupResult* result, // Set a real local property, even if it is READ_ONLY. If the property is not -// present, add it with attributes NONE. This code is the same as in -// SetProperty, except for the check for IsReadOnly and the check for a -// callback setter. -Object* JSObject::IgnoreAttributesAndSetLocalProperty(String* name, - Object* value) { +// present, add it with attributes NONE. This code is an exact clone of +// SetProperty, with the check for IsReadOnly and the check for a +// callback setter removed. The two lines looking up the LookupResult +// result are also added. If one of the functions is changed, the other +// should be. +Object* JSObject::IgnoreAttributesAndSetLocalProperty( + String* name, + Object* value, + PropertyAttributes attributes) { // Make sure that the top context does not change when doing callbacks or // interceptor calls. AssertNoContextChange ncc; - - LookupResult result; - LocalLookup(name, &result); - + // ADDED TO CLONE + LookupResult result_struct; + LocalLookup(name, &result_struct); + LookupResult* result = &result_struct; + // END ADDED TO CLONE // Check access rights if needed. - if (IsAccessCheckNeeded() && - !Top::MayNamedAccess(this, name, v8::ACCESS_SET)) { - Top::ReportFailedAccessCheck(this, v8::ACCESS_SET); - return value; + if (IsAccessCheckNeeded() + && !Top::MayNamedAccess(this, name, v8::ACCESS_SET)) { + return SetPropertyWithFailedAccessCheck(result, name, value); } - - if (result.IsValid()) { - switch (result.type()) { - case NORMAL: - property_dictionary()->ValueAtPut(result.GetDictionaryEntry(), value); - return value; - case FIELD: - properties()->set(result.GetFieldIndex(), value); - return value; - case MAP_TRANSITION: - return AddFastPropertyUsingMap(result.GetTransitionMap(), name, value); - case CONSTANT_FUNCTION: - return ReplaceConstantFunctionProperty(name, value); - case CALLBACKS: - return SetPropertyWithCallback(result.GetCallbackObject(), name, value, - result.holder()); - case INTERCEPTOR: - return SetPropertyWithInterceptor(name, value, NONE); - case CONSTANT_TRANSITION: - case NULL_DESCRIPTOR: - UNREACHABLE(); - break; + /* + REMOVED FROM CLONE + if (result->IsNotFound() || !result->IsProperty()) { + // We could not find a local property so let's check whether there is an + // accessor that wants to handle the property. + LookupResult accessor_result; + LookupCallbackSetterInPrototypes(name, &accessor_result); + if (accessor_result.IsValid()) { + return SetPropertyWithCallback(accessor_result.GetCallbackObject(), + name, + value, + accessor_result.holder()); + } } + */ + if (result->IsNotFound()) { + return AddProperty(name, value, attributes); } - - // The property was not found - return AddProperty(name, value, NONE); + if (!result->IsLoaded()) { + return SetLazyProperty(result, name, value, attributes); + } + /* + REMOVED FROM CLONE + if (result->IsReadOnly() && result->IsProperty()) return value; + */ + // This is a real property that is not read-only, or it is a + // transition or null descriptor and there are no setters in the prototypes. + switch (result->type()) { + case NORMAL: + property_dictionary()->ValueAtPut(result->GetDictionaryEntry(), value); + return value; + case FIELD: + properties()->set(result->GetFieldIndex(), value); + return value; + case MAP_TRANSITION: + if (attributes == result->GetAttributes()) { + // Only use map transition if the attributes match. + return AddFastPropertyUsingMap(result->GetTransitionMap(), + name, + value); + } else { + return AddFastProperty(name, value, attributes); + } + case CONSTANT_FUNCTION: + if (value == result->GetConstantFunction()) return value; + // Only replace the function if necessary. + return ReplaceConstantFunctionProperty(name, value); + case CALLBACKS: + return SetPropertyWithCallback(result->GetCallbackObject(), + name, + value, + result->holder()); + case INTERCEPTOR: + return SetPropertyWithInterceptor(name, value, attributes); + case CONSTANT_TRANSITION: + // Replace with a MAP_TRANSITION to a new map with a FIELD, even + // if the value is a function. + // AddProperty has been extended to do this, in this case. + return AddFastProperty(name, value, attributes); + case NULL_DESCRIPTOR: + UNREACHABLE(); + default: + UNREACHABLE(); + } + UNREACHABLE(); + return value; } diff --git a/src/objects.h b/src/objects.h index 85a8122..9566fca 100644 --- a/src/objects.h +++ b/src/objects.h @@ -1137,7 +1137,8 @@ class JSObject: public HeapObject { Object* value, PropertyAttributes attributes); Object* IgnoreAttributesAndSetLocalProperty(String* key, - Object* value); + Object* value, + PropertyAttributes attributes); // Sets a property that currently has lazy loading. Object* SetLazyProperty(LookupResult* result, diff --git a/src/runtime.cc b/src/runtime.cc index 9b79145..9ac04bb 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -397,7 +397,7 @@ static Object* Runtime_DeclareGlobals(Arguments args) { // of callbacks in the prototype chain (this rules out using // SetProperty). Also, we must use the handle-based version to // avoid GC issues. - AddProperty(global, name, value, attributes); + IgnoreAttributesAndSetLocalProperty(global, name, value, attributes); } } // Done. @@ -1416,7 +1416,7 @@ static Object* Runtime_IgnoreAttributesAndSetProperty(Arguments args) { CONVERT_CHECKED(JSObject, object, args[0]); CONVERT_CHECKED(String, name, args[1]); - return object->IgnoreAttributesAndSetLocalProperty(name, args[2]); + return object->IgnoreAttributesAndSetLocalProperty(name, args[2], NONE); } -- 2.7.4