[api] Throw in defineProperty() when necessary.

test/cctest/test-api-interceptors.cc

 // Copyright 2015 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <stdlib.h>
 
 #include "test/cctest/test-api.h"
 
 #include "include/v8-util.h"
 #include "src/api.h"
@@ skipping 470 matching lines @@
 
 bool get_was_called = false;
 bool set_was_called = false;
 
 namespace {
 void GetterCallback(Local<Name> property,
                     const v8::PropertyCallbackInfo<v8::Value>& info) {
   get_was_called = true;
 }
 
-void SetterCallback(Local<Name> property, Local<Value> value,
-                    const v8::PropertyCallbackInfo<v8::Value>& info) {
-  set_was_called = true;
-}
-
 }  // namespace
 
 // Check that get callback is called in defineProperty with accessor descriptor.
 THREADED_TEST(DefinerCallbackAccessorInterceptor) {
   v8::HandleScope scope(CcTest::isolate());
   v8::Local<v8::FunctionTemplate> templ =
       v8::FunctionTemplate::New(CcTest::isolate());
   templ->InstanceTemplate()->SetHandler(
-      v8::NamedPropertyHandlerConfiguration(GetterCallback, SetterCallback));
+      v8::NamedPropertyHandlerConfiguration(GetterCallback));
   LocalContext env;
   env->Global()
       ->Set(env.local(), v8_str("obj"), templ->GetFunction(env.local())
                                             .ToLocalChecked()
                                             ->NewInstance(env.local())
                                             .ToLocalChecked())
       .FromJust();
 
   CHECK_EQ(get_was_called, false);
-  CHECK_EQ(set_was_called, false);
 
   v8_compile("Object.defineProperty(obj, 'x', {set: function() {return 17;}});")
       ->Run(env.local())
       .ToLocalChecked();
   CHECK_EQ(get_was_called, true);
-  CHECK_EQ(set_was_called, false);
 }
 
 bool get_was_called_in_order = false;
 bool define_was_called_in_order = false;
 
 namespace {
 
 void GetterCallbackOrder(Local<Name> property,
                          const v8::PropertyCallbackInfo<v8::Value>& info) {
   get_was_called_in_order = true;
   CHECK_EQ(define_was_called_in_order, true);
   info.GetReturnValue().Set(property);
 }
 
 void DefinerCallbackOrder(Local<Name> property,
                           const v8::PropertyDescriptor& desc,
                           const v8::PropertyCallbackInfo<v8::Value>& info) {
   CHECK_EQ(get_was_called_in_order, false);  // Define called before get.
   define_was_called_in_order = true;
 }
 
 }  // namespace
 
 // Check that definer callback is called before getter callback.
 THREADED_TEST(DefinerCallbackGetAndDefine) {
   v8::HandleScope scope(CcTest::isolate());
   v8::Local<v8::FunctionTemplate> templ =
       v8::FunctionTemplate::New(CcTest::isolate());
   templ->InstanceTemplate()->SetHandler(v8::NamedPropertyHandlerConfiguration(
-      GetterCallbackOrder, SetterCallback, 0, 0, 0, DefinerCallbackOrder));
+      GetterCallbackOrder, 0, 0, 0, 0, DefinerCallbackOrder));
   LocalContext env;
   env->Global()
       ->Set(env.local(), v8_str("obj"), templ->GetFunction(env.local())
                                             .ToLocalChecked()
                                             ->NewInstance(env.local())
                                             .ToLocalChecked())
       .FromJust();
 
   CHECK_EQ(get_was_called_in_order, false);
   CHECK_EQ(define_was_called_in_order, false);
@@ skipping 754 matching lines @@
             .ToLocalChecked()
             ->BooleanValue(env.local())
             .FromJust());
   CHECK(v8_compile("delete obj.myProperty")
             ->Run(env.local())
             .ToLocalChecked()
             ->BooleanValue(env.local())
             .FromJust());
 }
 
