Title: [122930] trunk
- Revision
- 122930
- Author
- [email protected]
- Date
- 2012-07-18 01:16:31 -0700 (Wed, 18 Jul 2012)
Log Message
REGRESSION(r122345): HTMLCollection::length() sometimes returns a wrong value
https://bugs.webkit.org/show_bug.cgi?id=91587
Reviewed by Benjamin Poulain.
Source/WebCore:
The bug was caused by my douchey code that set the length cache to be the *offset*
of the last item in a HTMLCollection. Clearly, the length of a collection that contains
the last item at offset n is n + 1. Fixed that.
Also removed the call to setLengthCache in HTMLCollection::length since it must have set
by previous calls to itemBeforeOrAfterCachedItem already. This will allow us to catch
regressions like this in ports that use JSC bindings as well.
Test: fast/dom/htmlcollection-length-after-item.html
* html/HTMLCollection.cpp:
(WebCore::HTMLCollection::length):
(WebCore::HTMLCollection::itemBeforeOrAfterCachedItem):
LayoutTests:
Add a regression test. It only fails on Chromium port before the patch is applied because JSC binding code
has a bug (?) that it always checks index > length() to throw an exception before accessing an item in HTMLCollection.
* fast/dom/htmlcollection-length-after-item-expected.txt: Added.
* fast/dom/htmlcollection-length-after-item.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (122929 => 122930)
--- trunk/LayoutTests/ChangeLog 2012-07-18 08:14:32 UTC (rev 122929)
+++ trunk/LayoutTests/ChangeLog 2012-07-18 08:16:31 UTC (rev 122930)
@@ -1,3 +1,16 @@
+2012-07-18 Ryosuke Niwa <[email protected]>
+
+ REGRESSION(r122345): HTMLCollection::length() sometimes returns a wrong value
+ https://bugs.webkit.org/show_bug.cgi?id=91587
+
+ Reviewed by Benjamin Poulain.
+
+ Add a regression test. It only fails on Chromium port before the patch is applied because JSC binding code
+ has a bug (?) that it always checks index > length() to throw an exception before accessing an item in HTMLCollection.
+
+ * fast/dom/htmlcollection-length-after-item-expected.txt: Added.
+ * fast/dom/htmlcollection-length-after-item.html: Added.
+
2012-07-18 Vsevolod Vlasov <[email protected]>
Unreviewed chromium gardening, rebaselined tests.
Added: trunk/LayoutTests/fast/dom/htmlcollection-length-after-item-expected.txt (0 => 122930)
--- trunk/LayoutTests/fast/dom/htmlcollection-length-after-item-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/htmlcollection-length-after-item-expected.txt 2012-07-18 08:16:31 UTC (rev 122930)
@@ -0,0 +1,9 @@
+This tests accessing the length after accessing (length + 1)-th item in HTMLCollection doesn't cache a wrong length.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS children = container.children; children.length is 0
+PASS container.appendChild(span); children[1]; children.length is 1
+PASS container.removeChild(span); children.length is 0
+
Added: trunk/LayoutTests/fast/dom/htmlcollection-length-after-item.html (0 => 122930)
--- trunk/LayoutTests/fast/dom/htmlcollection-length-after-item.html (rev 0)
+++ trunk/LayoutTests/fast/dom/htmlcollection-length-after-item.html 2012-07-18 08:16:31 UTC (rev 122930)
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script>
+
+description("This tests accessing the length after accessing (length + 1)-th item in HTMLCollection doesn't cache a wrong length.");
+
+var container = document.createElement('div');
+var span = document.createElement('span');
+var children;
+shouldBe("children = container.children; children.length", "0");
+shouldBe("container.appendChild(span); children[1]; children.length", "1");
+shouldBe("container.removeChild(span); children.length", "0");
+
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (122929 => 122930)
--- trunk/Source/WebCore/ChangeLog 2012-07-18 08:14:32 UTC (rev 122929)
+++ trunk/Source/WebCore/ChangeLog 2012-07-18 08:16:31 UTC (rev 122930)
@@ -1,3 +1,24 @@
+2012-07-18 Ryosuke Niwa <[email protected]>
+
+ REGRESSION(r122345): HTMLCollection::length() sometimes returns a wrong value
+ https://bugs.webkit.org/show_bug.cgi?id=91587
+
+ Reviewed by Benjamin Poulain.
+
+ The bug was caused by my douchey code that set the length cache to be the *offset*
+ of the last item in a HTMLCollection. Clearly, the length of a collection that contains
+ the last item at offset n is n + 1. Fixed that.
+
+ Also removed the call to setLengthCache in HTMLCollection::length since it must have set
+ by previous calls to itemBeforeOrAfterCachedItem already. This will allow us to catch
+ regressions like this in ports that use JSC bindings as well.
+
+ Test: fast/dom/htmlcollection-length-after-item.html
+
+ * html/HTMLCollection.cpp:
+ (WebCore::HTMLCollection::length):
+ (WebCore::HTMLCollection::itemBeforeOrAfterCachedItem):
+
2012-07-18 Yoshifumi Inoue <[email protected]>
Decimal::toString should not round integer value.
Modified: trunk/Source/WebCore/html/HTMLCollection.cpp (122929 => 122930)
--- trunk/Source/WebCore/html/HTMLCollection.cpp 2012-07-18 08:14:32 UTC (rev 122929)
+++ trunk/Source/WebCore/html/HTMLCollection.cpp 2012-07-18 08:16:31 UTC (rev 122930)
@@ -324,8 +324,8 @@
do {
offset++;
} while (itemBeforeOrAfterCachedItem(offset));
+ ASSERT(isLengthCacheValid());
- setLengthCache(offset);
return offset;
}
@@ -397,7 +397,8 @@
}
}
- setLengthCache(currentOffset);
+ unsigned offsetOfLastItem = currentOffset;
+ setLengthCache(offsetOfLastItem + 1);
return 0;
}
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes
