Move HTMLSelectElement::ListBoxOnChange() to SelectType

This CL has no behavior changes.

Bug: 1052232
Change-Id: Ib6e3b3522b405f75cdd3164c4117155198837279
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2106979
Commit-Queue: Kent Tamura <tkent@chromium.org>
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Auto-Submit: Kent Tamura <tkent@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#751239}

third_party/blink/renderer/core/html/forms/html_select_element.cc

@@ -290,8 +290,8 @@ void HTMLSelectElement::setValue(const String& value, bool send_events) {
     flags |= kDispatchInputAndChangeEventFlag;
   SelectOption(option, flags);
 
-  if (send_events && previous_selected_option != option && !UsesMenuList())
-    ListBoxOnChange();
+  if (send_events && previous_selected_option != option)
+    select_type_->ListBoxOnChange();
 }
 
 String HTMLSelectElement::SuggestedValue() const {
@@ -532,37 +532,6 @@ void HTMLSelectElement::SetActiveSelectionEnd(HTMLOptionElement* option) {
   active_selection_end_ = option;
 }
 
-void HTMLSelectElement::ListBoxOnChange() {
-  DCHECK(!UsesMenuList());
-
-  const ListItems& items = GetListItems();
-
-  // If the cached selection list is empty, or the size has changed, then fire
-  // dispatchFormControlChangeEvent, and return early.
-  // FIXME: Why? This looks unreasonable.
-  if (last_on_change_selection_.IsEmpty() ||
-      last_on_change_selection_.size() != items.size()) {
-    DispatchChangeEvent();
-    return;
-  }
-
-  // Update last_on_change_selection_ and fire a 'change' event.
-  bool fire_on_change = false;
-  for (unsigned i = 0; i < items.size(); ++i) {
-    HTMLElement* element = items[i];
-    auto* option_element = DynamicTo<HTMLOptionElement>(element);
-    bool selected = option_element && option_element->Selected();
-    if (selected != last_on_change_selection_[i])
-      fire_on_change = true;
-    last_on_change_selection_[i] = selected;
-  }
-
-  if (fire_on_change) {
-    DispatchInputEvent();
-    DispatchChangeEvent();
-  }
-}
-
 void HTMLSelectElement::ScrollToSelection() {
   if (!IsFinishedParsingChildren())
     return;
@@ -1223,8 +1192,7 @@ void HTMLSelectElement::TypeAheadFind(const KeyboardEvent& event) {
   SelectOption(OptionAtListIndex(index), kDeselectOtherOptionsFlag |
                                              kMakeOptionDirtyFlag |
                                              kDispatchInputAndChangeEventFlag);
-  if (!UsesMenuList())
-    ListBoxOnChange();
+  select_type_->ListBoxOnChange();
 }
 
 void HTMLSelectElement::SelectOptionByAccessKey(HTMLOptionElement* option) {
@@ -1250,7 +1218,7 @@ void HTMLSelectElement::SelectOptionByAccessKey(HTMLOptionElement* option) {
   option->SetDirty(true);
   if (UsesMenuList())
     return;
-  ListBoxOnChange();
+  select_type_->ListBoxOnChange();
   ScrollToSelection();
 }
 

third_party/blink/renderer/core/html/forms/html_select_element.h

@@ -127,7 +127,6 @@ class CORE_EXPORT HTMLSelectElement final
 
   bool CanSelectAll() const;
   void SelectAll();
-  void ListBoxOnChange();
   int ActiveSelectionEndListIndex() const;
   HTMLOptionElement* ActiveSelectionEnd() const;
   void SetActiveSelectionAnchor(HTMLOptionElement*);

third_party/blink/renderer/core/html/forms/select_type.cc

@@ -612,6 +612,7 @@ class ListBoxSelectType final : public SelectType {
   void SelectAll() override;
   void SaveListboxActiveSelection() override;
   void HandleMouseRelease() override;
+  void ListBoxOnChange() override;
 
  private:
   HTMLOptionElement* NextSelectableOptionPageAway(HTMLOptionElement*,
@@ -649,7 +650,7 @@ bool ListBoxSelectType::DefaultEventHandler(const Event& event) {
         UpdateSelectedState(option, gesture_event->shiftKey()
                                         ? SelectionMode::kRange
                                         : SelectionMode::kNotChangeOthers);
-        select_->ListBoxOnChange();
+        ListBoxOnChange();
       }
       return true;
     }
@@ -846,7 +847,7 @@ bool ListBoxSelectType::DefaultEventHandler(const Event& event) {
       if (select_new_item || is_in_non_contiguous_selection_) {
         if (select_new_item) {
           UpdateListBoxSelection(deselect_others);
-          select_->ListBoxOnChange();
+          ListBoxOnChange();
         }
         UpdateMultiSelectFocus();
       } else {
@@ -919,7 +920,7 @@ void ListBoxSelectType::SelectAll() {
   select_->SetActiveSelectionEnd(PreviousSelectableOption(nullptr));
 
   UpdateListBoxSelection(false, false);
-  select_->ListBoxOnChange();
+  ListBoxOnChange();
   select_->SetNeedsValidityCheck();
 }
 
@@ -947,7 +948,7 @@ HTMLOptionElement* ListBoxSelectType::NextSelectableOptionPageAway(
 void ListBoxSelectType::ToggleSelection(HTMLOptionElement& option) {
   active_selection_state_ = !active_selection_state_;
   UpdateSelectedState(&option, SelectionMode::kNotChangeOthers);
-  select_->ListBoxOnChange();
+  ListBoxOnChange();
 }
 
 void ListBoxSelectType::UpdateSelectedState(HTMLOptionElement* clicked_option,
@@ -1058,7 +1059,36 @@ void ListBoxSelectType::HandleMouseRelease() {
   // We didn't start this click/drag on any options.
   if (select_->last_on_change_selection_.IsEmpty())
     return;
-  select_->ListBoxOnChange();
+  ListBoxOnChange();
+}
+
+void ListBoxSelectType::ListBoxOnChange() {
+  const auto& items = select_->GetListItems();
+
+  // If the cached selection list is empty, or the size has changed, then fire
+  // 'change' event, and return early.
+  // FIXME: Why? This looks unreasonable.
+  if (select_->last_on_change_selection_.IsEmpty() ||
+      select_->last_on_change_selection_.size() != items.size()) {
+    select_->DispatchChangeEvent();
+    return;
+  }
+
+  // Update last_on_change_selection_ and fire a 'change' event.
+  bool fire_on_change = false;
+  for (unsigned i = 0; i < items.size(); ++i) {
+    HTMLElement* element = items[i];
+    auto* option_element = DynamicTo<HTMLOptionElement>(element);
+    bool selected = option_element && option_element->Selected();
+    if (selected != select_->last_on_change_selection_[i])
+      fire_on_change = true;
+    select_->last_on_change_selection_[i] = selected;
+  }
+
+  if (fire_on_change) {
+    select_->DispatchInputEvent();
+    select_->DispatchChangeEvent();
+  }
 }
 
 // ============================================================================
@@ -1119,6 +1149,8 @@ void SelectType::SaveListboxActiveSelection() {}
 
 void SelectType::HandleMouseRelease() {}
 
+void SelectType::ListBoxOnChange() {}
+
 void SelectType::ShowPopup() {
   NOTREACHED();
 }

third_party/blink/renderer/core/html/forms/select_type.h

@@ -47,6 +47,7 @@ class SelectType : public GarbageCollected<SelectType> {
   virtual void SelectAll();
   virtual void SaveListboxActiveSelection();
   virtual void HandleMouseRelease();
+  virtual void ListBoxOnChange();
 
   virtual void ShowPopup();
   virtual void HidePopup();