[Test Automation] Kombucha verbs for selecting combobox items.

This CL:
 - Adds SelectItem() command to InteractionTestUtil.
 - Adds SelectItem() verb to InteractiveBrowserTestApi.
 - Implements SelectItem() for views::Combobox and
   views::EditableCombobox in views::InteractionTestUtilSimulatorViews.
 - Migrates existing Kombucha tests to use the new verb.
 - Removes a bunch of *ForTest() methods in views::EditableCombobox in
   favor of a couple of friend directives.

Because of the complexity of EditableCombobox and its event handling,
I did need to friend InteractionTestUtilSimulatorViews.

Unlike previous test code, this new simulator code actually opens the
combobox to select an item instead of forcing the selected index or
text in code; this hits more of the actual [Editable]Combobox selection
logic and makes the tests more "realistic".

SHERIFF NOTE:

There is a small chance that the views_unittests tests for the simulator
will flake because comboboxu popups may not be reliable in multiprocess/
heavily-sharded environments; in this case, we'll move the unit tests to
interactive_ui_tests, but they should still run very quickly.

Change-Id: I6dce5ba7d41850f90b6d56006af49702dd21af45
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4007335
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Dana Fried <dfried@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1068926}

chrome/browser/ui/views/passwords/password_bubble_interactive_uitest.cc

@@ -31,6 +31,8 @@
 #include "content/public/test/browser_test.h"
 #include "content/public/test/focus_changed_observer.h"
 #include "testing/gmock/include/gmock/gmock.h"
+#include "ui/views/controls/editable_combobox/editable_combobox.h"
+#include "ui/views/focus/focus_manager.h"
 
 using net::test_server::BasicHttpResponse;
 using net::test_server::HttpRequest;
@@ -50,10 +52,11 @@ bool IsBubbleShowing() {
              ->IsVisible();
 }
 
-views::View* GetUsernameTextfield(const PasswordBubbleViewBase* bubble) {
+views::EditableCombobox* GetUsernameDropdown(
+    const PasswordBubbleViewBase* bubble) {
   const PasswordSaveUpdateView* save_bubble =
       static_cast<const PasswordSaveUpdateView*>(bubble);
-  return save_bubble->GetUsernameTextfieldForTest();
+  return save_bubble->username_dropdown_for_testing();
 }
 
 }  // namespace
@@ -95,8 +98,8 @@ IN_PROC_BROWSER_TEST_F(PasswordBubbleInteractiveUiTest, BasicOpenAndClose) {
   bubble = PasswordBubbleViewBase::manage_password_bubble();
   // A pending password with empty username should initially focus on the
   // username field.
-  EXPECT_EQ(GetUsernameTextfield(bubble),
-            bubble->GetFocusManager()->GetFocusedView());
+  EXPECT_TRUE(GetUsernameDropdown(bubble)->Contains(
+      bubble->GetFocusManager()->GetFocusedView()));
   PasswordBubbleViewBase::CloseCurrentBubble();
   EXPECT_FALSE(IsBubbleShowing());
 }

chrome/browser/ui/views/passwords/password_save_update_view.cc

@@ -456,10 +456,6 @@ PasswordSaveUpdateView::~PasswordSaveUpdateView() {
   CloseIPHBubbleIfOpen();
 }
 
-views::View* PasswordSaveUpdateView::GetUsernameTextfieldForTest() const {
-  return username_dropdown_->GetTextfieldForTest();
-}
-
 PasswordBubbleControllerBase* PasswordSaveUpdateView::GetController() {
   return &controller_;
 }

chrome/browser/ui/views/passwords/password_save_update_view.h

@@ -39,7 +39,9 @@ class PasswordSaveUpdateView : public PasswordBubbleViewBase,
     return destination_dropdown_;
   }
 
-  views::View* GetUsernameTextfieldForTest() const;
+  views::EditableCombobox* username_dropdown_for_testing() const {
+    return username_dropdown_.get();
+  }
 
  private:
   // Type of the IPH to show.

chrome/browser/ui/views/translate/translate_bubble_view_interactive_uitest.cc

@@ -144,16 +144,6 @@ class TranslateBubbleViewUITest
     return steps;
   }
 
