js accessor creation on Template

src/objects.cc

Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index 38d8647b525c8cb56549af9758848ed887c194dc..03afbd43e01eb2e072906fedcfec65e31c5c237a 100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -513,6 +513,12 @@ MaybeObject* JSObject::GetPropertyWithFailedAccessCheck(
             return result->holder()->GetPropertyWithCallback(
                 receiver, result->GetCallbackObject(), name);
           }
+        } else if (obj->IsAccessorPair()) {

[Sven Panne, 2013/08/21 10:46:25]

Just curious: Can we reach this code path with Foreign callbacks? Is it OK that
they are not handled at all? Depending on that, we might want to ASSERT
something here, but I am not sure what. ;-)

[dcarney, 2013/08/21 10:59:33]

there's no access overrides for foreign callbacks, so we can't ASSERT anything,
they just fall through

+          AccessorPair* pair = AccessorPair::cast(obj);
+          if (pair->all_can_read()) {
+            return result->holder()->GetPropertyWithCallback(
+                receiver, result->GetCallbackObject(), name);
+          }
         }
         break;
       }
@@ -573,6 +579,11 @@ PropertyAttributes JSObject::GetPropertyAttributeWithFailedAccessCheck(
           if (info->all_can_read()) {
             return result->GetAttributes();
           }
+        } else if (obj->IsAccessorPair()) {

[Sven Panne, 2013/08/21 10:46:25]

Same here.

+          AccessorPair* pair = AccessorPair::cast(obj);
+          if (pair->all_can_read()) {
+            return result->GetAttributes();
+          }
         }
         break;
       }
@@ -3361,6 +3372,15 @@ MaybeObject* JSObject::SetPropertyWithFailedAccessCheck(
                                              result->holder(),
                                              strict_mode);
             }
+          } else if (obj->IsAccessorPair()) {

[Sven Panne, 2013/08/21 10:46:25]

... and here.

+            AccessorPair* pair = AccessorPair::cast(obj);
+            if (pair->all_can_read()) {
+              return SetPropertyWithCallback(result->GetCallbackObject(),
+                                             name,
+                                             value,
+                                             result->holder(),
+                                             strict_mode);
+            }
           }
           break;
         }
@@ -6012,11 +6032,13 @@ void JSObject::DefinePropertyAccessor(Handle<JSObject> object,
                                       Handle<Name> name,
                                       Handle<Object> getter,
                                       Handle<Object> setter,
-                                      PropertyAttributes attributes) {
+                                      PropertyAttributes attributes,
+                                      v8::AccessControl access_control) {
   // We could assert that the property is configurable here, but we would need
   // to do a lookup, which seems to be a bit of overkill.
   bool only_attribute_changes = getter->IsNull() && setter->IsNull();
   if (object->HasFastProperties() && !only_attribute_changes &&
+      access_control == v8::DEFAULT &&
       (object->map()->NumberOfOwnDescriptors() <
        DescriptorArray::kMaxNumberOfDescriptors)) {
     bool getterOk = getter->IsNull() ||
@@ -6028,6 +6050,7 @@ void JSObject::DefinePropertyAccessor(Handle<JSObject> object,
 
   Handle<AccessorPair> accessors = CreateAccessorPairFor(object, name);
   accessors->SetComponents(*getter, *setter);
+  accessors->set_access_flags(access_control);
 
   CALL_HEAP_FUNCTION_VOID(
       object->GetIsolate(),
@@ -6132,7 +6155,8 @@ void JSObject::DefineAccessor(Handle<JSObject> object,
                               Handle<Name> name,
                               Handle<Object> getter,
                               Handle<Object> setter,
-                              PropertyAttributes attributes) {
+                              PropertyAttributes attributes,
+                              v8::AccessControl access_control) {
   Isolate* isolate = object->GetIsolate();
   // Check access rights if needed.
   if (object->IsAccessCheckNeeded() &&
@@ -6145,8 +6169,12 @@ void JSObject::DefineAccessor(Handle<JSObject> object,
     Handle<Object> proto(object->GetPrototype(), isolate);
     if (proto->IsNull()) return;
     ASSERT(proto->IsJSGlobalObject());
-    DefineAccessor(
-        Handle<JSObject>::cast(proto), name, getter, setter, attributes);
+    DefineAccessor(Handle<JSObject>::cast(proto),
+                   name,
+                   getter,
+                   setter,
+                   attributes,
+                   access_control);
     return;
   }
 
@@ -6182,9 +6210,11 @@ void JSObject::DefineAccessor(Handle<JSObject> object,
   }
 
   if (is_element) {
+    ASSERT_EQ(v8::DEFAULT, access_control);

[Sven Panne, 2013/08/21 10:46:25]

Perhaps we should document this restriction in v8.h.

[dcarney, 2013/08/21 10:59:33]

it's not a restriction, i just want to get this working

     DefineElementAccessor(object, index, getter, setter, attributes);
   } else {
-    DefinePropertyAccessor(object, name, getter, setter, attributes);
+    DefinePropertyAccessor(
+        object, name, getter, setter, attributes, access_control);
   }
 
   if (is_observed) {