Fixes Object::SetAccessor to return false if the prop is unconfigurable.

http://www.ecma-international.org/ecma-262/7.0/#sec-validateandapplypropertydescriptor
says that [[DefineProperty]] should return false if the property is
already defined and it's unconfigurable (exactly speaking, the condition
in the spec is more complicated, but roughly speaking, it's when the
property is unconfigurable).

BUG=chromium:670651

Review-Url: https://codereview.chromium.org/2680353004
Cr-Commit-Position: refs/heads/master@{#43080}

src/api.cc

@@ -4678,7 +4678,7 @@ static Maybe<bool> ObjectSetAccessor(Local<Context> context, Object* self,
   has_pending_exception =
       !i::JSObject::SetAccessor(obj, info).ToHandle(&result);
   RETURN_ON_FAILED_EXECUTION_PRIMITIVE(bool);
-  if (result->IsUndefined(obj->GetIsolate())) return Nothing<bool>();
+  if (result->IsUndefined(obj->GetIsolate())) return Just(false);
   if (fast) {
     i::JSObject::MigrateSlowToFast(obj, 0, "APISetAccessor");
   }

test/cctest/test-api.cc

@@ -6459,14 +6459,14 @@ THREADED_TEST(DefineAPIAccessorOnObject) {
   ExpectString("obj1.x", "z");
   ExpectString("obj2.x", "z");
 
-  CHECK(GetGlobalProperty(&context, "obj1")
+  CHECK(!GetGlobalProperty(&context, "obj1")
             ->SetAccessor(context.local(), v8_str("x"), GetXValue, NULL,
                           v8_str("donut"))
-            .IsNothing());
-  CHECK(GetGlobalProperty(&context, "obj2")
+            .FromJust());
+  CHECK(!GetGlobalProperty(&context, "obj2")
             ->SetAccessor(context.local(), v8_str("x"), GetXValue, NULL,
                           v8_str("donut"))
-            .IsNothing());
+            .FromJust());
 
   ExpectString("obj1.x", "z");
   ExpectString("obj2.x", "z");
@@ -6500,14 +6500,14 @@ THREADED_TEST(DontDeleteAPIAccessorsCannotBeOverriden) {
   ExpectTrue("!Object.getOwnPropertyDescriptor(obj1, 'x').configurable");
   ExpectTrue("!Object.getOwnPropertyDescriptor(obj2, 'x').configurable");
 
-  CHECK(GetGlobalProperty(&context, "obj1")
+  CHECK(!GetGlobalProperty(&context, "obj1")
             ->SetAccessor(context.local(), v8_str("x"), GetXValue, NULL,
                           v8_str("donut"))
-            .IsNothing());
-  CHECK(GetGlobalProperty(&context, "obj2")
+            .FromJust());
+  CHECK(!GetGlobalProperty(&context, "obj2")
             ->SetAccessor(context.local(), v8_str("x"), GetXValue, NULL,
                           v8_str("donut"))
-            .IsNothing());
+            .FromJust());
 
   {
     v8::TryCatch try_catch(isolate);

test/unittests/BUILD.gn

@@ -12,6 +12,7 @@ v8_executable("unittests") {
     "../../testing/gtest-support.h",
     "api/exception-unittest.cc",
     "api/isolate-unittest.cc",
+    "api/v8-object-unittest.cc",
     "base/atomic-utils-unittest.cc",
     "base/bits-unittest.cc",
     "base/cpu-unittest.cc",

test/unittests/api/v8-object-unittest.cc

+// Copyright 2017 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 "include/v8.h"
+#include "test/unittests/test-utils.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace v8 {
+namespace {
+
+using ObjectTest = TestWithContext;
+
+void accessor_name_getter_callback(Local<Name>,
+                                   const PropertyCallbackInfo<Value>&) {}
+
+TEST_F(ObjectTest, SetAccessorWhenUnconfigurablePropAlreadyDefined) {
+  Context::Scope context_scope(context());
+
+  TryCatch try_catch(isolate());
+
+  Local<Object> global = context()->Global();
+  Local<String> property_name =
+      String::NewFromUtf8(isolate(), "foo", NewStringType::kNormal)
+          .ToLocalChecked();
+
+  PropertyDescriptor prop_desc;
+  prop_desc.set_configurable(false);
+  global->DefineProperty(context(), property_name, prop_desc).ToChecked();
+
+  Maybe<bool> result = global->SetAccessor(context(), property_name,
+                                           accessor_name_getter_callback);
+  ASSERT_TRUE(result.IsJust());
+  ASSERT_FALSE(result.FromJust());
+  ASSERT_FALSE(try_catch.HasCaught());
+}
+
+}  // namespace
+}  // namespace v8

test/unittests/unittests.gyp

@@ -10,6 +10,7 @@
     'unittests_sources': [  ### gcmole(all) ###
       'api/exception-unittest.cc',
       'api/isolate-unittest.cc',
+      'api/v8-object-unittest.cc',
       'base/atomic-utils-unittest.cc',
       'base/bits-unittest.cc',
       'base/cpu-unittest.cc',