Source/JavaScriptCore: <<<<<<< .mine
authorbarraclough@apple.com <barraclough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 16 Feb 2012 19:06:03 +0000 (19:06 +0000)
committerbarraclough@apple.com <barraclough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 16 Feb 2012 19:06:03 +0000 (19:06 +0000)
LayoutTests: <<<<<<< .mine

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@107956 268f45cc-cd09-0410-ab3c-d52691b4dbfc

15 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/js/Object-defineProperties-expected.txt
LayoutTests/fast/js/Object-defineProperty-expected.txt
LayoutTests/fast/js/preventExtensions-expected.txt
LayoutTests/fast/js/property-getters-and-setters-expected.txt
LayoutTests/fast/js/script-tests/Object-defineProperty.js
LayoutTests/fast/js/script-tests/preventExtensions.js
LayoutTests/fast/js/script-tests/property-getters-and-setters.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/JSArray.cpp
Source/JavaScriptCore/runtime/JSArray.h
Source/JavaScriptCore/runtime/JSObject.cpp
Source/JavaScriptCore/runtime/ObjectConstructor.cpp
Source/JavaScriptCore/runtime/PropertyDescriptor.cpp
Source/JavaScriptCore/runtime/Structure.cpp

index d93d942..96e0c14 100644 (file)
@@ -1,3 +1,33 @@
+<<<<<<< .mine
+2012-02-15  Gavin Barraclough  <barraclough@apple.com>
+
+        Numerous trivial bugs in Object.defineProperty
+        https://bugs.webkit.org/show_bug.cgi?id=78777
+
+        Reviewed by Sam Weinig.
+
+        There are a handful of really trivial bugs, related to Object.defineProperty:
+            * Redefining an accessor with different attributes changes the attributes, but not the get/set functions!
+            * Calling an undefined setter should only throw in strict mode.
+            * When redefining an accessor to a data decriptor, if writable is not specified we should default to false.
+            * Any attempt to redefine a non-configurable property of an array as configurable should be rejected.
+            * Object.defineProperties should call toObject on 'Properties' argument, rather than throwing if it is not an object.
+            * If preventExtensions has been called on an array, subsequent assignment beyond array bounds should fail.
+            * 'isFrozen' shouldn't be checking the ReadOnly bit for accessor descriptors (we presently always keep this bit as 'false').
+            * Should be able to redefine an non-writable, non-configurable property, with the same value and attributes.
+            * Should be able to define an non-configurable accessor.
+        These are mostly all one-line changes, e.g. inverted boolean checks, masking against wrong attribute.
+
+        * fast/js/Object-defineProperties-expected.txt:
+        * fast/js/Object-defineProperty-expected.txt:
+        * fast/js/preventExtensions-expected.txt:
+        * fast/js/property-getters-and-setters-expected.txt:
+        * fast/js/script-tests/Object-defineProperty.js:
+        * fast/js/script-tests/preventExtensions.js:
+        * fast/js/script-tests/property-getters-and-setters.js:
+            - Update result & add new test cases for all bugs fixed.
+
+=======
 2012-02-16  Terry Anderson  <tdanderson@chromium.org>
 
         WebKit does not support DOM 3 Events FocusEvent
         * platform/chromium/compositing/overflow/overflow-scrolling-touch-stacking-context-expected.txt: Added.
         * platform/chromium/compositing/overflow/overflow-scrolling-touch-stacking-context.html: Added.
 
+>>>>>>> .r107955
 2012-02-15  Alexey Proskuryakov  <ap@apple.com>
 
         Crash after trying to use FileReader in a document with null origin string
index 8d4f817..573da23 100644 (file)
@@ -5,7 +5,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 
 PASS Object.defineProperties() threw exception TypeError: Properties can only be defined on Objects..
 PASS Object.defineProperties('a string') threw exception TypeError: Properties can only be defined on Objects..
-PASS Object.defineProperties({}, 'a string') threw exception TypeError: Property descriptor list must be an Object..
+PASS Object.defineProperties({}, 'a string') threw exception TypeError: Property description must be an object..
 PASS JSON.stringify(Object.defineProperties({},{property:{value:'foo', enumerable:true}, property2:{value:'foo', enumerable:true}})) is '{"property":"foo","property2":"foo"}'
 PASS JSON.stringify(Object.defineProperties({},{property:{value:'foo'}, property2:{value:'foo', enumerable:true}})) is '{"property2":"foo"}'
 PASS JSON.stringify(Object.defineProperties({property:'foo'},{property:{value:'foo', enumerable:true}, property2:{value:'foo', enumerable:true}})) is '{"property":"foo","property2":"foo"}'
