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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@3294 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/handles.cc

@@ -301,7 +301,9 @@ Handle<Object> GetPrototype(Handle<Object> obj) {
 
 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 @@ Handle<Object> GetHiddenProperties(Handle<JSObject> obj,
     // 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 @@ Handle<Object> GetHiddenProperties(Handle<JSObject> obj,
   // 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());
 }
 
 

src/objects-inl.h

@@ -2994,6 +2994,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) {
   return HasElementWithReceiver(this, index);

src/objects.cc

@@ -6170,6 +6170,17 @@ Object* JSObject::GetPropertyPostInterceptor(JSObject* receiver,
   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(
     JSObject* receiver,

src/objects.h

@@ -1407,6 +1407,9 @@ class JSObject: public HeapObject {
   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 @@ class JSObject: public HeapObject {
     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,

test/cctest/test-api.cc

@@ -1344,17 +1344,10 @@ THREADED_TEST(HiddenProperties) {
 }
 
 
+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 @@ THREADED_TEST(HiddenPropertiesWithInterceptors) {
   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 @@ THREADED_TEST(HiddenPropertiesWithInterceptors) {
   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);
 }