Fix the issue with layout tests.

The problem was I incorrectly treated NULL result as failure to fetch
a property with a getter.  However, if getter returns zero, it is
manifested as NULL pointer (see added test case).

Good news: that gives another boost as before this CL if getter returned
0, I did another slow lookup.

Review URL: http://codereview.chromium.org/119172

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

src/objects.cc

@@ -5617,10 +5617,11 @@ Object* JSObject::GetPropertyPostInterceptor(JSObject* receiver,
 }
 
 
-Object* JSObject::GetPropertyWithInterceptorProper(
+bool JSObject::GetPropertyWithInterceptorProper(
     JSObject* receiver,
     String* name,
-    PropertyAttributes* attributes) {
+    PropertyAttributes* attributes,
+    Object** result_object) {
   HandleScope scope;
   Handle<InterceptorInfo> interceptor(GetNamedInterceptor());
   Handle<JSObject> receiver_handle(receiver);
@@ -5641,14 +5642,17 @@ Object* JSObject::GetPropertyWithInterceptorProper(
       VMState state(EXTERNAL);
       result = getter(v8::Utils::ToLocal(name_handle), info);
     }
-    RETURN_IF_SCHEDULED_EXCEPTION();
+    if (Top::has_scheduled_exception()) {
+      return false;
+    }
     if (!result.IsEmpty()) {
       *attributes = NONE;
-      return *v8::Utils::OpenHandle(*result);
+      *result_object = *v8::Utils::OpenHandle(*result);
+      return true;
     }
   }
 
-  return NULL;
+  return false;
 }
 
 