index 90242e7..6398cfe 100644 (file)
@@ -58,6 +58,20 @@ PASS Object.defineProperty(Object.defineProperty({}, 'foo', {value: 1}), 'foo',
 PASS Object.defineProperty(Object.defineProperty({}, 'foo', {value: 1}), 'foo', {set: setter1}) threw exception TypeError: Attempting to change access mechanism for an unconfigurable property..
 PASS Object.defineProperty(Object.defineProperty({}, 'foo', {value: 1, configurable: true}), 'foo', {get: getter1}).foo threw exception called getter1.
 PASS Object.defineProperty(Object.defineProperty({}, 'foo', {value: 1, configurable: true}), 'foo', {set: setter1}).foo='test' threw exception called setter1.
+PASS Object.defineProperty(Object.defineProperty({}, 'foo', {value: 1, configurable: true, writable: false}), 'foo', {value: 2, configurable: true, writable: true}).foo is 2
+PASS Object.defineProperty(Object.defineProperty({}, 'foo', {value: 1, configurable: true, writable: false}), 'foo', {value: 2, configurable: true, writable: false}).foo is 2
+PASS Object.defineProperty(Object.defineProperty({}, 'foo', {value: 1, configurable: false, writable: false}), 'foo', {value: 1, configurable: false, writable: false}).foo is 1
+PASS Object.defineProperty(Object.defineProperty({}, 'foo', {get: function() { return 1; }, configurable: true, enumerable: true}), 'foo', {get: function() { return 2; }, configurable: true, enumerable: false}).foo is 2
+PASS Object.defineProperty(Object.defineProperty({}, 'foo', {get: function() { return 1; }, configurable: true, enumerable: false}), 'foo', {get: function() { return 2; }, configurable: true, enumerable: false}).foo is 2
+PASS var result = false; var o = Object.defineProperty(Object.defineProperty({}, 'foo', {get: function() { return 1; }, configurable: true, enumerable: true}), 'foo', {get: function() { return 2; }, configurable: true, enumerable: false}); for (x in o) result = true; result is false
+PASS var result = false; var o = Object.defineProperty(Object.defineProperty({}, 'foo', {get: function() { return 1; }, configurable: true, enumerable: false}), 'foo', {get: function() { return 2; }, configurable: true, enumerable: true}); for (x in o) result = true; result is true
+PASS var o = Object.defineProperty({}, 'foo', {get: function() { return 1; }, configurable: true}); delete o.foo; o.foo is undefined.
+PASS var o = Object.defineProperty({}, 'foo', {get: function() { return 1; }, configurable: false}); delete o.foo; o.foo is 1
+PASS Object.defineProperty(Object.defineProperty({}, 'foo', {get: function() { return 1; }, configurable: true}), 'foo', {value:2}).foo is 2
+PASS Object.defineProperty(Object.defineProperty({}, 'foo', {get: function() { return 1; }, configurable: false}), 'foo', {value:2}).foo threw exception TypeError: Attempting to change access mechanism for an unconfigurable property..
+PASS var o = Object.defineProperty(Object.defineProperty({}, 'foo', {get: function() { return 1; }, configurable: true}), 'foo', {value:2}); o.foo = 3; o.foo is 2
+PASS Object.defineProperty(Object.defineProperty({}, 'foo', {value: 1}), 'foo', {value:2, configurable: true}) threw exception TypeError: Attempting to configurable attribute of unconfigurable property..
+PASS Object.defineProperty(Object.defineProperty([], 'foo', {value: 1}), 'foo', {value:2, configurable: true}) threw exception TypeError: Attempting to configurable attribute of unconfigurable property..
 PASS var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}); o.foo is 42
 PASS var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}); o.foo = 42; o.result; is 42
 PASS var o = Object.defineProperty({}, 'foo', {get:undefined, set:function(x){this.result = x;}}); o.foo is undefined
@@ -65,11 +79,11 @@ PASS var o = Object.defineProperty({}, 'foo', {get:undefined, set:function(x){th
 PASS var o = Object.defineProperty({}, 'foo', {set:function(x){this.result = x;}}); o.foo is undefined
 PASS var o = Object.defineProperty({}, 'foo', {set:function(x){this.result = x;}}); o.foo = 42; o.result; is 42
 PASS var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}, set:undefined}); o.foo is 42
-PASS var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}, set:undefined}); o.foo = 42; o.result; threw exception TypeError: setting a property that has only a getter.
+PASS var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}, set:undefined}); o.foo = 42; o.result; is undefined
 PASS var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}}); o.foo is 42
-PASS var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}}); o.foo = 42; o.result; threw exception TypeError: setting a property that has only a getter.
+PASS var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}}); o.foo = 42; o.result; is undefined
 PASS var o = Object.defineProperty({}, 'foo', {get:undefined, set:undefined}); o.foo is undefined
