Only HTMLAllCollection needs stateful named item traversal.
authorandreas.kling@nokia.com <andreas.kling@nokia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 13 Nov 2011 14:53:48 +0000 (14:53 +0000)
committerandreas.kling@nokia.com <andreas.kling@nokia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 13 Nov 2011 14:53:48 +0000 (14:53 +0000)
<http://webkit.org/b/71987>

Reviewed by Antti Koivisto.

After the ability to call collections (excluding document.all) was removed
in <http://webkit.org/b/67579>, chunks of code remained in HTMLCollection
and HTMLFormCollection that was only used for this purpose.

Restrict the nextNamedItem() mechanism to HTMLAllCollection, since it's no
longer used by any of the other collections.

* html/HTMLCollection.h:

    Moved nextNamedItem() to HTMLAllCollection (and made it non-virtual.)
    Promoted itemAfter() and checkForNameMatch() to protected methods so
    they can be called from HTMLAllCollection.

* html/HTMLAllCollection.h:
* html/HTMLAllCollection.cpp:
(WebCore::HTMLAllCollection::HTMLAllCollection):
(WebCore::HTMLAllCollection::nextNamedItem):
* html/HTMLCollection.cpp:
(WebCore::HTMLCollection::HTMLCollection):
(WebCore::HTMLCollection::namedItem):
* html/HTMLFormCollection.cpp:
(WebCore::HTMLFormCollection::namedItem):
* html/HTMLFormCollection.h:

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

Source/WebCore/ChangeLog
Source/WebCore/html/HTMLAllCollection.cpp
Source/WebCore/html/HTMLAllCollection.h
Source/WebCore/html/HTMLCollection.cpp
Source/WebCore/html/HTMLCollection.h
Source/WebCore/html/HTMLFormCollection.cpp
Source/WebCore/html/HTMLFormCollection.h

index 58b2329..1f1274f 100644 (file)
@@ -1,3 +1,34 @@
+2011-11-11  Andreas Kling  <kling@webkit.org>
+
+        Only HTMLAllCollection needs stateful named item traversal.
+        <http://webkit.org/b/71987>
+
+        Reviewed by Antti Koivisto.
+
+        After the ability to call collections (excluding document.all) was removed
+        in <http://webkit.org/b/67579>, chunks of code remained in HTMLCollection
+        and HTMLFormCollection that was only used for this purpose.
+
+        Restrict the nextNamedItem() mechanism to HTMLAllCollection, since it's no
+        longer used by any of the other collections.
+
+        * html/HTMLCollection.h:
+
+            Moved nextNamedItem() to HTMLAllCollection (and made it non-virtual.)
+            Promoted itemAfter() and checkForNameMatch() to protected methods so
+            they can be called from HTMLAllCollection.
+
+        * html/HTMLAllCollection.h:
+        * html/HTMLAllCollection.cpp:
+        (WebCore::HTMLAllCollection::HTMLAllCollection):
+        (WebCore::HTMLAllCollection::nextNamedItem):
+        * html/HTMLCollection.cpp:
+        (WebCore::HTMLCollection::HTMLCollection):
+        (WebCore::HTMLCollection::namedItem):
+        * html/HTMLFormCollection.cpp:
+        (WebCore::HTMLFormCollection::namedItem):
+        * html/HTMLFormCollection.h:
+
 2011-11-13  Caio Marcelo de Oliveira Filho  <caio.oliveira@openbossa.org>
 
         Update binding generator tests to use jsCast
index dbfed28..d6272b7 100644 (file)
@@ -26,7 +26,8 @@
 #include "config.h"
 #include "HTMLAllCollection.h"
 
