Clean up (Get|Set)Property(Attributes)WithFailedAccessChecks

BUG=
R=ishell@chromium.org

Review URL: https://codereview.chromium.org/309653003

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

src/objects.cc

@@ -540,62 +540,43 @@ MaybeHandle<Object> Object::SetPropertyWithDefinedSetter(
 }
 
 
-// Only deal with CALLBACKS and INTERCEPTOR
+static bool FindAllCanReadHolder(LookupResult* result,
+                                 Handle<Name> name,
+                                 bool check_prototype) {
+  if (result->IsInterceptor()) {
+    result->holder()->LookupOwnRealNamedProperty(name, result);
+  }
+
+  while (result->IsProperty()) {
+    if (result->type() == CALLBACKS) {
+      Object* callback_obj = result->GetCallbackObject();
+      if (callback_obj->IsAccessorInfo()) {
+        if (AccessorInfo::cast(callback_obj)->all_can_read()) return true;
+      } else if (callback_obj->IsAccessorPair()) {
+        if (AccessorPair::cast(callback_obj)->all_can_read()) return true;
+      }
+    }
+    if (!check_prototype) break;
+    result->holder()->LookupRealNamedPropertyInPrototypes(name, result);
+  }
+  return false;
+}
+
+
 MaybeHandle<Object> JSObject::GetPropertyWithFailedAccessCheck(
     Handle<JSObject> object,
     Handle<Object> receiver,
     LookupResult* result,
     Handle<Name> name,
     PropertyAttributes* attributes) {
-  Isolate* isolate = name->GetIsolate();
-  if (result->IsProperty()) {
-    switch (result->type()) {
-      case CALLBACKS: {
-        // Only allow API accessors.
-        Handle<Object> callback_obj(result->GetCallbackObject(), isolate);
-        if (callback_obj->IsAccessorInfo()) {
-          if (!AccessorInfo::cast(*callback_obj)->all_can_read()) break;
+  if (FindAllCanReadHolder(result, name, true)) {
     *attributes = result->GetAttributes();
-          // Fall through to GetPropertyWithCallback.
-        } else if (callback_obj->IsAccessorPair()) {
-          if (!AccessorPair::cast(*callback_obj)->all_can_read()) break;
-          // Fall through to GetPropertyWithCallback.
-        } else {
-          break;
-        }
-        Handle<JSObject> holder(result->holder(), isolate);
-        return GetPropertyWithCallback(receiver, name, holder, callback_obj);
-      }
-      case NORMAL:
-      case FIELD:
-      case CONSTANT: {
-        // Search ALL_CAN_READ accessors in prototype chain.
-        LookupResult r(isolate);
-        result->holder()->LookupRealNamedPropertyInPrototypes(name, &r);
-        if (r.IsProperty()) {
-          return GetPropertyWithFailedAccessCheck(
-              object, receiver, &r, name, attributes);
-        }
-        break;
-      }
-      case INTERCEPTOR: {
-        // If the object has an interceptor, try real named properties.
-        // No access check in GetPropertyAttributeWithInterceptor.
-        LookupResult r(isolate);
-        result->holder()->LookupRealNamedProperty(name, &r);
-        if (r.IsProperty()) {
-          return GetPropertyWithFailedAccessCheck(
-              object, receiver, &r, name, attributes);
-        }
-        break;
-      }
-      default:
-        UNREACHABLE();
-    }
+    Handle<JSObject> holder(result->holder());
+    Handle<Object> callbacks(result->GetCallbackObject(), result->isolate());
+    return GetPropertyWithCallback(receiver, name, holder, callbacks);
   }
-
-  // No accessible property found.
   *attributes = ABSENT;
+  Isolate* isolate = result->isolate();
   isolate->ReportFailedAccessCheck(object, v8::ACCESS_GET);
   RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(isolate, Object);
   return isolate->factory()->undefined_value();
@@ -606,63 +587,61 @@ PropertyAttributes JSObject::GetPropertyAttributeWithFailedAccessCheck(
     Handle<JSObject> object,
     LookupResult* result,
     Handle<Name> name,
-    bool continue_search) {
-  if (result->IsProperty()) {
-    switch (result->type()) {
-      case CALLBACKS: {
-        // Only allow API accessors.
-        Handle<Object> obj(result->GetCallbackObject(), object->GetIsolate());
-        if (obj->IsAccessorInfo()) {
-          Handle<AccessorInfo> info = Handle<AccessorInfo>::cast(obj);
-          if (info->all_can_read()) {
-            return result->GetAttributes();
-          }
-        } else if (obj->IsAccessorPair()) {
-          Handle<AccessorPair> pair = Handle<AccessorPair>::cast(obj);
-          if (pair->all_can_read()) {
+    bool check_prototype) {
+  if (FindAllCanReadHolder(result, name, check_prototype)) {
     return result->GetAttributes();
   }
-        }
-        break;
-      }
+  result->isolate()->ReportFailedAccessCheck(object, v8::ACCESS_HAS);
+  // TODO(yangguo): Issue 3269, check for scheduled exception missing?
+  return ABSENT;
+}
 
-      case NORMAL:
-      case FIELD:
-      case CONSTANT: {
-        if (!continue_search) break;
-        // Search ALL_CAN_READ accessors in prototype chain.
-        LookupResult r(object->GetIsolate());
-        result->holder()->LookupRealNamedPropertyInPrototypes(name, &r);
-        if (r.IsProperty()) {
-          return GetPropertyAttributeWithFailedAccessCheck(
-              object, &r, name, continue_search);
-        }
-        break;
+
+static bool FindAllCanWriteHolder(LookupResult* result,
+                                  Handle<Name> name,
+                                  bool check_prototype) {
+  if (result->IsInterceptor()) {
+    result->holder()->LookupOwnRealNamedProperty(name, result);
   }
 
-      case INTERCEPTOR: {
-        // If the object has an interceptor, try real named properties.
-        // No access check in GetPropertyAttributeWithInterceptor.
-        LookupResult r(object->GetIsolate());
-        if (continue_search) {
-          result->holder()->LookupRealNamedProperty(name, &r);
-        } else {
-          result->holder()->LookupOwnRealNamedProperty(name, &r);
+  while (result->IsProperty()) {
+    if (result->type() == CALLBACKS) {
+      Object* callback_obj = result->GetCallbackObject();
+      if (callback_obj->IsAccessorInfo()) {
+        if (AccessorInfo::cast(callback_obj)->all_can_write()) return true;
+      } else if (callback_obj->IsAccessorPair()) {
+        if (AccessorPair::cast(callback_obj)->all_can_write()) return true;
+      }
     }
-        if (!r.IsFound()) break;
-        return GetPropertyAttributeWithFailedAccessCheck(
-            object, &r, name, continue_search);
+    if (!check_prototype) break;
+    result->holder()->LookupRealNamedPropertyInPrototypes(name, result);
   }
+  return false;
+}
 
-      case HANDLER:
-      case NONEXISTENT:
-        UNREACHABLE();
+
+MaybeHandle<Object> JSObject::SetPropertyWithFailedAccessCheck(
+    Handle<JSObject> object,
+    LookupResult* result,
+    Handle<Name> name,
+    Handle<Object> value,
+    bool check_prototype,
+    StrictMode strict_mode) {
+  if (check_prototype && !result->IsProperty()) {
+    object->LookupRealNamedPropertyInPrototypes(name, result);
   }
+
+  if (FindAllCanWriteHolder(result, name, check_prototype)) {
+    Handle<JSObject> holder(result->holder());
+    Handle<Object> callbacks(result->GetCallbackObject(), result->isolate());
+    return SetPropertyWithCallback(
+        object, name, value, holder, callbacks, strict_mode);
   }
 
-  object->GetIsolate()->ReportFailedAccessCheck(object, v8::ACCESS_HAS);
-  // TODO(yangguo): Issue 3269, check for scheduled exception missing?
-  return ABSENT;
+  Isolate* isolate = object->GetIsolate();
+  isolate->ReportFailedAccessCheck(object, v8::ACCESS_SET);
+  RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(isolate, Object);
+  return value;
 }
 
 
@@ -834,11 +813,11 @@ MaybeHandle<Object> Object::GetProperty(Handle<Object> object,
   if (!result->IsHandler()) {
     ASSERT(*object != object->GetPrototype(isolate));
     Handle<Object> last = result->IsProperty()
-        ? Handle<Object>(result->holder(), isolate)
+        ? handle(result->holder()->GetPrototype(), isolate)
         : Handle<Object>::cast(factory->null_value());
     for (Handle<Object> current = object;
-         true;
-         current = Handle<Object>(current->GetPrototype(isolate), isolate)) {
+         !current.is_identical_to(last);
+         current = Object::GetPrototype(isolate, current)) {
       if (current->IsAccessCheckNeeded()) {
         // Check if we're allowed to read from the current object. Note
         // that even though we may not actually end up loading the named
@@ -850,10 +829,6 @@ MaybeHandle<Object> Object::GetProperty(Handle<Object> object,
               checked, receiver, result, name, attributes);
         }
       }
-      // Stop traversing the chain once we reach the last object in the
-      // chain; either the holder of the result or null in case of an
-      // absent property.
-      if (current.is_identical_to(last)) break;
     }
   }
 
@@ -3603,69 +3578,6 @@ void JSObject::LookupRealNamedPropertyInPrototypes(Handle<Name> name,
 }
 
 
-// We only need to deal with CALLBACKS and INTERCEPTORS
-MaybeHandle<Object> JSObject::SetPropertyWithFailedAccessCheck(
-    Handle<JSObject> object,
-    LookupResult* result,
-    Handle<Name> name,
-    Handle<Object> value,
-    bool check_prototype,
-    StrictMode strict_mode) {
-  if (check_prototype && !result->IsProperty()) {
-    object->LookupRealNamedPropertyInPrototypes(name, result);
-  }
-
-  if (result->IsProperty()) {
-    if (!result->IsReadOnly()) {
-      switch (result->type()) {
-        case CALLBACKS: {
-          Object* obj = result->GetCallbackObject();
-          if (obj->IsAccessorInfo()) {
-            Handle<AccessorInfo> info(AccessorInfo::cast(obj));
-            if (info->all_can_write()) {
-              return SetPropertyWithCallback(object, name, value,
-                                             handle(result->holder()),
-                                             info, strict_mode);
-            }
-          } else if (obj->IsAccessorPair()) {
-            Handle<AccessorPair> pair(AccessorPair::cast(obj));
-            if (pair->all_can_read()) {
-              return SetPropertyWithCallback(object, name, value,
-                                             handle(result->holder()),
-                                             pair, strict_mode);
-            }
-          }
-          break;
-        }
-        case INTERCEPTOR: {
-          // Try lookup real named properties. Note that only property can be
-          // set is callbacks marked as ALL_CAN_WRITE on the prototype chain.
-          LookupResult r(object->GetIsolate());
-          object->LookupRealNamedProperty(name, &r);
-          if (r.IsProperty()) {
-            return SetPropertyWithFailedAccessCheck(object,
-                                                    &r,
-                                                    name,
-                                                    value,
-                                                    check_prototype,
-                                                    strict_mode);
-          }
-          break;
-        }
-        default: {
-          break;
-        }
-      }
-    }
-  }
-
-  Isolate* isolate = object->GetIsolate();
-  isolate->ReportFailedAccessCheck(object, v8::ACCESS_SET);
-  RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(isolate, Object);
-  return value;
-}
-
-
 MaybeHandle<Object> JSReceiver::SetProperty(Handle<JSReceiver> object,
                                             LookupResult* result,
                                             Handle<Name> key,
@@ -4505,14 +4417,14 @@ PropertyAttributes JSObject::GetPropertyAttributePostInterceptor(
     Handle<JSObject> object,
     Handle<JSObject> receiver,
     Handle<Name> name,
-    bool continue_search) {
+    bool check_prototype) {
   // Check own property, ignore interceptor.
   Isolate* isolate = object->GetIsolate();
   LookupResult result(isolate);
   object->LookupOwnRealNamedProperty(name, &result);
   if (result.IsFound()) return result.GetAttributes();
 
-  if (continue_search) {
+  if (check_prototype) {
     // Continue searching via the prototype chain.
     Handle<Object> proto(object->GetPrototype(), isolate);
     if (!proto->IsNull()) {
@@ -4528,7 +4440,7 @@ PropertyAttributes JSObject::GetPropertyAttributeWithInterceptor(
     Handle<JSObject> object,
     Handle<JSObject> receiver,
     Handle<Name> name,
-    bool continue_search) {
+    bool check_prototype) {
   // TODO(rossberg): Support symbols in the API.
   if (name->IsSymbol()) return ABSENT;
 
@@ -4563,7 +4475,7 @@ PropertyAttributes JSObject::GetPropertyAttributeWithInterceptor(
     if (!result.IsEmpty()) return DONT_ENUM;
   }
   return GetPropertyAttributePostInterceptor(
-      object, receiver, name, continue_search);
+      object, receiver, name, check_prototype);
 }
 
 
@@ -4588,14 +4500,14 @@ PropertyAttributes JSReceiver::GetPropertyAttributeForResult(
     Handle<JSReceiver> receiver,
     LookupResult* lookup,
     Handle<Name> name,
-    bool continue_search) {
+    bool check_prototype) {
   // Check access rights if needed.
   if (object->IsAccessCheckNeeded()) {
     Heap* heap = object->GetHeap();
     Handle<JSObject> obj = Handle<JSObject>::cast(object);
     if (!heap->isolate()->MayNamedAccess(obj, name, v8::ACCESS_HAS)) {
       return JSObject::GetPropertyAttributeWithFailedAccessCheck(
-          obj, lookup, name, continue_search);
+          obj, lookup, name, check_prototype);
     }
   }
   if (lookup->IsFound()) {
@@ -4614,7 +4526,7 @@ PropertyAttributes JSReceiver::GetPropertyAttributeForResult(
             handle(lookup->holder()),
             Handle<JSObject>::cast(receiver),
             name,
-            continue_search);
+            check_prototype);
       case NONEXISTENT:
         UNREACHABLE();
     }
@@ -4641,7 +4553,7 @@ PropertyAttributes JSObject::GetElementAttributeWithReceiver(
     Handle<JSObject> object,
     Handle<JSReceiver> receiver,
     uint32_t index,
-    bool continue_search) {
+    bool check_prototype) {
   Isolate* isolate = object->GetIsolate();
 
   // Check access rights if needed.
@@ -4658,17 +4570,17 @@ PropertyAttributes JSObject::GetElementAttributeWithReceiver(
     if (proto->IsNull()) return ABSENT;
     ASSERT(proto->IsJSGlobalObject());
     return JSObject::GetElementAttributeWithReceiver(
-        Handle<JSObject>::cast(proto), receiver, index, continue_search);
+        Handle<JSObject>::cast(proto), receiver, index, check_prototype);
   }
 
   // Check for lookup interceptor except when bootstrapping.
   if (object->HasIndexedInterceptor() && !isolate->bootstrapper()->IsActive()) {
     return JSObject::GetElementAttributeWithInterceptor(
-        object, receiver, index, continue_search);
+        object, receiver, index, check_prototype);
   }
 
   return GetElementAttributeWithoutInterceptor(
-      object, receiver, index, continue_search);
+      object, receiver, index, check_prototype);
 }
 
 
@@ -4676,7 +4588,7 @@ PropertyAttributes JSObject::GetElementAttributeWithInterceptor(
     Handle<JSObject> object,
     Handle<JSReceiver> receiver,
     uint32_t index,
-    bool continue_search) {
+    bool check_prototype) {
   Isolate* isolate = object->GetIsolate();
   HandleScope scope(isolate);
 
@@ -4706,7 +4618,7 @@ PropertyAttributes JSObject::GetElementAttributeWithInterceptor(
   }
 
   return GetElementAttributeWithoutInterceptor(
-       object, receiver, index, continue_search);
+       object, receiver, index, check_prototype);
 }
 
 
@@ -4714,7 +4626,7 @@ PropertyAttributes JSObject::GetElementAttributeWithoutInterceptor(
     Handle<JSObject> object,
     Handle<JSReceiver> receiver,
     uint32_t index,
-    bool continue_search) {
+    bool check_prototype) {
   PropertyAttributes attr = object->GetElementsAccessor()->GetAttributes(
       receiver, object, index);
   if (attr != ABSENT) return attr;
@@ -4724,7 +4636,7 @@ PropertyAttributes JSObject::GetElementAttributeWithoutInterceptor(
     return static_cast<PropertyAttributes>(READ_ONLY | DONT_DELETE);
   }
 
-  if (!continue_search) return ABSENT;
+  if (!check_prototype) return ABSENT;
 
   Handle<Object> proto(object->GetPrototype(), object->GetIsolate());
   if (proto->IsJSProxy()) {

src/objects.h

@@ -2222,22 +2222,22 @@ class JSObject: public JSReceiver {
       Handle<JSObject> object,
       Handle<JSObject> receiver,
       Handle<Name> name,
-      bool continue_search);
+      bool check_prototype);
   static PropertyAttributes GetPropertyAttributeWithInterceptor(
       Handle<JSObject> object,
       Handle<JSObject> receiver,
       Handle<Name> name,
-      bool continue_search);
+      bool check_prototype);
   static PropertyAttributes GetPropertyAttributeWithFailedAccessCheck(
       Handle<JSObject> object,
       LookupResult* result,
       Handle<Name> name,
-      bool continue_search);
+      bool check_prototype);
   static PropertyAttributes GetElementAttributeWithReceiver(
       Handle<JSObject> object,
       Handle<JSReceiver> receiver,
       uint32_t index,
-      bool continue_search);
+      bool check_prototype);
 
   // Retrieves an AccessorPair property from the given object. Might return
   // undefined if the property doesn't exist or is of a different kind.