-PASS var o = Object.defineProperty({}, 'foo', {get:undefined, set:undefined}); o.foo = 42; o.result; threw exception TypeError: setting a property that has only a getter.
+PASS var o = Object.defineProperty({}, 'foo', {get:undefined, set:undefined}); o.foo = 42; o.result; is undefined
 PASS var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {get:function(){return 13;}}); o.foo is 13
 PASS var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {get:function(){return 13;}}); o.foo = 42; o.result; is 42
 PASS var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {get:undefined}); o.foo is undefined
@@ -77,7 +91,27 @@ PASS var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:fu
 PASS var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {set:function(){this.result = 13;}}); o.foo is 42
 PASS var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {set:function(){this.result = 13;}}); o.foo = 42; o.result; is 13
 PASS var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {set:undefined}); o.foo is 42
-PASS var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {set:undefined}); o.foo = 42; o.result; threw exception TypeError: setting a property that has only a getter.
+PASS var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {set:undefined}); o.foo = 42; o.result; is undefined
+PASS 'use strict'; var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}); o.foo is 42
+PASS 'use strict'; var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}); o.foo = 42; o.result; is 42
+PASS 'use strict'; var o = Object.defineProperty({}, 'foo', {get:undefined, set:function(x){this.result = x;}}); o.foo is undefined
+PASS 'use strict'; var o = Object.defineProperty({}, 'foo', {get:undefined, set:function(x){this.result = x;}}); o.foo = 42; o.result; is 42
+PASS 'use strict'; var o = Object.defineProperty({}, 'foo', {set:function(x){this.result = x;}}); o.foo is undefined
+PASS 'use strict'; var o = Object.defineProperty({}, 'foo', {set:function(x){this.result = x;}}); o.foo = 42; o.result; is 42
+PASS 'use strict'; var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}, set:undefined}); o.foo is 42
+PASS 'use strict'; var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}, set:undefined}); o.foo = 42; o.result; threw exception TypeError: setting a property that has only a getter.
+PASS 'use strict'; var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}}); o.foo is 42
+PASS 'use strict'; var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}}); o.foo = 42; o.result; threw exception TypeError: setting a property that has only a getter.
+PASS 'use strict'; var o = Object.defineProperty({}, 'foo', {get:undefined, set:undefined}); o.foo is undefined
+PASS 'use strict'; var o = Object.defineProperty({}, 'foo', {get:undefined, set:undefined}); o.foo = 42; o.result; threw exception TypeError: setting a property that has only a getter.
+PASS 'use strict'; var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {get:function(){return 13;}}); o.foo is 13
+PASS 'use strict'; var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {get:function(){return 13;}}); o.foo = 42; o.result; is 42
+PASS 'use strict'; var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {get:undefined}); o.foo is undefined
+PASS 'use strict'; var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {get:undefined}); o.foo = 42; o.result; is 42
+PASS 'use strict'; var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {set:function(){this.result = 13;}}); o.foo is 42
+PASS 'use strict'; var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {set:function(){this.result = 13;}}); o.foo = 42; o.result; is 13
+PASS 'use strict'; var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {set:undefined}); o.foo is 42
+PASS 'use strict'; var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {set:undefined}); o.foo = 42; o.result; threw exception TypeError: setting a property that has only a getter.
 PASS successfullyParsed is true
 
 TEST COMPLETE
index fe43cb3..8295029 100644 (file)
@@ -14,6 +14,8 @@ PASS Object.preventExtensions(Math.sin) is Math.sin
 PASS var o = {}; Object.preventExtensions(o); o.__proto__ = { newProp: "Should not see this" }; o.newProp; is undefined.
 PASS "use strict"; var o = {}; Object.preventExtensions(o); o.__proto__ = { newProp: "Should not see this" }; threw exception TypeError: Attempted to assign to readonly property..
 PASS Object.preventExtensions(Math); Math.sqrt(4) is 2
+PASS var arr = Object.preventExtensions([]); arr[0] = 42; arr[0] is undefined.
+PASS var arr = Object.preventExtensions([]); arr[0] = 42; arr.length is 0
 PASS successfullyParsed is true
 
 TEST COMPLETE
index 32ba0f3..9d578dd 100644 (file)
@@ -10,7 +10,7 @@ __defineGetter__ and __defineSetter__
 PASS o2.b is 8
 PASS o2.b is 11
 Setting a value without having a setter
-PASS o3.x = 10; threw exception TypeError: setting a property that has only a getter.
+PASS o3.x = 10; o3.x is 42
 Getting a value without having a getter
 PASS o4.x is undefined.
 __lookupGetter__ and __lookupSetter__
