Reviewers: Yang,

Message:
PTAL

Description:
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=

Please review this at https://codereview.chromium.org/240573004/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files (+4, -10 lines):
  M src/runtime.cc
  M test/cctest/test-api.cc


Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index 67ac754fc3de9bd50ba45a669a71ac6040fc9bb0..bbe4bbed83bb81637d4f76a0a5eca3e18e30ecc5 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -5140,17 +5140,12 @@ RUNTIME_FUNCTION(MaybeObject*, 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,
Index: test/cctest/test-api.cc
diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index 3f6351c7fe08b912dbf4ebe66e75cfb362ad015a..8509ebd1ccf90ba7a1a38ae7124b7e912b5a6075 100644
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -9396,9 +9396,8 @@ 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");
+  value = CompileRun("other.accessible_prop == -1");
   CHECK(value->IsTrue());
 }



--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to