-#include "Node.h"
+#include "CollectionCache.h"
+#include "Element.h"
 
 namespace WebCore {
 
@@ -37,6 +38,7 @@ PassRefPtr<HTMLAllCollection> HTMLAllCollection::create(PassRefPtr<Node> base)
 
 HTMLAllCollection::HTMLAllCollection(PassRefPtr<Node> base)
     : HTMLCollection(base, DocAll)
+    , m_idsDone(false)
 {
 }
 
@@ -44,4 +46,32 @@ HTMLAllCollection::~HTMLAllCollection()
 {
 }
 
+Node* HTMLAllCollection::nextNamedItem(const AtomicString& name) const
+{
+    resetCollectionInfo();
+    info()->checkConsistency();
+
+    for (Element* e = itemAfter(info()->current); e; e = itemAfter(e)) {
+        if (checkForNameMatch(e, m_idsDone, name)) {
+            info()->current = e;
+            return e;
+        }
+    }
+
+    if (m_idsDone) {
+        info()->current = 0;
+        return 0;
+    }
+    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;
+}
+
 } // namespace WebCore
index 1dd3ede..3da086c 100644 (file)
@@ -35,8 +35,13 @@ public:
     static PassRefPtr<HTMLAllCollection> create(PassRefPtr<Node>);
     virtual ~HTMLAllCollection();
 
+    Node* nextNamedItem(const AtomicString& name) const; // In case of multiple items named the same way.
+
 private:
     HTMLAllCollection(PassRefPtr<Node>);
+
+    mutable bool m_idsDone; // for nextNamedItem()
+
 };
 
 } // namespace WebCore
index 163c6a1..c886abd 100644 (file)
@@ -37,8 +37,7 @@ namespace WebCore {
 using namespace HTMLNames;
 
 HTMLCollection::HTMLCollection(PassRefPtr<Node> base, CollectionType type)
-    : m_idsDone(false)
-    , m_ownsInfo(false)
+    : m_ownsInfo(false)
     , m_type(type)
     , m_base(base)
     , m_info(m_base->isDocumentNode() ? static_cast<Document*>(m_base.get())->collectionInfo(type) : 0)
@@ -46,8 +45,7 @@ HTMLCollection::HTMLCollection(PassRefPtr<Node> base, CollectionType type)
 }
 
 HTMLCollection::HTMLCollection(PassRefPtr<Node> base, CollectionType type, CollectionCache* info)
-    : m_idsDone(false)
-    , m_ownsInfo(false)
+    : m_ownsInfo(false)
     , m_type(type)
     , m_base(base)
     , m_info(info)
@@ -288,19 +286,16 @@ Node* HTMLCollection::namedItem(const AtomicString& name) const
     // object with a matching name attribute, but only on those elements
     // that are allowed a name attribute.
     resetCollectionInfo();