index a7b049a..8bdc166 100644 (file)
@@ -81,6 +81,34 @@ shouldThrow("Object.defineProperty(Object.defineProperty({}, 'foo', {value: 1}),
 shouldThrow("Object.defineProperty(Object.defineProperty({}, 'foo', {value: 1, configurable: true}), 'foo', {get: getter1}).foo", "'called getter1'");
 shouldThrow("Object.defineProperty(Object.defineProperty({}, 'foo', {value: 1, configurable: true}), 'foo', {set: setter1}).foo='test'", "'called setter1'");
 
+// Should be able to redefine an non-writable property, if it is configurable.
+shouldBe("Object.defineProperty(Object.defineProperty({}, 'foo', {value: 1, configurable: true, writable: false}), 'foo', {value: 2, configurable: true, writable: true}).foo", "2");
+shouldBe("Object.defineProperty(Object.defineProperty({}, 'foo', {value: 1, configurable: true, writable: false}), 'foo', {value: 2, configurable: true, writable: false}).foo", "2");
+
+// Should be able to redefine an non-writable, non-configurable property, with the same value and attributes.
+shouldBe("Object.defineProperty(Object.defineProperty({}, 'foo', {value: 1, configurable: false, writable: false}), 'foo', {value: 1, configurable: false, writable: false}).foo", "1");
+
+// Should be able to redefine a configurable accessor, with the same or with different attributes.
+// Check the accessor function changed.
+shouldBe("Object.defineProperty(Object.defineProperty({}, 'foo', {get: function() { return 1; }, configurable: true, enumerable: true}), 'foo', {get: function() { return 2; }, configurable: true, enumerable: false}).foo", "2");
+shouldBe("Object.defineProperty(Object.defineProperty({}, 'foo', {get: function() { return 1; }, configurable: true, enumerable: false}), 'foo', {get: function() { return 2; }, configurable: true, enumerable: false}).foo", "2");
+// Check the attributes changed.
+shouldBeFalse("var result = false; var o = Object.defineProperty(Object.defineProperty({}, 'foo', {get: function() { return 1; }, configurable: true, enumerable: true}), 'foo', {get: function() { return 2; }, configurable: true, enumerable: false}); for (x in o) result = true; result");
+shouldBeTrue("var result = false; var o = Object.defineProperty(Object.defineProperty({}, 'foo', {get: function() { return 1; }, configurable: true, enumerable: false}), 'foo', {get: function() { return 2; }, configurable: true, enumerable: true}); for (x in o) result = true; result");
+
+// Should be able to define an non-configurable accessor.
+shouldBeUndefined("var o = Object.defineProperty({}, 'foo', {get: function() { return 1; }, configurable: true}); delete o.foo; o.foo");
+shouldBe("var o = Object.defineProperty({}, 'foo', {get: function() { return 1; }, configurable: false}); delete o.foo; o.foo", '1');
+shouldBe("Object.defineProperty(Object.defineProperty({}, 'foo', {get: function() { return 1; }, configurable: true}), 'foo', {value:2}).foo", "2");
+shouldThrow("Object.defineProperty(Object.defineProperty({}, 'foo', {get: function() { return 1; }, configurable: false}), 'foo', {value:2}).foo");
+
+// Defining a data descriptor over an accessor should result in a read only property.
+shouldBe("var o = Object.defineProperty(Object.defineProperty({}, 'foo', {get: function() { return 1; }, configurable: true}), 'foo', {value:2}); o.foo = 3; o.foo", "2");
+
+// Trying to redefine a non-configurable property as configurable should throw.
+shouldThrow("Object.defineProperty(Object.defineProperty({}, 'foo', {value: 1}), 'foo', {value:2, configurable: true})");
+shouldThrow("Object.defineProperty(Object.defineProperty([], 'foo', {value: 1}), 'foo', {value:2, configurable: true})");
+
 // Test an object with a getter setter.
 // Either accessor may be omitted or replaced with undefined, or both may be replaced with undefined.
 shouldBe("var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}); o.foo", '42')
@@ -90,11 +118,11 @@ shouldBe("var o = Object.defineProperty({}, 'foo', {get:undefined, set:function(
 shouldBe("var o = Object.defineProperty({}, 'foo', {set:function(x){this.result = x;}}); o.foo", 'undefined')
 shouldBe("var o = Object.defineProperty({}, 'foo', {set:function(x){this.result = x;}}); o.foo = 42; o.result;", '42');
 shouldBe("var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}, set:undefined}); o.foo", '42')
-shouldThrow("var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}, set:undefined}); o.foo = 42; o.result;");
+shouldBe("var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}, set:undefined}); o.foo = 42; o.result;", 'undefined');
 shouldBe("var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}}); o.foo", '42')
-shouldThrow("var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}}); o.foo = 42; o.result;");
+shouldBe("var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}}); o.foo = 42; o.result;", 'undefined');
 shouldBe("var o = Object.defineProperty({}, 'foo', {get:undefined, set:undefined}); o.foo", 'undefined')
-shouldThrow("var o = Object.defineProperty({}, 'foo', {get:undefined, set:undefined}); o.foo = 42; o.result;");
+shouldBe("var o = Object.defineProperty({}, 'foo', {get:undefined, set:undefined}); o.foo = 42; o.result;", 'undefined');
 // Test replacing or removing either the getter or setter individually.
 shouldBe("var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {get:function(){return 13;}}); o.foo", '13')
 shouldBe("var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {get:function(){return 13;}}); o.foo = 42; o.result;", '42')
@@ -103,4 +131,28 @@ shouldBe("var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {g
 shouldBe("var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {set:function(){this.result = 13;}}); o.foo", '42')
 shouldBe("var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {set:function(){this.result = 13;}}); o.foo = 42; o.result;", '13')
 shouldBe("var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {set:undefined}); o.foo", '42')
-shouldThrow("var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {set:undefined}); o.foo = 42; o.result;")
+shouldBe("var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {set:undefined}); o.foo = 42; o.result;", 'undefined')
+
+// Repeat of the above, in strict mode.
+// Either accessor may be omitted or replaced with undefined, or both may be replaced with undefined.
+shouldBe("'use strict'; var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}); o.foo", '42')
+shouldBe("'use strict'; var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}); o.foo = 42; o.result;", '42');
+shouldBe("'use strict'; var o = Object.defineProperty({}, 'foo', {get:undefined, set:function(x){this.result = x;}}); o.foo", 'undefined')
+shouldBe("'use strict'; var o = Object.defineProperty({}, 'foo', {get:undefined, set:function(x){this.result = x;}}); o.foo = 42; o.result;", '42');
+shouldBe("'use strict'; var o = Object.defineProperty({}, 'foo', {set:function(x){this.result = x;}}); o.foo", 'undefined')
+shouldBe("'use strict'; var o = Object.defineProperty({}, 'foo', {set:function(x){this.result = x;}}); o.foo = 42; o.result;", '42');
+shouldBe("'use strict'; var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}, set:undefined}); o.foo", '42')
+shouldThrow("'use strict'; var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}, set:undefined}); o.foo = 42; o.result;");
+shouldBe("'use strict'; var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}}); o.foo", '42')
+shouldThrow("'use strict'; var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}}); o.foo = 42; o.result;");
+shouldBe("'use strict'; var o = Object.defineProperty({}, 'foo', {get:undefined, set:undefined}); o.foo", 'undefined')
+shouldThrow("'use strict'; var o = Object.defineProperty({}, 'foo', {get:undefined, set:undefined}); o.foo = 42; o.result;");
+// Test replacing or removing either the getter or setter individually.
+shouldBe("'use strict'; var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {get:function(){return 13;}}); o.foo", '13')
+shouldBe("'use strict'; var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {get:function(){return 13;}}); o.foo = 42; o.result;", '42')
+shouldBe("'use strict'; var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {get:undefined}); o.foo", 'undefined')
+shouldBe("'use strict'; var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {get:undefined}); o.foo = 42; o.result;", '42')
+shouldBe("'use strict'; var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {set:function(){this.result = 13;}}); o.foo", '42')
+shouldBe("'use strict'; var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {set:function(){this.result = 13;}}); o.foo = 42; o.result;", '13')
+shouldBe("'use strict'; var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {set:undefined}); o.foo", '42')
+shouldThrow("'use strict'; var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {set:undefined}); o.foo = 42; o.result;")
index 65562e0..ce48f0e 100644 (file)
@@ -4,7 +4,8 @@ description(
 
 function obj()
 {
-    return { a: 1, b: 2 };
+    // Add an accessor property to check 'isFrozen' returns the correct result for objects with accessors.
+    return Object.defineProperty({ a: 1, b: 2 }, 'g', { get: function() { return "getter"; } });
 }
 
 function test(obj)
@@ -73,3 +74,7 @@ shouldThrow('"use strict"; var o = {}; Object.preventExtensions(o); o.__proto__
 
 // check that we can still access static properties on an object after calling preventExtensions.
 shouldBe('Object.preventExtensions(Math); Math.sqrt(4)', '2');
+
+// Should not be able to add properties to a preventExtensions array.
+shouldBeUndefined('var arr = Object.preventExtensions([]); arr[0] = 42; arr[0]');
+shouldBe('var arr = Object.preventExtensions([]); arr[0] = 42; arr.length', '0');
index b63370d..1fea46c 100644 (file)
@@ -19,7 +19,7 @@ shouldBe("o2.b", "11");
 
 debug("Setting a value without having a setter");
 var o3 = { get x() { return 42; } }
-shouldThrow("o3.x = 10;");
+shouldBe("o3.x = 10; o3.x", "42");
 
 debug("Getting a value without having a getter");
 var o4 = { set x(y) { }}
index cdd2012..dfc6338 100644 (file)
@@ -1,3 +1,59 @@
+<<<<<<< .mine
+2012-02-15  Gavin Barraclough  <barraclough@apple.com>
+
+        Numerous trivial bugs in Object.defineProperty
+        https://bugs.webkit.org/show_bug.cgi?id=78777
+
+        Reviewed by Sam Weinig.
+
+        There are a handful of really trivial bugs, related to Object.defineProperty:
+            * Redefining an accessor with different attributes changes the attributes, but not the get/set functions!
+            * Calling an undefined setter should only throw in strict mode.
+            * When redefining an accessor to a data decriptor, if writable is not specified we should default to false.
+            * Any attempt to redefine a non-configurable property of an array as configurable should be rejected.
+            * Object.defineProperties should call toObject on 'Properties' argument, rather than throwing if it is not an object.
+            * If preventExtensions has been called on an array, subsequent assignment beyond array bounds should fail.
+            * 'isFrozen' shouldn't be checking the ReadOnly bit for accessor descriptors (we presently always keep this bit as 'false').
+            * Should be able to redefine an non-writable, non-configurable property, with the same value and attributes.
+            * Should be able to define an non-configurable accessor.
+        These are mostly all one-line changes, e.g. inverted boolean checks, masking against wrong attribute.
+
+        * runtime/JSArray.cpp:
+        (JSC::SparseArrayValueMap::put):
+            - Added ASSERT.
+            - Calling an undefined setter should only throw in strict mode.
+        (JSC::JSArray::putDescriptor):
+            - Should be able to define an non-configurable accessor.
+        (JSC::JSArray::defineOwnNumericProperty):
+            - Any attempt to redefine a non-configurable property of an array as configurable should be rejected.
+        (JSC::JSArray::putByIndexBeyondVectorLength):
+            - If preventExtensions has been called on an array, subsequent assignment beyond array bounds should fail.
+        * runtime/JSArray.h:
+        (JSArray):
+            - made enterDictionaryMode public, called from JSObject.
+        * runtime/JSObject.cpp:
+        (JSC::JSObject::put):
+            - Calling an undefined setter should only throw in strict mode.
+        (JSC::JSObject::preventExtensions):
+            - Put array objects into dictionary mode to handle this!
+        (JSC::JSObject::defineOwnProperty):
+            - Should be able to redefine an non-writable, non-configurable property, with the same value and attributes.
+            - Redefining an accessor with different attributes changes the attributes, but not the get/set functions!
+        * runtime/ObjectConstructor.cpp:
+        (JSC::objectConstructorDefineProperties):
+            - Object.defineProperties should call toObject on 'Properties' argument, rather than throwing if it is not an object.
+        * runtime/PropertyDescriptor.cpp:
+        (JSC::PropertyDescriptor::attributesWithOverride):
+            - When redefining an accessor to a data decriptor, if writable is not specified we should default to false.
+        (JSC::PropertyDescriptor::attributesOverridingCurrent):
+            - When redefining an accessor to a data decriptor, if writable is not specified we should default to false.
+        * runtime/Structure.cpp:
+        (JSC::Structure::freezeTransition):
+            - 'freezeTransition' shouldn't be setting the ReadOnly bit for accessor descriptors (we presently always keep this bit as 'false').
+        (JSC::Structure::isFrozen):
+            - 'isFrozen' shouldn't be checking the ReadOnly bit for accessor descriptors (we presently always keep this bit as 'false').
+
+=======
 2012-02-13  Filip Pizlo  <fpizlo@apple.com>
 
         DFG should not check the types of arguments that are dead
@@ -14,6 +70,7 @@
         * dfg/DFGSpeculativeJIT.cpp:
         (JSC::DFG::SpeculativeJIT::checkArgumentTypes):
 
+>>>>>>> .r107954
 2012-02-15  Oliver Hunt  <oliver@apple.com>
 
         Ensure that the DFG JIT always plants a CodeOrigin when making calls
index 71d678e..f1a430e 100644 (file)
@@ -223,6 +223,9 @@ inline std::pair<SparseArrayValueMap::iterator, bool> SparseArrayValueMap::add(J
 
 inline void SparseArrayValueMap::put(ExecState* exec, JSArray* array, unsigned i, JSValue value)
 {
+    // If the array is not extensible, we shouldn't get here!
+    ASSERT(array->isExtensible());
+
     SparseArrayEntry& entry = add(array, i).first->second;
 
     if (!(entry.attributes & Accessor)) {
@@ -241,7 +244,8 @@ inline void SparseArrayValueMap::put(ExecState* exec, JSArray* array, unsigned i
     JSObject* setter = asGetterSetter(accessor)->setter();
     
     if (!setter) {
-        throwTypeError(exec, "setting a property that has only a getter");
+        // FIXME: should throw if being called from strict mode.
+        // throwTypeError(exec, "setting a property that has only a getter");
         return;
     }
 
@@ -384,7 +388,7 @@ void JSArray::putDescriptor(ExecState* exec, SparseArrayEntry* entryInMap, Prope
             accessor->setSetter(exec->globalData(), setter);
 
         entryInMap->set(exec->globalData(), this, accessor);
-        entryInMap->attributes = descriptor.attributesOverridingCurrent(oldDescriptor) & ~DontDelete;
+        entryInMap->attributes = descriptor.attributesOverridingCurrent(oldDescriptor) & ~ReadOnly;
         return;
     }
 
@@ -465,7 +469,7 @@ bool JSArray::defineOwnNumericProperty(ExecState* exec, unsigned index, Property
     // 7. If the [[Configurable]] field of current is false then
     if (!current.configurable()) {
         // 7.a. Reject, if the [[Configurable]] field of Desc is true.
-        if (descriptor.configurablePresent() && !descriptor.configurable())
+        if (descriptor.configurablePresent() && descriptor.configurable())
             return reject(exec, throwException, "Attempting to change configurable attribute of unconfigurable property.");
         // 7.b. Reject, if the [[Enumerable]] field of Desc is present and the [[Enumerable]] fields of current and Desc are the Boolean negation of each other.
         if (descriptor.enumerablePresent() && current.enumerable() != descriptor.enumerable())
@@ -785,6 +789,9 @@ NEVER_INLINE void JSArray::putByIndexBeyondVectorLength(ExecState* exec, unsigne
 
     // First, handle cases where we don't currently have a sparse map.
     if (LIKELY(!map)) {
+        // If the array is not extensible, we should have entered dictionary mode, and created the spare map.
+        ASSERT(isExtensible());
+    
         // Update m_length if necessary.
         if (i >= storage->m_length)
             storage->m_length = i + 1;
@@ -808,7 +815,7 @@ NEVER_INLINE void JSArray::putByIndexBeyondVectorLength(ExecState* exec, unsigne
     unsigned length = storage->m_length;
     if (i >= length) {
         // Prohibit growing the array if length is not writable.
-        if (map->lengthIsReadOnly()) {
+        if (map->lengthIsReadOnly() || !isExtensible()) {
             // FIXME: should throw in strict mode.
             return;
         }
index 68e38cf..df75084 100644 (file)
@@ -244,6 +244,8 @@ namespace JSC {
 
         JS_EXPORT_PRIVATE static void visitChildren(JSCell*, SlotVisitor&);
 
+        void enterDictionaryMode(JSGlobalData&);
+
     protected:
         static const unsigned StructureFlags = OverridesGetOwnPropertySlot | OverridesVisitChildren | OverridesGetPropertyNames | JSObject::StructureFlags;
         static void put(JSCell*, ExecState*, const Identifier& propertyName, JSValue, PutPropertySlot&);
@@ -265,7 +267,6 @@ namespace JSC {
         void setLengthWritable(ExecState*, bool writable);
         void putDescriptor(ExecState*, SparseArrayEntry*, PropertyDescriptor&, PropertyDescriptor& old);
         bool defineOwnNumericProperty(ExecState*, unsigned, PropertyDescriptor&, bool throwException);
-        void enterDictionaryMode(JSGlobalData&);
         void allocateSparseMap(JSGlobalData&);
         void deallocateSparseMap();
 
index 4d90f5a..285c6fa 100644 (file)
@@ -172,7 +172,8 @@ void JSObject::put(JSCell* cell, ExecState* exec, const Identifier& propertyName
             if (gs.isGetterSetter()) {
                 JSObject* setterFunc = asGetterSetter(gs)->setter();        
                 if (!setterFunc) {
-                    throwSetterError(exec);
+                    if (slot.isStrictMode())
+                        throwSetterError(exec);
                     return;
                 }
                 
@@ -465,6 +466,8 @@ void JSObject::freeze(JSGlobalData& globalData)
 
 void JSObject::preventExtensions(JSGlobalData& globalData)
 {
+    if (isJSArray(this))
+        asArray(this)->enterDictionaryMode(globalData);
     if (isExtensible())
         setStructure(globalData, Structure::preventExtensionsTransition(globalData, structure()));
 }
@@ -697,21 +700,15 @@ bool JSObject::defineOwnProperty(JSObject* object, ExecState* exec, const Identi
                 return false;
             }
             if (!current.writable()) {
-                if (descriptor.value() || !sameValue(exec, current.value(), descriptor.value())) {
+                if (descriptor.value() && !sameValue(exec, current.value(), descriptor.value())) {
                     if (throwException)
                         throwError(exec, createTypeError(exec, "Attempting to change value of a readonly property."));
                     return false;
                 }
             }
-        } else if (current.attributesEqual(descriptor)) {
-            if (!descriptor.value())
-                return true;
-            PutPropertySlot slot;
-            object->methodTable()->put(object, exec, propertyName, descriptor.value(), slot);
-            if (exec->hadException())
-                return false;
-            return true;
         }
+        if (current.attributesEqual(descriptor) && !descriptor.value())
+            return true;
         object->methodTable()->deleteProperty(object, exec, propertyName);
         return putDescriptor(exec, object, propertyName, descriptor, current.attributesWithOverride(descriptor), current);
     }
@@ -734,15 +731,14 @@ bool JSObject::defineOwnProperty(JSObject* object, ExecState* exec, const Identi
     if (!accessor)
         return false;
     GetterSetter* getterSetter = asGetterSetter(accessor);
-    if (current.attributesEqual(descriptor)) {
-        if (descriptor.setterPresent())
-            getterSetter->setSetter(exec->globalData(), descriptor.setterObject());
-        if (descriptor.getterPresent())
-            getterSetter->setGetter(exec->globalData(), descriptor.getterObject());
+    if (descriptor.setterPresent())
+        getterSetter->setSetter(exec->globalData(), descriptor.setterObject());
+    if (descriptor.getterPresent())
+        getterSetter->setGetter(exec->globalData(), descriptor.getterObject());
+    if (current.attributesEqual(descriptor))
         return true;
-    }
     object->methodTable()->deleteProperty(object, exec, propertyName);
-    unsigned attrs = current.attributesWithOverride(descriptor);
+    unsigned attrs = descriptor.attributesOverridingCurrent(current);
     object->putDirectAccessor(exec->globalData(), propertyName, getterSetter, attrs | Accessor);
     return true;
 }
index d96c1de..bb0303e 100644 (file)
@@ -342,9 +342,7 @@ EncodedJSValue JSC_HOST_CALL objectConstructorDefineProperties(ExecState* exec)
 {
     if (!exec->argument(0).isObject())
         return throwVMError(exec, createTypeError(exec, "Properties can only be defined on Objects."));
-    if (!exec->argument(1).isObject())
-        return throwVMError(exec, createTypeError(exec, "Property descriptor list must be an Object."));
-    return JSValue::encode(defineProperties(exec, asObject(exec->argument(0)), asObject(exec->argument(1))));
+    return JSValue::encode(defineProperties(exec, asObject(exec->argument(0)), exec->argument(1).toObject(exec)));
 }
 
 EncodedJSValue JSC_HOST_CALL objectConstructorCreate(ExecState* exec)
index e3458e4..0cb6295 100644 (file)
@@ -217,12 +217,16 @@ unsigned PropertyDescriptor::attributesWithOverride(const PropertyDescriptor& ot
         newAttributes ^= DontDelete;
     if (sharedSeen & EnumerablePresent && mismatch & DontEnum)
         newAttributes ^= DontEnum;
+    if (isAccessorDescriptor() && other.isDataDescriptor())
+        newAttributes |= ReadOnly;
     return newAttributes;
 }
 
 unsigned PropertyDescriptor::attributesOverridingCurrent(const PropertyDescriptor& current) const
 {
     unsigned currentAttributes = current.m_attributes;
+    if (isDataDescriptor() && current.isAccessorDescriptor())
+        currentAttributes |= ReadOnly;
     unsigned overrideMask = 0;
     if (writablePresent())
         overrideMask |= ReadOnly;
index 978e3d3..a9688f0 100644 (file)
@@ -469,7 +469,7 @@ Structure* Structure::freezeTransition(JSGlobalData& globalData, Structure* stru
     if (transition->m_propertyTable) {
         PropertyTable::iterator end = transition->m_propertyTable->end();
         for (PropertyTable::iterator iter = transition->m_propertyTable->begin(); iter != end; ++iter)
-            iter->attributes |= (DontDelete | ReadOnly);
+            iter->attributes |= iter->attributes & Accessor ? DontDelete : (DontDelete | ReadOnly);
     }
 
     return transition;
@@ -520,7 +520,9 @@ bool Structure::isFrozen(JSGlobalData& globalData)
 
     PropertyTable::iterator end = m_propertyTable->end();
     for (PropertyTable::iterator iter = m_propertyTable->begin(); iter != end; ++iter) {
-        if ((iter->attributes & (DontDelete | ReadOnly)) != (DontDelete | ReadOnly))
+        if (!(iter->attributes & DontDelete))
+            return false;
+        if (!(iter->attributes & (ReadOnly | Accessor)))
             return false;
     }
     return true;