Revision: 7169
Author: [email protected]
Date: Tue Mar 15 02:16:12 2011
Log: Do not set value on host objects in Object.defineProperty (fixes issue 1250).

To be compatible with safari we should not change the value on API
objects in Object.defineProperty (e.g., the window.location object).

Review URL: http://codereview.chromium.org/6673042
http://code.google.com/p/v8/source/detail?r=7169

Modified:
 /branches/bleeding_edge/src/runtime.cc
 /branches/bleeding_edge/test/cctest/test-api.cc

=======================================
--- /branches/bleeding_edge/src/runtime.cc      Mon Mar 14 10:46:37 2011
+++ /branches/bleeding_edge/src/runtime.cc      Tue Mar 15 02:16:12 2011
@@ -3761,6 +3761,14 @@

   LookupResult result;
   js_object->LookupRealNamedProperty(*name, &result);
+
+  // To be compatible with safari we do not change the value on API objects
+ // in defineProperty. Firefox disagrees here, and actually changes the value.
+  if (result.IsProperty() &&
+      (result.type() == CALLBACKS) &&
+      result.GetCallbackObject()->IsAccessorInfo()) {
+    return Heap::undefined_value();
+  }

   // Take special care when attributes are different and there is already
   // a property. For simplicity we normalize the property which enables us
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc     Wed Mar  9 07:01:16 2011
+++ /branches/bleeding_edge/test/cctest/test-api.cc     Tue Mar 15 02:16:12 2011
@@ -5659,6 +5659,14 @@
   global_template->SetAccessCheckCallbacks(NamedAccessBlocker,
                                            IndexedAccessBlocker);

+  // Add accessible accessor.
+  global_template->SetAccessor(
+      v8_str("accessible_prop"),
+      EchoGetter, EchoSetter,
+      v8::Handle<Value>(),
+      v8::AccessControl(v8::ALL_CAN_READ | v8::ALL_CAN_WRITE));
+
+
   // Add an accessor that is not accessible by cross-domain JS code.
   global_template->SetAccessor(v8_str("blocked_prop"),
                                UnreachableGetter, UnreachableSetter,
@@ -5699,6 +5707,18 @@

   CompileRun("Object.seal(other)");
   ExpectTrue("Object.isExtensible(other)");
+
+  // Regression test for issue 1250.
+  // Make sure that we can set the accessible accessors value using normal
+  // assignment.
+  CompileRun("other.accessible_prop = 42");
+  CHECK_EQ(42, g_echo_value);
+
+  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());
 }


--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to