-  // Callback that selects a specific row in a combobox.
-  static auto SelectComboboxRow(int row) {
-    return base::BindOnce(
-        [](int row, ui::TrackedElement* element) {
-          auto* const combobox = AsView<views::Combobox>(element);
-          combobox->SetSelectedRow(row);
-        },
-        row);
-  }
-
   std::unique_ptr<net::test_server::HttpResponse> HandleRequest(
       const net::test_server::HttpRequest& request) {
     if (request.GetURL().path() != "/mock_translate_script.js")
@@ -260,8 +250,7 @@ IN_PROC_BROWSER_TEST_P(TranslateBubbleViewUITest, ChooseAnotherLanguage) {
       // languages.
       AfterHide(TranslateBubbleView::kChangeTargetLanguage, base::DoNothing()),
       // P4. Select a language from the list and select translate.
-      AfterShow(TranslateBubbleView::kTargetLanguageCombobox,
-                SelectComboboxRow(0)),
+      SelectDropdownItem(TranslateBubbleView::kTargetLanguageCombobox, 0),
       PressButton(TranslateBubbleView::kTargetLanguageDoneButton),
       // V2. Verify that the language list will be dismissed, the target
       // language tab shows updated target language. Source language tab is
@@ -300,8 +289,8 @@ IN_PROC_BROWSER_TEST_P(TranslateBubbleViewUITest,
       // languages.
       AfterHide(TranslateBubbleView::kChangeSourceLanguage, base::DoNothing()),
       // P4. Select a language from the list and select translate.
-      AfterShow(TranslateBubbleView::kSourceLanguageCombobox,
-                SelectComboboxRow(1)),  // 0 = Detected Language
+      // Item 0 is the detected language.
+      SelectDropdownItem(TranslateBubbleView::kSourceLanguageCombobox, 1),
       PressButton(TranslateBubbleView::kSourceLanguageDoneButton),
       // V2. The language list will be dismissed, the source language tab
       // shows updated source language. Source language tab is no longer

ui/base/interaction/interaction_test_util.cc

@@ -26,6 +26,12 @@ bool InteractionTestUtil::Simulator::SelectTab(TrackedElement*,
   return false;
 }
 
+bool InteractionTestUtil::Simulator::SelectDropdownItem(TrackedElement*,
+                                                        size_t,
+                                                        InputType) {
+  return false;
+}
+
 InteractionTestUtil::InteractionTestUtil() = default;
 InteractionTestUtil::~InteractionTestUtil() = default;
 
@@ -78,4 +84,17 @@ void InteractionTestUtil::SelectTab(TrackedElement* tab_collection,
   NOTREACHED();
 }
 
+void InteractionTestUtil::SelectDropdownItem(TrackedElement* dropdown,
+                                             size_t index,
+                                             InputType input_type) {
+  for (const auto& simulator : simulators_) {
+    if (simulator->SelectDropdownItem(dropdown, index, input_type))
+      return;
+  }
+
+  // If a test has requested an invalid operation on an element, then this is
+  // an error.
+  NOTREACHED();
+}
+
 }  // namespace ui::test

ui/base/interaction/interaction_test_util.h

@@ -97,6 +97,12 @@ class InteractionTestUtil {
     [[nodiscard]] virtual bool SelectTab(TrackedElement* tab_collection,
                                          size_t index,
                                          InputType input_type);
+
+    // Tries to select item `index` in `dropdown`. The collection could be
+    // a listbox, combobox, or similar. Note that `index` is zero-indexed.
+    [[nodiscard]] virtual bool SelectDropdownItem(TrackedElement* dropdown,
+                                                  size_t index,
+                                                  InputType input_type);
   };
 
   InteractionTestUtil();
@@ -129,6 +135,21 @@ class InteractionTestUtil {
                  size_t index,
                  InputType input_type = InputType::kDontCare);
 
+  // Simulate selecting item `index` in `dropdown`. The collection could be
+  // a listbox, combobox, or similar. Will fail if the target object is not a
+  // supported type, if `index` is out of bounds, or if `input_type` is not
+  // supported.
+  //
+  // Note that if `input_type` is kDontCare, the approach with the broadest
+  // possible compatibility will be used, possibly bypassing the dropdown menu
+  // associated with the element. This is because dropdown menus vary in
+  // implementation across platforms and can be a source of flakiness. Options
+  // other than kDontCare may not be supported on all platforms for this reason;
+  // if they are not, an error message will be printed and the test will fail.
+  void SelectDropdownItem(TrackedElement* dropdown,
+                          size_t index,
+                          InputType input_type = InputType::kDontCare);
+
   // Simulate the default action for `element` - typically whatever happens when
   // the user clicks or taps on it. Will fail if `input_type` is not supported.
   // Prefer PressButton() for buttons and SelectMenuItem() for menu items.

ui/base/interaction/interaction_test_util_unittest.cc

@@ -27,6 +27,14 @@ class MockInteractionSimulator : public InteractionTestUtil::Simulator {
                bool(TrackedElement* element, InputType input_type));
   MOCK_METHOD2(DoDefaultAction,
                bool(TrackedElement* element, InputType input_type));
+  MOCK_METHOD3(SelectTab,
+               bool(TrackedElement* tab_collection,
+                    size_t index,
+                    InputType input_type));
+  MOCK_METHOD3(SelectDropdownItem,
+               bool(TrackedElement* dropdown,
+                    size_t index,
+                    InputType input_type));
 };
 
 }  // namespace