-    m_idsDone = false;
 
     for (Element* e = itemAfter(0); e; e = itemAfter(e)) {
-        if (checkForNameMatch(e, m_idsDone, name)) {
+        if (checkForNameMatch(e, /* checkName */ false, name)) {
             m_info->current = e;
             return e;
         }
     }
-        
-    m_idsDone = true;
 
     for (Element* e = itemAfter(0); e; e = itemAfter(e)) {
-        if (checkForNameMatch(e, m_idsDone, name)) {
+        if (checkForNameMatch(e, /* checkName */ true, name)) {
             m_info->current = e;
             return e;
         }
@@ -370,35 +365,6 @@ void HTMLCollection::namedItems(const AtomicString& name, Vector<RefPtr<Node> >&
         result.append(nameResults->at(i));
 }
 
-
-Node* HTMLCollection::nextNamedItem(const AtomicString& name) const
-{
-    resetCollectionInfo();
-    m_info->checkConsistency();
-
-    for (Element* e = itemAfter(m_info->current); e; e = itemAfter(e)) {
-        if (checkForNameMatch(e, m_idsDone, name)) {
-            m_info->current = e;
-            return e;
-        }
-    }
-    
-    if (m_idsDone) {
-        m_info->current = 0; 
-        return 0;
-    }
-    m_idsDone = true;
-
-    for (Element* e = itemAfter(m_info->current); e; e = itemAfter(e)) {
-        if (checkForNameMatch(e, m_idsDone, name)) {
-            m_info->current = e;
-            return e;
-        }
-    }
-
-    return 0;
-}
-
 PassRefPtr<NodeList> HTMLCollection::tags(const String& name)
 {
     return m_base->getElementsByTagName(name);
index f3f855e..8522f5b 100644 (file)
@@ -48,7 +48,6 @@ public:
     virtual Node* nextItem() const;
 
     virtual Node* namedItem(const AtomicString& name) const;
-    virtual Node* nextNamedItem(const AtomicString& name) const; // In case of multiple items named the same way
 
     Node* firstItem() const;
 
@@ -66,14 +65,13 @@ protected:
     CollectionCache* info() const { return m_info; }
     void resetCollectionInfo() const;
 
-    mutable bool m_idsDone : 1; // for nextNamedItem()
+    virtual Element* itemAfter(Element*) const;
+    bool checkForNameMatch(Element*, bool checkName, const AtomicString& name) const;
 
 private:
-    virtual Element* itemAfter(Element*) const;
     virtual unsigned calcLength() const;
     virtual void updateNameCache() const;
 
-    bool checkForNameMatch(Element*, bool checkName, const AtomicString& name) const;
     mutable bool m_ownsInfo : 1;
     unsigned m_type : 5; // CollectionType
 
index 4f52b9d..d0720c1 100644 (file)
@@ -139,18 +139,6 @@ Node* HTMLFormCollection::nextItem() const
     return item(info()->position + 1);
 }
 
-Element* HTMLFormCollection::nextNamedItemInternal(const String &name) const
-{
-    Element* retval = getNamedFormItem(m_idsDone ? nameAttr : idAttr, name, ++info()->position);
-    if (retval)
-        return retval;
-    if (m_idsDone) // we're done
-        return 0;
-    // After doing id, do name
-    m_idsDone = true;
-    return getNamedItem(nameAttr, name);
-}
-
 Node* HTMLFormCollection::namedItem(const AtomicString& name) const
 {
     // http://msdn.microsoft.com/workshop/author/dhtml/reference/methods/nameditem.asp
@@ -159,29 +147,13 @@ Node* HTMLFormCollection::namedItem(const AtomicString& name) const
     // object with a matching name attribute, but only on those elements
     // that are allowed a name attribute.
     resetCollectionInfo();
-    m_idsDone = false;
     info()->current = getNamedItem(idAttr, name);
     if (info()->current)
         return info()->current;
-    m_idsDone = true;
     info()->current = getNamedItem(nameAttr, name);
     return info()->current;
 }
 
-Node* HTMLFormCollection::nextNamedItem(const AtomicString& name) const
-{
-    // The nextNamedItemInternal function can return the same item twice if it has
-    // both an id and name that are equal to the name parameter. So this function
-    // checks if we are on the nameAttr half of the iteration and skips over any
-    // that also have the same idAttributeName.
-    Element* impl = nextNamedItemInternal(name);
-    if (m_idsDone) {
-        while (impl && impl->getIdAttribute() == name)
-            impl = nextNamedItemInternal(name);
-    }
-    return impl;
-}
-
 void HTMLFormCollection::updateNameCache() const
 {
     if (info()->hasNameCache)
index a204446..e2fee91 100644 (file)
@@ -43,7 +43,6 @@ public:
     virtual Node* nextItem() const;
 
     virtual Node* namedItem(const AtomicString& name) const;
-    virtual Node* nextNamedItem(const AtomicString& name) const;
 
 private:
     HTMLFormCollection(PassRefPtr<HTMLFormElement>);
@@ -54,8 +53,6 @@ private:
     static CollectionCache* formCollectionInfo(HTMLFormElement*);
 
     Element* getNamedItem(const QualifiedName& attrName, const AtomicString& name) const;
-    Element* nextNamedItemInternal(const String& name) const;
-
     Element* getNamedFormItem(const QualifiedName& attrName, const String& name, int duplicateNumber) const;
 
     mutable int currentPos;