Revision: 10293
Author:   [email protected]
Date:     Wed Dec 21 08:14:38 2011
Log:      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.

[email protected]
BUG=v8:1651,chromium:94666
TEST=cctest/test-api

Review URL: http://codereview.chromium.org/9021019
http://code.google.com/p/v8/source/detail?r=10293

Modified:
 /branches/bleeding_edge/src/objects.cc
 /branches/bleeding_edge/test/cctest/test-api.cc

=======================================
--- /branches/bleeding_edge/src/objects.cc      Fri Dec 16 04:54:08 2011
+++ /branches/bleeding_edge/src/objects.cc      Wed Dec 21 08:14:38 2011
@@ -4352,7 +4352,7 @@
   } 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.
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc     Wed Dec  7 08:08:09 2011
+++ /branches/bleeding_edge/test/cctest/test-api.cc     Wed Dec 21 08:14:38 2011
@@ -1423,6 +1423,40 @@

 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 @@
   ExpectInt32("child.accessor_age", 10000);
 }

-THREADED_TEST(SwitchFromAccessorToInterceptor) {
+THREADED_TEST(SwitchFromAccessorToInterceptorWithInheritance) {
   v8::HandleScope scope;
   Handle<FunctionTemplate> parent = FunctionTemplate::New();
   Handle<FunctionTemplate> child = FunctionTemplate::New();
@@ -1458,6 +1492,54 @@
   // The last i goes to the interceptor.
   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;

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to