SELECT element: Avoid to use listItems() in HTMLSelectElement::selectOption()

Remove listItems() usage from:
 - HTMLOptionElement::index()
 - HTMLSelectElement::saveListboxActiveSelection()
 - HTMLSelectElement::selectedOption()
 - HTMLSelectElement::deselectItemsWithoutValidation()
We use optionList() instead.

These changes don't improve computational complexity. However we can avoid to
update m_listItems when we add/remove OPTIONs.

Other changes:
 - HTMLSelectElement::updateListBoxSelectioN() needs to be updated to synchronize
  with saveListboxActiveSelection().  m_cachedStateForActiveSelection now
  represents OPTIONs instead of listItems, which includes OPTGROUP and HR.
 - Change the arguments of OptionList and OptionListIterator to |const| so that
  we can create them in const member functions of HTMLSelectElement.
 - Remove the optimized fast path in HTMLSelectElement::setRecalcListItems(). We
  don't use m_listItems in critical paths any longer.  So the optimization isn't
  necessary.

This CL improves performance because of the removal of the fast path.
blink_perf.dom:
select-multiple-add: 1,772 runs/s ->  1,912 runs/s
select-single-add: 867.9 runs/s -> 944.7 runs/s
select-single-remove: 137.9 runs/s -> 145.5 runs/.s

BUG=577989

