- Revision
- 103186
- Author
- [email protected]
- Date
- 2011-12-18 12:43:35 -0800 (Sun, 18 Dec 2011)
Log Message
HTMLAllCollection: Get rid of stateful namedItem traversal.
<http://webkit.org/b/74803>
Reviewed by Sam Weinig.
Source/WebCore:
Add a namedItemWithIndex() function to HTMLAllCollection to cover the
document.all(name, index) use-case. This moves the collection traversal
into WebCore and allows us to remove some complexity.
This incidentally fixes a bug where the CollectionCache would point to
the last node returned by document.all(name, index) without the correct
associated node index (because info()->current was getting set without
updating info()->position.) Added a layout test for that.
Test: fast/dom/htmlallcollection-call-with-index-caching-bug.html
* bindings/js/JSHTMLAllCollectionCustom.cpp:
(WebCore::callHTMLAllCollection):
* bindings/v8/custom/V8HTMLAllCollectionCustom.cpp:
(WebCore::V8HTMLAllCollection::callAsFunctionCallback):
Replace collection traversal by calls to namedItemWithIndex().
* html/HTMLCollection.h:
Promoted updateNameCache() to protected (for HTMLAllCollection.)
Demoted checkForNameMatch() to private.
* html/HTMLAllCollection.cpp:
(WebCore::HTMLAllCollection::namedItemWithIndex):
Added for document.all(name, index). Uses the name/id cache.
* html/HTMLAllCollection.cpp:
* html/HTMLAllCollection.h:
(WebCore::HTMLAllCollection::HTMLAllCollection):
Removed m_idsDone, HTMLAllCollection is now stateless.
LayoutTests:
* fast/dom/htmlallcollection-call-with-index-caching-bug-expected.txt: Added.
* fast/dom/htmlallcollection-call-with-index-caching-bug.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (103185 => 103186)
--- trunk/LayoutTests/ChangeLog 2011-12-18 20:27:44 UTC (rev 103185)
+++ trunk/LayoutTests/ChangeLog 2011-12-18 20:43:35 UTC (rev 103186)
@@ -1,3 +1,13 @@
+2011-12-18 Andreas Kling <[email protected]>
+
+ HTMLAllCollection: Get rid of stateful namedItem traversal.
+ <http://webkit.org/b/74803>
+
+ Reviewed by Sam Weinig.
+
+ * fast/dom/htmlallcollection-call-with-index-caching-bug-expected.txt: Added.
+ * fast/dom/htmlallcollection-call-with-index-caching-bug.html: Added.
+
2011-12-18 Peter Rybin <[email protected]>
Web Inspector: Switch to type-safe JSON ConsoleMessage.cpp, InspectorDOMAgent.cpp, InspectorDebuggerAgent.cpp, ScriptCallFrame.cpp
Added: trunk/LayoutTests/fast/dom/htmlallcollection-call-with-index-caching-bug-expected.txt (0 => 103186)
--- trunk/LayoutTests/fast/dom/htmlallcollection-call-with-index-caching-bug-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/htmlallcollection-call-with-index-caching-bug-expected.txt 2011-12-18 20:43:35 UTC (rev 103186)
@@ -0,0 +1,12 @@
+This tests verifies that calling document.all(name, index) doesn't affect subsequent calls to document.all.item(index)
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS document.all.item(0) is document.documentElement
+Calling document.all('foo', 0).
+PASS document.all.item(0) is document.documentElement
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/dom/htmlallcollection-call-with-index-caching-bug.html (0 => 103186)
--- trunk/LayoutTests/fast/dom/htmlallcollection-call-with-index-caching-bug.html (rev 0)
+++ trunk/LayoutTests/fast/dom/htmlallcollection-call-with-index-caching-bug.html 2011-12-18 20:43:35 UTC (rev 103186)
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html>
+<head id="foo">
+<meta charset="utf-8">
+<script src=""
+</head>
+<body>
+<script>
+
+description("This tests verifies that calling document.all(name, index) doesn't affect subsequent calls to document.all.item(index)");
+
+shouldBe("document.all.item(0)", "document.documentElement");
+debug("Calling document.all('foo', 0).");
+document.all('foo', 0)
+shouldBe("document.all.item(0)", "document.documentElement");
+
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (103185 => 103186)
--- trunk/Source/WebCore/ChangeLog 2011-12-18 20:27:44 UTC (rev 103185)
+++ trunk/Source/WebCore/ChangeLog 2011-12-18 20:43:35 UTC (rev 103186)
@@ -1,3 +1,45 @@
+2011-12-18 Andreas Kling <[email protected]>
+
+ HTMLAllCollection: Get rid of stateful namedItem traversal.
+ <http://webkit.org/b/74803>
+
+ Reviewed by Sam Weinig.
+
+ Add a namedItemWithIndex() function to HTMLAllCollection to cover the
+ document.all(name, index) use-case. This moves the collection traversal
+ into WebCore and allows us to remove some complexity.
+
+ This incidentally fixes a bug where the CollectionCache would point to
+ the last node returned by document.all(name, index) without the correct
+ associated node index (because info()->current was getting set without
+ updating info()->position.) Added a layout test for that.
+
+ Test: fast/dom/htmlallcollection-call-with-index-caching-bug.html
+
+ * bindings/js/JSHTMLAllCollectionCustom.cpp:
+ (WebCore::callHTMLAllCollection):
+ * bindings/v8/custom/V8HTMLAllCollectionCustom.cpp:
+ (WebCore::V8HTMLAllCollection::callAsFunctionCallback):
+
+ Replace collection traversal by calls to namedItemWithIndex().
+
+ * html/HTMLCollection.h:
+
+ Promoted updateNameCache() to protected (for HTMLAllCollection.)
+ Demoted checkForNameMatch() to private.
+
+ * html/HTMLAllCollection.cpp:
+ (WebCore::HTMLAllCollection::namedItemWithIndex):
+
+ Added for document.all(name, index). Uses the name/id cache.
+
+ * html/HTMLAllCollection.cpp:
+ * html/HTMLAllCollection.h:
+ (WebCore::HTMLAllCollection::HTMLAllCollection):
+
+ Removed m_idsDone, HTMLAllCollection is now stateless.
+
+
2011-12-18 Anders Carlsson <[email protected]>
The scrolling coordinator should know about the main frame scroll layer
Modified: trunk/Source/WebCore/bindings/js/JSHTMLAllCollectionCustom.cpp (103185 => 103186)
--- trunk/Source/WebCore/bindings/js/JSHTMLAllCollectionCustom.cpp 2011-12-18 20:27:44 UTC (rev 103185)
+++ trunk/Source/WebCore/bindings/js/JSHTMLAllCollectionCustom.cpp 2011-12-18 20:43:35 UTC (rev 103186)
@@ -86,14 +86,8 @@
UString string = exec->argument(0).toString(exec);
unsigned index = Identifier::toUInt32(exec->argument(1).toString(exec), ok);
if (ok) {
- AtomicString pstr = ustringToAtomicString(string);
- Node* node = collection->namedItem(pstr);
- while (node) {
- if (!index)
- return JSValue::encode(toJS(exec, jsCollection->globalObject(), node));
- node = collection->nextNamedItem(pstr);
- --index;
- }
+ if (Node* node = collection->namedItemWithIndex(ustringToAtomicString(string), index))
+ return JSValue::encode(toJS(exec, jsCollection->globalObject(), node));
}
return JSValue::encode(jsUndefined());
Modified: trunk/Source/WebCore/bindings/v8/custom/V8HTMLAllCollectionCustom.cpp (103185 => 103186)
--- trunk/Source/WebCore/bindings/v8/custom/V8HTMLAllCollectionCustom.cpp 2011-12-18 20:27:44 UTC (rev 103185)
+++ trunk/Source/WebCore/bindings/v8/custom/V8HTMLAllCollectionCustom.cpp 2011-12-18 20:43:35 UTC (rev 103186)
@@ -120,16 +120,9 @@
if (index.IsEmpty())
return v8::Undefined();
- unsigned current = index->Uint32Value();
- Node* node = imp->namedItem(name);
- while (node) {
- if (!current)
- return toV8(node);
+ if (Node* node = imp->namedItemWithIndex(name, index->Uint32Value()))
+ return toV8(node);
- node = imp->nextNamedItem(name);
- current--;
- }
-
return v8::Undefined();
}
Modified: trunk/Source/WebCore/html/HTMLAllCollection.cpp (103185 => 103186)
--- trunk/Source/WebCore/html/HTMLAllCollection.cpp 2011-12-18 20:27:44 UTC (rev 103185)
+++ trunk/Source/WebCore/html/HTMLAllCollection.cpp 2011-12-18 20:43:35 UTC (rev 103186)
@@ -38,7 +38,6 @@
HTMLAllCollection::HTMLAllCollection(Document* document)
: HTMLCollection(document, DocAll)
- , m_idsDone(false)
{
}
@@ -46,31 +45,23 @@
{
}
-Node* HTMLAllCollection::nextNamedItem(const AtomicString& name) const
+Node* HTMLAllCollection::namedItemWithIndex(const AtomicString& name, unsigned index) const
{
resetCollectionInfo();
+ updateNameCache();
info()->checkConsistency();
- for (Element* e = itemAfter(info()->current); e; e = itemAfter(e)) {
- if (checkForNameMatch(e, m_idsDone, name)) {
- info()->current = e;
- return e;
- }
+ if (Vector<Element*>* idCache = info()->idCache.get(name.impl())) {
+ if (index < idCache->size())
+ return idCache->at(index);
+ index -= idCache->size();
}
- if (m_idsDone) {
- info()->current = 0;
- return 0;
+ if (Vector<Element*>* nameCache = info()->nameCache.get(name.impl())) {
+ if (index < nameCache->size())
+ return nameCache->at(index);
}
- m_idsDone = true;
- for (Element* e = itemAfter(info()->current); e; e = itemAfter(e)) {
- if (checkForNameMatch(e, m_idsDone, name)) {
- info()->current = e;
- return e;
- }
- }
-
return 0;
}
Modified: trunk/Source/WebCore/html/HTMLAllCollection.h (103185 => 103186)
--- trunk/Source/WebCore/html/HTMLAllCollection.h 2011-12-18 20:27:44 UTC (rev 103185)
+++ trunk/Source/WebCore/html/HTMLAllCollection.h 2011-12-18 20:43:35 UTC (rev 103186)
@@ -35,13 +35,10 @@
static PassRefPtr<HTMLAllCollection> create(Document*);
virtual ~HTMLAllCollection();
- Node* nextNamedItem(const AtomicString& name) const; // In case of multiple items named the same way.
+ Node* namedItemWithIndex(const AtomicString& name, unsigned index) const;
private:
HTMLAllCollection(Document*);
-
- mutable bool m_idsDone; // for nextNamedItem()
-
};
} // namespace WebCore
Modified: trunk/Source/WebCore/html/HTMLCollection.h (103185 => 103186)
--- trunk/Source/WebCore/html/HTMLCollection.h 2011-12-18 20:27:44 UTC (rev 103185)
+++ trunk/Source/WebCore/html/HTMLCollection.h 2011-12-18 20:43:35 UTC (rev 103186)
@@ -67,14 +67,14 @@
CollectionCache* info() const { return m_info; }
void resetCollectionInfo() const;
+ virtual void updateNameCache() const;
virtual Element* itemAfter(Element*) const;
- bool checkForNameMatch(Element*, bool checkName, const AtomicString& name) const;
private:
static bool shouldIncludeChildren(CollectionType);
+ bool checkForNameMatch(Element*, bool checkName, const AtomicString& name) const;
virtual unsigned calcLength() const;
- virtual void updateNameCache() const;
bool isAcceptableElement(Element*) const;
