Move NextValidOption() and *SelectableOption() of HTMLSelectElement to SelectType.

Also, move NextSelectableOptionPageAway() to ListBoxSelectType.
This CL has no behavior changes.

Bug: 1052232
Change-Id: Icc0d4e3c3f651473170c9904dbf6db47bb66a6de
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2068070Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#743467}

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

@@ -490,82 +490,6 @@ HTMLOptionElement* HTMLSelectElement::OptionAtListIndex(int list_index) const {
   return DynamicTo<HTMLOptionElement>(items[list_index].Get());
 }
 
-// Returns the 1st valid OPTION |skip| items from |listIndex| in direction
-// |direction| if there is one.
-// Otherwise, it returns the valid OPTION closest to that boundary which is past
-// |listIndex| if there is one.
-// Otherwise, it returns nullptr.
-// Valid means that it is enabled and visible.
-HTMLOptionElement* HTMLSelectElement::NextValidOption(int list_index,
-                                                      SkipDirection direction,
-                                                      int skip) const {
-  DCHECK(direction == kSkipBackwards || direction == kSkipForwards);
-  const ListItems& list_items = GetListItems();
-  HTMLOptionElement* last_good_option = nullptr;
-  int size = list_items.size();
-  for (list_index += direction; list_index >= 0 && list_index < size;
-       list_index += direction) {
-    --skip;
-    HTMLElement* element = list_items[list_index];
-    auto* option_element = DynamicTo<HTMLOptionElement>(element);
-    if (!option_element)
-      continue;
-    if (option_element->IsDisplayNone())
-      continue;
-    if (element->IsDisabledFormControl())
-      continue;
-    if (!UsesMenuList() && !element->GetLayoutObject())
-      continue;
-    last_good_option = option_element;
-    if (skip <= 0)
-      break;
-  }
-  return last_good_option;
-}
-
-HTMLOptionElement* HTMLSelectElement::NextSelectableOption(
-    HTMLOptionElement* start_option) const {
-  return NextValidOption(start_option ? start_option->ListIndex() : -1,
-                         kSkipForwards, 1);
-}
-
-HTMLOptionElement* HTMLSelectElement::PreviousSelectableOption(
-    HTMLOptionElement* start_option) const {
-  return NextValidOption(
-      start_option ? start_option->ListIndex() : GetListItems().size(),
-      kSkipBackwards, 1);
-}
-
-HTMLOptionElement* HTMLSelectElement::FirstSelectableOption() const {
-  return NextValidOption(-1, kSkipForwards, 1);
-}
-
-HTMLOptionElement* HTMLSelectElement::LastSelectableOption() const {
-  return NextValidOption(GetListItems().size(), kSkipBackwards, 1);
-}
-
-// Returns the index of the next valid item one page away from |startIndex| in
-// direction |direction|.
-HTMLOptionElement* HTMLSelectElement::NextSelectableOptionPageAway(
-    HTMLOptionElement* start_option,
-    SkipDirection direction) const {
-  DCHECK(!UsesMenuList());
-  const ListItems& items = GetListItems();
-  // -1 so we still show context.
-  int page_size = ListBoxSize() - 1;
-
-  // One page away, but not outside valid bounds.
-  // If there is a valid option item one page away, the index is chosen.
-  // If there is no exact one page away valid option, returns startIndex or
-  // the most far index.
-  int start_index = start_option ? start_option->ListIndex() : -1;
-  int edge_index = (direction == kSkipForwards) ? 0 : (items.size() - 1);
-  int skip_amount =
-      page_size +
-      ((direction == kSkipForwards) ? start_index : (edge_index - start_index));
-  return NextValidOption(edge_index, direction, skip_amount);
-}
-
 void HTMLSelectElement::SelectAll() {
   select_type_->SelectAll();
 }
@@ -1532,7 +1456,7 @@ HTMLOptionElement* HTMLSelectElement::SpatialNavigationFocusedOption() {
     return nullptr;
   HTMLOptionElement* focused_option = ActiveSelectionEnd();
   if (!focused_option)
-    focused_option = FirstSelectableOption();
+    focused_option = select_type_->FirstSelectableOption();
   return focused_option;
 }
 

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

