Revision: 10279
Author: [email protected]
Date: Tue Dec 20 00:49:51 2011
Log: Fix handling of foreign callbacks in DefineOwnProperty.
We use foreign callbacks to make some properties shadow internal values
but still behave as data properties from within JavaScript. This means
when a value is passed to Object.defineProperty() on such a property,
it should update the internal value instead of redefinind the property
and destroying the shadowing.
[email protected]
BUG=v8:1530
TEST=mjsunit/regress/regress-1530,test262/S15.3.3.1_A4
Review URL: http://codereview.chromium.org/8996008
http://code.google.com/p/v8/source/detail?r=10279
Added:
/branches/bleeding_edge/test/mjsunit/regress/regress-1530.js
Modified:
/branches/bleeding_edge/src/runtime.cc
/branches/bleeding_edge/test/test262/test262.status
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/regress-1530.js Tue Dec 20
00:49:51 2011
@@ -0,0 +1,69 @@
+// 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.
+
+// Test that redefining the 'prototype' property of a function object
+// does actually set the internal value and does not screw up any
+// shadowing between said property and the internal value.
+
+var f = function() {};
+
+// Verify that normal assignment of 'prototype' property works properly
+// and updates the internal value.
+var x = { foo: 'bar' };
+f.prototype = x;
+assertSame(f.prototype, x);
+assertSame(f.prototype.foo, 'bar');
+assertSame(new f().foo, 'bar');
+assertSame(Object.getPrototypeOf(new f()), x);
+assertSame(Object.getOwnPropertyDescriptor(f, 'prototype').value, x);
+
+// Verify that 'prototype' behaves like a data property when it comes to
+// redefining with Object.defineProperty() and the internal value gets
+// updated.
+var y = { foo: 'baz' };
+Object.defineProperty(f, 'prototype', { value: y, writable: true });
+assertSame(f.prototype, y);
+assertSame(f.prototype.foo, 'baz');
+assertSame(new f().foo, 'baz');
+assertSame(Object.getPrototypeOf(new f()), y);
+assertSame(Object.getOwnPropertyDescriptor(f, 'prototype').value, y);
+
+// Verify that the previous redefinition didn't screw up callbacks and
+// the internal value still gets updated.
+var z = { foo: 'other' };
+f.prototype = z;
+assertSame(f.prototype, z);
+assertSame(f.prototype.foo, 'other');
+assertSame(new f().foo, 'other');
+assertSame(Object.getPrototypeOf(new f()), z);
+assertSame(Object.getOwnPropertyDescriptor(f, 'prototype').value, z);
+
+// Verify that non-writability of other properties is respected.
+assertThrows("Object.defineProperty(f, 'name', { value: {} })");
+assertThrows("Object.defineProperty(f, 'length', { value: {} })");
+assertThrows("Object.defineProperty(f, 'caller', { value: {} })");
+assertThrows("Object.defineProperty(f, 'arguments', { value: {} })");
=======================================
--- /branches/bleeding_edge/src/runtime.cc Fri Dec 16 04:54:08 2011
+++ /branches/bleeding_edge/src/runtime.cc Tue Dec 20 00:49:51 2011
@@ -4326,12 +4326,26 @@
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 Object.defineProperty(). Firefox disagrees here, and actually
changes
+ // the value.
+ 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(),
+ kStrictMode);
+ }
}
// Take special care when attributes are different and there is already
=======================================
--- /branches/bleeding_edge/test/test262/test262.status Fri Dec 16 04:54:08
2011
+++ /branches/bleeding_edge/test/test262/test262.status Tue Dec 20 00:49:51
2011
@@ -39,9 +39,6 @@
# V8 Bug: http://code.google.com/p/v8/issues/detail?id=1624
S10.4.2.1_A1: FAIL
-# V8 Bug: http://code.google.com/p/v8/issues/detail?id=1530
-S15.3.3.1_A4: FAIL
-
# V8 Bug: http://code.google.com/p/v8/issues/detail?id=1475
15.2.3.6-4-405: FAIL
15.2.3.6-4-410: FAIL
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