@@ -35,7 +43,7 @@ TEST(InteractionTestUtilTest, PressButton) {
   TestElement element(kTestElementIdentifier, kTestElementContext);
   InteractionTestUtil util;
   auto* const mock = util.AddSimulator(
-      std::make_unique<testing::NiceMock<MockInteractionSimulator>>());
+      std::make_unique<testing::StrictMock<MockInteractionSimulator>>());
   EXPECT_CALL(*mock,
               PressButton(&element, InteractionTestUtil::InputType::kDontCare))
       .WillOnce(testing::Return(true));
@@ -46,7 +54,7 @@ TEST(InteractionTestUtilTest, SelectMenuItem) {
   TestElement element(kTestElementIdentifier, kTestElementContext);
   InteractionTestUtil util;
   auto* const mock = util.AddSimulator(
-      std::make_unique<testing::NiceMock<MockInteractionSimulator>>());
+      std::make_unique<testing::StrictMock<MockInteractionSimulator>>());
   EXPECT_CALL(*mock, SelectMenuItem(&element,
                                     InteractionTestUtil::InputType::kDontCare))
       .WillOnce(testing::Return(true));
@@ -57,11 +65,34 @@ TEST(InteractionTestUtilTest, DoDefaultAction) {
   TestElement element(kTestElementIdentifier, kTestElementContext);
   InteractionTestUtil util;
   auto* const mock = util.AddSimulator(
-      std::make_unique<testing::NiceMock<MockInteractionSimulator>>());
+      std::make_unique<testing::StrictMock<MockInteractionSimulator>>());
   EXPECT_CALL(*mock, DoDefaultAction(&element,
                                      InteractionTestUtil::InputType::kDontCare))
       .WillOnce(testing::Return(true));
   util.DoDefaultAction(&element);
 }
 
+TEST(InteractionTestUtilTest, SelectTab) {
+  TestElement element(kTestElementIdentifier, kTestElementContext);
+  InteractionTestUtil util;
+  auto* const mock = util.AddSimulator(
+      std::make_unique<testing::StrictMock<MockInteractionSimulator>>());
+  EXPECT_CALL(
+      *mock, SelectTab(&element, 1U, InteractionTestUtil::InputType::kDontCare))
+      .WillOnce(testing::Return(true));
+  util.SelectTab(&element, 1U);
+}
+
+TEST(InteractionTestUtilTest, SelectDropdownItem) {
+  TestElement element(kTestElementIdentifier, kTestElementContext);
+  InteractionTestUtil util;
+  auto* const mock = util.AddSimulator(
+      std::make_unique<testing::StrictMock<MockInteractionSimulator>>());
+  EXPECT_CALL(*mock,
+              SelectDropdownItem(&element, 1U,
+                                 InteractionTestUtil::InputType::kDontCare))
+      .WillOnce(testing::Return(true));
+  util.SelectDropdownItem(&element, 1U);
+}
+
 }  // namespace ui::test

