Reviewers: antonm, Description: Make sure that we don't actually overwrite a property that has failed access checsk with Object.defineProperty.
Please review this at http://codereview.chromium.org/6246103/ SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ Affected files: M src/runtime.cc M src/v8natives.js Index: src/runtime.cc =================================================================== --- src/runtime.cc (revision 6628) +++ src/runtime.cc (working copy) @@ -841,7 +841,7 @@ } if (!CheckAccess(*obj, *name, &result, v8::ACCESS_HAS)) { - return Heap::undefined_value(); + return Heap::false_value(); } elms->set(ENUMERABLE_INDEX, Heap::ToBoolean(!result.IsDontEnum())); Index: src/v8natives.js =================================================================== --- src/v8natives.js (revision 6628) +++ src/v8natives.js (working copy) @@ -490,29 +490,25 @@ return this.hasSetter_; } +// 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) { + 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; } @@ -534,10 +530,26 @@ return IS_UNDEFINED(desc) ? false : true; } +// 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. + 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; + + var current = ConvertDescriptorArrayToDescriptor(current_or_access); var extensible = %IsExtensible(ToObject(obj)); // Error handling according to spec. -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
