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

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

src/runtime.cc

@@ -3762,6 +3762,14 @@ static MaybeObject* Runtime_DefineOrRedefineDataProperty(Arguments args) {
   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
   // to not worry about changing the instance_descriptor and creating a new

test/cctest/test-api.cc

@@ -5659,6 +5659,14 @@ TEST(AccessControlES5) {
   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 @@ TEST(AccessControlES5) {
 
   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());
 }