Review-Url: https://codereview.chromium.org/2131073002
Cr-Commit-Position: refs/heads/master@{#404591}

third_party/WebKit/Source/core/html/HTMLOptionElement.cpp

@@ -183,13 +183,8 @@ int HTMLOptionElement::index() const
         return 0;
 
     int optionIndex = 0;
-
-    const HeapVector<Member<HTMLElement>>& items = selectElement->listItems();
-    size_t length = items.size();
-    for (size_t i = 0; i < length; ++i) {
-        if (!isHTMLOptionElement(*items[i]))
-            continue;
-        if (items[i].get() == this)
+    for (const auto& option : selectElement->optionList()) {
+        if (option == this)
             return optionIndex;
         ++optionIndex;
     }

third_party/WebKit/Source/core/html/HTMLSelectElement.cpp

@@ -644,8 +644,8 @@ void HTMLSelectElement::saveListboxActiveSelection()
     //   m_activeSelectionEndIndex = 3, options at 1-3 indices are selected.
     //   updateListBoxSelection needs to clear selection of the fifth OPTION.
     m_cachedStateForActiveSelection.resize(0);
-    for (auto& element : listItems()) {
-        m_cachedStateForActiveSelection.append(isHTMLOptionElement(*element) && toHTMLOptionElement(element)->selected());
+    for (const auto& option : optionList()) {
+        m_cachedStateForActiveSelection.append(option->selected());
     }
 }
 
@@ -659,27 +659,27 @@ void HTMLSelectElement::updateListBoxSelection(bool deselectOtherOptions, bool s
     ASSERT(layoutObject());
     ASSERT(layoutObject()->isListBox() || m_multiple);
 
-    int activeSelectionAnchorIndex = m_activeSelectionAnchor ? m_activeSelectionAnchor->listIndex() : -1;
-    int activeSelectionEndIndex = m_activeSelectionEnd ? m_activeSelectionEnd->listIndex() : -1;
+    int activeSelectionAnchorIndex = m_activeSelectionAnchor ? m_activeSelectionAnchor->index() : -1;
+    int activeSelectionEndIndex = m_activeSelectionEnd ? m_activeSelectionEnd->index() : -1;
     int start = std::min(activeSelectionAnchorIndex, activeSelectionEndIndex);
     int end = std::max(activeSelectionAnchorIndex, activeSelectionEndIndex);
 
-    const ListItems& items = listItems();
-    for (int i = 0; i < static_cast<int>(items.size()); ++i) {
-        if (!isHTMLOptionElement(*items[i]))
-            continue;
-        HTMLOptionElement& option = toHTMLOptionElement(*items[i]);
-        if (option.isDisabledFormControl() || !option.layoutObject())
+    int i = 0;
+    for (const auto& option : optionList()) {
+        if (option->isDisabledFormControl() || !option->layoutObject()) {
+            ++i;
             continue;
+        }
         if (i >= start && i <= end) {
-            option.setSelectedState(m_activeSelectionState);
-            option.setDirty(true);
+            option->setSelectedState(m_activeSelectionState);
+            option->setDirty(true);
         } else if (deselectOtherOptions || i >= static_cast<int>(m_cachedStateForActiveSelection.size())) {
-            option.setSelectedState(false);
-            option.setDirty(true);
+            option->setSelectedState(false);
+            option->setDirty(true);
         } else {
-            option.setSelectedState(m_cachedStateForActiveSelection[i]);
+            option->setSelectedState(m_cachedStateForActiveSelection[i]);
         }
+        ++i;
     }
 
     setNeedsValidityCheck();
@@ -770,54 +770,12 @@ void HTMLSelectElement::invalidateSelectedItems()
         collection->invalidateCache();
 }
 
-void HTMLSelectElement::setRecalcListItems(HTMLElement& subject)
+void HTMLSelectElement::setRecalcListItems()
 {
     // FIXME: This function does a bunch of confusing things depending on if it
     // is in the document or not.
 
-    bool shouldRecalc = true;
-    if (!m_shouldRecalcListItems && !isHTMLOptGroupElement(subject)) {
-        if (firstChild() == &subject) {
-            // The subject was prepended. This doesn't handle elements in an
-            // OPTGROUP.
-            DCHECK(m_listItems.size() == 0 || m_listItems[0] != &subject);
-            m_listItems.prepend(&subject);
-            shouldRecalc = false;
-        } else if (lastChild() == &subject) {
-            // The subject was appended. This doesn't handle elements in an
-            // OPTGROUP.
-            DCHECK(m_listItems.size() == 0 || m_listItems.last() != &subject);
-            m_listItems.append(&subject);
-            shouldRecalc = false;
-        } else if (!subject.isDescendantOf(this)) {
-            // |subject| was removed from this. This logic works well with
-            // SELECT children and OPTGROUP children.
-
-            // m_listItems might be empty, or might not have the OPTION.
-            //   1. Remove an OPTGROUP with OPTION children from a SELECT.
-            //   2. This function is called for the OPTGROUP removal.
-            //   3. m_shouldRecalcListItems becomes true.
-            //   4. recalcListItems() happens.  m_listItems has no OPTGROUP and
-            //      no its children.  m_shouldRecalcListItems becomes false.
-            //   5. This function is called for the removal of an OPTION child
-            //      of the OPTGROUP.
-            if (m_listItems.size() > 0) {
-                size_t index;
-                // Avoid Vector::find() in typical cases.
-                if (m_listItems.first() == &subject)
-                    index = 0;
-                else if (m_listItems.last() == &subject)
-                    index = m_listItems.size() - 1;
-                else
-                    index = m_listItems.find(&subject);
-                if (index != WTF::kNotFound) {
-                    m_listItems.remove(index);
-                    shouldRecalc = false;
-                }
-            }
-        }
-    }
-    m_shouldRecalcListItems = shouldRecalc;
+    m_shouldRecalcListItems = true;
 
     setOptionsChangedOnLayoutObject();
     if (!inShadowIncludingDocument()) {
@@ -918,9 +876,9 @@ void HTMLSelectElement::resetToDefaultSelection(ResetReason reason)
 
 HTMLOptionElement* HTMLSelectElement::selectedOption() const
 {
-    for (const auto& element : listItems()) {
-        if (isHTMLOptionElement(*element) && toHTMLOptionElement(*element).selected())
-            return toHTMLOptionElement(element);
+    for (const auto option : optionList()) {
+        if (option->selected())
+            return option;
     }
     return nullptr;
 }
@@ -1004,7 +962,7 @@ void HTMLSelectElement::optionSelectionStateChanged(HTMLOptionElement* option, b
 void HTMLSelectElement::optionInserted(HTMLOptionElement& option, bool optionIsSelected)
 {
     ASSERT(option.ownerSelectElement() == this);
-    setRecalcListItems(option);
+    setRecalcListItems();
     if (optionIsSelected) {
         selectOption(&option, multiple() ? 0 : DeselectOtherOptions);
     } else {
@@ -1018,7 +976,7 @@ void HTMLSelectElement::optionInserted(HTMLOptionElement& option, bool optionIsS
 
 void HTMLSelectElement::optionRemoved(HTMLOptionElement& option)
 {
-    setRecalcListItems(option);
+    setRecalcListItems();
     if (option.selected())
         resetToDefaultSelection(ResetReasonSelectedOptionRemoved);
     else if (!m_lastOnChangeOption)
@@ -1041,14 +999,14 @@ void HTMLSelectElement::optionRemoved(HTMLOptionElement& option)
 
 void HTMLSelectElement::optGroupInsertedOrRemoved(HTMLOptGroupElement& optgroup)
 {
-    setRecalcListItems(optgroup);
+    setRecalcListItems();
     setNeedsValidityCheck();
     m_lastOnChangeSelection.clear();
 }
 
 void HTMLSelectElement::hrInsertedOrRemoved(HTMLHRElement& hr)
 {
-    setRecalcListItems(hr);
+    setRecalcListItems();
     m_lastOnChangeSelection.clear();
 }
 
@@ -1174,15 +1132,15 @@ void HTMLSelectElement::dispatchBlurEvent(Element* newFocusedElement, WebFocusTy
     HTMLFormControlElementWithState::dispatchBlurEvent(newFocusedElement, type, sourceCapabilities);
 }
 
-void HTMLSelectElement::deselectItemsWithoutValidation(HTMLElement* excludeElement)
+void HTMLSelectElement::deselectItemsWithoutValidation(HTMLOptionElement* excludeElement)
 {
     if (!multiple() && usesMenuList() && m_lastOnChangeOption && m_lastOnChangeOption != excludeElement) {
         m_lastOnChangeOption->setSelectedState(false);
         return;
     }
-    for (auto& element : listItems()) {
-        if (element != excludeElement && isHTMLOptionElement(*element))
-            toHTMLOptionElement(element)->setSelectedState(false);
+    for (const auto& option : optionList()) {
+        if (option != excludeElement)
+            option->setSelectedState(false);
     }
 }
 

third_party/WebKit/Source/core/html/HTMLSelectElement.h

@@ -93,7 +93,7 @@ public:
     // This is similar to |options| HTMLCollection.  But this is safe in
     // HTMLOptionElement::removedFrom() and insertedInto().
     // OptionList supports only forward iteration.
-    OptionList optionList() { return OptionList(*this); }
+    OptionList optionList() const { return OptionList(*this); }
 
     void optionElementChildrenChanged(const HTMLOptionElement&);
 
@@ -204,8 +204,7 @@ private:
 
     void dispatchInputAndChangeEventForMenuList();
 
-    // |subject| is an element which was inserted or removed.
-    void setRecalcListItems(HTMLElement& subject);
+    void setRecalcListItems();
     void recalcListItems() const;
     enum ResetReason {
         ResetReasonSelectedOptionRemoved,
@@ -230,7 +229,7 @@ private:
     };
     typedef unsigned SelectOptionFlags;
     void selectOption(HTMLOptionElement*, SelectOptionFlags);
-    void deselectItemsWithoutValidation(HTMLElement* elementToExclude = 0);
+    void deselectItemsWithoutValidation(HTMLOptionElement* elementToExclude = nullptr);
     void parseMultipleAttribute(const AtomicString&);
     HTMLOptionElement* lastSelectedOption() const;
     void updateSelectedState(HTMLOptionElement*, bool multi, bool shift);

third_party/WebKit/Source/core/html/forms/OptionList.cpp

@@ -28,7 +28,7 @@ void OptionListIterator::advance(HTMLOptionElement* previous)
             m_current = toHTMLOptionElement(current);
             return;
         }
-        if (isHTMLOptGroupElement(current) && current->parentNode() == m_select) {
+        if (isHTMLOptGroupElement(current) && current->parentNode() == m_select.get()) {
             if ((m_current = Traversal<HTMLOptionElement>::firstChild(*current)))
                 return;
         }

third_party/WebKit/Source/core/html/forms/OptionList.h

@@ -16,7 +16,7 @@ class HTMLOptionElement;
 class CORE_EXPORT OptionListIterator final {
     STACK_ALLOCATED();
 public:
-    explicit OptionListIterator(HTMLSelectElement* select) : m_select(select)
+    explicit OptionListIterator(const HTMLSelectElement* select) : m_select(select)
     {
         if (m_select)
             advance(nullptr);
@@ -33,7 +33,7 @@ public:
 private:
     void advance(HTMLOptionElement* current);
 
-    Member<HTMLSelectElement> m_select;
+    Member<const HTMLSelectElement> m_select;
     Member<HTMLOptionElement> m_current; // nullptr means we reached to the end.
 };
 
@@ -41,13 +41,13 @@ private:
 class OptionList final {
     STACK_ALLOCATED();
 public:
-    explicit OptionList(HTMLSelectElement& select) : m_select(select) {}
+    explicit OptionList(const HTMLSelectElement& select) : m_select(select) {}
     using Iterator = OptionListIterator;
     Iterator begin() { return Iterator(m_select); }
     Iterator end() { return Iterator(nullptr); }
 
 private:
-    Member<HTMLSelectElement> m_select;
+    Member<const HTMLSelectElement> m_select;
 };
 
 } // namespace blink