@@ -268,16 +268,7 @@ class CORE_EXPORT HTMLSelectElement final
   // Returns nullptr if listIndex is out of bounds, or it doesn't point an
   // HTMLOptionElement.
   HTMLOptionElement* OptionAtListIndex(int list_index) const;
-  enum SkipDirection { kSkipBackwards = -1, kSkipForwards = 1 };
-  HTMLOptionElement* NextValidOption(int list_index,
-                                     SkipDirection,
-                                     int skip) const;
-  HTMLOptionElement* NextSelectableOption(HTMLOptionElement*) const;
-  HTMLOptionElement* PreviousSelectableOption(HTMLOptionElement*) const;
-  HTMLOptionElement* FirstSelectableOption() const;
-  HTMLOptionElement* LastSelectableOption() const;
-  HTMLOptionElement* NextSelectableOptionPageAway(HTMLOptionElement*,
-                                                  SkipDirection) const;
+
   AutoscrollController* GetAutoscrollController() const;
   LayoutBox* AutoscrollBox() override;
   void StopAutoscroll() override;
@@ -328,10 +319,7 @@ class CORE_EXPORT HTMLSelectElement final
   friend class ListBoxSelectType;
   friend class MenuListSelectType;
   friend class SelectType;
-  FRIEND_TEST_ALL_PREFIXES(HTMLSelectElementTest, FirstSelectableOption);
-  FRIEND_TEST_ALL_PREFIXES(HTMLSelectElementTest, LastSelectableOption);
-  FRIEND_TEST_ALL_PREFIXES(HTMLSelectElementTest, NextSelectableOption);
-  FRIEND_TEST_ALL_PREFIXES(HTMLSelectElementTest, PreviousSelectableOption);
+  friend class HTMLSelectElementTest;
 };
 
 }  // namespace blink

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

