Bug 121769 – CTTE: StaticNodeLists often contain only Elements, we shouldn't store them as Vector<RefPtr<Node>> in those cases

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Sam Weinig]

CTTE: StaticNodeLists often contain only Elements, we shouldn't store them as Vector<RefPtr<Node>> in those cases

[Sam Weinig]

Created attachment 212307 [details]
Patch

[WebKit Commit Bot]

Attachment 212307 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/accessibility/AccessibilityRenderObject.cpp', u'Source/WebCore/bindings/js/JSHTMLAllCollectionCustom.cpp', u'Source/WebCore/bindings/js/JSHTMLCollectionCustom.cpp', u'Source/WebCore/bindings/js/JSHTMLFormControlsCollectionCustom.cpp', u'Source/WebCore/bindings/js/JSHTMLFormElementCustom.cpp', u'Source/WebCore/dom/SelectorQuery.cpp', u'Source/WebCore/dom/StaticNodeList.cpp', u'Source/WebCore/dom/StaticNodeList.h', u'Source/WebCore/dom/WebKitNamedFlow.cpp', u'Source/WebCore/html/HTMLCollection.cpp', u'Source/WebCore/html/HTMLCollection.h', u'Source/WebCore/html/HTMLFormElement.cpp', u'Source/WebCore/html/HTMLFormElement.h', u'Source/WebCore/svg/SVGSVGElement.cpp']" exit_code: 1
Source/WebCore/html/HTMLCollection.cpp:649:  Missing spaces around >>  [whitespace/operators] [3]
Source/WebCore/html/HTMLFormElement.cpp:676:  Missing spaces around >>  [whitespace/operators] [3]
Source/WebCore/html/HTMLCollection.h:46:  Missing spaces around >>  [whitespace/operators] [3]
Source/WebCore/html/HTMLFormElement.h:51:  Missing spaces around >>  [whitespace/operators] [3]
Source/WebCore/dom/StaticNodeList.h:42:  Missing spaces around >>  [whitespace/operators] [3]
Source/WebCore/dom/StaticNodeList.h:66:  Missing spaces around >>  [whitespace/operators] [3]
Total errors found: 6 in 14 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Andreas Kling]

Comment on attachment 212307 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=212307&action=review

r=me

> Source/WebCore/bindings/js/JSHTMLAllCollectionCustom.cpp:46
> +    Vector<Ref<Element>> namedItems;
>      collection->impl()->namedItems(propertyNameToAtomicString(propertyName), namedItems);

Would be nice to make namedItems() return a vector instead of taking an out-parameter.

> Source/WebCore/dom/StaticNodeList.cpp:69
> +    return 0;

nullptr

> Source/WebCore/dom/StaticNodeList.cpp:75
> +    size_t length = m_elements.size();
> +    for (size_t i = 0; i < length; ++i) {

Modern WebKit style would be:
for (unsigned i = 0, length = m_elements.size(); i < length; ++i) {

> Source/WebCore/dom/StaticNodeList.cpp:81
> +    return 0;

nullptr

> Source/WebCore/dom/StaticNodeList.h:80
> +    virtual Node* item(unsigned index) const OVERRIDE;
> +    virtual Node* namedItem(const AtomicString&) const OVERRIDE;

Can we make these return Element* instead? (C++ supports covariant return types for virtuals, no?)

[Sam Weinig]

Committed r156251: <http://trac.webkit.org/changeset/156251>