Fix JavaScript accessors on objects with interceptors.

This fixes how Object.defineProperty() defines JavaScript accessors on
objects with installed API interceptors. The definition itself does not
cause any interceptors to be called, whereas any subsequent accesses on
said object will still fire the interceptor. This behavior is in sync
with API accessors.

R=rossberg@chromium.org
BUG=v8:1651,chromium:94666
TEST=cctest/test-api

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

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

diff --git a/src/objects.cc b/src/objects.cc
index e193188..ae5aa78 100644 (file)
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -4352,7 +4352,7 @@ MaybeObject* JSObject::DefineGetterSetter(String* name,
   } else {
     // Lookup the name.
     LookupResult result(heap->isolate());
-    LocalLookup(name, &result);
+    LocalLookupRealNamedProperty(name, &result);
     if (result.IsProperty()) {
       // TODO(mstarzinger): We should check for result.IsDontDelete() here once
       // we only call into the runtime once to set both getter and setter.

diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index 8b618d4..dab8b7c 100644 (file)
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -1423,6 +1423,40 @@ THREADED_TEST(EmptyInterceptorDoesNotAffectJSProperties) {
 
 THREADED_TEST(SwitchFromInterceptorToAccessor) {
   v8::HandleScope scope;
+  Handle<FunctionTemplate> templ = FunctionTemplate::New();
+  AddAccessor(templ, v8_str("age"),
+              SimpleAccessorGetter, SimpleAccessorSetter);
+  AddInterceptor(templ, InterceptorGetter, InterceptorSetter);
+  LocalContext env;
+  env->Global()->Set(v8_str("Obj"), templ->GetFunction());
+  CompileRun("var obj = new Obj;"
+             "function setAge(i){ obj.age = i; };"
+             "for(var i = 0; i <= 10000; i++) setAge(i);");
+  // All i < 10000 go to the interceptor.
+  ExpectInt32("obj.interceptor_age", 9999);
+  // The last i goes to the accessor.
+  ExpectInt32("obj.accessor_age", 10000);
+}
+
+THREADED_TEST(SwitchFromAccessorToInterceptor) {
+  v8::HandleScope scope;
+  Handle<FunctionTemplate> templ = FunctionTemplate::New();
+  AddAccessor(templ, v8_str("age"),
+              SimpleAccessorGetter, SimpleAccessorSetter);
+  AddInterceptor(templ, InterceptorGetter, InterceptorSetter);
+  LocalContext env;
+  env->Global()->Set(v8_str("Obj"), templ->GetFunction());
+  CompileRun("var obj = new Obj;"
+             "function setAge(i){ obj.age = i; };"
+             "for(var i = 20000; i >= 9999; i--) setAge(i);");
+  // All i >= 10000 go to the accessor.
+  ExpectInt32("obj.accessor_age", 10000);
+  // The last i goes to the interceptor.
+  ExpectInt32("obj.interceptor_age", 9999);
+}
+
+THREADED_TEST(SwitchFromInterceptorToAccessorWithInheritance) {
+  v8::HandleScope scope;
   Handle<FunctionTemplate> parent = FunctionTemplate::New();
   Handle<FunctionTemplate> child = FunctionTemplate::New();
   child->Inherit(parent);
@@ -1440,7 +1474,7 @@ THREADED_TEST(SwitchFromInterceptorToAccessor) {
   ExpectInt32("child.accessor_age", 10000);
 }
 
-THREADED_TEST(SwitchFromAccessorToInterceptor) {
+THREADED_TEST(SwitchFromAccessorToInterceptorWithInheritance) {
   v8::HandleScope scope;
   Handle<FunctionTemplate> parent = FunctionTemplate::New();
   Handle<FunctionTemplate> child = FunctionTemplate::New();
@@ -1459,6 +1493,54 @@ THREADED_TEST(SwitchFromAccessorToInterceptor) {
   ExpectInt32("child.interceptor_age", 9999);
 }
 
+THREADED_TEST(SwitchFromInterceptorToJSAccessor) {
+  v8::HandleScope scope;
+  Handle<FunctionTemplate> templ = FunctionTemplate::New();
+  AddInterceptor(templ, InterceptorGetter, InterceptorSetter);
+  LocalContext env;
+  env->Global()->Set(v8_str("Obj"), templ->GetFunction());
+  CompileRun("var obj = new Obj;"
+             "function setter(i) { this.accessor_age = i; };"
+             "function getter() { return this.accessor_age; };"
+             "function setAge(i) { obj.age = i; };"
+             "Object.defineProperty(obj, 'age', { get:getter, set:setter });"
+             "for(var i = 0; i <= 10000; i++) setAge(i);");
+  // All i < 10000 go to the interceptor.
+  ExpectInt32("obj.interceptor_age", 9999);
+  // The last i goes to the JavaScript accessor.
+  ExpectInt32("obj.accessor_age", 10000);
+  // The installed JavaScript getter is still intact.
+  // This last part is a regression test for issue 1651 and relies on the fact
+  // that both interceptor and accessor are being installed on the same object.
+  ExpectInt32("obj.age", 10000);
+  ExpectBoolean("obj.hasOwnProperty('age')", true);
+  ExpectUndefined("Object.getOwnPropertyDescriptor(obj, 'age').value");
+}
+
+THREADED_TEST(SwitchFromJSAccessorToInterceptor) {
+  v8::HandleScope scope;
+  Handle<FunctionTemplate> templ = FunctionTemplate::New();
+  AddInterceptor(templ, InterceptorGetter, InterceptorSetter);
+  LocalContext env;
+  env->Global()->Set(v8_str("Obj"), templ->GetFunction());
+  CompileRun("var obj = new Obj;"
+             "function setter(i) { this.accessor_age = i; };"
+             "function getter() { return this.accessor_age; };"
+             "function setAge(i) { obj.age = i; };"
+             "Object.defineProperty(obj, 'age', { get:getter, set:setter });"
+             "for(var i = 20000; i >= 9999; i--) setAge(i);");
+  // All i >= 10000 go to the accessor.
+  ExpectInt32("obj.accessor_age", 10000);
+  // The last i goes to the interceptor.
+  ExpectInt32("obj.interceptor_age", 9999);
+  // The installed JavaScript getter is still intact.
+  // This last part is a regression test for issue 1651 and relies on the fact
+  // that both interceptor and accessor are being installed on the same object.
+  ExpectInt32("obj.age", 10000);
+  ExpectBoolean("obj.hasOwnProperty('age')", true);
+  ExpectUndefined("Object.getOwnPropertyDescriptor(obj, 'age').value");
+}
+
 THREADED_TEST(SwitchFromInterceptorToProperty) {
   v8::HandleScope scope;
   Handle<FunctionTemplate> parent = FunctionTemplate::New();