Revision: 3294 Author: [email protected] Date: Thu Nov 12 08:34:52 2009 Log: Make accessors for hidden properties object not touch interceptors.
Interceptors cannot provide a meaningful result for hidden_symbol anyway and some of them crash on empty property name. Related Chromium issue: http://code.google.com/p/chromium/issues/detail?id=27385 Review URL: http://codereview.chromium.org/390020 http://code.google.com/p/v8/source/detail?r=3294 Modified: /branches/bleeding_edge/src/handles.cc /branches/bleeding_edge/src/objects-inl.h /branches/bleeding_edge/src/objects.cc /branches/bleeding_edge/src/objects.h /branches/bleeding_edge/test/cctest/test-api.cc ======================================= --- /branches/bleeding_edge/src/handles.cc Wed Nov 11 01:50:06 2009 +++ /branches/bleeding_edge/src/handles.cc Thu Nov 12 08:34:52 2009 @@ -301,7 +301,9 @@ Handle<Object> GetHiddenProperties(Handle<JSObject> obj, bool create_if_needed) { - Handle<String> key = Factory::hidden_symbol(); + Object* holder = obj->BypassGlobalProxy(); + if (holder->IsUndefined()) return Factory::undefined_value(); + obj = Handle<JSObject>(JSObject::cast(holder)); if (obj->HasFastProperties()) { // If the object has fast properties, check whether the first slot @@ -310,7 +312,7 @@ // code zero) it will always occupy the first entry if present. DescriptorArray* descriptors = obj->map()->instance_descriptors(); if ((descriptors->number_of_descriptors() > 0) && - (descriptors->GetKey(0) == *key) && + (descriptors->GetKey(0) == Heap::hidden_symbol()) && descriptors->IsProperty(0)) { ASSERT(descriptors->GetType(0) == FIELD); return Handle<Object>(obj->FastPropertyAt(descriptors->GetFieldIndex(0))); @@ -320,17 +322,17 @@ // Only attempt to find the hidden properties in the local object and not // in the prototype chain. Note that HasLocalProperty() can cause a GC in // the general case in the presence of interceptors. - if (!obj->HasLocalProperty(*key)) { + if (!obj->HasHiddenPropertiesObject()) { // Hidden properties object not found. Allocate a new hidden properties // object if requested. Otherwise return the undefined value. if (create_if_needed) { Handle<Object> hidden_obj = Factory::NewJSObject(Top::object_function()); - return SetProperty(obj, key, hidden_obj, DONT_ENUM); + return Handle<Object>(obj->SetHiddenPropertiesObject(*hidden_obj)); } else { return Factory::undefined_value(); } } - return GetProperty(obj, key); + return Handle<Object>(obj->GetHiddenPropertiesObject()); } ======================================= --- /branches/bleeding_edge/src/objects-inl.h Wed Nov 11 01:50:06 2009 +++ /branches/bleeding_edge/src/objects-inl.h Thu Nov 12 08:34:52 2009 @@ -2993,6 +2993,43 @@ PropertyAttributes JSObject::GetPropertyAttribute(String* key) { return GetPropertyAttributeWithReceiver(this, key); } + +// TODO(504): this may be useful in other places too where JSGlobalProxy +// is used. +Object* JSObject::BypassGlobalProxy() { + if (IsJSGlobalProxy()) { + Object* proto = GetPrototype(); + if (proto->IsNull()) return Heap::undefined_value(); + ASSERT(proto->IsJSGlobalObject()); + return proto; + } + return this; +} + + +bool JSObject::HasHiddenPropertiesObject() { + ASSERT(!IsJSGlobalProxy()); + return GetPropertyAttributePostInterceptor(this, + Heap::hidden_symbol(), + false) != ABSENT; +} + + +Object* JSObject::GetHiddenPropertiesObject() { + ASSERT(!IsJSGlobalProxy()); + PropertyAttributes attributes; + return GetLocalPropertyPostInterceptor(this, + Heap::hidden_symbol(), + &attributes); +} + + +Object* JSObject::SetHiddenPropertiesObject(Object* hidden_obj) { + ASSERT(!IsJSGlobalProxy()); + return SetPropertyPostInterceptor(Heap::hidden_symbol(), + hidden_obj, + DONT_ENUM); +} bool JSObject::HasElement(uint32_t index) { ======================================= --- /branches/bleeding_edge/src/objects.cc Wed Nov 11 01:50:06 2009 +++ /branches/bleeding_edge/src/objects.cc Thu Nov 12 08:34:52 2009 @@ -6169,6 +6169,17 @@ if (pt == Heap::null_value()) return Heap::undefined_value(); return pt->GetPropertyWithReceiver(receiver, name, attributes); } + +Object* JSObject::GetLocalPropertyPostInterceptor( + JSObject* receiver, + String* name, + PropertyAttributes* attributes) { + // Check local property in holder, ignore interceptor. + LookupResult result; + LocalLookupRealNamedProperty(name, &result); + if (!result.IsValid()) return Heap::undefined_value(); + return GetProperty(receiver, &result, name, attributes); +} Object* JSObject::GetPropertyWithInterceptor( ======================================= --- /branches/bleeding_edge/src/objects.h Wed Nov 11 01:50:06 2009 +++ /branches/bleeding_edge/src/objects.h Thu Nov 12 08:34:52 2009 @@ -1407,6 +1407,9 @@ Object* GetPropertyPostInterceptor(JSObject* receiver, String* name, PropertyAttributes* attributes); + Object* GetLocalPropertyPostInterceptor(JSObject* receiver, + String* name, + PropertyAttributes* attributes); Object* GetLazyProperty(Object* receiver, LookupResult* result, String* name, @@ -1428,6 +1431,27 @@ return GetLocalPropertyAttribute(name) != ABSENT; } + // If the receiver is a JSGlobalProxy this method will return its prototype, + // otherwise the result is the receiver itself. + inline Object* BypassGlobalProxy(); + + // Accessors for hidden properties object. + // + // Hidden properties are not local properties of the object itself. + // Instead they are stored on an auxiliary JSObject stored as a local + // property with a special name Heap::hidden_symbol(). But if the + // receiver is a JSGlobalProxy then the auxiliary object is a property + // of its prototype. + // + // Has/Get/SetHiddenPropertiesObject methods don't allow the holder to be + // a JSGlobalProxy. Use BypassGlobalProxy method above to get to the real + // holder. + // + // These accessors do not touch interceptors or accessors. + inline bool HasHiddenPropertiesObject(); + inline Object* GetHiddenPropertiesObject(); + inline Object* SetHiddenPropertiesObject(Object* hidden_obj); + Object* DeleteProperty(String* name, DeleteMode mode); Object* DeleteElement(uint32_t index, DeleteMode mode); Object* DeleteLazyProperty(LookupResult* result, ======================================= --- /branches/bleeding_edge/test/cctest/test-api.cc Wed Nov 11 01:50:06 2009 +++ /branches/bleeding_edge/test/cctest/test-api.cc Thu Nov 12 08:34:52 2009 @@ -1344,17 +1344,10 @@ } +static bool interceptor_for_hidden_properties_called; static v8::Handle<Value> InterceptorForHiddenProperties( Local<String> name, const AccessorInfo& info) { - // Make sure objects move. - bool saved_always_compact = i::FLAG_always_compact; - if (!i::FLAG_never_compact) { - i::FLAG_always_compact = true; - } - // The whole goal of this interceptor is to cause a GC during local property - // lookup. - i::Heap::CollectAllGarbage(false); - i::FLAG_always_compact = saved_always_compact; + interceptor_for_hidden_properties_called = true; return v8::Handle<Value>(); } @@ -1363,6 +1356,8 @@ v8::HandleScope scope; LocalContext context; + interceptor_for_hidden_properties_called = false; + v8::Local<v8::String> key = v8_str("api-test::hidden-key"); // Associate an interceptor with an object and start setting hidden values. @@ -1373,6 +1368,7 @@ Local<v8::Object> obj = function->NewInstance(); CHECK(obj->SetHiddenValue(key, v8::Integer::New(2302))); CHECK_EQ(2302, obj->GetHiddenValue(key)->Int32Value()); + CHECK(!interceptor_for_hidden_properties_called); } --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