@@ -124,28 +124,21 @@ bool MenuListSelectType::DefaultEventHandler(const Event& event) {
 
     const String& key = key_event->key();
     bool handled = true;
-    const HTMLSelectElement::ListItems& list_items = select_->GetListItems();
     HTMLOptionElement* option = select_->SelectedOption();
     int list_index = option ? option->ListIndex() : -1;
 
     if (key == "ArrowDown" || key == "ArrowRight") {
-      option = select_->NextValidOption(list_index,
-                                        HTMLSelectElement::kSkipForwards, 1);
+      option = NextValidOption(list_index, kSkipForwards, 1);
     } else if (key == "ArrowUp" || key == "ArrowLeft") {
-      option = select_->NextValidOption(list_index,
-                                        HTMLSelectElement::kSkipBackwards, 1);
+      option = NextValidOption(list_index, kSkipBackwards, 1);
     } else if (key == "PageDown") {
-      option = select_->NextValidOption(list_index,
-                                        HTMLSelectElement::kSkipForwards, 3);
+      option = NextValidOption(list_index, kSkipForwards, 3);
     } else if (key == "PageUp") {
-      option = select_->NextValidOption(list_index,
-                                        HTMLSelectElement::kSkipBackwards, 3);
+      option = NextValidOption(list_index, kSkipBackwards, 3);
     } else if (key == "Home") {
-      option =
-          select_->NextValidOption(-1, HTMLSelectElement::kSkipForwards, 1);
+      option = FirstSelectableOption();
     } else if (key == "End") {
-      option = select_->NextValidOption(list_items.size(),
-                                        HTMLSelectElement::kSkipBackwards, 1);
+      option = LastSelectableOption();
     } else {
       handled = false;
     }
@@ -386,6 +379,9 @@ class ListBoxSelectType final : public SelectType {
   void SelectAll() override;
 
  private:
+  HTMLOptionElement* NextSelectableOptionPageAway(HTMLOptionElement*,
+                                                  SkipDirection) const;
+
   bool is_in_non_contiguous_selection_ = false;
 };
 
@@ -504,48 +500,45 @@ bool ListBoxSelectType::DefaultEventHandler(const Event& event) {
         HTMLOptionElement* start_option = select_->LastSelectedOption();
         handled = true;
         if (key == "ArrowDown") {
-          end_option = select_->NextSelectableOption(start_option);
+          end_option = NextSelectableOption(start_option);
         } else {
-          end_option = select_->NextSelectableOptionPageAway(
-              start_option, HTMLSelectElement::kSkipForwards);
+          end_option =
+              NextSelectableOptionPageAway(start_option, kSkipForwards);
         }
       } else if (key == "ArrowUp" || key == "PageUp") {
         HTMLOptionElement* start_option = select_->SelectedOption();
         handled = true;
         if (key == "ArrowUp") {
-          end_option = select_->PreviousSelectableOption(start_option);
+          end_option = PreviousSelectableOption(start_option);
         } else {
-          end_option = select_->NextSelectableOptionPageAway(
-              start_option, HTMLSelectElement::kSkipBackwards);
+          end_option =
+              NextSelectableOptionPageAway(start_option, kSkipBackwards);
         }
       }
     } else {
       // Set the end index based on the current end index.
       if (key == "ArrowDown") {
-        end_option =
-            select_->NextSelectableOption(select_->active_selection_end_.Get());
+        end_option = NextSelectableOption(select_->active_selection_end_.Get());
         handled = true;
       } else if (key == "ArrowUp") {
-        end_option = select_->PreviousSelectableOption(
-            select_->active_selection_end_.Get());
+        end_option =
+            PreviousSelectableOption(select_->active_selection_end_.Get());
         handled = true;
       } else if (key == "PageDown") {
-        end_option = select_->NextSelectableOptionPageAway(
-            select_->active_selection_end_.Get(),
-            HTMLSelectElement::kSkipForwards);
+        end_option = NextSelectableOptionPageAway(
+            select_->active_selection_end_.Get(), kSkipForwards);
         handled = true;
       } else if (key == "PageUp") {
-        end_option = select_->NextSelectableOptionPageAway(
-            select_->active_selection_end_.Get(),
-            HTMLSelectElement::kSkipBackwards);
+        end_option = NextSelectableOptionPageAway(
+            select_->active_selection_end_.Get(), kSkipBackwards);
         handled = true;
       }
     }
     if (key == "Home") {
-      end_option = select_->FirstSelectableOption();
+      end_option = FirstSelectableOption();
       handled = true;
     } else if (key == "End") {
-      end_option = select_->LastSelectableOption();
+      end_option = LastSelectableOption();
       handled = true;
     }
 
