Make sure that we don't actually overwrite a property that has failed access ...

src/v8natives.js

Index: src/v8natives.js
===================================================================
--- src/v8natives.js	(revision 6628)
+++ src/v8natives.js	(working copy)
@@ -491,28 +491,25 @@
 }
 
 
+// Converts an array returned from Runtime_GetOwnProperty to an actual
+// property descriptor. For a description of the array layout please
+// see the runtime.cc file.
+function ConvertDescriptorArrayToDescriptor(desc_array) {

[antonm, 2011/02/04 11:41:33]

maybe you want to add something like assert(desc_array !== false) (in the form
if (desc_array === false) throw 'Internal error: invalid desc_array)

[Rico, 2011/02/04 12:14:18]

Done.

+  if (IS_UNDEFINED(desc_array)) {
+    return void 0;
+  }
 
-// ES5 section 8.12.1.
-function GetOwnProperty(obj, p) {
   var desc = new PropertyDescriptor();
-
-  // GetOwnProperty returns an array indexed by the constants
-  // defined in macros.py.
-  // If p is not a property on obj undefined is returned.
-  var props = %GetOwnProperty(ToObject(obj), ToString(p));
-
-  if (IS_UNDEFINED(props)) return void 0;
-
-  // This is an accessor
-  if (props[IS_ACCESSOR_INDEX]) {
-    desc.setGet(props[GETTER_INDEX]);
-    desc.setSet(props[SETTER_INDEX]);
+  // This is an accessor.
+  if (desc_array[IS_ACCESSOR_INDEX]) {
+    desc.setGet(desc_array[GETTER_INDEX]);
+    desc.setSet(desc_array[SETTER_INDEX]);
   } else {
-    desc.setValue(props[VALUE_INDEX]);
-    desc.setWritable(props[WRITABLE_INDEX]);
+    desc.setValue(desc_array[VALUE_INDEX]);
+    desc.setWritable(desc_array[WRITABLE_INDEX]);
   }
-  desc.setEnumerable(props[ENUMERABLE_INDEX]);
-  desc.setConfigurable(props[CONFIGURABLE_INDEX]);
+  desc.setEnumerable(desc_array[ENUMERABLE_INDEX]);
+  desc.setConfigurable(desc_array[CONFIGURABLE_INDEX]);
 
   return desc;
 }
@@ -535,9 +532,25 @@
 }
 
 
+// ES5 section 8.12.1.
+function GetOwnProperty(obj, p) {
+  // GetOwnProperty returns an array indexed by the constants
+  // defined in macros.py.
+  // If p is not a property on obj undefined is returned.

[antonm, 2011/02/04 11:41:33]

you might want to clarify what returned false means

[Rico, 2011/02/04 12:14:18]

Done.

+  var props = %GetOwnProperty(ToObject(obj), ToString(p));
+
+  if (props == false) return void 0;
+
+  return ConvertDescriptorArrayToDescriptor(props);
+}
+
+
 // ES5 8.12.9.
 function DefineOwnProperty(obj, p, desc, should_throw) {
-  var current = GetOwnProperty(obj, p);
+  var current_or_access = %GetOwnProperty(ToObject(obj), ToString(p));
+  if (current_or_access == false) return void 0;

[antonm, 2011/02/04 11:41:33]

===?

[Rico, 2011/02/04 12:14:18]

No, this will not return true on the return value (because we return the wrapper
boolean)

+
+  var current = ConvertDescriptorArrayToDescriptor(current_or_access);
   var extensible = %IsExtensible(ToObject(obj));
 
   // Error handling according to spec.