Use SameValue to compare property descriptor values
authorbarraclough@apple.com <barraclough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 10 Jan 2012 19:22:54 +0000 (19:22 +0000)
committerbarraclough@apple.com <barraclough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 10 Jan 2012 19:22:54 +0000 (19:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=75975

Reviewed by Sam Weinig.

Source/JavaScriptCore:

Rather than strictEqual.

* runtime/JSArray.cpp:
(JSC::JSArray::defineOwnNumericProperty):
    - Missing configurablePresent() check.
* runtime/JSObject.cpp:
(JSC::JSObject::defineOwnProperty):
    - call sameValue.
* runtime/PropertyDescriptor.cpp:
(JSC::sameValue):
    - Moved from JSArray.cpp, fix NaN comparison.
(JSC::PropertyDescriptor::equalTo):
    - call sameValue.
* runtime/PropertyDescriptor.h:
    - Added declaration for sameValue.

LayoutTests:

* fast/js/array-defineOwnProperty-expected.txt:
* fast/js/script-tests/array-defineOwnProperty.js:
    - Add new test cases.

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

LayoutTests/ChangeLog
LayoutTests/fast/js/array-defineOwnProperty-expected.txt
LayoutTests/fast/js/script-tests/array-defineOwnProperty.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/JSArray.cpp
Source/JavaScriptCore/runtime/JSObject.cpp
Source/JavaScriptCore/runtime/PropertyDescriptor.cpp
Source/JavaScriptCore/runtime/PropertyDescriptor.h

index e37b0b3..b7bb5e2 100644 (file)
@@ -1,3 +1,14 @@
+2012-01-10  Gavin Barraclough  <barraclough@apple.com>
+
+        Use SameValue to compare property descriptor values
+        https://bugs.webkit.org/show_bug.cgi?id=75975
+
+        Reviewed by Sam Weinig.
+
+        * fast/js/array-defineOwnProperty-expected.txt:
+        * fast/js/script-tests/array-defineOwnProperty.js:
+            - Add new test cases.
+
 2012-01-10  Tony Chang  <tony@chromium.org>
 
         [chromium] Fix expectations for worker-script-error.html on Mac/Linux
index c1f2c29..3a0837e 100644 (file)
@@ -17,7 +17,23 @@ FAIL var a = Object.defineProperty([42], '0', { configurable: false }); a.length
 PASS var foo = 0; Object.defineProperty([], '0', { set:function(x){foo = x;} })[0] = 42; foo is 42
 PASS Object.defineProperty([], '0', { get:function(){return true;} })[0] is true
 PASS Object.defineProperty(Object.defineProperty([true], '0', { configurable:true, writable: false }), '0', { writable: true })[0] is true
-PASS Object.defineProperty(Object.defineProperty([true], '0', { configurable:false, writable: false }), '0', { writable: true })[0] threw exception TypeError: Attempting to change configurable attribute of unconfigurable property..
+PASS Object.defineProperty(Object.defineProperty([true], '0', { configurable:false, writable: false }), '0', { writable: true })[0] threw exception TypeError: Attempting to change writable attribute of unconfigurable property..
+PASS Object.defineProperty(Object.defineProperty([], '0', { value: 1, writable:true }), '0', { value: 2 })[0] is 2
+PASS Object.defineProperty(Object.defineProperty([], '0', { value: 1 }), '0', { value: 1 })[0] is 1
+PASS Object.defineProperty(Object.defineProperty([], '0', { value: Number.NaN }), '0', { value: -Number.NaN })[0] is Number.NaN
+PASS Object.defineProperty(Object.defineProperty([], '0', { value: 'okay'.substring(0,2) }), '0', { value: 'not ok'.substring(4,6) })[0] is "ok"
+PASS Object.defineProperty(Object.defineProperty([], '0', { value: true }), '0', { value: true })[0] is true
+PASS Object.defineProperty(Object.defineProperty([], '0', { value: false }), '0', { value: false })[0] is false
+PASS Object.defineProperty(Object.defineProperty([], '0', { value: null }), '0', { value: null })[0] is null
+PASS Object.defineProperty(Object.defineProperty([], '0', { value: undefined }), '0', { value: undefined })[0] is undefined
+PASS Object.defineProperty(Object.defineProperty([], '0', { value: Math }), '0', { value: Math })[0] is Math
+PASS Object.defineProperty(Object.defineProperty([], '0', { value: 1 }), '0', { value: 2 })[0] threw exception TypeError: Attempting to change value of a readonly property..
+PASS Object.defineProperty(Object.defineProperty([], '0', { value: 'okay' }), '0', { value: 'not ok' })[0] threw exception TypeError: Attempting to change value of a readonly property..
+PASS Object.defineProperty(Object.defineProperty([], '0', { value: true }), '0', { value: false })[0] threw exception TypeError: Attempting to change value of a readonly property..
+PASS Object.defineProperty(Object.defineProperty([], '0', { value: false }), '0', { value: true })[0] threw exception TypeError: Attempting to change value of a readonly property..
+PASS Object.defineProperty(Object.defineProperty([], '0', { value: Math }), '0', { value: Object })[0] threw exception TypeError: Attempting to change value of a readonly property..
+PASS Object.defineProperty(Object.defineProperty([], '0', { value: null }), '0', { value: undefined })[0] threw exception TypeError: Attempting to change value of a readonly property..
+PASS Object.defineProperty(Object.defineProperty([], '0', { value: undefined }), '0', { value: null })[0] threw exception TypeError: Attempting to change value of a readonly property..
 PASS successfullyParsed is true
 
 TEST COMPLETE
index aeba42a..fef5414 100644 (file)
@@ -30,5 +30,25 @@ shouldBeTrue("Object.defineProperty(Object.defineProperty([true], '0', { configu
 //
 shouldThrow("Object.defineProperty(Object.defineProperty([true], '0', { configurable:false, writable: false }), '0', { writable: true })[0]");
 
+// Reassigning the value is okay if the property is writable.
+shouldBe("Object.defineProperty(Object.defineProperty([], '0', { value: 1, writable:true }), '0', { value: 2 })[0]", '2');
+// Reassigning the value is okay if the value doesn't change.
+shouldBe("Object.defineProperty(Object.defineProperty([], '0', { value: 1 }), '0', { value: 1 })[0]", '1');
+shouldBe("Object.defineProperty(Object.defineProperty([], '0', { value: Number.NaN }), '0', { value: -Number.NaN })[0]", 'Number.NaN');
+shouldBe("Object.defineProperty(Object.defineProperty([], '0', { value: 'okay'.substring(0,2) }), '0', { value: 'not ok'.substring(4,6) })[0]", '"ok"');
+shouldBe("Object.defineProperty(Object.defineProperty([], '0', { value: true }), '0', { value: true })[0]", 'true');
+shouldBe("Object.defineProperty(Object.defineProperty([], '0', { value: false }), '0', { value: false })[0]", 'false');
+shouldBe("Object.defineProperty(Object.defineProperty([], '0', { value: null }), '0', { value: null })[0]", 'null');
+shouldBe("Object.defineProperty(Object.defineProperty([], '0', { value: undefined }), '0', { value: undefined })[0]", 'undefined');
+shouldBe("Object.defineProperty(Object.defineProperty([], '0', { value: Math }), '0', { value: Math })[0]", 'Math');
+// Reassigning the value is not okay if the value changes.
+shouldThrow("Object.defineProperty(Object.defineProperty([], '0', { value: 1 }), '0', { value: 2 })[0]");
+shouldThrow("Object.defineProperty(Object.defineProperty([], '0', { value: 'okay' }), '0', { value: 'not ok' })[0]");
+shouldThrow("Object.defineProperty(Object.defineProperty([], '0', { value: true }), '0', { value: false })[0]");
+shouldThrow("Object.defineProperty(Object.defineProperty([], '0', { value: false }), '0', { value: true })[0]");
+shouldThrow("Object.defineProperty(Object.defineProperty([], '0', { value: Math }), '0', { value: Object })[0]");
+// Reassigning the value is not okay if the type changes.
+shouldThrow("Object.defineProperty(Object.defineProperty([], '0', { value: null }), '0', { value: undefined })[0]");
+shouldThrow("Object.defineProperty(Object.defineProperty([], '0', { value: undefined }), '0', { value: null })[0]");
 
 successfullyParsed = true;
index 79f8428..0697137 100644 (file)
@@ -1,3 +1,25 @@
+2012-01-10  Gavin Barraclough  <barraclough@apple.com>
+
+        Use SameValue to compare property descriptor values
+        https://bugs.webkit.org/show_bug.cgi?id=75975
+
+        Reviewed by Sam Weinig.
+
+        Rather than strictEqual.
+
+        * runtime/JSArray.cpp:
+        (JSC::JSArray::defineOwnNumericProperty):
+            - Missing configurablePresent() check.
+        * runtime/JSObject.cpp:
+        (JSC::JSObject::defineOwnProperty):
+            - call sameValue.
+        * runtime/PropertyDescriptor.cpp:
+        (JSC::sameValue):
+            - Moved from JSArray.cpp, fix NaN comparison.
+        (JSC::PropertyDescriptor::equalTo):
+            - call sameValue.
+        * runtime/PropertyDescriptor.h:
+            - Added declaration for sameValue.
 2012-01-09  Gavin Barraclough  <barraclough@apple.com>
 
         Error handling : in ISO8601 timezone
index 75759a1..f2653d4 100644 (file)
@@ -381,13 +381,6 @@ void JSArray::putDescriptor(ExecState* exec, SparseArrayEntry* entryInMap, Prope
     entryInMap->attributes = descriptor.attributesOverridingCurrent(oldDescriptor);
 }
 
-static bool sameValue(ExecState* exec, JSValue a, JSValue b)
-{
-    if (a.isNumber())
-        return b.isNumber() && bitwise_cast<uint64_t>(a.asNumber()) == bitwise_cast<uint64_t>(b.asNumber());
-    return JSValue::strictEqual(exec, a, b);
-}
-
 static bool reject(ExecState* exec, bool throwException, const char* message)
 {
     if (throwException)
@@ -461,7 +454,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.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())
index 15d3dd2..a813e84 100644 (file)
@@ -786,7 +786,7 @@ bool JSObject::defineOwnProperty(JSObject* object, ExecState* exec, const Identi
                 return false;
             }
             if (!current.writable()) {
-                if (descriptor.value() || !JSValue::strictEqual(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;
index cb4c003..c664952 100644 (file)
@@ -162,16 +162,30 @@ void PropertyDescriptor::setGetter(JSValue getter)
     m_attributes &= ~ReadOnly;
 }
 
+// See ES5.1 9.12
+bool sameValue(ExecState* exec, JSValue a, JSValue b)
+{
+    if (!a.isNumber())
+        return JSValue::strictEqual(exec, a, b);
+    if (!b.isNumber())
+        return false;
+    double x = a.asNumber();
+    double y = b.asNumber();
+    if (isnan(x))
+        return isnan(y);
+    return bitwise_cast<uint64_t>(x) == bitwise_cast<uint64_t>(y);
+}
+
 bool PropertyDescriptor::equalTo(ExecState* exec, const PropertyDescriptor& other) const
 {
     if (!other.m_value == m_value ||
         !other.m_getter == m_getter ||
         !other.m_setter == m_setter)
         return false;
-    return (!m_value || JSValue::strictEqual(exec, other.m_value, m_value)) && 
-           (!m_getter || JSValue::strictEqual(exec, other.m_getter, m_getter)) && 
-           (!m_setter || JSValue::strictEqual(exec, other.m_setter, m_setter)) &&
-           attributesEqual(other);
+    return (!m_value || sameValue(exec, other.m_value, m_value))
+        && (!m_getter || JSValue::strictEqual(exec, other.m_getter, m_getter))
+        && (!m_setter || JSValue::strictEqual(exec, other.m_setter, m_setter))
+        && attributesEqual(other);
 }
 
 bool PropertyDescriptor::attributesEqual(const PropertyDescriptor& other) const
index 8bf36c8..3d481b2 100644 (file)
@@ -31,6 +31,9 @@
 namespace JSC {
     class GetterSetter;
 
+    // See ES5.1 9.12
+    bool sameValue(ExecState*, JSValue, JSValue);
+
     class PropertyDescriptor {
     public:
         PropertyDescriptor()