Revision: 6683
Author: [email protected]
Date: Tue Feb 8 08:31:58 2011
Log: Fix issues with using defineProperty on the global proxy object.
Review URL: http://codereview.chromium.org/6452004
http://code.google.com/p/v8/source/detail?r=6683
Added:
/branches/bleeding_edge/test/mjsunit/regress/regress-1112.js
Modified:
/branches/bleeding_edge/src/runtime.cc
/branches/bleeding_edge/test/cctest/test-api.cc
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/regress-1112.js Tue Feb 8
08:31:58 2011
@@ -0,0 +1,36 @@
+// Copyright 2011 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+// * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+// * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following
+// disclaimer in the documentation and/or other materials provided
+// with the distribution.
+// * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived
+// from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Regression test making sure that defineProperty on the global proxy
+// defines the property on the global object.
+
+Object.defineProperty(this,
+ 1,
+ { configurable: true, enumerable: true, value: 3 });
+assertEquals(3, this[1]);
+assertTrue(this.hasOwnProperty("1"));
+
=======================================
--- /branches/bleeding_edge/src/runtime.cc Tue Feb 8 06:04:27 2011
+++ /branches/bleeding_edge/src/runtime.cc Tue Feb 8 08:31:58 2011
@@ -3685,14 +3685,20 @@
if (((unchecked & (DONT_DELETE | DONT_ENUM | READ_ONLY)) != 0) &&
is_element) {
// Normalize the elements to enable attributes on the property.
- if (!js_object->IsJSGlobalProxy()) {
- NormalizeElements(js_object);
- }
+ if (js_object->IsJSGlobalProxy()) {
+ Handle<Object> proto(js_object->GetPrototype());
+ // If proxy is detached, ignore the assignment. Alternatively,
+ // we could throw an exception.
+ if (proto->IsNull()) return *obj_value;
+ js_object = Handle<JSObject>::cast(proto);
+ }
+ NormalizeElements(js_object);
Handle<NumberDictionary> dictionary(js_object->element_dictionary());
// Make sure that we never go back to fast case.
dictionary->set_requires_slow_elements();
PropertyDetails details = PropertyDetails(attr, NORMAL);
NumberDictionarySet(dictionary, index, obj_value, details);
+ return *obj_value;
}
LookupResult result;
@@ -3707,9 +3713,12 @@
if (result.IsProperty() &&
(attr != result.GetAttributes() || result.type() == CALLBACKS)) {
// New attributes - normalize to avoid writing to instance descriptor
- if (!js_object->IsJSGlobalProxy()) {
- NormalizeProperties(js_object, CLEAR_INOBJECT_PROPERTIES, 0);
- }
+ if (js_object->IsJSGlobalProxy()) {
+ // Since the result is a property, the prototype will exist so
+ // we don't have to check for null.
+ js_object =
Handle<JSObject>(JSObject::cast(js_object->GetPrototype()));
+ }
+ NormalizeProperties(js_object, CLEAR_INOBJECT_PROPERTIES, 0);
// Use IgnoreAttributes version since a readonly property may be
// overridden and SetProperty does not allow this.
return js_object->SetLocalPropertyIgnoreAttributes(*name,
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc Fri Feb 4 05:43:38 2011
+++ /branches/bleeding_edge/test/cctest/test-api.cc Tue Feb 8 08:31:58 2011
@@ -12549,3 +12549,19 @@
CHECK_EQ(1, result->Length());
CHECK_EQ(v8_str("universalAnswer"), result->Get(0));
}
+
+
+TEST(DefinePropertyPostDetach) {
+ v8::HandleScope scope;
+ LocalContext context;
+ v8::Handle<v8::Object> proxy = context->Global();
+ v8::Handle<v8::Function> define_property =
+ CompileRun("(function() {"
+ " Object.defineProperty("
+ " this,"
+ " 1,"
+ " { configurable: true, enumerable: true, value: 3 });"
+ "})").As<Function>();
+ context->DetachGlobal();
+ define_property->Call(proxy, 0, NULL);
+}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
