Revision: 14818
Author: [email protected]
Date: Mon May 27 03:56:27 2013
Log: Fix Object.freeze for objects with mixed accessors and data
properties
The bug in the existing code was that it modified the |attributes|
local variable on its way through the loop in CopyUpToAddAttributes.
But that affected any properties updated after an accessor property.
The code now sets up a mask each time and applies that instead of
mutating |attributes|.
[email protected]
Review URL: https://chromiumcodereview.appspot.com/16051002
Patch from Adam Klein <[email protected]>.
http://code.google.com/p/v8/source/detail?r=14818
Modified:
/branches/bleeding_edge/src/objects.cc
/branches/bleeding_edge/test/mjsunit/object-freeze.js
=======================================
--- /branches/bleeding_edge/src/objects.cc Thu May 23 06:53:49 2013
+++ /branches/bleeding_edge/src/objects.cc Mon May 27 03:56:27 2013
@@ -6744,11 +6744,14 @@
for (int i = 0; i < size; ++i) {
Object* value = GetValue(i);
PropertyDetails details = GetDetails(i);
+ int mask = DONT_DELETE | DONT_ENUM;
// READ_ONLY is an invalid attribute for JS setters/getters.
- if (details.type() == CALLBACKS && value->IsAccessorPair()) {
- attributes = static_cast<PropertyAttributes>(attributes &
~READ_ONLY);
+ if (details.type() != CALLBACKS || !value->IsAccessorPair()) {
+ mask |= READ_ONLY;
}
- Descriptor desc(GetKey(i), value,
details.CopyAddAttributes(attributes));
+ details = details.CopyAddAttributes(
+ static_cast<PropertyAttributes>(attributes & mask));
+ Descriptor desc(GetKey(i), value, details);
descriptors->Set(i, &desc, witness);
}
} else {
=======================================
--- /branches/bleeding_edge/test/mjsunit/object-freeze.js Thu May 23
00:05:58 2013
+++ /branches/bleeding_edge/test/mjsunit/object-freeze.js Mon May 27
03:56:27 2013
@@ -296,3 +296,21 @@
accessorDidRun = false;
obj.accessor = 'ignored value';
assertTrue(accessorDidRun);
+
+// Test for regression in mixed accessor/data property objects.
+// The strict function is one such object.
+assertTrue(Object.isFrozen(Object.freeze(function(){"use strict";})));
+
+// Also test a simpler case
+obj = {};
+Object.defineProperty(obj, 'accessor', {
+ get: function() { return 42 },
+ set: function() { accessorDidRun = true },
+ configurable: true,
+ enumerable: true
+});
+obj.data = 'foo';
+assertTrue(%HasFastProperties(obj));
+Object.freeze(obj);
+assertTrue(%HasFastProperties(obj));
+assertTrue(Object.isFrozen(obj));
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
