Allow Object.defineProperty to update value of an API accessor.

This is needed for converting internal accessors to API accessors and can break blink tests.

BUG=
R=yangguo@chromium.org

Review URL: https://codereview.chromium.org/240573004

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

src/runtime.cc

@@ -5229,17 +5229,12 @@ RUNTIME_FUNCTION(Runtime_DefineOrRedefineDataProperty) {
   // Special case for callback properties.
   if (lookup.IsPropertyCallbacks()) {
     Handle<Object> callback(lookup.GetCallbackObject(), isolate);
-    // To be compatible with Safari we do not change the value on API objects
-    // in Object.defineProperty(). Firefox disagrees here, and actually changes
-    // the value.
-    if (callback->IsAccessorInfo()) {
-      return isolate->heap()->undefined_value();
-    }
-    // Avoid redefining foreign callback as data property, just use the stored
+    // Avoid redefining callback as data property, just use the stored
     // setter to update the value instead.
     // TODO(mstarzinger): So far this only works if property attributes don't
     // change, this should be fixed once we cleanup the underlying code.
-    if (callback->IsForeign() && lookup.GetAttributes() == attr) {
+    if ((callback->IsForeign() || callback->IsAccessorInfo()) &&
+        lookup.GetAttributes() == attr) {
       Handle<Object> result_object;
       ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
           isolate, result_object,

test/cctest/test-api.cc

@@ -9396,7 +9396,6 @@ TEST(AccessControlES5) {
   CHECK_EQ(42, g_echo_value_1);
 
   v8::Handle<Value> value;
-  // We follow Safari in ignoring assignments to host object accessors.
   CompileRun("Object.defineProperty(other, 'accessible_prop', {value: -1})");
   value = CompileRun("other.accessible_prop == 42");
   CHECK(value->IsTrue());