@@ -5662,8 +5666,12 @@ Object* JSObject::GetInterceptorPropertyWithLookupHint(
   Handle<JSObject> holder_handle(this);
   Handle<String> name_handle(name);
 
-  Object* result = GetPropertyWithInterceptorProper(receiver, name, attributes);
-  if (result) return result;
+  Object* result = NULL;
+  if (GetPropertyWithInterceptorProper(receiver, name, attributes, &result)) {
+    return result;
+  } else {
+    RETURN_IF_SCHEDULED_EXCEPTION();
+  }
 
   int property_index = lookup_hint->value();
   if (property_index >= 0) {
@@ -5708,8 +5716,12 @@ Object* JSObject::GetPropertyWithInterceptor(
   Handle<JSObject> holder_handle(this);
   Handle<String> name_handle(name);
 
-  Object* result = GetPropertyWithInterceptorProper(receiver, name, attributes);
-  if (result) return result;
+  Object* result = NULL;
+  if (GetPropertyWithInterceptorProper(receiver, name, attributes, &result)) {
+    return result;
+  } else {
+    RETURN_IF_SCHEDULED_EXCEPTION();
+  }
 
   result = holder_handle->GetPropertyPostInterceptor(
       *receiver_handle,

src/objects.h

@@ -1548,9 +1548,13 @@ class JSObject: public HeapObject {
 
   void LookupInDescriptor(String* name, LookupResult* result);
 
-  Object* GetPropertyWithInterceptorProper(JSObject* receiver,
-                                           String* name,
-                                           PropertyAttributes* attributes);
+  // Attempts to get property with a named interceptor getter.  Returns
+  // |true| and stores result into |result| if succesful, otherwise
+  // returns |false|
+  bool GetPropertyWithInterceptorProper(JSObject* receiver,
+                                        String* name,
+                                        PropertyAttributes* attributes,
+                                        Object** result);
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(JSObject);
 };

test/cctest/test-api.cc

@@ -4827,6 +4827,23 @@ THREADED_TEST(InterceptorHasOwnPropertyCausingGC) {
 }
 
 
+typedef v8::Handle<Value> (*NamedPropertyGetter)(Local<String> property,
+                                                 const AccessorInfo& info);
+
+
+static void CheckInterceptorLoadIC(NamedPropertyGetter getter,
+                                   const char* source,
+                                   int expected) {
+  v8::HandleScope scope;
+  v8::Handle<v8::ObjectTemplate> templ = ObjectTemplate::New();
+  templ->SetNamedPropertyHandler(getter);
+  LocalContext context;
+  context->Global()->Set(v8_str("o"), templ->NewInstance());
+  v8::Handle<Value> value = CompileRun(source);
+  CHECK_EQ(expected, value->Int32Value());
+}
+
+
 static v8::Handle<Value> InterceptorLoadICGetter(Local<String> name,
                                                  const AccessorInfo& info) {
   ApiTestFuzzer::Fuzz();
@@ -4837,17 +4854,12 @@ static v8::Handle<Value> InterceptorLoadICGetter(Local<String> name,
 
 // This test should hit the load IC for the interceptor case.
 THREADED_TEST(InterceptorLoadIC) {
-  v8::HandleScope scope;
-  v8::Handle<v8::ObjectTemplate> templ = ObjectTemplate::New();
-  templ->SetNamedPropertyHandler(InterceptorLoadICGetter);
-  LocalContext context;
-  context->Global()->Set(v8_str("o"), templ->NewInstance());
-  v8::Handle<Value> value = CompileRun(
+  CheckInterceptorLoadIC(InterceptorLoadICGetter,
     "var result = 0;"
     "for (var i = 0; i < 1000; i++) {"
     "  result = o.x;"
-    "}");
-  CHECK_EQ(42, value->Int32Value());
+    "}",
+    42);
 }
 
 
@@ -4863,19 +4875,8 @@ static v8::Handle<Value> InterceptorLoadXICGetter(Local<String> name,
 }
 
 
-static void CheckInterceptorLoadIC(const char* source, int expected) {
-  v8::HandleScope scope;
-  v8::Handle<v8::ObjectTemplate> templ = ObjectTemplate::New();
-  templ->SetNamedPropertyHandler(InterceptorLoadXICGetter);
-  LocalContext context;
-  context->Global()->Set(v8_str("o"), templ->NewInstance());
-  v8::Handle<Value> value = CompileRun(source);
-  CHECK_EQ(expected, value->Int32Value());
-}
-
-
 THREADED_TEST(InterceptorLoadICWithFieldOnHolder) {
-  CheckInterceptorLoadIC(
+  CheckInterceptorLoadIC(InterceptorLoadXICGetter,
     "var result = 0;"
     "o.y = 239;"
     "for (var i = 0; i < 1000; i++) {"
@@ -4886,7 +4887,7 @@ THREADED_TEST(InterceptorLoadICWithFieldOnHolder) {
 
 
 THREADED_TEST(InterceptorLoadICWithSubstitutedProto) {
-  CheckInterceptorLoadIC(
+  CheckInterceptorLoadIC(InterceptorLoadXICGetter,
     "var result = 0;"
     "o.__proto__ = { 'y': 239 };"
     "for (var i = 0; i < 1000; i++) {"
@@ -4897,7 +4898,7 @@ THREADED_TEST(InterceptorLoadICWithSubstitutedProto) {
 
 
 THREADED_TEST(InterceptorLoadICWithPropertyOnProto) {
-  CheckInterceptorLoadIC(
+  CheckInterceptorLoadIC(InterceptorLoadXICGetter,
     "var result = 0;"
     "o.__proto__.y = 239;"
     "for (var i = 0; i < 1000; i++) {"
@@ -4908,7 +4909,7 @@ THREADED_TEST(InterceptorLoadICWithPropertyOnProto) {
 
 
 THREADED_TEST(InterceptorLoadICUndefined) {
-  CheckInterceptorLoadIC(
+  CheckInterceptorLoadIC(InterceptorLoadXICGetter,
     "var result = 0;"
     "for (var i = 0; i < 1000; i++) {"
     "  result = (o.y == undefined) ? 239 : 42;"
@@ -4918,7 +4919,7 @@ THREADED_TEST(InterceptorLoadICUndefined) {
 
 
 THREADED_TEST(InterceptorLoadICWithOverride) {
-  CheckInterceptorLoadIC(
+  CheckInterceptorLoadIC(InterceptorLoadXICGetter,
     "fst = new Object();  fst.__proto__ = o;"
     "snd = new Object();  snd.__proto__ = fst;"
     "var result1 = 0;"
@@ -4935,6 +4936,21 @@ THREADED_TEST(InterceptorLoadICWithOverride) {
 }
 
 
+static v8::Handle<Value> InterceptorLoadICGetter0(Local<String> name,
+                                                  const AccessorInfo& info) {
+  ApiTestFuzzer::Fuzz();
+  CHECK(v8_str("x")->Equals(name));
+  return v8::Integer::New(0);
+}
+
+
+THREADED_TEST(InterceptorReturningZero) {
+  CheckInterceptorLoadIC(InterceptorLoadICGetter0,
+     "o.x == undefined ? 1 : 0",
+     0);
+}
+
+
 static v8::Handle<Value> InterceptorStoreICSetter(
     Local<String> key, Local<Value> value, const AccessorInfo&) {
   CHECK(v8_str("x")->Equals(key));