ui/base/interaction/interactive_test.cc

@@ -84,6 +84,21 @@ InteractionSequence::StepBuilder InteractiveTestApi::SelectTab(
   return builder;
 }
 
+InteractionSequence::StepBuilder InteractiveTestApi::SelectDropdownItem(
+    ElementSpecifier collection,
+    size_t item,
+    InputType input_type) {
+  StepBuilder builder;
+  internal::SpecifyElement(builder, collection);
+  builder.SetStartCallback(base::BindOnce(
+      [](size_t item, InputType input_type, InteractiveTestApi* test,
+         InteractionSequence*, TrackedElement* el) {
+        test->test_util().SelectDropdownItem(el, item, input_type);
+      },
+      item, input_type, base::Unretained(this)));
+  return builder;
+}
+
 InteractiveTestApi::StepBuilder InteractiveTestApi::Check(
     CheckCallback check_callback) {
   StepBuilder builder;

ui/base/interaction/interactive_test.h

@@ -92,6 +92,10 @@ class InteractiveTestApi {
       ElementSpecifier tab_collection,
       size_t tab_index,
       InputType input_type = InputType::kDontCare);
+  [[nodiscard]] StepBuilder SelectDropdownItem(
+      ElementSpecifier collection,
+      size_t item,
+      InputType input_type = InputType::kDontCare);
 
   // Specifies a test action that is not tied to any one UI element.
   // Returns true on success, false on failure (which will fail the test).

ui/base/interaction/interactive_test_unittest.cc

@@ -24,7 +24,8 @@ enum class ActionType {
   kPressButton,
   kSelectMenuItem,
   kDoDefaultAction,
-  kSelectTab
+  kSelectTab,
+  kSelectDropdownItem
 };
 
 using ActionRecord = std::tuple<ActionType,
@@ -69,6 +70,13 @@ class TestSimulator : public InteractionTestUtil::Simulator {
     return true;
   }
 
+  bool SelectDropdownItem(TrackedElement* collection,
+                          size_t item,
+                          InputType input_type) override {
+    DoAction(ActionType::kSelectDropdownItem, collection, input_type);
+    return true;
+  }
+
   const std::vector<ActionRecord>& records() const { return records_; }
 
  private:
@@ -130,22 +138,25 @@ TEST_F(InteractiveTestTest, InteractionVerbs) {
   e2.Show();
   e3.Show();
   e4.Show();
-  RunTestSequenceInContext(kTestContext1,
-                           PressButton(kTestId1, InputType::kDontCare),
-                           SelectMenuItem(kTestId2, InputType::kKeyboard),
-                           DoDefaultAction(kTestId3, InputType::kMouse),
-                           SelectTab(kTestId4, 3U, InputType::kTouch));
-
-  EXPECT_THAT(
-      simulator()->records(),
-      testing::ElementsAre(ActionRecord{ActionType::kPressButton, kTestId1,
-                                        kTestContext1, InputType::kDontCare},
-                           ActionRecord{ActionType::kSelectMenuItem, kTestId2,
-                                        kTestContext1, InputType::kKeyboard},
-                           ActionRecord{ActionType::kDoDefaultAction, kTestId3,
-                                        kTestContext1, InputType::kMouse},
-                           ActionRecord{ActionType::kSelectTab, kTestId4,
-                                        kTestContext1, InputType::kTouch}));
+  RunTestSequenceInContext(
+      kTestContext1, PressButton(kTestId1, InputType::kDontCare),
+      SelectMenuItem(kTestId2, InputType::kKeyboard),
+      DoDefaultAction(kTestId3, InputType::kMouse),
+      SelectTab(kTestId4, 3U, InputType::kTouch),
+      SelectDropdownItem(kTestId1, 2U, InputType::kDontCare));
+
+  EXPECT_THAT(simulator()->records(),
+              testing::ElementsAre(
+                  ActionRecord{ActionType::kPressButton, kTestId1,
+                               kTestContext1, InputType::kDontCare},
+                  ActionRecord{ActionType::kSelectMenuItem, kTestId2,
+                               kTestContext1, InputType::kKeyboard},
+                  ActionRecord{ActionType::kDoDefaultAction, kTestId3,
+                               kTestContext1, InputType::kMouse},
+                  ActionRecord{ActionType::kSelectTab, kTestId4, kTestContext1,
+                               InputType::kTouch},
+                  ActionRecord{ActionType::kSelectDropdownItem, kTestId1,
+                               kTestContext1, InputType::kDontCare}));
 }
 
 TEST_F(InteractiveTestTest, InteractionVerbsInAnyContext) {
@@ -161,18 +172,21 @@ TEST_F(InteractiveTestTest, InteractionVerbsInAnyContext) {
       kTestContext2, InAnyContext(PressButton(kTestId1, InputType::kDontCare)),
       InAnyContext(SelectMenuItem(kTestId2, InputType::kKeyboard)),
       InAnyContext(DoDefaultAction(kTestId3, InputType::kMouse)),
-      InAnyContext(SelectTab(kTestId4, 3U, InputType::kTouch)));
-
-  EXPECT_THAT(
-      simulator()->records(),
-      testing::ElementsAre(ActionRecord{ActionType::kPressButton, kTestId1,
-                                        kTestContext1, InputType::kDontCare},
-                           ActionRecord{ActionType::kSelectMenuItem, kTestId2,
-                                        kTestContext1, InputType::kKeyboard},
-                           ActionRecord{ActionType::kDoDefaultAction, kTestId3,
-                                        kTestContext1, InputType::kMouse},
-                           ActionRecord{ActionType::kSelectTab, kTestId4,
-                                        kTestContext1, InputType::kTouch}));
+      InAnyContext(SelectTab(kTestId4, 3U, InputType::kTouch)),
+      InAnyContext(SelectDropdownItem(kTestId1, 2U, InputType::kDontCare)));
+
+  EXPECT_THAT(simulator()->records(),
+              testing::ElementsAre(
+                  ActionRecord{ActionType::kPressButton, kTestId1,
+                               kTestContext1, InputType::kDontCare},
+                  ActionRecord{ActionType::kSelectMenuItem, kTestId2,
+                               kTestContext1, InputType::kKeyboard},
+                  ActionRecord{ActionType::kDoDefaultAction, kTestId3,
+                               kTestContext1, InputType::kMouse},
+                  ActionRecord{ActionType::kSelectTab, kTestId4, kTestContext1,
+                               InputType::kTouch},
+                  ActionRecord{ActionType::kSelectDropdownItem, kTestId1,
+                               kTestContext1, InputType::kDontCare}));
 }
 
 TEST_F(InteractiveTestTest, Do) {

ui/views/controls/combobox/combobox.cc

@@ -136,11 +136,7 @@ Combobox::Combobox(std::unique_ptr<ui::ComboboxModel> model,
 }
 
 Combobox::Combobox(ui::ComboboxModel* model, int text_context, int text_style)
-    : text_context_(text_context),
-      text_style_(text_style),
-      arrow_button_(new TransparentButton(
-          base::BindRepeating(&Combobox::ArrowButtonPressed,
-                              base::Unretained(this)))) {
+    : text_context_(text_context), text_style_(text_style) {
   SetModel(model);
 #if BUILDFLAG(IS_MAC)
   SetFocusBehavior(FocusBehavior::ACCESSIBLE_ONLY);
@@ -151,8 +147,9 @@ Combobox::Combobox(ui::ComboboxModel* model, int text_context, int text_style)
   SetBackgroundColorId(ui::kColorTextfieldBackground);
   UpdateBorder();
 
-  arrow_button_->SetVisible(should_show_arrow_);
-  AddChildView(arrow_button_.get());
+  arrow_button_ =
+      AddChildView(std::make_unique<TransparentButton>(base::BindRepeating(
+          &Combobox::ArrowButtonPressed, base::Unretained(this))));
 
   // A layer is applied to make sure that canvas bounds are snapped to pixel
   // boundaries (for the sake of drawing the arrow).