From aae7033ba1b502ac756031dcbd9377bd4faae843 Mon Sep 17 00:00:00 2001 From: "mstarzinger@chromium.org" Date: Wed, 21 Dec 2011 16:14:38 +0000 Subject: [PATCH] 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 --- src/objects.cc | 2 +- test/cctest/test-api.cc | 84 ++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 84 insertions(+), 2 deletions(-) diff --git a/src/objects.cc b/src/objects.cc index e193188..ae5aa78 100644 --- 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 --- 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 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 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 parent = FunctionTemplate::New(); Handle 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 parent = FunctionTemplate::New(); Handle child = FunctionTemplate::New(); @@ -1459,6 +1493,54 @@ THREADED_TEST(SwitchFromAccessorToInterceptor) { ExpectInt32("child.interceptor_age", 9999); } +THREADED_TEST(SwitchFromInterceptorToJSAccessor) { + v8::HandleScope scope; + Handle 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 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 parent = FunctionTemplate::New(); -- 2.7.4