Simplify protocol between SELECT elements and popups.

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

Index: third_party/WebKit/Source/core/html/HTMLSelectElement.cpp
diff --git a/third_party/WebKit/Source/core/html/HTMLSelectElement.cpp b/third_party/WebKit/Source/core/html/HTMLSelectElement.cpp
index c39608b845960f171043cbd4dbd028054d7e37a1..b3d3b5262429c28b0e6886db4de4d485cb7081e1 100644
--- a/third_party/WebKit/Source/core/html/HTMLSelectElement.cpp
+++ b/third_party/WebKit/Source/core/html/HTMLSelectElement.cpp
@@ -208,18 +208,19 @@ String HTMLSelectElement::defaultToolTip() const
     return validationMessage();
 }
 
-void HTMLSelectElement::listBoxSelectItem(int listIndex, bool allowMultiplySelections, bool fireOnChangeNow)
+void HTMLSelectElement::selectMultipleOptionsByPopup(const Vector<int>& listIndices)
 {
-    if (!multiple()) {
-        optionSelectedByUser(listToOptionIndex(listIndex), fireOnChangeNow);
-    } else {
-        HTMLElement* element = listItems()[listIndex];
+    DCHECK(usesMenuList());
+    DCHECK(!multiple());

[wychen, 2016/09/29 02:42:57]

Should this line be DCHECK(multiple())? I'm a bit confused.

[tkent, 2016/09/29 03:14:55]

I think so :)

+    for (size_t i = 0; i < listIndices.size(); ++i) {
+        bool addSelectionIfNotFirst = i > 0;
+        HTMLElement* element = listItems()[listIndices[i]];
         if (isHTMLOptionElement(element))
-            updateSelectedState(toHTMLOptionElement(element), allowMultiplySelections, false);
-        setNeedsValidityCheck();
-        if (fireOnChangeNow)
-            listBoxOnChange();
+            updateSelectedState(toHTMLOptionElement(element), addSelectionIfNotFirst, false);
     }
+    setNeedsValidityCheck();
+    // TODO(tkent): Using listBoxOnChange() is very confusing.
+    listBoxOnChange();
 }
 
 bool HTMLSelectElement::usesMenuList() const
@@ -1267,8 +1268,8 @@ void HTMLSelectElement::handlePopupOpenKeyboardEvent(Event* event)
         return;
     // Save the selection so it can be compared to the new selection when
     // dispatching change events during selectOption, which gets called from
-    // valueChanged, which gets called after the user makes a selection from the
-    // menu.
+    // selectOptionByPopup, which gets called after the user makes a selection
+    // from the menu.
     saveLastSelection();
     showPopup();
     event->setDefaultHandled();
@@ -1386,8 +1387,8 @@ void HTMLSelectElement::menuListDefaultEventHandler(Event* event)
             } else {
                 // Save the selection so it can be compared to the new selection
                 // when we call onChange during selectOption, which gets called
-                // from valueChanged, which gets called after the user makes a
-                // selection from the menu.
+                // from selectOptionByPopup, which gets called after the user
+                // makes a selection from the menu.
                 saveLastSelection();
                 // TODO(lanwei): Will check if we need to add
                 // InputDeviceCapabilities here when select menu list gets
@@ -1923,8 +1924,9 @@ HTMLOptionElement* HTMLSelectElement::optionToBeShown() const
     return m_lastOnChangeOption;
 }
 
-void HTMLSelectElement::valueChanged(unsigned listIndex)
+void HTMLSelectElement::selectOptionByPopup(int listIndex)
 {
+    DCHECK(usesMenuList());
     // Check to ensure a page navigation has not occurred while the popup was
     // up.
     Document& doc = document();
@@ -1938,7 +1940,7 @@ void HTMLSelectElement::valueChanged(unsigned listIndex)
 void HTMLSelectElement::popupDidCancel()
 {
     if (m_indexToSelectOnCancel >= 0)
-        valueChanged(m_indexToSelectOnCancel);
+        selectOptionByPopup(m_indexToSelectOnCancel);
 }
 
 void HTMLSelectElement::provisionalSelectionChanged(unsigned listIndex)