+// See https://bugs.chromium.org/p/chromium/issues/detail?id=645542
+THREADED_TEST(PropertySetterCallbackPresentDuringDefineProperty) {
+  {  // defineProperty with a data descriptor is OK in presence of named
+    // setter callback.
+    v8::HandleScope scope(CcTest::isolate());
+    LocalContext env;
+    v8::Local<v8::FunctionTemplate> templ =
+        v8::FunctionTemplate::New(CcTest::isolate());
+    templ->InstanceTemplate()->SetHandler(
+        v8::NamedPropertyHandlerConfiguration(0, EmptyInterceptorSetter));
+    env->Global()
+        ->Set(env.local(), v8_str("obj"), templ->GetFunction(env.local())
+                                              .ToLocalChecked()
+                                              ->NewInstance(env.local())
+                                              .ToLocalChecked())
+        .FromJust();
+    const char* code =
+        "Object.defineProperty(obj, 'x', {value: 42});"
+        "obj.x;";
+    CHECK_EQ(42, v8_compile(code)
+                     ->Run(env.local())
+                     .ToLocalChecked()
+                     ->Int32Value(env.local())
+                     .FromJust());
+  }
+
+  {  // defineProperty with a descriptor that is not a data descriptor should
+     // throw if object has a
+     // named setter callback.
+    v8::HandleScope scope(CcTest::isolate());
+    LocalContext env;
+    v8::Local<v8::FunctionTemplate> templ =
+        v8::FunctionTemplate::New(CcTest::isolate());
+    templ->InstanceTemplate()->SetHandler(
+        v8::NamedPropertyHandlerConfiguration(0, EmptyInterceptorSetter));
+    env->Global()
+        ->Set(env.local(), v8_str("obj"), templ->GetFunction(env.local())
+                                              .ToLocalChecked()
+                                              ->NewInstance(env.local())
+                                              .ToLocalChecked())
+        .FromJust();
+
+    v8::TryCatch try_catch(env->GetIsolate());
+    CompileRun("Object.defineProperty(obj, 'x', {get: function() {}});");
+    CHECK(try_catch.HasCaught());
+  }
+
+  {  // defineProperty with a descriptor that is not a data descriptor, e.g.,
+     // empty descriptor, should throw if object has a named setter callback.
+    v8::HandleScope scope(CcTest::isolate());
+    LocalContext env;
+    v8::Local<v8::FunctionTemplate> templ =
+        v8::FunctionTemplate::New(CcTest::isolate());
+    templ->InstanceTemplate()->SetHandler(
+        v8::NamedPropertyHandlerConfiguration(0, EmptyInterceptorSetter));
+    env->Global()
+        ->Set(env.local(), v8_str("obj"), templ->GetFunction(env.local())
+                                              .ToLocalChecked()
+                                              ->NewInstance(env.local())
+                                              .ToLocalChecked())
+        .FromJust();
+
+    v8::TryCatch try_catch(env->GetIsolate());
+    CompileRun("Object.defineProperty(obj, 'x', {});");
+    CHECK(try_catch.HasCaught());
+  }
+}
+
 namespace {
 void NotInterceptingPropertyDefineCallback(
     Local<Name> name, const v8::PropertyDescriptor& desc,
     const v8::PropertyCallbackInfo<v8::Value>& info) {
   // Do not intercept by not calling info.GetReturnValue().Set().
 }
 
 void InterceptingPropertyDefineCallback(
     Local<Name> name, const v8::PropertyDescriptor& desc,
     const v8::PropertyCallbackInfo<v8::Value>& info) {
@@ skipping 864 matching lines @@
   // All i >= 10000 go to the accessor.
   ExpectInt32("child.accessor_age", 10000);
   // The last i goes to the interceptor.
   ExpectInt32("child.interceptor_age", 9999);
 }
 
 
 THREADED_TEST(SwitchFromInterceptorToJSAccessor) {
   v8::HandleScope scope(CcTest::isolate());
   Local<FunctionTemplate> templ = FunctionTemplate::New(CcTest::isolate());
-  AddInterceptor(templ, InterceptorGetter, InterceptorSetter);
+  AddInterceptor(templ, InterceptorGetter, nullptr);
   LocalContext env;
   env->Global()
       ->Set(env.local(), v8_str("Obj"),
             templ->GetFunction(env.local()).ToLocalChecked())
       .FromJust();
   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(CcTest::isolate());
   Local<FunctionTemplate> templ = FunctionTemplate::New(CcTest::isolate());
-  AddInterceptor(templ, InterceptorGetter, InterceptorSetter);
+  AddInterceptor(templ, InterceptorGetter, nullptr);
   LocalContext env;
   env->Global()
       ->Set(env.local(), v8_str("Obj"),
             templ->GetFunction(env.local()).ToLocalChecked())
       .FromJust();
   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);
+  ExpectInt32("obj.accessor_age", 9999);
+  ExpectInt32("obj.age", 9999);
   ExpectBoolean("obj.hasOwnProperty('age')", true);
   ExpectUndefined("Object.getOwnPropertyDescriptor(obj, 'age').value");
 }
 
 
 THREADED_TEST(SwitchFromInterceptorToProperty) {
   v8::HandleScope scope(CcTest::isolate());
   Local<FunctionTemplate> parent = FunctionTemplate::New(CcTest::isolate());
   Local<FunctionTemplate> child = FunctionTemplate::New(CcTest::isolate());
   child->Inherit(parent);
@@ skipping 215 matching lines @@
 
 static void IndexedPropertyGetter(
     uint32_t index, const v8::PropertyCallbackInfo<v8::Value>& info) {
   ApiTestFuzzer::Fuzz();
   if (index == 37) {
     info.GetReturnValue().Set(v8_num(625));
   }
 }
 
 
-static void IndexedPropertySetter(
-    uint32_t index, Local<Value> value,
-    const v8::PropertyCallbackInfo<v8::Value>& info) {
-  ApiTestFuzzer::Fuzz();
-  if (index == 39) {
-    info.GetReturnValue().Set(value);
-  }
-}
-
-
 THREADED_TEST(IndexedInterceptorWithIndexedAccessor) {
   v8::Isolate* isolate = CcTest::isolate();
   v8::HandleScope scope(isolate);
   Local<ObjectTemplate> templ = ObjectTemplate::New(isolate);
-  templ->SetHandler(v8::IndexedPropertyHandlerConfiguration(
-      IndexedPropertyGetter, IndexedPropertySetter));
+  templ->SetHandler(
+      v8::IndexedPropertyHandlerConfiguration(IndexedPropertyGetter, nullptr));
   LocalContext context;
   context->Global()
       ->Set(context.local(), v8_str("obj"),
             templ->NewInstance(context.local()).ToLocalChecked())
       .FromJust();
   Local<Script> getter_script =
       v8_compile("obj.__defineGetter__(\"3\", function(){return 5;});obj[3];");
   Local<Script> setter_script = v8_compile(
       "obj.__defineSetter__(\"17\", function(val){this.foo = val;});"
       "obj[17] = 23;"
       "obj.foo;");
-  Local<Script> interceptor_setter_script = v8_compile(
-      "obj.__defineSetter__(\"39\", function(val){this.foo = \"hit\";});"
-      "obj[39] = 47;"
-      "obj.foo;");  // This setter should not run, due to the interceptor.
   Local<Script> interceptor_getter_script = v8_compile("obj[37];");
   Local<Value> result = getter_script->Run(context.local()).ToLocalChecked();
   CHECK(v8_num(5)->Equals(context.local(), result).FromJust());
   result = setter_script->Run(context.local()).ToLocalChecked();
   CHECK(v8_num(23)->Equals(context.local(), result).FromJust());
-  result = interceptor_setter_script->Run(context.local()).ToLocalChecked();
-  CHECK(v8_num(23)->Equals(context.local(), result).FromJust());
   result = interceptor_getter_script->Run(context.local()).ToLocalChecked();
   CHECK(v8_num(625)->Equals(context.local(), result).FromJust());
 }
 
 
 static void UnboxedDoubleIndexedPropertyGetter(
     uint32_t index, const v8::PropertyCallbackInfo<v8::Value>& info) {
   ApiTestFuzzer::Fuzz();
   if (index < 25) {
     info.GetReturnValue().Set(v8_num(index));
@@ skipping 1415 matching lines @@
       "%OptimizeFunctionOnNextCall(f);"
       "f(o);");
   ExpectBoolean("%GetOptimizationStatus(f) != 2", true);
 }
 
 
 THREADED_TEST(CrankshaftInterceptorSetter) {
   i::FLAG_allow_natives_syntax = true;
   v8::HandleScope scope(CcTest::isolate());
   Local<FunctionTemplate> templ = FunctionTemplate::New(CcTest::isolate());
-  AddInterceptor(templ, InterceptorGetter, InterceptorSetter);
+  AddInterceptor(templ, InterceptorGetter, nullptr);
   LocalContext env;
   env->Global()
       ->Set(env.local(), v8_str("Obj"),
             templ->GetFunction(env.local()).ToLocalChecked())
       .FromJust();
   CompileRun(
       "var obj = new Obj;"
       // Initialize fields to avoid transitions later.
       "obj.age = 0;"
       "obj.accessor_age = 42;"
       "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 });"
       "setAge(1);"
       "setAge(2);"
       "setAge(3);"
       "%OptimizeFunctionOnNextCall(setAge);"
       "setAge(4);");
-  // All stores went through the interceptor.
-  ExpectInt32("obj.interceptor_age", 4);
-  ExpectInt32("obj.accessor_age", 42);
+  ExpectUndefined("obj.interceptor_age");
+  ExpectInt32("obj.accessor_age", 4);
 }
 
 
 THREADED_TEST(CrankshaftInterceptorGetter) {
   i::FLAG_allow_natives_syntax = true;
   v8::HandleScope scope(CcTest::isolate());
   Local<FunctionTemplate> templ = FunctionTemplate::New(CcTest::isolate());
   AddInterceptor(templ, InterceptorGetter, InterceptorSetter);
   LocalContext env;
   env->Global()
@@ skipping 760 matching lines @@
             ->Set(env.local(), v8_str("Fun"),
                   fun_templ->GetFunction(env.local()).ToLocalChecked())
             .FromJust());
 
   CompileRun(
       "var f = new Fun();"
       "Number.prototype.__proto__ = f;"
       "var a = 42;"
       "for (var i = 0; i<3; i++) { a.foo; }");
 }