- Revision
- 129317
- Author
- [email protected]
- Date
- 2012-09-23 16:37:01 -0700 (Sun, 23 Sep 2012)
Log Message
Sorting a non-array creates propreties (spec-violation)
https://bugs.webkit.org/show_bug.cgi?id=25477
Reviewed by Oliver Hunt.
Source/_javascript_Core:
We're just calling get() to get properties, which is converting missing properties to
undefined. Hole values should be retained, and moved to the end of the array.
* runtime/ArrayPrototype.cpp:
(JSC::getOrHole):
- Helper function, returns JSValue() instead of undefined for missing properties.
(JSC::arrayProtoFuncSort):
- Implemented per 15.4.4.11, see comments above.
LayoutTests:
Added test cases.
* fast/js/array-sort-sparse-expected.txt: Added.
* fast/js/array-sort-sparse.html: Added.
* fast/js/script-tests/array-sort-sparse.js: Added.
(testSort):
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (129316 => 129317)
--- trunk/LayoutTests/ChangeLog 2012-09-23 22:48:19 UTC (rev 129316)
+++ trunk/LayoutTests/ChangeLog 2012-09-23 23:37:01 UTC (rev 129317)
@@ -1,3 +1,17 @@
+2012-09-23 Gavin Barraclough <[email protected]>
+
+ Sorting a non-array creates propreties (spec-violation)
+ https://bugs.webkit.org/show_bug.cgi?id=25477
+
+ Reviewed by Oliver Hunt.
+
+ Added test cases.
+
+ * fast/js/array-sort-sparse-expected.txt: Added.
+ * fast/js/array-sort-sparse.html: Added.
+ * fast/js/script-tests/array-sort-sparse.js: Added.
+ (testSort):
+
2012-09-22 Zan Dobersek <[email protected]>
Unreviewed GTK gardening.
Added: trunk/LayoutTests/fast/js/array-sort-sparse-expected.txt (0 => 129317)
--- trunk/LayoutTests/fast/js/array-sort-sparse-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/js/array-sort-sparse-expected.txt 2012-09-23 23:37:01 UTC (rev 129317)
@@ -0,0 +1,11 @@
+This tests that arrays and array like objects containing holes are sorted correctly.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS testSort([,undefined,0,1]) is true
+PASS testSort({length:4,1:undefined,2:0,3:1}) is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/js/array-sort-sparse.html (0 => 129317)
--- trunk/LayoutTests/fast/js/array-sort-sparse.html (rev 0)
+++ trunk/LayoutTests/fast/js/array-sort-sparse.html 2012-09-23 23:37:01 UTC (rev 129317)
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script src=""
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/fast/js/script-tests/array-sort-sparse.js (0 => 129317)
--- trunk/LayoutTests/fast/js/script-tests/array-sort-sparse.js (rev 0)
+++ trunk/LayoutTests/fast/js/script-tests/array-sort-sparse.js 2012-09-23 23:37:01 UTC (rev 129317)
@@ -0,0 +1,12 @@
+description(
+"This tests that arrays and array like objects containing holes are sorted correctly."
+);
+
+function testSort(x)
+{
+ [].sort.call(x)
+ return x[0] < x[1] && x[2] === undefined && !(3 in x) && x.length == 4;
+}
+
+shouldBeTrue("testSort([,undefined,0,1])");
+shouldBeTrue("testSort({length:4,1:undefined,2:0,3:1})");
Modified: trunk/Source/_javascript_Core/ChangeLog (129316 => 129317)
--- trunk/Source/_javascript_Core/ChangeLog 2012-09-23 22:48:19 UTC (rev 129316)
+++ trunk/Source/_javascript_Core/ChangeLog 2012-09-23 23:37:01 UTC (rev 129317)
@@ -1,3 +1,19 @@
+2012-09-23 Gavin Barraclough <[email protected]>
+
+ Sorting a non-array creates propreties (spec-violation)
+ https://bugs.webkit.org/show_bug.cgi?id=25477
+
+ Reviewed by Oliver Hunt.
+
+ We're just calling get() to get properties, which is converting missing properties to
+ undefined. Hole values should be retained, and moved to the end of the array.
+
+ * runtime/ArrayPrototype.cpp:
+ (JSC::getOrHole):
+ - Helper function, returns JSValue() instead of undefined for missing properties.
+ (JSC::arrayProtoFuncSort):
+ - Implemented per 15.4.4.11, see comments above.
+
2012-09-23 Geoffrey Garen <[email protected]>
CSE for access to closure variables (get_/put_scoped_var)
Modified: trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp (129316 => 129317)
--- trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp 2012-09-23 22:48:19 UTC (rev 129316)
+++ trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp 2012-09-23 23:37:01 UTC (rev 129317)
@@ -629,6 +629,15 @@
return JSValue::encode(result);
}
+inline JSValue getOrHole(JSObject* obj, ExecState* exec, unsigned propertyName)
+{
+ PropertySlot slot(obj);
+ if (obj->getPropertySlot(exec, propertyName, slot))
+ return slot.getValue(exec, propertyName);
+
+ return JSValue();
+}
+
EncodedJSValue JSC_HOST_CALL arrayProtoFuncSort(ExecState* exec)
{
JSObject* thisObj = exec->hostThisValue().toObject(exec);
@@ -653,17 +662,21 @@
// "Min" sort. Not the fastest, but definitely less code than heapsort
// or quicksort, and much less swapping than bubblesort/insertionsort.
for (unsigned i = 0; i < length - 1; ++i) {
- JSValue iObj = thisObj->get(exec, i);
+ JSValue iObj = getOrHole(thisObj, exec, i);
if (exec->hadException())
return JSValue::encode(jsUndefined());
unsigned themin = i;
JSValue minObj = iObj;
for (unsigned j = i + 1; j < length; ++j) {
- JSValue jObj = thisObj->get(exec, j);
+ JSValue jObj = getOrHole(thisObj, exec, j);
if (exec->hadException())
return JSValue::encode(jsUndefined());
double compareResult;
- if (jObj.isUndefined())
+ if (!jObj)
+ compareResult = 1;
+ else if (!minObj)
+ compareResult = -1;
+ else if (jObj.isUndefined())
compareResult = 1; // don't check minObj because there's no need to differentiate == (0) from > (1)
else if (minObj.isUndefined())
compareResult = -1;
@@ -682,12 +695,22 @@
}
// Swap themin and i
if (themin > i) {
- thisObj->methodTable()->putByIndex(thisObj, exec, i, minObj, true);
- if (exec->hadException())
+ if (minObj) {
+ thisObj->methodTable()->putByIndex(thisObj, exec, i, minObj, true);
+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
+ } else if (!thisObj->methodTable()->deletePropertyByIndex(thisObj, exec, i)) {
+ throwTypeError(exec, "Unable to delete property.");
return JSValue::encode(jsUndefined());
- thisObj->methodTable()->putByIndex(thisObj, exec, themin, iObj, true);
- if (exec->hadException())
+ }
+ if (iObj) {
+ thisObj->methodTable()->putByIndex(thisObj, exec, themin, iObj, true);
+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
+ } else if (!thisObj->methodTable()->deletePropertyByIndex(thisObj, exec, themin)) {
+ throwTypeError(exec, "Unable to delete property.");
return JSValue::encode(jsUndefined());
+ }
}
}
return JSValue::encode(thisObj);
