Fix handling of foreign callbacks in DefineOwnProperty.

src/runtime.cc

Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index b0e1a057da4b4384cedff7372fd0a04775c14988..09c1a1b88da4d9b3fc4af7595b70705747289c8d 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -4326,12 +4326,25 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DefineOrRedefineDataProperty) {
   LookupResult result(isolate);
   js_object->LocalLookupRealNamedProperty(*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 isolate->heap()->undefined_value();
+  // Special case for callback properties.
+  if (result.IsProperty() && result.type() == CALLBACKS) {
+    Object* callback = result.GetCallbackObject();
+    // To be compatible with safari we do not change the value on API objects
+    // in defineProperty. Firefox disagrees here, and actually changes the value.

[rossberg, 2011/12/19 16:47:44]

80 characters

[Michael Starzinger, 2011/12/20 08:50:16]

Done.

+    if (callback->IsAccessorInfo()) {
+      return isolate->heap()->undefined_value();
+    }
+    // Avoid redefining foreign 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() && result.GetAttributes() == attr) {
+      return js_object->SetPropertyWithCallback(callback,
+                                                *name,
+                                                *obj_value,
+                                                result.holder(),
+                                                kNonStrictMode);

[rossberg, 2011/12/19 16:47:44]

This should be KStrictMode, I think.

[Michael Starzinger, 2011/12/20 08:50:16]

Done. On this code path it doesn't actually matter. The mode is only considered
for JavaScript accessors, but not for API nor foreign callbacks.

+    }
   }
 
   // Take special care when attributes are different and there is already