@@ -629,7 +622,7 @@ bool ListBoxSelectType::DefaultEventHandler(const Event& event) {
       // If there's no active selection,
       // act as if "ArrowDown" had been pressed.
       if (!option)
-        option = select_->NextSelectableOption(select_->LastSelectedOption());
+        option = NextSelectableOption(select_->LastSelectedOption());
       if (option) {
         // Use space to toggle selection change.
         select_->ToggleSelection(*option);
@@ -664,14 +657,35 @@ void ListBoxSelectType::SelectAll() {
   select_->SaveLastSelection();
 
   select_->active_selection_state_ = true;
-  select_->SetActiveSelectionAnchor(select_->NextSelectableOption(nullptr));
-  select_->SetActiveSelectionEnd(select_->PreviousSelectableOption(nullptr));
+  select_->SetActiveSelectionAnchor(NextSelectableOption(nullptr));
+  select_->SetActiveSelectionEnd(PreviousSelectableOption(nullptr));
 
   select_->UpdateListBoxSelection(false, false);
   select_->ListBoxOnChange();
   select_->SetNeedsValidityCheck();
 }
 
+// Returns the index of the next valid item one page away from |start_option|
+// in direction |direction|.
+HTMLOptionElement* ListBoxSelectType::NextSelectableOptionPageAway(
+    HTMLOptionElement* start_option,
+    SkipDirection direction) const {
+  const auto& items = select_->GetListItems();
+  // -1 so we still show context.
+  int page_size = select_->ListBoxSize() - 1;
+
+  // One page away, but not outside valid bounds.
+  // If there is a valid option item one page away, the index is chosen.
+  // If there is no exact one page away valid option, returns start_index or
+  // the most far index.
+  int start_index = start_option ? start_option->ListIndex() : -1;
+  int edge_index = (direction == kSkipForwards) ? 0 : (items.size() - 1);
+  int skip_amount =
+      page_size +
+      ((direction == kSkipForwards) ? start_index : (edge_index - start_index));
+  return NextValidOption(edge_index, direction, skip_amount);
+}
+
 // ============================================================================
 
 SelectType::SelectType(HTMLSelectElement& select) : select_(select) {}
@@ -713,4 +727,58 @@ void SelectType::SelectAll() {
   NOTREACHED();
 }
 
+// Returns the 1st valid OPTION |skip| items from |list_index| in direction
+// |direction| if there is one.
+// Otherwise, it returns the valid OPTION closest to that boundary which is past
+// |list_index| if there is one.
+// Otherwise, it returns nullptr.
+// Valid means that it is enabled and visible.
+HTMLOptionElement* SelectType::NextValidOption(int list_index,
+                                               SkipDirection direction,
+                                               int skip) const {
+  DCHECK(direction == kSkipBackwards || direction == kSkipForwards);
+  const auto& list_items = select_->GetListItems();
+  HTMLOptionElement* last_good_option = nullptr;
+  int size = list_items.size();
+  for (list_index += direction; list_index >= 0 && list_index < size;
+       list_index += direction) {
+    --skip;
+    HTMLElement* element = list_items[list_index];
+    auto* option_element = DynamicTo<HTMLOptionElement>(element);
+    if (!option_element)
+      continue;
+    if (option_element->IsDisplayNone())
+      continue;
+    if (element->IsDisabledFormControl())
+      continue;
+    if (!select_->UsesMenuList() && !element->GetLayoutObject())
+      continue;
+    last_good_option = option_element;
+    if (skip <= 0)
+      break;
+  }
+  return last_good_option;
+}
+
+HTMLOptionElement* SelectType::NextSelectableOption(
+    HTMLOptionElement* start_option) const {
+  return NextValidOption(start_option ? start_option->ListIndex() : -1,
+                         kSkipForwards, 1);
+}
+
+HTMLOptionElement* SelectType::PreviousSelectableOption(
+    HTMLOptionElement* start_option) const {
+  return NextValidOption(
+      start_option ? start_option->ListIndex() : select_->GetListItems().size(),
+      kSkipBackwards, 1);
+}
+
+HTMLOptionElement* SelectType::FirstSelectableOption() const {
+  return NextValidOption(-1, kSkipForwards, 1);
+}
+
+HTMLOptionElement* SelectType::LastSelectableOption() const {
+  return NextValidOption(select_->GetListItems().size(), kSkipBackwards, 1);
+}
+
 }  // namespace blink

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

@@ -5,6 +5,7 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_HTML_FORMS_SELECT_TYPE_H_
 #define THIRD_PARTY_BLINK_RENDERER_CORE_HTML_FORMS_SELECT_TYPE_H_
 
+#include "third_party/blink/renderer/core/core_export.h"
 #include "third_party/blink/renderer/core/html/forms/html_select_element.h"
 #include "third_party/blink/renderer/platform/heap/handle.h"
 
@@ -41,10 +42,18 @@ class SelectType : public GarbageCollected<SelectType> {
 
   virtual void SelectAll();
 
-  // TODO(crbug.com/1052232): Add more virtual functions.
+  enum SkipDirection { kSkipBackwards = -1, kSkipForwards = 1 };
+  CORE_EXPORT HTMLOptionElement* NextSelectableOption(HTMLOptionElement*) const;
+  CORE_EXPORT HTMLOptionElement* PreviousSelectableOption(
+      HTMLOptionElement*) const;
+  CORE_EXPORT HTMLOptionElement* FirstSelectableOption() const;
+  CORE_EXPORT HTMLOptionElement* LastSelectableOption() const;
 
  protected:
   explicit SelectType(HTMLSelectElement& select);
+  HTMLOptionElement* NextValidOption(int list_index,
+                                     SkipDirection direction,
+                                     int skip) const;
 
   const Member<HTMLSelectElement> select_;
   bool will_be_destroyed_ = false;