Minor tweaks to HTMLCollection
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 11 Nov 2014 17:35:29 +0000 (17:35 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 11 Nov 2014 17:35:29 +0000 (17:35 +0000)
https://bugs.webkit.org/show_bug.cgi?id=138556

Reviewed by Chris Dumez.

* accessibility/AccessibilityRenderObject.cpp:
(WebCore::AccessibilityRenderObject::addRadioButtonGroupMembers):
Update for namedElements function that now returns a Vector; also use
a new style for loop to iterate it.

* bindings/js/JSHTMLAllCollectionCustom.cpp: Removed some unneeded includes.
(WebCore::namedItems): Updated name to match WebKit coding style, and also
updated to use the return value from namedItems, which now returns a Vector.
(WebCore::callHTMLAllCollection): Updated for namedItems name change.
Also removed explicit Node* type from result of namedItemWithIndex, since that
function now returns a more specific type.
(WebCore::JSHTMLAllCollection::nameGetter): Update for namedItems name change.
(WebCore::JSHTMLAllCollection::item): Ditto.
(WebCore::JSHTMLAllCollection::namedItem): Ditto.

* bindings/js/JSHTMLFormControlsCollectionCustom.cpp: Removed some unneeded includes.
(WebCore::namedItems): Updated name to match WebKit coding style, and also
updated to use the return value from namedItems, which now returns a Vector.
(WebCore::JSHTMLFormControlsCollection::nameGetter): Update for namedItems name change.
(WebCore::JSHTMLFormControlsCollection::namedItem): Ditto.

* bindings/js/JSHTMLFormElementCustom.cpp:
(WebCore::JSHTMLFormElement::nameGetter): Updated to use the return value from
namedItems, which now returns a Vector.

* html/HTMLAllCollection.cpp:
(WebCore::HTMLAllCollection::HTMLAllCollection): Marked the constructor inline,
since it's only used in one place, the create function.
(WebCore::HTMLAllCollection::~HTMLAllCollection): Deleted. No need to have an
explicit destructor since there's nothing special to implement, and includers of
the header file have everything they ened to compile the compiler-generated one.
(WebCore::HTMLAllCollection::namedItemWithIndex): Changed return type to Element.

* html/HTMLAllCollection.h: Removed unneeded explicit declaration of destructor.
Chagned return type of namedItemWithIndex to Element.

* html/HTMLCollection.cpp:
(WebCore::HTMLCollection::rootTypeFromCollectionType): Marked this inline. Also
changed this to be a static member function so it can use the RootType enum, which
is now private to the class.
(WebCore::isMatchingHTMLElement): Marked this function static so it will get
internal linkage.
(WebCore::isMatchingElement): Ditto.
(WebCore::previousElement): Marked this function inline since it's called in only
one place. Changed argument type to a reference since it can never be null.
(WebCore::HTMLCollection::iterateForPreviousElement): Changed argument name and
also updated for above changes.
(WebCore::firstMatchingElement): Marked this function static so it will get
internal linkage.
(WebCore::nextMatchingElement): Ditto. Changed argument type to a reference
since it can never be null.
(WebCore::HTMLCollection::item): Changed return type to Element.
(WebCore::nameShouldBeVisibleInDocumentAll): Added an overload that takes an
Element. This streamlines the code below that calls it so it fits on one line.
(WebCore::firstMatchingChildElement): Marked this function static so it will get
internal linkage.
(WebCore::nextMatchingSiblingElement): Ditto. Changed argument type to a reference
since it can never be null.
(WebCore::HTMLCollection::usesCustomForwardOnlyTraversal): Moved here from the
header since, although it's marked inline, it's only used inside this file.
(WebCore::HTMLCollection::traverseForward): Restructured the code a little bit
to make the function smaller and possibly easier to read. This does add one
redundant null check, but it seems OK to do that.
(WebCore::HTMLCollection::collectionTraverseBackward): Tweaked foramtting a bit.
(WebCore::HTMLCollection::namedItem): Changed return type to Element. Tightened
the code that calls nameShouldBeVisibleInDocumentAll so it fits better on one line.
Changed code that handles m_shouldOnlyIncludeDirectChildren to use a nested if
instead of an && since it makes the code a little easier to read.
(WebCore::HTMLCollection::updateNamedElementCache): Tweaked code a little bit,
using shorter variable names, and using references instead of pointers. Also removed
the call to didPopulate, since setNamedItemCache now takes care of that.
(WebCore::HTMLCollection::namedItems): Changed to return a Vector instead of
appending to an existing one. Also use reserveInitialCapacity and uncheckedAppend
for better performance. Added a FIXME, because there seems to be something wrong
here about this being non-virtual. Made other small tweaks to streamline the code.
(WebCore::HTMLCollection::customElementAfter): Moved this here from the header.
There is no reason to need to inline this.

* html/HTMLCollection.h: Removed unneeded includes. Moved function bodies out
of the class definitions so the class definitions are easier to read. Made some
functions that were formerly public or protected be private instead. Added a call
to didPopulate to setNamedItemCache so the callers don't have to do it.

* html/HTMLFormControlsCollection.cpp:
(WebCore::HTMLFormControlsCollection::namedItem): Changed return value to Element.
Tweaked coding style a little bit.
(WebCore::HTMLFormControlsCollection::updateNamedElementCache): Rearranged to
simplify a bit. Don't build the foundInputElements set when the owner is not a
form element, since we don't use the set in that case. Use shorter variable names,
and modern for loops. Also removed the call to didPopulate, since setNamedItemCache
now takes care of that.

* html/HTMLFormControlsCollection.h: Removed some uneeded forward declarations.
Updated return type for namedItem, and also made the override private.

* html/HTMLFormElement.cpp:
(WebCore::HTMLFormElement::namedElements): Changed to return a Vector and updated
function name accordingly.
* html/HTMLFormElement.h: Ditto.

* html/HTMLNameCollection.h: Removed a stray blank line.

* html/HTMLSelectElement.cpp:
(WebCore::HTMLSelectElement::namedItem): Changed return value to Element.
(WebCore::HTMLSelectElement::item): Ditto.
* html/HTMLSelectElement.h: Ditto.

* page/scrolling/AxisScrollSnapOffsets.cpp:
(WebCore::appendChildSnapOffsets): Rewrote loop as a for loop rather than a while
loop. Removed unwanted use of children()->collectionBegin() to get the first element
child of an HTMLElement. This function uses a mix of DOM and rendering functions that
is probably incorrect, but I did not tackle fixing that at this time.

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

17 files changed:
Source/WebCore/ChangeLog
Source/WebCore/accessibility/AccessibilityRenderObject.cpp
Source/WebCore/bindings/js/JSHTMLAllCollectionCustom.cpp
Source/WebCore/bindings/js/JSHTMLFormControlsCollectionCustom.cpp
Source/WebCore/bindings/js/JSHTMLFormElementCustom.cpp
Source/WebCore/html/HTMLAllCollection.cpp
Source/WebCore/html/HTMLAllCollection.h
Source/WebCore/html/HTMLCollection.cpp
Source/WebCore/html/HTMLCollection.h
Source/WebCore/html/HTMLFormControlsCollection.cpp
Source/WebCore/html/HTMLFormControlsCollection.h
Source/WebCore/html/HTMLFormElement.cpp
Source/WebCore/html/HTMLFormElement.h
Source/WebCore/html/HTMLNameCollection.h
Source/WebCore/html/HTMLSelectElement.cpp
Source/WebCore/html/HTMLSelectElement.h
Source/WebCore/page/scrolling/AxisScrollSnapOffsets.cpp

index ca9d24a..242dd62 100644 (file)
@@ -1,3 +1,123 @@
+2014-11-09  Darin Adler  <darin@apple.com>
+
+        Minor tweaks to HTMLCollection
+        https://bugs.webkit.org/show_bug.cgi?id=138556
+
+        Reviewed by Chris Dumez.
+
+        * accessibility/AccessibilityRenderObject.cpp:
+        (WebCore::AccessibilityRenderObject::addRadioButtonGroupMembers):
+        Update for namedElements function that now returns a Vector; also use
+        a new style for loop to iterate it.
+
+        * bindings/js/JSHTMLAllCollectionCustom.cpp: Removed some unneeded includes.
+        (WebCore::namedItems): Updated name to match WebKit coding style, and also
+        updated to use the return value from namedItems, which now returns a Vector.
+        (WebCore::callHTMLAllCollection): Updated for namedItems name change.
+        Also removed explicit Node* type from result of namedItemWithIndex, since that
+        function now returns a more specific type.
+        (WebCore::JSHTMLAllCollection::nameGetter): Update for namedItems name change.
+        (WebCore::JSHTMLAllCollection::item): Ditto.
+        (WebCore::JSHTMLAllCollection::namedItem): Ditto.
+
+        * bindings/js/JSHTMLFormControlsCollectionCustom.cpp: Removed some unneeded includes.
+        (WebCore::namedItems): Updated name to match WebKit coding style, and also
+        updated to use the return value from namedItems, which now returns a Vector.
+        (WebCore::JSHTMLFormControlsCollection::nameGetter): Update for namedItems name change.
+        (WebCore::JSHTMLFormControlsCollection::namedItem): Ditto.
+
+        * bindings/js/JSHTMLFormElementCustom.cpp:
+        (WebCore::JSHTMLFormElement::nameGetter): Updated to use the return value from
+        namedItems, which now returns a Vector.
+
+        * html/HTMLAllCollection.cpp:
+        (WebCore::HTMLAllCollection::HTMLAllCollection): Marked the constructor inline,
+        since it's only used in one place, the create function.
+        (WebCore::HTMLAllCollection::~HTMLAllCollection): Deleted. No need to have an
+        explicit destructor since there's nothing special to implement, and includers of
+        the header file have everything they ened to compile the compiler-generated one.
+        (WebCore::HTMLAllCollection::namedItemWithIndex): Changed return type to Element.
+
+        * html/HTMLAllCollection.h: Removed unneeded explicit declaration of destructor.
+        Chagned return type of namedItemWithIndex to Element.
+
+        * html/HTMLCollection.cpp:
+        (WebCore::HTMLCollection::rootTypeFromCollectionType): Marked this inline. Also
+        changed this to be a static member function so it can use the RootType enum, which
+        is now private to the class.
+        (WebCore::isMatchingHTMLElement): Marked this function static so it will get
+        internal linkage.
+        (WebCore::isMatchingElement): Ditto.
+        (WebCore::previousElement): Marked this function inline since it's called in only
+        one place. Changed argument type to a reference since it can never be null.
+        (WebCore::HTMLCollection::iterateForPreviousElement): Changed argument name and
+        also updated for above changes.
+        (WebCore::firstMatchingElement): Marked this function static so it will get
+        internal linkage.
+        (WebCore::nextMatchingElement): Ditto. Changed argument type to a reference
+        since it can never be null.
+        (WebCore::HTMLCollection::item): Changed return type to Element.
+        (WebCore::nameShouldBeVisibleInDocumentAll): Added an overload that takes an
+        Element. This streamlines the code below that calls it so it fits on one line.
+        (WebCore::firstMatchingChildElement): Marked this function static so it will get
+        internal linkage.
+        (WebCore::nextMatchingSiblingElement): Ditto. Changed argument type to a reference
+        since it can never be null.
+        (WebCore::HTMLCollection::usesCustomForwardOnlyTraversal): Moved here from the
+        header since, although it's marked inline, it's only used inside this file.
+        (WebCore::HTMLCollection::traverseForward): Restructured the code a little bit
+        to make the function smaller and possibly easier to read. This does add one
+        redundant null check, but it seems OK to do that.
+        (WebCore::HTMLCollection::collectionTraverseBackward): Tweaked foramtting a bit.
+        (WebCore::HTMLCollection::namedItem): Changed return type to Element. Tightened
+        the code that calls nameShouldBeVisibleInDocumentAll so it fits better on one line.
+        Changed code that handles m_shouldOnlyIncludeDirectChildren to use a nested if
+        instead of an && since it makes the code a little easier to read.
+        (WebCore::HTMLCollection::updateNamedElementCache): Tweaked code a little bit,
+        using shorter variable names, and using references instead of pointers. Also removed
+        the call to didPopulate, since setNamedItemCache now takes care of that.
+        (WebCore::HTMLCollection::namedItems): Changed to return a Vector instead of
+        appending to an existing one. Also use reserveInitialCapacity and uncheckedAppend
+        for better performance. Added a FIXME, because there seems to be something wrong
+        here about this being non-virtual. Made other small tweaks to streamline the code.
+        (WebCore::HTMLCollection::customElementAfter): Moved this here from the header.
+        There is no reason to need to inline this.
+
+        * html/HTMLCollection.h: Removed unneeded includes. Moved function bodies out
+        of the class definitions so the class definitions are easier to read. Made some
+        functions that were formerly public or protected be private instead. Added a call
+        to didPopulate to setNamedItemCache so the callers don't have to do it.
+
+        * html/HTMLFormControlsCollection.cpp:
+        (WebCore::HTMLFormControlsCollection::namedItem): Changed return value to Element.
+        Tweaked coding style a little bit.
+        (WebCore::HTMLFormControlsCollection::updateNamedElementCache): Rearranged to
+        simplify a bit. Don't build the foundInputElements set when the owner is not a
+        form element, since we don't use the set in that case. Use shorter variable names,
+        and modern for loops. Also removed the call to didPopulate, since setNamedItemCache
+        now takes care of that.
+
+        * html/HTMLFormControlsCollection.h: Removed some uneeded forward declarations.
+        Updated return type for namedItem, and also made the override private.
+
+        * html/HTMLFormElement.cpp:
+        (WebCore::HTMLFormElement::namedElements): Changed to return a Vector and updated
+        function name accordingly.
+        * html/HTMLFormElement.h: Ditto.
+
+        * html/HTMLNameCollection.h: Removed a stray blank line.
+
+        * html/HTMLSelectElement.cpp:
+        (WebCore::HTMLSelectElement::namedItem): Changed return value to Element.
+        (WebCore::HTMLSelectElement::item): Ditto.
+        * html/HTMLSelectElement.h: Ditto.
+
+        * page/scrolling/AxisScrollSnapOffsets.cpp:
+        (WebCore::appendChildSnapOffsets): Rewrote loop as a for loop rather than a while
+        loop. Removed unwanted use of children()->collectionBegin() to get the first element
+        child of an HTMLElement. This function uses a mix of DOM and rendering functions that
+        is probably incorrect, but I did not tackle fixing that at this time.
+
 2014-11-11  Dan Bernstein  <mitz@apple.com>
 
         [Mac] WebCore includes unused cursor image resources
index 85f856a..f1f9067 100644 (file)
@@ -949,10 +949,7 @@ void AccessibilityRenderObject::addRadioButtonGroupMembers(AccessibilityChildren
     HTMLInputElement& input = downcast<HTMLInputElement>(*node);
     // if there's a form, then this is easy
     if (input.form()) {
-        Vector<Ref<Element>> formElements;
-        input.form()->getNamedElements(input.name(), formElements);
-        
-        for (auto& associateElement : formElements) {
+        for (auto& associateElement : input.form()->namedElements(input.name())) {
             if (AccessibilityObject* object = axObjectCache()->getOrCreate(&associateElement.get()))
                 linkedUIElements.append(object);        
         } 
index 21935e1..8122507 100644 (file)
 #include "JSHTMLAllCollection.h"
 
 #include "HTMLAllCollection.h"
-#include "JSDOMBinding.h"
 #include "JSNode.h"
 #include "JSNodeList.h"
-#include "Node.h"
 #include "StaticNodeList.h"
 #include <runtime/IdentifierInlines.h>
-#include <runtime/JSCJSValue.h>
-#include <wtf/Vector.h>
-#include <wtf/text/AtomicString.h>
 
 using namespace JSC;
 
 namespace WebCore {
 
-static JSValue getNamedItems(ExecState* exec, JSHTMLAllCollection* collection, PropertyName propertyName)
+static JSValue namedItems(ExecState* exec, JSHTMLAllCollection* collection, PropertyName propertyName)
 {
-    Vector<Ref<Element>> namedItems;
-    collection->impl().namedItems(propertyNameToAtomicString(propertyName), namedItems);
+    Vector<Ref<Element>> namedItems = collection->impl().namedItems(propertyNameToAtomicString(propertyName));
 
     if (namedItems.isEmpty())
         return jsUndefined();
@@ -76,15 +70,15 @@ static EncodedJSValue JSC_HOST_CALL callHTMLAllCollection(ExecState* exec)
             return JSValue::encode(toJS(exec, jsCollection->globalObject(), collection.item(index)));
 
         // Support for document.images('<name>') etc.
-        return JSValue::encode(getNamedItems(exec, jsCollection, Identifier(exec, string)));
+        return JSValue::encode(namedItems(exec, jsCollection, Identifier(exec, string)));
     }
 
     // The second arg, if set, is the index of the item we want
     String string = exec->argument(0).toString(exec)->value(exec);
     unsigned index = toUInt32FromStringImpl(exec->argument(1).toWTFString(exec).impl());
     if (index != PropertyName::NotAnIndex) {
-        if (Node* node = collection.namedItemWithIndex(string, index))
-            return JSValue::encode(toJS(exec, jsCollection->globalObject(), node));
+        if (auto* item = collection.namedItemWithIndex(string, index))
+            return JSValue::encode(toJS(exec, jsCollection->globalObject(), item));
     }
 
     return JSValue::encode(jsUndefined());
@@ -104,7 +98,7 @@ bool JSHTMLAllCollection::canGetItemsForName(ExecState*, HTMLAllCollection* coll
 EncodedJSValue JSHTMLAllCollection::nameGetter(ExecState* exec, JSObject* slotBase, EncodedJSValue, PropertyName propertyName)
 {
     JSHTMLAllCollection* thisObj = jsCast<JSHTMLAllCollection*>(slotBase);
-    return JSValue::encode(getNamedItems(exec, thisObj, propertyName));
+    return JSValue::encode(namedItems(exec, thisObj, propertyName));
 }
 
 JSValue JSHTMLAllCollection::item(ExecState* exec)
@@ -112,12 +106,12 @@ JSValue JSHTMLAllCollection::item(ExecState* exec)
     uint32_t index = toUInt32FromStringImpl(exec->argument(0).toString(exec)->value(exec).impl());
     if (index != PropertyName::NotAnIndex)
         return toJS(exec, globalObject(), impl().item(index));
-    return getNamedItems(exec, this, Identifier(exec, exec->argument(0).toString(exec)->value(exec)));
+    return namedItems(exec, this, Identifier(exec, exec->argument(0).toString(exec)->value(exec)));
 }
 
 JSValue JSHTMLAllCollection::namedItem(ExecState* exec)
 {
-    JSValue value = getNamedItems(exec, this, Identifier(exec, exec->argument(0).toString(exec)->value(exec)));
+    JSValue value = namedItems(exec, this, Identifier(exec, exec->argument(0).toString(exec)->value(exec)));
     return value.isUndefined() ? jsNull() : value;
 }
 
index 41ea63d..2867d60 100644 (file)
  */
 
 #include "config.h"
-#include "HTMLFormControlsCollection.h"
-
-
-#include "HTMLAllCollection.h"
-#include "JSDOMBinding.h"
-#include "JSHTMLCollection.h"
 #include "JSHTMLFormControlsCollection.h"
+
+#include "HTMLFormControlsCollection.h"
 #include "JSNode.h"
-#include "JSNodeList.h"
 #include "JSRadioNodeList.h"
-#include "Node.h"
 #include "RadioNodeList.h"
 #include <runtime/IdentifierInlines.h>
-#include <wtf/Vector.h>
-#include <wtf/text/AtomicString.h>
 
 using namespace JSC;
 
 namespace WebCore {
 
-static JSValue getNamedItems(ExecState* exec, JSHTMLFormControlsCollection* collection, PropertyName propertyName)
+static JSValue namedItems(ExecState* exec, JSHTMLFormControlsCollection* collection, PropertyName propertyName)
 {
-    Vector<Ref<Element>> namedItems;
     const AtomicString& name = propertyNameToAtomicString(propertyName);
-    collection->impl().namedItems(name, namedItems);
+    Vector<Ref<Element>> namedItems = collection->impl().namedItems(name);
 
     if (namedItems.isEmpty())
         return jsUndefined();
@@ -61,12 +52,12 @@ bool JSHTMLFormControlsCollection::canGetItemsForName(ExecState*, HTMLFormContro
 EncodedJSValue JSHTMLFormControlsCollection::nameGetter(ExecState* exec, JSObject* slotBase, EncodedJSValue, PropertyName propertyName)
 {
     JSHTMLFormControlsCollection* thisObj = jsCast<JSHTMLFormControlsCollection*>(slotBase);
-    return JSValue::encode(getNamedItems(exec, thisObj, propertyName));
+    return JSValue::encode(namedItems(exec, thisObj, propertyName));
 }
 
 JSValue JSHTMLFormControlsCollection::namedItem(ExecState* exec)
 {
-    JSValue value = getNamedItems(exec, this, Identifier(exec, exec->argument(0).toString(exec)->value(exec)));
+    JSValue value = namedItems(exec, this, Identifier(exec, exec->argument(0).toString(exec)->value(exec)));
     return value.isUndefined() ? jsNull() : value;
 }
 
index 95cbc70..3d39bd1 100644 (file)
@@ -47,8 +47,7 @@ EncodedJSValue JSHTMLFormElement::nameGetter(ExecState* exec, JSObject* slotBase
     JSHTMLFormElement* jsForm = jsCast<JSHTMLFormElement*>(slotBase);
     HTMLFormElement& form = jsForm->impl();
 
-    Vector<Ref<Element>> namedItems;
-    form.getNamedElements(propertyNameToAtomicString(propertyName), namedItems);
+    Vector<Ref<Element>> namedItems = form.namedElements(propertyNameToAtomicString(propertyName));
     
     if (namedItems.isEmpty())
         return JSValue::encode(jsUndefined());
index f2afbab..82d670d 100644 (file)
@@ -35,16 +35,12 @@ PassRef<HTMLAllCollection> HTMLAllCollection::create(Document& document, Collect
     return adoptRef(*new HTMLAllCollection(document, type));
 }
 
-HTMLAllCollection::HTMLAllCollection(Document& document, CollectionType type)
+inline HTMLAllCollection::HTMLAllCollection(Document& document, CollectionType type)
     : HTMLCollection(document, type)
 {
 }
 
-HTMLAllCollection::~HTMLAllCollection()
-{
-}
-
-Node* HTMLAllCollection::namedItemWithIndex(const AtomicString& name, unsigned index) const
+Element* HTMLAllCollection::namedItemWithIndex(const AtomicString& name, unsigned index) const
 {
     updateNamedElementCache();
     const CollectionNamedElementCache& cache = namedItemCaches();
@@ -60,7 +56,7 @@ Node* HTMLAllCollection::namedItemWithIndex(const AtomicString& name, unsigned i
             return elements->at(index);
     }
 
-    return 0;
+    return nullptr;
 }
 
 } // namespace WebCore
index 24bbf3d..bdf4ff1 100644 (file)
@@ -33,9 +33,8 @@ namespace WebCore {
 class HTMLAllCollection final : public HTMLCollection {
 public:
     static PassRef<HTMLAllCollection> create(Document&, CollectionType);
-    virtual ~HTMLAllCollection();
 
-    Node* namedItemWithIndex(const AtomicString& name, unsigned index) const;
+    Element* namedItemWithIndex(const AtomicString& name, unsigned index) const;
 
 private:
     HTMLAllCollection(Document&, CollectionType);
index a290054..8e9656f 100644 (file)
@@ -65,7 +65,7 @@ static bool shouldOnlyIncludeDirectChildren(CollectionType type)
     return false;
 }
 
-static HTMLCollection::RootType rootTypeFromCollectionType(CollectionType type)
+inline auto HTMLCollection::rootTypeFromCollectionType(CollectionType type) -> RootType
 {
     switch (type) {
     case DocImages:
@@ -169,7 +169,7 @@ ContainerNode& HTMLCollection::rootNode() const
     return ownerNode();
 }
 
-inline bool isMatchingHTMLElement(const HTMLCollection& collection, HTMLElement& element)
+static inline bool isMatchingHTMLElement(const HTMLCollection& collection, HTMLElement& element)
 {
     switch (collection.type()) {
     case DocImages:
@@ -218,7 +218,7 @@ inline bool isMatchingHTMLElement(const HTMLCollection& collection, HTMLElement&
     return false;
 }
 
-inline bool isMatchingElement(const HTMLCollection& collection, Element& element)
+static inline bool isMatchingElement(const HTMLCollection& collection, Element& element)
 {
     // Collection types that deal with any type of Elements, not just HTMLElements.
     switch (collection.type()) {
@@ -233,23 +233,23 @@ inline bool isMatchingElement(const HTMLCollection& collection, Element& element
     }
 }
 
-static Element* previousElement(ContainerNode& base, Element* previous, bool onlyIncludeDirectChildren)
+static inline Element* previousElement(ContainerNode& base, Element& element, bool onlyIncludeDirectChildren)
 {
-    return onlyIncludeDirectChildren ? ElementTraversal::previousSibling(previous) : ElementTraversal::previous(previous, &base);
+    return onlyIncludeDirectChildren ? ElementTraversal::previousSibling(&element) : ElementTraversal::previous(&element, &base);
 }
 
-ALWAYS_INLINE Element* HTMLCollection::iterateForPreviousElement(Element* current) const
+ALWAYS_INLINE Element* HTMLCollection::iterateForPreviousElement(Element* element) const
 {
     bool onlyIncludeDirectChildren = m_shouldOnlyIncludeDirectChildren;
     ContainerNode& rootNode = this->rootNode();
-    for (; current; current = previousElement(rootNode, current, onlyIncludeDirectChildren)) {
-        if (isMatchingElement(*this, *current))
-            return current;
+    for (; element; element = previousElement(rootNode, *element, onlyIncludeDirectChildren)) {
+        if (isMatchingElement(*this, *element))
+            return element;
     }
     return nullptr;
 }
 
-inline Element* firstMatchingElement(const HTMLCollection& collection, ContainerNode& root)
+static inline Element* firstMatchingElement(const HTMLCollection& collection, ContainerNode& root)
 {
     Element* element = ElementTraversal::firstWithin(&root);
     while (element && !isMatchingElement(collection, *element))
@@ -257,12 +257,13 @@ inline Element* firstMatchingElement(const HTMLCollection& collection, Container
     return element;
 }
 
-inline Element* nextMatchingElement(const HTMLCollection& collection, Element* current, ContainerNode& root)
+static inline Element* nextMatchingElement(const HTMLCollection& collection, Element& element, ContainerNode& root)
 {
+    Element* next = &element;
     do {
-        current = ElementTraversal::next(current, &root);
-    } while (current && !isMatchingElement(collection, *current));
-    return current;
+        next = ElementTraversal::next(next, &root);
+    } while (next && !isMatchingElement(collection, *next));
+    return next;
 }
 
 unsigned HTMLCollection::length() const
@@ -270,7 +271,7 @@ unsigned HTMLCollection::length() const
     return m_indexCache.nodeCount(*this);
 }
 
-Node* HTMLCollection::item(unsigned offset) const
+Element* HTMLCollection::item(unsigned offset) const
 {
     return m_indexCache.nodeAt(*this, offset);
 }
@@ -288,7 +289,12 @@ static inline bool nameShouldBeVisibleInDocumentAll(HTMLElement& element)
         || element.hasTagName(selectTag);
 }
 
-inline Element* firstMatchingChildElement(const HTMLCollection& nodeList, ContainerNode& root)
+static inline bool nameShouldBeVisibleInDocumentAll(Element& element)
+{
+    return is<HTMLElement>(element) && nameShouldBeVisibleInDocumentAll(downcast<HTMLElement>(element));
+}
+
+static inline Element* firstMatchingChildElement(const HTMLCollection& nodeList, ContainerNode& root)
 {
     Element* element = ElementTraversal::firstWithin(&root);
     while (element && !isMatchingElement(nodeList, *element))
@@ -296,12 +302,18 @@ inline Element* firstMatchingChildElement(const HTMLCollection& nodeList, Contai
     return element;
 }
 
-inline Element* nextMatchingSiblingElement(const HTMLCollection& nodeList, Element* current)
+static inline Element* nextMatchingSiblingElement(const HTMLCollection& nodeList, Element& element)
 {
+    Element* next = &element;
     do {
-        current = ElementTraversal::nextSibling(current);
-    } while (current && !isMatchingElement(nodeList, *current));
-    return current;
+        next = ElementTraversal::nextSibling(next);
+    } while (next && !isMatchingElement(nodeList, *next));
+    return next;
+}
+
+inline bool HTMLCollection::usesCustomForwardOnlyTraversal() const
+{
+    return m_usesCustomForwardOnlyTraversal;
 }
 
 inline Element* HTMLCollection::firstElement(ContainerNode& root) const
@@ -317,25 +329,14 @@ inline Element* HTMLCollection::traverseForward(Element& current, unsigned count
 {
     Element* element = &current;
     if (usesCustomForwardOnlyTraversal()) {
-        for (traversedCount = 0; traversedCount < count; ++traversedCount) {
+        for (traversedCount = 0; element && traversedCount < count; ++traversedCount)
             element = customElementAfter(element);
-            if (!element)
-                return nullptr;
-        }
-        return element;
-    }
-    if (m_shouldOnlyIncludeDirectChildren) {
-        for (traversedCount = 0; traversedCount < count; ++traversedCount) {
-            element = nextMatchingSiblingElement(*this, element);
-            if (!element)
-                return nullptr;
-        }
-        return element;
-    }
-    for (traversedCount = 0; traversedCount < count; ++traversedCount) {
-        element = nextMatchingElement(*this, element, root);
-        if (!element)
-            return nullptr;
+    } else if (m_shouldOnlyIncludeDirectChildren) {
+        for (traversedCount = 0; element && traversedCount < count; ++traversedCount)
+            element = nextMatchingSiblingElement(*this, *element);
+    } else {
+        for (traversedCount = 0; element && traversedCount < count; ++traversedCount)
+            element = nextMatchingElement(*this, *element, root);
     }
     return element;
 }
@@ -361,13 +362,13 @@ void HTMLCollection::collectionTraverseForward(Element*& current, unsigned count
 void HTMLCollection::collectionTraverseBackward(Element*& current, unsigned count) const
 {
     // FIXME: This should be optimized similarly to the forward case.
-    auto& root = rootNode();
     if (m_shouldOnlyIncludeDirectChildren) {
-        for (; count && current ; --count)
+        for (; count && current; --count)
             current = iterateForPreviousElement(ElementTraversal::previousSibling(current));
         return;
     }
-    for (; count && current ; --count)
+    auto& root = rootNode();
+    for (; count && current; --count)
         current = iterateForPreviousElement(ElementTraversal::previous(current, &root));
 }
 
@@ -388,7 +389,7 @@ void HTMLCollection::invalidateNamedElementCache(Document& document) const
     m_namedElementCache = nullptr;
 }
 
-Node* HTMLCollection::namedItem(const AtomicString& name) const
+Element* HTMLCollection::namedItem(const AtomicString& name) const
 {
     // http://msdn.microsoft.com/workshop/author/dhtml/reference/methods/nameditem.asp
     // This method first searches for an object with a matching id
@@ -401,23 +402,25 @@ Node* HTMLCollection::namedItem(const AtomicString& name) const
 
     ContainerNode& root = rootNode();
     if (!usesCustomForwardOnlyTraversal() && root.isInTreeScope()) {
-        TreeScope& treeScope = root.treeScope();
         Element* candidate = nullptr;
+
+        TreeScope& treeScope = root.treeScope();
         if (treeScope.hasElementWithId(*name.impl())) {
             if (!treeScope.containsMultipleElementsWithId(name))
                 candidate = treeScope.getElementById(name);
         } else if (treeScope.hasElementWithName(*name.impl())) {
             if (!treeScope.containsMultipleElementsWithName(name)) {
                 candidate = treeScope.getElementByName(name);
-                if (candidate && type() == DocAll && (!is<HTMLElement>(*candidate) || !nameShouldBeVisibleInDocumentAll(downcast<HTMLElement>(*candidate))))
+                if (candidate && type() == DocAll && !nameShouldBeVisibleInDocumentAll(*candidate))
                     candidate = nullptr;
             }
         } else
             return nullptr;
 
-        if (candidate && isMatchingElement(*this, *candidate)
-            && (m_shouldOnlyIncludeDirectChildren ? candidate->parentNode() == &root : candidate->isDescendantOf(&root)))
-            return candidate;
+        if (candidate && isMatchingElement(*this, *candidate)) {
+            if (m_shouldOnlyIncludeDirectChildren ? candidate->parentNode() == &root : candidate->isDescendantOf(&root))
+                return candidate;
+        }
     }
 
     // The pathological case. We need to walk the entire subtree.
@@ -442,25 +445,22 @@ void HTMLCollection::updateNamedElementCache() const
     if (hasNamedElementCache())
         return;
 
-
     auto cache = std::make_unique<CollectionNamedElementCache>();
 
     unsigned size = m_indexCache.nodeCount(*this);
-    for (unsigned i = 0; i < size; i++) {
-        Element* element = m_indexCache.nodeAt(*this, i);
-        ASSERT(element);
-        const AtomicString& idAttrVal = element->getIdAttribute();
-        if (!idAttrVal.isEmpty())
-            cache->appendIdCache(idAttrVal, element);
-        if (!is<HTMLElement>(*element))
+    for (unsigned i = 0; i < size; ++i) {
+        Element& element = *m_indexCache.nodeAt(*this, i);
+        const AtomicString& id = element.getIdAttribute();
+        if (!id.isEmpty())
+            cache->appendToIdCache(id, element);
+        if (!is<HTMLElement>(element))
             continue;
-        const AtomicString& nameAttrVal = element->getNameAttribute();
-        if (!nameAttrVal.isEmpty() && idAttrVal != nameAttrVal && (type() != DocAll || nameShouldBeVisibleInDocumentAll(downcast<HTMLElement>(*element))))
-            cache->appendNameCache(nameAttrVal, element);
+        const AtomicString& name = element.getNameAttribute();
+        if (!name.isEmpty() && id != name && (type() != DocAll || nameShouldBeVisibleInDocumentAll(downcast<HTMLElement>(element))))
+            cache->appendToNameCache(name, element);
     }
 
-    cache->didPopulate();
-    setNameItemCache(WTF::move(cache));
+    setNamedItemCache(WTF::move(cache));
 }
 
 bool HTMLCollection::hasNamedItem(const AtomicString& name) const
@@ -469,23 +469,34 @@ bool HTMLCollection::hasNamedItem(const AtomicString& name) const
     return namedItem(name);
 }
 
-void HTMLCollection::namedItems(const AtomicString& name, Vector<Ref<Element>>& result) const
+Vector<Ref<Element>> HTMLCollection::namedItems(const AtomicString& name) const
 {
-    ASSERT(result.isEmpty());
+    // FIXME: This non-virtual function can't possibly be doing the correct thing for
+    // any derived class that overrides the virtual namedItem function.
+
+    Vector<Ref<Element>> elements;
+
     if (name.isEmpty())
-        return;
+        return elements;
 
     updateNamedElementCache();
     ASSERT(m_namedElementCache);
 
-    const Vector<Element*>* idResults = m_namedElementCache->findElementsWithId(name);
-    const Vector<Element*>* nameResults = m_namedElementCache->findElementsWithName(name);
+    auto* elementsWithId = m_namedElementCache->findElementsWithId(name);
+    auto* elementsWithName = m_namedElementCache->findElementsWithName(name);
 
-    for (unsigned i = 0; idResults && i < idResults->size(); ++i)
-        result.append(*idResults->at(i));
+    elements.reserveInitialCapacity((elementsWithId ? elementsWithId->size() : 0) + (elementsWithName ? elementsWithName->size() : 0));
 
-    for (unsigned i = 0; nameResults && i < nameResults->size(); ++i)
-        result.append(*nameResults->at(i));
+    if (elementsWithId) {
+        for (auto& element : *elementsWithId)
+            elements.uncheckedAppend(*element);
+    }
+    if (elementsWithName) {
+        for (auto& element : *elementsWithName)
+            elements.uncheckedAppend(*element);
+    }
+
+    return elements;
 }
 
 PassRefPtr<NodeList> HTMLCollection::tags(const String& name)
@@ -493,4 +504,10 @@ PassRefPtr<NodeList> HTMLCollection::tags(const String& name)
     return ownerNode().getElementsByTagName(name);
 }
 
+Element* HTMLCollection::customElementAfter(Element*) const
+{
+    ASSERT_NOT_REACHED();
+    return nullptr;
+}
+
 } // namespace WebCore
index f40417e..08f123b 100644 (file)
 #define HTMLCollection_h
 
 #include "CollectionIndexCache.h"
-#include "CollectionType.h"
-#include "ContainerNode.h"
-#include "Document.h"
 #include "HTMLNames.h"
 #include "LiveNodeList.h"
 #include "ScriptWrappable.h"
-#include <memory>
-#include <wtf/Forward.h>
 #include <wtf/HashMap.h>
-#include <wtf/TypeCasts.h>
-#include <wtf/Vector.h>
 
 namespace WebCore {
 
@@ -42,47 +35,26 @@ class Element;
 
 class CollectionNamedElementCache {
 public:
-#ifndef ASSERT_DISABLED
-    CollectionNamedElementCache : m_didPopulateCalled(false) { }
-#endif
-
-    const Vector<Element*>* findElementsWithId(const AtomicString& id) const { return find(m_idToElementsMap, id); }
-    const Vector<Element*>* findElementsWithName(const AtomicString& name) const { return find(m_nameToElementsMap, name); }
+    const Vector<Element*>* findElementsWithId(const AtomicString& id) const;
+    const Vector<Element*>* findElementsWithName(const AtomicString& name) const;
 
-    void appendIdCache(const AtomicString& id, Element* element) { return append(m_idToElementsMap, id, element); }
-    void appendNameCache(const AtomicString& name, Element* element)  { return append(m_nameToElementsMap, name, element); }
+    void appendToIdCache(const AtomicString& id, Element&);
+    void appendToNameCache(const AtomicString& name, Element&);
+    void didPopulate();
 
-    void didPopulate()
-    {
-#ifndef ASSERT_DISABLED
-        m_didPopulateCalled = true;
-#endif
-        if (size_t cost = memoryCost())
-            reportExtraMemoryCostForCollectionIndexCache(cost);
-    }
-    size_t memoryCost() const { return (m_idToElementsMap.size() + m_nameToElementsMap.size()) * sizeof(Element*); }
+    size_t memoryCost() const;
 
 private:
     typedef HashMap<AtomicStringImpl*, Vector<Element*>> StringToElementsMap;
 
-    const Vector<Element*>* find(const StringToElementsMap& map, const AtomicString& key) const
-    {
-#ifndef ASSERT_DISABLED
-        ASSERT(m_didPopulateCalled);
-#endif
-        auto it = map.find(key.impl());
-        return it != map.end() ? &it->value : nullptr;
-    }
-
-    static void append(StringToElementsMap& map, const AtomicString& key, Element* element)
-    {
-        map.add(key.impl(), Vector<Element*>()).iterator->value.append(element);
-    }
-
-    StringToElementsMap m_idToElementsMap;
-    StringToElementsMap m_nameToElementsMap;
-#ifndef ASSERT_DISABLED
-    bool m_didPopulateCalled;
+    const Vector<Element*>* find(const StringToElementsMap&, const AtomicString& key) const;
+    static void append(StringToElementsMap&, const AtomicString& key, Element&);
+
+    StringToElementsMap m_idMap;
+    StringToElementsMap m_nameMap;
+
+#if !ASSERT_DISABLED
+    bool m_didPopulate { false };
 #endif
 };
 
@@ -93,42 +65,32 @@ public:
 
     // DOM API
     unsigned length() const;
-    Node* item(unsigned offset) const;
-    virtual Node* namedItem(const AtomicString& name) const;
+    Element* item(unsigned offset) const;
+    virtual Element* namedItem(const AtomicString& name) const;
     PassRefPtr<NodeList> tags(const String&);
 
     // Non-DOM API
     bool hasNamedItem(const AtomicString& name) const;
-    void namedItems(const AtomicString& name, Vector<Ref<Element>>&) const;
-    size_t memoryCost() const { return m_indexCache.memoryCost() + (m_namedElementCache ? m_namedElementCache->memoryCost() : 0); }
-
-    enum RootType {
-        IsRootedAtNode,
-        IsRootedAtDocument
-    };
-    bool isRootedAtDocument() const { return m_rootType == IsRootedAtDocument; }
-    NodeListInvalidationType invalidationType() const { return static_cast<NodeListInvalidationType>(m_invalidationType); }
-    CollectionType type() const { return static_cast<CollectionType>(m_collectionType); }
-    ContainerNode& ownerNode() const { return const_cast<ContainerNode&>(m_ownerNode.get()); }
-    void invalidateCache(const QualifiedName* attrName) const
-    {
-        if (!attrName || shouldInvalidateTypeOnAttributeChange(invalidationType(), *attrName))
-            invalidateCache(document());
-        else if (hasNamedElementCache() && (*attrName == HTMLNames::idAttr || *attrName == HTMLNames::nameAttr))
-            invalidateNamedElementCache(document());
-    }
+    Vector<Ref<Element>> namedItems(const AtomicString& name) const;
+    size_t memoryCost() const;
+
+    bool isRootedAtDocument() const;
+    NodeListInvalidationType invalidationType() const;
+    CollectionType type() const;
+    ContainerNode& ownerNode() const;
+    void invalidateCache(const QualifiedName* attributeName) const;
     virtual void invalidateCache(Document&) const;
 
-    // For CollectionIndexCache
+    // For CollectionIndexCache; do not use elsewhere.
     Element* collectionBegin() const;
     Element* collectionLast() const;
-    Element* collectionEnd() const { return nullptr; }
+    Element* collectionEnd() const;
     void collectionTraverseForward(Element*&, unsigned count, unsigned& traversedCount) const;
     void collectionTraverseBackward(Element*&, unsigned count) const;
-    bool collectionCanTraverseBackward() const { return !m_usesCustomForwardOnlyTraversal; }
-    void willValidateIndexCache() const { document().registerCollection(const_cast<HTMLCollection&>(*this)); }
+    bool collectionCanTraverseBackward() const;
+    void willValidateIndexCache() const;
 
-    bool hasNamedElementCache() const { return !!m_namedElementCache; }
+    bool hasNamedElementCache() const;
 
 protected:
     enum ElementTraversalType { NormalTraversal, CustomForwardOnlyTraversal };
@@ -136,34 +98,25 @@ protected:
 
     virtual void updateNamedElementCache() const;
 
-    Document& document() const { return m_ownerNode->document(); }
-    ContainerNode& rootNode() const;
-    bool usesCustomForwardOnlyTraversal() const { return m_usesCustomForwardOnlyTraversal; }
-
-    RootType rootType() const { return static_cast<RootType>(m_rootType); }
-
-    void setNameItemCache(std::unique_ptr<CollectionNamedElementCache> cache) const
-    {
-        ASSERT(!m_namedElementCache);
-        m_namedElementCache = WTF::move(cache);
-        document().collectionCachedIdNameMap(*this);
-    }
-
-    const CollectionNamedElementCache& namedItemCaches() const
-    {
-        ASSERT(!!m_namedElementCache);
-        return *m_namedElementCache;
-    }
+    void setNamedItemCache(std::unique_ptr<CollectionNamedElementCache>) const;
+    const CollectionNamedElementCache& namedItemCaches() const;
 
 private:
-    Element* iterateForPreviousElement(Element* current) const;
+    Document& document() const;
+    ContainerNode& rootNode() const;
+    bool usesCustomForwardOnlyTraversal() const;
+
+    Element* iterateForPreviousElement(Element*) const;
     Element* firstElement(ContainerNode& root) const;
-    Element* traverseForward(Element& current, unsigned count, unsigned& traversedCount, ContainerNode& root) const;
+    Element* traverseForward(Element&, unsigned count, unsigned& traversedCount, ContainerNode& root) const;
+
+    virtual Element* customElementAfter(Element*) const;
 
-    virtual Element* customElementAfter(Element*) const { ASSERT_NOT_REACHED(); return nullptr; }
-    
     void invalidateNamedElementCache(Document&) const;
 
+    enum RootType { IsRootedAtNode, IsRootedAtDocument };
+    static RootType rootTypeFromCollectionType(CollectionType);
+
     Ref<ContainerNode> m_ownerNode;
 
     mutable CollectionIndexCache<HTMLCollection, Element*> m_indexCache;
@@ -176,6 +129,125 @@ private:
     const unsigned m_usesCustomForwardOnlyTraversal : 1;
 };
 
+inline const Vector<Element*>* CollectionNamedElementCache::findElementsWithId(const AtomicString& id) const
+{
+    return find(m_idMap, id);
+}
+
+inline const Vector<Element*>* CollectionNamedElementCache::findElementsWithName(const AtomicString& name) const
+{
+    return find(m_nameMap, name);
+}
+
+inline void CollectionNamedElementCache::appendToIdCache(const AtomicString& id, Element& element)
+{
+    return append(m_idMap, id, element);
+}
+
+inline void CollectionNamedElementCache::appendToNameCache(const AtomicString& name, Element& element)
+{
+    return append(m_nameMap, name, element);
+}
+
+inline size_t CollectionNamedElementCache::memoryCost() const
+{
+    return (m_idMap.size() + m_nameMap.size()) * sizeof(Element*);
+}
+
+inline void CollectionNamedElementCache::didPopulate()
+{
+#if !ASSERT_DISABLED
+    m_didPopulate = true;
+#endif
+    if (size_t cost = memoryCost())
+        reportExtraMemoryCostForCollectionIndexCache(cost);
+}
+
+inline const Vector<Element*>* CollectionNamedElementCache::find(const StringToElementsMap& map, const AtomicString& key) const
+{
+    ASSERT(m_didPopulate);
+    auto it = map.find(key.impl());
+    return it != map.end() ? &it->value : nullptr;
+}
+
+inline void CollectionNamedElementCache::append(StringToElementsMap& map, const AtomicString& key, Element& element)
+{
+    map.add(key.impl(), Vector<Element*>()).iterator->value.append(&element);
+}
+
+inline size_t HTMLCollection::memoryCost() const
+{
+    return m_indexCache.memoryCost() + (m_namedElementCache ? m_namedElementCache->memoryCost() : 0);
+}
+
+inline bool HTMLCollection::isRootedAtDocument() const
+{
+    return m_rootType == IsRootedAtDocument;
+}
+
+inline NodeListInvalidationType HTMLCollection::invalidationType() const
+{
+    return static_cast<NodeListInvalidationType>(m_invalidationType);
+}
+
+inline CollectionType HTMLCollection::type() const
+{
+    return static_cast<CollectionType>(m_collectionType);
+}
+
+inline ContainerNode& HTMLCollection::ownerNode() const
+{
+    return const_cast<ContainerNode&>(m_ownerNode.get());
+}
+
+inline Document& HTMLCollection::document() const
+{
+    return m_ownerNode->document();
+}
+
+inline void HTMLCollection::invalidateCache(const QualifiedName* attributeName) const
+{
+    if (!attributeName || shouldInvalidateTypeOnAttributeChange(invalidationType(), *attributeName))
+        invalidateCache(document());
+    else if (hasNamedElementCache() && (*attributeName == HTMLNames::idAttr || *attributeName == HTMLNames::nameAttr))
+        invalidateNamedElementCache(document());
+}
+
+inline Element* HTMLCollection::collectionEnd() const
+{
+    return nullptr;
+}
+
+inline bool HTMLCollection::collectionCanTraverseBackward() const
+{
+    return !m_usesCustomForwardOnlyTraversal;
+}
+
+inline void HTMLCollection::willValidateIndexCache() const
+{
+    document().registerCollection(const_cast<HTMLCollection&>(*this));
+}
+
+inline bool HTMLCollection::hasNamedElementCache() const
+{
+    return !!m_namedElementCache;
+}
+
+inline void HTMLCollection::setNamedItemCache(std::unique_ptr<CollectionNamedElementCache> cache) const
+{
+    ASSERT(cache);
+    ASSERT(!m_namedElementCache);
+    cache->didPopulate();
+    m_namedElementCache = WTF::move(cache);
+    document().collectionCachedIdNameMap(*this);
+}
+
+inline const CollectionNamedElementCache& HTMLCollection::namedItemCaches() const
+{
+    ASSERT(!!m_namedElementCache);
+    return *m_namedElementCache;
+}
+
 } // namespace WebCore
 
 #define SPECIALIZE_TYPE_TRAITS_HTMLCOLLECTION(ClassName, Type) \
index 05248ac..38ec31d 100644 (file)
@@ -121,18 +121,17 @@ static HTMLElement* firstNamedItem(const Vector<FormAssociatedElement*>& element
     return nullptr;
 }
 
-Node* HTMLFormControlsCollection::namedItem(const AtomicString& name) const
+HTMLElement* HTMLFormControlsCollection::namedItem(const AtomicString& name) const
 {
     // http://msdn.microsoft.com/workshop/author/dhtml/reference/methods/nameditem.asp
     // This method first searches for an object with a matching id
     // attribute. If a match is not found, the method then searches for an
     // object with a matching name attribute, but only on those elements
     // that are allowed a name attribute.
-    const Vector<HTMLImageElement*>* imagesElements = is<HTMLFieldSetElement>(ownerNode()) ? nullptr : &formImageElements();
-    if (HTMLElement* item = firstNamedItem(formControlElements(), imagesElements, idAttr, name))
+    auto* imageElements = is<HTMLFieldSetElement>(ownerNode()) ? nullptr : &formImageElements();
+    if (HTMLElement* item = firstNamedItem(formControlElements(), imageElements, idAttr, name))
         return item;
-
-    return firstNamedItem(formControlElements(), imagesElements, nameAttr, name);
+    return firstNamedItem(formControlElements(), imageElements, nameAttr, name);
 }
 
 void HTMLFormControlsCollection::updateNamedElementCache() const
@@ -141,41 +140,41 @@ void HTMLFormControlsCollection::updateNamedElementCache() const
         return;
 
     auto cache = std::make_unique<CollectionNamedElementCache>();
+
+    bool ownerIsFormElement = is<HTMLFormElement>(ownerNode());
     HashSet<AtomicStringImpl*> foundInputElements;
-    const Vector<FormAssociatedElement*>& elementsArray = formControlElements();
 
-    for (unsigned i = 0; i < elementsArray.size(); ++i) {
-        FormAssociatedElement& associatedElement = *elementsArray[i];
+    for (auto& elementPtr : formControlElements()) {
+        FormAssociatedElement& associatedElement = *elementPtr;
         if (associatedElement.isEnumeratable()) {
             HTMLElement& element = associatedElement.asHTMLElement();
-            const AtomicString& idAttrVal = element.getIdAttribute();
-            const AtomicString& nameAttrVal = element.getNameAttribute();
-            if (!idAttrVal.isEmpty()) {
-                cache->appendIdCache(idAttrVal, &element);
-                foundInputElements.add(idAttrVal.impl());
+            const AtomicString& id = element.getIdAttribute();
+            if (!id.isEmpty()) {
+                cache->appendToIdCache(id, element);
+                if (ownerIsFormElement)
+                    foundInputElements.add(id.impl());
             }
-            if (!nameAttrVal.isEmpty() && idAttrVal != nameAttrVal) {
-                cache->appendNameCache(nameAttrVal, &element);
-                foundInputElements.add(nameAttrVal.impl());
+            const AtomicString& name = element.getNameAttribute();
+            if (!name.isEmpty() && id != name) {
+                cache->appendToNameCache(name, element);
+                if (ownerIsFormElement)
+                    foundInputElements.add(name.impl());
             }
         }
     }
-
-    if (is<HTMLFormElement>(ownerNode())) {
-        const Vector<HTMLImageElement*>& imageElementsArray = formImageElements();
-        for (unsigned i = 0; i < imageElementsArray.size(); ++i) {
-            HTMLImageElement& element = *imageElementsArray[i];
-            const AtomicString& idAttrVal = element.getIdAttribute();
-            const AtomicString& nameAttrVal = element.getNameAttribute();
-            if (!idAttrVal.isEmpty() && !foundInputElements.contains(idAttrVal.impl()))
-                cache->appendIdCache(idAttrVal, &element);
-            if (!nameAttrVal.isEmpty() && idAttrVal != nameAttrVal && !foundInputElements.contains(nameAttrVal.impl()))
-                cache->appendNameCache(nameAttrVal, &element);
+    if (ownerIsFormElement) {
+        for (auto* elementPtr : formImageElements()) {
+            HTMLImageElement& element = *elementPtr;
+            const AtomicString& id = element.getIdAttribute();
+            if (!id.isEmpty() && !foundInputElements.contains(id.impl()))
+                cache->appendToIdCache(id, element);
+            const AtomicString& name = element.getNameAttribute();
+            if (!name.isEmpty() && id != name && !foundInputElements.contains(name.impl()))
+                cache->appendToNameCache(name, element);
         }
     }
 
-    cache->didPopulate();
-    setNameItemCache(WTF::move(cache));
+    setNamedItemCache(WTF::move(cache));
 }
 
 void HTMLFormControlsCollection::invalidateCache(Document& document) const
index caa6a0d..3162bd9 100644 (file)
 #define HTMLFormControlsCollection_h
 
 #include "HTMLCollection.h"
+#include "HTMLElement.h"
 
 namespace WebCore {
 
 class FormAssociatedElement;
-class HTMLElement;
 class HTMLImageElement;
-class QualifiedName;
 
 // This class is just a big hack to find form elements even in malformed HTML elements.
 // The famous <table><tr><form><td> problem.
@@ -38,14 +37,12 @@ class QualifiedName;
 class HTMLFormControlsCollection final : public HTMLCollection {
 public:
     static PassRef<HTMLFormControlsCollection> create(ContainerNode&, CollectionType);
-
     virtual ~HTMLFormControlsCollection();
 
-    virtual Node* namedItem(const AtomicString& name) const override;
-
 private:
     explicit HTMLFormControlsCollection(ContainerNode&);
 
+    virtual HTMLElement* namedItem(const AtomicString& name) const override;
     virtual void invalidateCache(Document&) const override;
     virtual void updateNamedElementCache() const override;
 
index 9260076..4eda58c 100644 (file)
@@ -792,17 +792,19 @@ bool HTMLFormElement::hasNamedElement(const AtomicString& name)
     return elements()->hasNamedItem(name) || elementFromPastNamesMap(name);
 }
 
-// FIXME: Use RefPtr<HTMLElement> for namedItems. elements()->namedItems never return non-HTMLElement nodes.
-void HTMLFormElement::getNamedElements(const AtomicString& name, Vector<Ref<Element>>& namedItems)
+// FIXME: Use Ref<HTMLElement> for the function result since there are no non-HTML elements returned here.
+Vector<Ref<Element>> HTMLFormElement::namedElements(const AtomicString& name)
 {
     // http://www.whatwg.org/specs/web-apps/current-work/multipage/forms.html#dom-form-nameditem
-    elements()->namedItems(name, namedItems);
+    Vector<Ref<Element>> namedItems = elements()->namedItems(name);
 
     HTMLElement* elementFromPast = elementFromPastNamesMap(name);
     if (namedItems.size() == 1 && namedItems.first().ptr() != elementFromPast)
         addToPastNamesMap(downcast<HTMLElement>(namedItems.first().get()).asFormNamedItem(), name);
     else if (elementFromPast && namedItems.isEmpty())
         namedItems.append(*elementFromPast);
+
+    return namedItems;
 }
 
 void HTMLFormElement::documentDidResumeFromPageCache()
index b4d6866..e545c77 100644 (file)
@@ -52,7 +52,7 @@ public:
 
     PassRefPtr<HTMLCollection> elements();
     bool hasNamedElement(const AtomicString&);
-    void getNamedElements(const AtomicString&, Vector<Ref<Element>>&);
+    Vector<Ref<Element>> namedElements(const AtomicString&);
 
     unsigned length() const;
     Node* item(unsigned index);
index 022ea30..a0f4bb5 100644 (file)
@@ -24,7 +24,6 @@
 #define HTMLNameCollection_h
 
 #include "HTMLCollection.h"
-
 #include <wtf/text/AtomicString.h>
 
 namespace WebCore {
index ea3043e..472b369 100644 (file)
@@ -427,14 +427,14 @@ void HTMLSelectElement::setSize(int size)
     setIntegralAttribute(sizeAttr, size);
 }
 
-Node* HTMLSelectElement::namedItem(const AtomicString& name)
+HTMLOptionElement* HTMLSelectElement::namedItem(const AtomicString& name)
 {
-    return options()->namedItem(name);
+    return downcast<HTMLOptionElement>(options()->namedItem(name));
 }
 
-Node* HTMLSelectElement::item(unsigned index)
+HTMLOptionElement* HTMLSelectElement::item(unsigned index)
 {
-    return options()->item(index);
+    return downcast<HTMLOptionElement>(options()->item(index));
 }
 
 void HTMLSelectElement::setOption(unsigned index, HTMLOptionElement* option, ExceptionCode& ec)
index da7f15a..dc00daa 100644 (file)
 
 #include "Event.h"
 #include "HTMLFormControlElementWithState.h"
+#include "HTMLOptionElement.h"
 #include "TypeAhead.h"
 #include <wtf/Vector.h>
 
 namespace WebCore {
 
-class HTMLOptionElement;
 class HTMLOptionsCollection;
 
 class HTMLSelectElement : public HTMLFormControlElementWithState, public TypeAheadDataSource {
@@ -87,8 +87,8 @@ public:
     void setOption(unsigned index, HTMLOptionElement*, ExceptionCode&);
     void setLength(unsigned, ExceptionCode&);
 
-    Node* namedItem(const AtomicString& name);
-    Node* item(unsigned index);
+    HTMLOptionElement* namedItem(const AtomicString& name);
+    HTMLOptionElement* item(unsigned index);
 
     void scrollToSelection();
 
index d139fe4..46b6842 100644 (file)
@@ -26,6 +26,7 @@
 #include "config.h"
 #include "AxisScrollSnapOffsets.h"
 
+#include "ElementChildIterator.h"
 #include "HTMLCollection.h"
 #include "HTMLElement.h"
 #include "RenderBox.h"
@@ -38,16 +39,16 @@ namespace WebCore {
 
 static void appendChildSnapOffsets(HTMLElement& parent, bool shouldAddHorizontalChildOffsets, Vector<LayoutUnit>& horizontalSnapOffsetSubsequence, bool shouldAddVerticalChildOffsets, Vector<LayoutUnit>& verticalSnapOffsetSubsequence)
 {
-    Element* child = parent.children()->collectionBegin();
     // FIXME: Instead of traversing all children, register children with snap coordinates before appending to snapOffsetSubsequence.
-    while (child) {
-        if (RenderBox* box = child->renderBox()) {
+    for (auto& child : childrenOfType<Element>(parent)) {
+        if (RenderBox* box = child.renderBox()) {
             LayoutUnit viewWidth = box->width();
             LayoutUnit viewHeight = box->height();
 #if PLATFORM(IOS)
-            // FIXME: Investigate why using localToContainerPoint gives the wrong offsets for iOS mainframe. Also, these offsets won't take transforms into account (make sure to test this!)
-            float left = child->offsetLeft();
-            float top = child->offsetTop();
+            // FIXME: Dangerous to call offsetLeft and offsetTop because they call updateLayoutIgnorePendingStylesheets, which can invalidate the RenderBox pointer we are holding.
+            // FIXME: Investigate why using localToContainerPoint gives the wrong offsets for iOS main frame. Also, these offsets won't take transforms into account (make sure to test this!).
+            float left = child.offsetLeft();
+            float top = child.offsetTop();
 #else
             // FIXME: Check that localToContainerPoint works with CSS rotations.
             FloatPoint position = box->localToContainerPoint(FloatPoint(), parent.renderBox());
@@ -64,7 +65,6 @@ static void appendChildSnapOffsets(HTMLElement& parent, bool shouldAddHorizontal
                     verticalSnapOffsetSubsequence.append(lastPotentialSnapPositionY);
             }
         }
-        child = child->nextElementSibling();
     }
 }