Title: [104602] trunk
Revision
104602
Author
barraclo...@apple.com
Date
2012-01-10 11:22:54 -0800 (Tue, 10 Jan 2012)

Log Message

Use SameValue to compare property descriptor values
https://bugs.webkit.org/show_bug.cgi?id=75975

Reviewed by Sam Weinig.

Source/_javascript_Core: 

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.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (104601 => 104602)


--- trunk/LayoutTests/ChangeLog	2012-01-10 19:21:14 UTC (rev 104601)
+++ trunk/LayoutTests/ChangeLog	2012-01-10 19:22:54 UTC (rev 104602)
@@ -1,3 +1,14 @@
+2012-01-10  Gavin Barraclough  <barraclo...@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  <t...@chromium.org>
 
         [chromium] Fix expectations for worker-script-error.html on Mac/Linux

Modified: trunk/LayoutTests/fast/js/array-defineOwnProperty-expected.txt (104601 => 104602)


--- trunk/LayoutTests/fast/js/array-defineOwnProperty-expected.txt	2012-01-10 19:21:14 UTC (rev 104601)
+++ trunk/LayoutTests/fast/js/array-defineOwnProperty-expected.txt	2012-01-10 19:22:54 UTC (rev 104602)
@@ -17,7 +17,23 @@
 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

Modified: trunk/LayoutTests/fast/js/script-tests/array-defineOwnProperty.js (104601 => 104602)


--- trunk/LayoutTests/fast/js/script-tests/array-defineOwnProperty.js	2012-01-10 19:21:14 UTC (rev 104601)
+++ trunk/LayoutTests/fast/js/script-tests/array-defineOwnProperty.js	2012-01-10 19:22:54 UTC (rev 104602)
@@ -30,5 +30,25 @@
 //
 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;

Modified: trunk/Source/_javascript_Core/ChangeLog (104601 => 104602)


--- trunk/Source/_javascript_Core/ChangeLog	2012-01-10 19:21:14 UTC (rev 104601)
+++ trunk/Source/_javascript_Core/ChangeLog	2012-01-10 19:22:54 UTC (rev 104602)
@@ -1,3 +1,25 @@
+2012-01-10  Gavin Barraclough  <barraclo...@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  <barraclo...@apple.com>
 
         Error handling : in ISO8601 timezone

Modified: trunk/Source/_javascript_Core/runtime/JSArray.cpp (104601 => 104602)


--- trunk/Source/_javascript_Core/runtime/JSArray.cpp	2012-01-10 19:21:14 UTC (rev 104601)
+++ trunk/Source/_javascript_Core/runtime/JSArray.cpp	2012-01-10 19:22:54 UTC (rev 104602)
@@ -381,13 +381,6 @@
     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 @@
     // 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())

Modified: trunk/Source/_javascript_Core/runtime/JSObject.cpp (104601 => 104602)


--- trunk/Source/_javascript_Core/runtime/JSObject.cpp	2012-01-10 19:21:14 UTC (rev 104601)
+++ trunk/Source/_javascript_Core/runtime/JSObject.cpp	2012-01-10 19:22:54 UTC (rev 104602)
@@ -786,7 +786,7 @@
                 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;

Modified: trunk/Source/_javascript_Core/runtime/PropertyDescriptor.cpp (104601 => 104602)


--- trunk/Source/_javascript_Core/runtime/PropertyDescriptor.cpp	2012-01-10 19:21:14 UTC (rev 104601)
+++ trunk/Source/_javascript_Core/runtime/PropertyDescriptor.cpp	2012-01-10 19:22:54 UTC (rev 104602)
@@ -162,16 +162,30 @@
     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

Modified: trunk/Source/_javascript_Core/runtime/PropertyDescriptor.h (104601 => 104602)


--- trunk/Source/_javascript_Core/runtime/PropertyDescriptor.h	2012-01-10 19:21:14 UTC (rev 104601)
+++ trunk/Source/_javascript_Core/runtime/PropertyDescriptor.h	2012-01-10 19:22:54 UTC (rev 104602)
@@ -31,6 +31,9 @@
 namespace JSC {
     class GetterSetter;
 
+    // See ES5.1 9.12
+    bool sameValue(ExecState*, JSValue, JSValue);
+
     class PropertyDescriptor {
     public:
         PropertyDescriptor()
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to