Fix VoiceInputToggle button on Assistant Embedded UI.

Each |AssistantButton| is identified using an enum |AssistantButtonId|.
These values were stored on the |AssistantButton| using views::View::SetID().

Recently we also started using views::View::SetID() to make it easier to find
views during unittests.
This caused a conflict where the |AssistantButtonId| was overwritten.

To solve this the |AssistantButton| now stores its id in a separate |id_|
property.
This is then passed to all interested parties using a newly introduced
|AssistantButtonListener| API.

       |ash_unittests --gtest_filter="AssistantPageViewTest.*"|,
       as well as manual testing of both the embedded and standalone UI.

Bug: b/144530695
Change-Id: Id3f231553d04fb0f8a8c7c64c7762d1a8b72cd59
Tests: Newly introduced unittests
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1919830
Commit-Queue: Jeroen Dhollander <jeroendh@google.com>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#717862}

ash/ambient/ui/ambient_assistant_dialog_plate.cc

@@ -32,10 +32,8 @@ const char* AmbientAssistantDialogPlate::GetClassName() const {
   return "AmbientAssistantDialogPlate";
 }
 
-void AmbientAssistantDialogPlate::ButtonPressed(views::Button* sender,
-                                                const ui::Event& event) {
-  delegate_->OnDialogPlateButtonPressed(
-      static_cast<AssistantButtonId>(sender->GetID()));
+void AmbientAssistantDialogPlate::OnButtonPressed(AssistantButtonId button_id) {
+  delegate_->OnDialogPlateButtonPressed(button_id);
 }
 
 void AmbientAssistantDialogPlate::OnCommittedQueryChanged(

ash/ambient/ui/ambient_assistant_dialog_plate.h

@@ -6,6 +6,7 @@
 #define ASH_AMBIENT_UI_AMBIENT_ASSISTANT_DIALOG_PLATE_H_
 
 #include "ash/assistant/model/assistant_interaction_model_observer.h"
+#include "ash/assistant/ui/base/assistant_button_listener.h"
 #include "base/macros.h"
 #include "ui/views/controls/button/button.h"
 #include "ui/views/view.h"
@@ -15,10 +16,9 @@ namespace ash {
 class AssistantQueryView;
 class AssistantViewDelegate;
 class MicView;
-enum class AssistantButtonId;
 
 class AmbientAssistantDialogPlate : public views::View,
-                                    public views::ButtonListener,
+                                    public AssistantButtonListener,
                                     public AssistantInteractionModelObserver {
  public:
   explicit AmbientAssistantDialogPlate(AssistantViewDelegate* delegate);
@@ -27,8 +27,8 @@ class AmbientAssistantDialogPlate : public views::View,
   // views::View:
   const char* GetClassName() const override;
 
-  // views::ButtonListener:
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
+  // AssistantButtonListener:
+  void OnButtonPressed(AssistantButtonId button_id) override;
 
   // AssistantInteractionModelObserver:
   void OnCommittedQueryChanged(const AssistantQuery& query) override;

ash/app_list/views/assistant/assistant_dialog_plate.cc

@@ -99,9 +99,9 @@ gfx::Size AssistantDialogPlate::CalculatePreferredSize() const {
   return gfx::Size(INT_MAX, GetHeightForWidth(INT_MAX));
 }
 
-void AssistantDialogPlate::ButtonPressed(views::Button* sender,
-                                         const ui::Event& event) {
-  OnButtonPressed(static_cast<ash::AssistantButtonId>(sender->GetID()));
+void AssistantDialogPlate::OnButtonPressed(AssistantButtonId button_id) {
+  delegate_->OnDialogPlateButtonPressed(button_id);
+  textfield_->SetText(base::string16());
 }
 
 bool AssistantDialogPlate::HandleKeyEvent(views::Textfield* textfield,
@@ -341,10 +341,10 @@ void AssistantDialogPlate::InitKeyboardLayoutContainer() {
   layout_manager->SetFlexForView(textfield_, 1);
 
   // Voice input toggle.
-  voice_input_toggle_ = ash::AssistantButton::Create(
-      this, ash::kMicIcon, kButtonSizeDip, kIconSizeDip,
+  voice_input_toggle_ =
+      AssistantButton::Create(this, kMicIcon, kButtonSizeDip, kIconSizeDip,
                               IDS_ASH_ASSISTANT_DIALOG_PLATE_MIC_ACCNAME,
-      ash::AssistantButtonId::kVoiceInputToggle,
+                              AssistantButtonId::kVoiceInputToggle,
                               IDS_ASH_ASSISTANT_DIALOG_PLATE_MIC_TOOLTIP);
   voice_input_toggle_->SetID(AssistantViewID::kVoiceInputToggle);
   keyboard_layout_container_->AddChildView(voice_input_toggle_);
@@ -406,11 +406,6 @@ void AssistantDialogPlate::InitVoiceLayoutContainer() {
   input_modality_layout_container_->AddChildView(voice_layout_container_);
 }
 
-void AssistantDialogPlate::OnButtonPressed(ash::AssistantButtonId id) {
-  delegate_->OnDialogPlateButtonPressed(id);
-  textfield_->SetText(base::string16());
-}
-
 void AssistantDialogPlate::OnAnimationStarted(
     const ui::CallbackLayerAnimationObserver& observer) {
   keyboard_layout_container_->set_can_process_events_within_subtree(false);

ash/app_list/views/assistant/assistant_dialog_plate.h

@@ -12,6 +12,7 @@
 #include "ash/assistant/model/assistant_interaction_model_observer.h"
 #include "ash/assistant/model/assistant_query_history.h"
 #include "ash/assistant/model/assistant_ui_model_observer.h"
+#include "ash/assistant/ui/base/assistant_button_listener.h"
 #include "base/component_export.h"
 #include "base/macros.h"
 #include "ui/views/controls/button/button.h"
@@ -27,7 +28,6 @@ class ImageButton;
 }  // namespace views
 
 namespace ash {
-enum class AssistantButtonId;
 class AssistantViewDelegate;
 class LogoView;
 class MicView;
@@ -42,9 +42,9 @@ class MicView;
 class APP_LIST_EXPORT AssistantDialogPlate
     : public views::View,
       public views::TextfieldController,
-      public ash::AssistantInteractionModelObserver,
-      public ash::AssistantUiModelObserver,
-      public views::ButtonListener {
+      public AssistantInteractionModelObserver,
+      public AssistantUiModelObserver,
+      public AssistantButtonListener {
  public:
   explicit AssistantDialogPlate(ash::AssistantViewDelegate* delegate);
   ~AssistantDialogPlate() override;
@@ -54,8 +54,8 @@ class APP_LIST_EXPORT AssistantDialogPlate
   gfx::Size CalculatePreferredSize() const override;
   void RequestFocus() override;
 
-  // views::ButtonListener:
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
+  // AssistantButtonListener:
+  void OnButtonPressed(AssistantButtonId button_id) override;
 
   // views::TextfieldController:
   bool HandleKeyEvent(views::Textfield* sender,
@@ -81,8 +81,6 @@ class APP_LIST_EXPORT AssistantDialogPlate
   void InitKeyboardLayoutContainer();
   void InitVoiceLayoutContainer();
 
-  void OnButtonPressed(ash::AssistantButtonId id);
-
   void OnAnimationStarted(const ui::CallbackLayerAnimationObserver& observer);
   bool OnAnimationEnded(const ui::CallbackLayerAnimationObserver& observer);
 

ash/app_list/views/assistant/assistant_page_view_unittest.cc

@@ -2,9 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "ash/assistant/model/assistant_ui_model.h"
 #include "ash/assistant/test/assistant_ash_test_base.h"
 #include "ash/assistant/ui/assistant_ui_constants.h"
 #include "base/run_loop.h"
+#include "chromeos/services/assistant/public/mojom/assistant.mojom-shared.h"
 #include "ui/views/controls/textfield/textfield.h"
 #include "ui/views/focus/focus_manager.h"
 
@@ -12,6 +14,29 @@ namespace ash {
 
 namespace {
 
+using chromeos::assistant::mojom::AssistantInteractionMetadata;
+using chromeos::assistant::mojom::AssistantInteractionType;
+
+#define EXPECT_INTERACTION_OF_TYPE(type_)                      \
+  ({                                                           \
+    base::Optional<AssistantInteractionMetadata> interaction = \
+        current_interaction();                                 \
+    ASSERT_TRUE(interaction.has_value());                      \
+    EXPECT_EQ(interaction->type, type_);                       \
+  })
+
+// Ensures that the given view has the focus. If it doesn't, this will print a
+// nice error message indicating which view has the focus instead.
+#define EXPECT_HAS_FOCUS(expected_)                                           \
+  ({                                                                          \
+    const views::View* actual =                                               \
+        main_view()->GetFocusManager()->GetFocusedView();                     \
+    EXPECT_TRUE(expected_->HasFocus())                                        \
+        << "Expected focus on '" << expected_->GetClassName()                 \
+        << "' but it is on '" << (actual ? actual->GetClassName() : "<null>") \
+        << "'.";                                                              \
+  })
+
 // Stubbed |FocusChangeListener| that simply remembers all the views that
 // received focus.
 class FocusChangeListenerStub : public views::FocusChangeListener {
@@ -44,22 +69,18 @@ class FocusChangeListenerStub : public views::FocusChangeListener {
   DISALLOW_COPY_AND_ASSIGN(FocusChangeListenerStub);
 };
 
-// Ensures that the given view has the focus. If it doesn't, this will print a
-// nice error message indicating which view has the focus instead.
-#define EXPECT_HAS_FOCUS(expected_)                                           \
-  ({                                                                          \
-    const views::View* actual =                                               \
-        main_view()->GetFocusManager()->GetFocusedView();                     \
-    EXPECT_TRUE(expected_->HasFocus())                                        \
-        << "Expected focus on '" << expected_->GetClassName()                 \
-        << "' but it is on '" << (actual ? actual->GetClassName() : "<null>") \
-        << "'.";                                                              \
-  })
-
 class AssistantPageViewTest : public AssistantAshTestBase {
  public:
   AssistantPageViewTest() = default;
 
+  void ShowAssistantUiInTextMode() {
+    ShowAssistantUi(AssistantEntryPoint::kUnspecified);
+  }
+
+  void ShowAssistantUiInVoiceMode() {
+    ShowAssistantUi(AssistantEntryPoint::kHotword);
+  }
+
  private:
   DISALLOW_COPY_AND_ASSIGN(AssistantPageViewTest);
 };
@@ -127,7 +148,7 @@ TEST_F(AssistantPageViewTest, ShouldNotRequestFocusWhenOtherAppWindowOpens) {
   }
 }
 
-TEST_F(AssistantPageViewTest, ShouldFocusTextDialogWhenOpeningWithHotkey) {
+TEST_F(AssistantPageViewTest, ShouldFocusTextFieldWhenOpeningWithHotkey) {
   ShowAssistantUi(AssistantEntryPoint::kHotkey);
 
   EXPECT_HAS_FOCUS(input_text_field());
@@ -201,6 +222,42 @@ TEST_F(AssistantPageViewTest,
   EXPECT_FALSE(greeting_label()->GetVisible());
 }
 
+TEST_F(AssistantPageViewTest, ShouldFocusMicViewWhenPressingVoiceInputToggle) {
+  ShowAssistantUiInTextMode();
+
+  ClickOnAndWait(voice_input_toggle());
+
+  EXPECT_HAS_FOCUS(mic_view());
+}
+
+TEST_F(AssistantPageViewTest,
+       ShouldStartVoiceInteractionWhenPressingVoiceInputToggle) {
+  ShowAssistantUiInTextMode();
+
+  ClickOnAndWait(voice_input_toggle());
+
+  EXPECT_INTERACTION_OF_TYPE(AssistantInteractionType::kVoice);
+}
+
+TEST_F(AssistantPageViewTest,
+       ShouldStopVoiceInteractionWhenPressingKeyboardInputToggle) {
+  ShowAssistantUiInVoiceMode();
+  EXPECT_INTERACTION_OF_TYPE(AssistantInteractionType::kVoice);
+
+  ClickOnAndWait(keyboard_input_toggle());
+
+  EXPECT_FALSE(current_interaction().has_value());
+}
+
+TEST_F(AssistantPageViewTest,
+       ShouldFocusTextFieldWhenPressingKeyboardInputToggle) {
+  ShowAssistantUiInVoiceMode();
+
+  ClickOnAndWait(keyboard_input_toggle());
+
+  EXPECT_HAS_FOCUS(input_text_field());
+}
+
 // Tests the |AssistantPageView| with tablet mode enabled.
 class AssistantPageViewTabletModeTest : public AssistantAshTestBase {
  public:
@@ -228,8 +285,7 @@ TEST_F(AssistantPageViewTabletModeTest, ShouldFocusMicWhenOpeningWithHotword) {
   EXPECT_HAS_FOCUS(mic_view());
 }
 
-TEST_F(AssistantPageViewTabletModeTest,
-       ShouldFocusTextDialogAfterSendingQuery) {
+TEST_F(AssistantPageViewTabletModeTest, ShouldFocusTextFieldAfterSendingQuery) {
   ShowAssistantUi();
 
   SendQueryThroughTextField("The query");
@@ -251,7 +307,7 @@ TEST_F(AssistantPageViewTabletModeTest,
   ShowAssistantUi();
   EXPECT_FALSE(IsKeyboardShowing());
 
-  TapOnTextField();
+  TapOnAndWait(input_text_field());
 
   EXPECT_TRUE(IsKeyboardShowing());
 }

ash/app_list/views/assistant/assistant_test_api_impl.cc

@@ -77,6 +77,14 @@ views::View* AssistantTestApiImpl::greeting_label() {
   return page_view()->GetViewByID(AssistantViewID::kGreetingLabel);
 }
 
+views::View* AssistantTestApiImpl::voice_input_toggle() {
+  return page_view()->GetViewByID(AssistantViewID::kVoiceInputToggle);
+}
+
+views::View* AssistantTestApiImpl::keyboard_input_toggle() {
+  return page_view()->GetViewByID(AssistantViewID::kKeyboardInputToggle);
+}
+
 aura::Window* AssistantTestApiImpl::window() {
   return main_view()->GetWidget()->GetNativeWindow();
 }

ash/app_list/views/assistant/assistant_test_api_impl.h

@@ -40,6 +40,8 @@ class AssistantTestApiImpl : public AssistantTestApi {
   views::Textfield* input_text_field() override;
   views::View* mic_view() override;
   views::View* greeting_label() override;
+  views::View* voice_input_toggle() override;
+  views::View* keyboard_input_toggle() override;
   aura::Window* window() override;
 
  private:

ash/assistant/test/assistant_ash_test_base.cc

@@ -40,6 +40,16 @@ bool CanProcessEvents(const views::View* view) {
   return true;
 }
 
+void CheckCanProcessEvents(const views::View* view) {
+  if (!view->IsDrawn()) {
+    ADD_FAILURE()
+        << view->GetClassName()
+        << " can not process events because it is not drawn on screen.";
+  } else if (!CanProcessEvents(view)) {
+    ADD_FAILURE() << view->GetClassName() << " can not process events.";
+  }
+}
+
 }  // namespace
 
 AssistantAshTestBase::AssistantAshTestBase()
@@ -139,11 +149,24 @@ void AssistantAshTestBase::SendQueryThroughTextField(const std::string& query) {
   test_api_->SendTextQuery(query);
 }
 
-void AssistantAshTestBase::TapOnTextField() {
-  if (!CanProcessEvents(input_text_field()))
-    ADD_FAILURE() << "TextField can not process tap events";
+void AssistantAshTestBase::TapOnAndWait(views::View* view) {
+  CheckCanProcessEvents(view);
+  GetEventGenerator()->GestureTapAt(GetPointInside(view));
+
+  base::RunLoop().RunUntilIdle();
+}
+
+void AssistantAshTestBase::ClickOnAndWait(views::View* view) {
+  CheckCanProcessEvents(view);
+  GetEventGenerator()->MoveMouseTo(GetPointInside(view));
+  GetEventGenerator()->ClickLeftButton();
 
-  GetEventGenerator()->GestureTapAt(GetPointInside(input_text_field()));
+  base::RunLoop().RunUntilIdle();
+}
+
+base::Optional<chromeos::assistant::mojom::AssistantInteractionMetadata>
+AssistantAshTestBase::current_interaction() {
+  return assistant_service()->current_interaction();
 }
 
 aura::Window* AssistantAshTestBase::SwitchToNewAppWindow() {
@@ -170,6 +193,14 @@ views::View* AssistantAshTestBase::greeting_label() {
   return test_api_->greeting_label();
 }
 
+views::View* AssistantAshTestBase::voice_input_toggle() {
+  return test_api_->voice_input_toggle();
+}
+
+views::View* AssistantAshTestBase::keyboard_input_toggle() {
+  return test_api_->keyboard_input_toggle();
+}
+
 void AssistantAshTestBase::ShowKeyboard() {
   auto* keyboard_controller = keyboard::KeyboardUIController::Get();
   keyboard_controller->ShowKeyboard(/*lock=*/false);

ash/assistant/test/assistant_ash_test_base.h

@@ -67,8 +67,18 @@ class AssistantAshTestBase : public AshTestBase {
   // Simulate the user entering a query followed by <return>.
   void SendQueryThroughTextField(const std::string& query);
 
-  // Simulate the user tapping on the text field.
-  void TapOnTextField();
+  // Simulate the user tapping on the given view.
+  // Waits for the event to be processed.
+  void TapOnAndWait(views::View* view);
+
+  // Simulate a mouse click on the given view.
+  // Waits for the event to be processed.
+  void ClickOnAndWait(views::View* view);
+
+  // Returns the current interaction. Returns |base::nullopt| if no interaction
+  // is in progress.
+  base::Optional<chromeos::assistant::mojom::AssistantInteractionMetadata>
+  current_interaction();
 
   // Create a new App window, and activate it. This will take the focus away
   // from the Assistant UI (and force it to close).
@@ -89,8 +99,15 @@ class AssistantAshTestBase : public AshTestBase {
   // Return the greeting label shown when you first open the Assistant.
   views::View* greeting_label();
 
+  // Return the button to enable voice mode.
+  views::View* voice_input_toggle();
+
+  // Return the button to enable text mode.
+  views::View* keyboard_input_toggle();
+
   // Show the on-screen keyboard.
   void ShowKeyboard();
+
   // Returns if the on-screen keyboard is being displayed.
   bool IsKeyboardShowing() const;
 

ash/assistant/test/test_assistant_service.cc

@@ -13,6 +13,7 @@
 namespace ash {
 
 using chromeos::assistant::mojom::AssistantInteractionMetadata;
+using chromeos::assistant::mojom::AssistantInteractionMetadataPtr;
 using chromeos::assistant::mojom::AssistantInteractionResolution;
 using chromeos::assistant::mojom::AssistantInteractionSubscriber;
 using chromeos::assistant::mojom::AssistantInteractionType;
@@ -33,9 +34,7 @@ class SanityCheckSubscriber : public AssistantInteractionSubscriber {
   }
 
   // AssistantInteractionSubscriber implementation:
-  void OnInteractionStarted(
-      chromeos::assistant::mojom::AssistantInteractionMetadataPtr metadata)
-      override {
+  void OnInteractionStarted(AssistantInteractionMetadataPtr metadata) override {
     if (current_state_ == ConversationState::kInProgress) {
       ADD_FAILURE()
           << "Cannot start a new Assistant interaction without finishing the "
@@ -112,6 +111,60 @@ class SanityCheckSubscriber : public AssistantInteractionSubscriber {
   DISALLOW_COPY_AND_ASSIGN(SanityCheckSubscriber);
 };
 
+// Subscriber that tracks the current interaction.
+class CurrentInteractionSubscriber : public AssistantInteractionSubscriber {
+ public:
+  CurrentInteractionSubscriber() : receiver_(this) {}
+  CurrentInteractionSubscriber(CurrentInteractionSubscriber&) = delete;
+  CurrentInteractionSubscriber& operator=(CurrentInteractionSubscriber&) =
+      delete;
+  ~CurrentInteractionSubscriber() override = default;
+
+  mojo::PendingRemote<AssistantInteractionSubscriber>
+  BindNewPipeAndPassRemote() {
+    return receiver_.BindNewPipeAndPassRemote();
+  }
+
+  // AssistantInteractionSubscriber implementation:
+  void OnInteractionStarted(AssistantInteractionMetadataPtr metadata) override {
+    current_interaction_ = *metadata;
+  }
+
+  void OnInteractionFinished(
+      AssistantInteractionResolution resolution) override {
+    current_interaction_ = base::nullopt;
+  }
+
+  void OnHtmlResponse(const std::string& response,
+                      const std::string& fallback) override {}
+  void OnSuggestionsResponse(
+      std::vector<chromeos::assistant::mojom::AssistantSuggestionPtr> response)
+      override {}
+  void OnTextResponse(const std::string& response) override {}
+  void OnOpenUrlResponse(const ::GURL& url, bool in_background) override {}
+  void OnOpenAppResponse(chromeos::assistant::mojom::AndroidAppInfoPtr app_info,
+                         OnOpenAppResponseCallback callback) override {}
+  void OnSpeechRecognitionStarted() override {}
+  void OnSpeechRecognitionIntermediateResult(
+      const std::string& high_confidence_text,
+      const std::string& low_confidence_text) override {}
+  void OnSpeechRecognitionEndOfUtterance() override {}
+  void OnSpeechRecognitionFinalResult(
+      const std::string& final_result) override {}
+  void OnSpeechLevelUpdated(float speech_level) override {}
+  void OnTtsStarted(bool due_to_error) override {}
+  void OnWaitStarted() override {}
+
+  base::Optional<AssistantInteractionMetadata> current_interaction() {
+    return current_interaction_;
+  }
+
+ private:
+  base::Optional<AssistantInteractionMetadata> current_interaction_ =
+      base::nullopt;
+  mojo::Receiver<AssistantInteractionSubscriber> receiver_;
+};
+
 class InteractionResponse::Response {
  public:
   Response() = default;
@@ -166,9 +219,13 @@ class ResolutionResponse : public InteractionResponse::Response {
 };
 
 TestAssistantService::TestAssistantService()
-    : sanity_check_subscriber_(std::make_unique<SanityCheckSubscriber>()) {
+    : sanity_check_subscriber_(std::make_unique<SanityCheckSubscriber>()),
+      current_interaction_subscriber_(
+          std::make_unique<CurrentInteractionSubscriber>()) {
   AddAssistantInteractionSubscriber(
       sanity_check_subscriber_->BindNewPipeAndPassRemote());
+  AddAssistantInteractionSubscriber(
+      current_interaction_subscriber_->BindNewPipeAndPassRemote());
 }
 
 TestAssistantService::~TestAssistantService() = default;
@@ -183,6 +240,11 @@ void TestAssistantService::SetInteractionResponse(
   interaction_response_ = std::move(response);
 }
 
+base::Optional<AssistantInteractionMetadata>
+TestAssistantService::current_interaction() {
+  return current_interaction_subscriber_->current_interaction();
+}
+
 void TestAssistantService::StartCachedScreenContextInteraction() {
   NOTIMPLEMENTED_LOG_ONCE();
 }

ash/assistant/test/test_assistant_service.h

@@ -16,6 +16,7 @@
 
 namespace ash {
 
+class CurrentInteractionSubscriber;
 class SanityCheckSubscriber;
 
 // A response issued when an Assistant interaction is started.
@@ -79,6 +80,10 @@ class TestAssistantService : public chromeos::assistant::mojom::Assistant {
   // Set the response that will be invoked when the next interaction starts.
   void SetInteractionResponse(InteractionResponse&& response);
 
+  // Returns the current interaction.
+  base::Optional<chromeos::assistant::mojom::AssistantInteractionMetadata>
+  current_interaction();
+
   // mojom::Assistant overrides:
   void StartCachedScreenContextInteraction() override;
   void StartEditReminderInteraction(const std::string& client_id) override;
@@ -121,6 +126,7 @@ class TestAssistantService : public chromeos::assistant::mojom::Assistant {
   mojo::RemoteSet<chromeos::assistant::mojom::AssistantInteractionSubscriber>
       interaction_subscribers_;
   std::unique_ptr<SanityCheckSubscriber> sanity_check_subscriber_;
+  std::unique_ptr<CurrentInteractionSubscriber> current_interaction_subscriber_;
   InteractionResponse interaction_response_;
 
   DISALLOW_COPY_AND_ASSIGN(TestAssistantService);

ash/assistant/ui/BUILD.gn

@@ -58,6 +58,7 @@ component("ui") {
     "assistant_web_view_delegate.h",
     "base/assistant_button.cc",
     "base/assistant_button.h",
+    "base/assistant_button_listener.h",
     "base/assistant_scroll_view.cc",
     "base/assistant_scroll_view.h",
     "base/stack_layout.cc",

ash/assistant/ui/base/assistant_button.cc

@@ -5,6 +5,7 @@
 #include "ash/assistant/ui/base/assistant_button.h"
 
 #include "ash/assistant/model/assistant_ui_model.h"
+#include "ash/assistant/ui/base/assistant_button_listener.h"
 #include "ash/assistant/util/histogram_util.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/color_utils.h"
@@ -23,9 +24,9 @@ constexpr int kInkDropInset = 2;
 
 }  // namespace
 
-AssistantButton::AssistantButton(views::ButtonListener* listener,
+AssistantButton::AssistantButton(AssistantButtonListener* listener,
                                  AssistantButtonId button_id)
-    : views::ImageButton(this), listener_(listener) {
+    : views::ImageButton(this), listener_(listener), id_(button_id) {
   constexpr SkColor kInkDropBaseColor = SK_ColorBLACK;
   constexpr float kInkDropVisibleOpacity = 0.06f;
 
@@ -45,14 +46,12 @@ AssistantButton::AssistantButton(views::ButtonListener* listener,
   set_has_ink_drop_action_on_click(true);
   set_ink_drop_base_color(kInkDropBaseColor);
   set_ink_drop_visible_opacity(kInkDropVisibleOpacity);
-
-  SetID(static_cast<int>(button_id));
 }
 
 AssistantButton::~AssistantButton() = default;
 
 // static
-views::ImageButton* AssistantButton::Create(views::ButtonListener* listener,
+AssistantButton* AssistantButton::Create(AssistantButtonListener* listener,
                                          const gfx::VectorIcon& icon,
                                          int size_in_dip,
                                          int icon_size_in_dip,
@@ -116,9 +115,8 @@ std::unique_ptr<views::InkDropRipple> AssistantButton::CreateInkDropRipple()
 
 void AssistantButton::ButtonPressed(views::Button* sender,
                                     const ui::Event& event) {
-  assistant::util::IncrementAssistantButtonClickCount(
-      static_cast<AssistantButtonId>(sender->GetID()));
-  listener_->ButtonPressed(sender, event);
+  assistant::util::IncrementAssistantButtonClickCount(id_);
+  listener_->OnButtonPressed(id_);
 }
 
 }  // namespace ash

ash/assistant/ui/base/assistant_button.h

@@ -19,24 +19,24 @@ struct VectorIcon;
 }  // namespace gfx
 
 namespace views {
-class ButtonListener;
 class ImageButton;
 }  // namespace views
 
 namespace ash {
 
+class AssistantButtonListener;
 enum class AssistantButtonId;
 
 class COMPONENT_EXPORT(ASSISTANT_UI) AssistantButton
     : public views::ImageButton,
       public views::ButtonListener {
  public:
-  AssistantButton(views::ButtonListener* listener, AssistantButtonId button_id);
+  AssistantButton(AssistantButtonListener* listener,
+                  AssistantButtonId button_id);
   ~AssistantButton() override;
 
-  // Creates an ImageButton with the default Assistant styles.
-  static views::ImageButton* Create(
-      views::ButtonListener* listener,
+  // Creates a button with the default Assistant styles.
+  static AssistantButton* Create(AssistantButtonListener* listener,
                                  const gfx::VectorIcon& icon,
                                  int size_in_dip,
                                  int icon_size_in_dip,
@@ -45,6 +45,8 @@ class COMPONENT_EXPORT(ASSISTANT_UI) AssistantButton
                                  base::Optional<int> tooltip_id = base::nullopt,
                                  SkColor icon_color = gfx::kGoogleGrey700);
 
+  AssistantButtonId GetAssistantButtonId() const { return id_; }
+
   // views::Button:
   const char* GetClassName() const override;
   void OnBoundsChanged(const gfx::Rect& previous_bounds) override;
@@ -58,7 +60,8 @@ class COMPONENT_EXPORT(ASSISTANT_UI) AssistantButton
   void ButtonPressed(views::Button* sender, const ui::Event& event) override;
 
  private:
-  views::ButtonListener* listener_;
+  AssistantButtonListener* listener_;
+  const AssistantButtonId id_;
 
   DISALLOW_COPY_AND_ASSIGN(AssistantButton);
 };

ash/assistant/ui/base/assistant_button_listener.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef ASH_ASSISTANT_UI_BASE_ASSISTANT_BUTTON_LISTENER_H_
+#define ASH_ASSISTANT_UI_BASE_ASSISTANT_BUTTON_LISTENER_H_
+
+namespace ash {
+
+enum class AssistantButtonId;
+
+// Listener to observe presses on |AssistantButton|.
+class AssistantButtonListener {
+ public:
+  AssistantButtonListener() = default;
+  virtual ~AssistantButtonListener() = default;
+
+  virtual void OnButtonPressed(AssistantButtonId button_id) = 0;
+};
+
+}  // namespace ash
+
+#endif  // ASH_ASSISTANT_UI_BASE_ASSISTANT_BUTTON_LISTENER_H_

ash/assistant/ui/caption_bar.cc

@@ -29,10 +29,10 @@ constexpr int kVectorIconSizeDip = 12;
 
 // CaptionButton ---------------------------------------------------------------
 
-views::ImageButton* CreateCaptionButton(const gfx::VectorIcon& icon,
+AssistantButton* CreateCaptionButton(const gfx::VectorIcon& icon,
                                      int accessible_name_id,
                                      AssistantButtonId button_id,
-                                        views::ButtonListener* listener) {
+                                     AssistantButtonListener* listener) {
   return AssistantButton::Create(listener, icon, kCaptionButtonSizeDip,
                                  kVectorIconSizeDip, accessible_name_id,
                                  button_id);
@@ -80,8 +80,8 @@ void CaptionBar::VisibilityChanged(views::View* starting_from, bool visible) {
       this, root_window, {ui::ET_KEY_PRESSED});
 }
 
-void CaptionBar::ButtonPressed(views::Button* sender, const ui::Event& event) {
-  HandleButton(static_cast<AssistantButtonId>(sender->GetID()));
+void CaptionBar::OnButtonPressed(AssistantButtonId button_id) {
+  HandleButton(button_id);
 }
 
 void CaptionBar::OnEvent(const ui::Event& event) {
@@ -104,7 +104,7 @@ void CaptionBar::OnEvent(const ui::Event& event) {
 }
 
 void CaptionBar::SetButtonVisible(AssistantButtonId id, bool visible) {
-  views::View* button = GetViewByID(static_cast<int>(id));
+  views::View* button = GetButtonWithId(id);
   if (button)
     button->SetVisible(visible);
 }
@@ -119,7 +119,7 @@ void CaptionBar::InitLayout() {
       views::BoxLayout::CrossAxisAlignment::kCenter);
 
   // Back.
-  AddChildView(CreateCaptionButton(kWindowControlBackIcon, IDS_APP_LIST_BACK,
+  AddButton(CreateCaptionButton(kWindowControlBackIcon, IDS_APP_LIST_BACK,
                                 AssistantButtonId::kBack, this));
 
   // Spacer.
@@ -129,18 +129,23 @@ void CaptionBar::InitLayout() {
   layout_manager->SetFlexForView(spacer, 1);
 
   // Minimize.
-  AddChildView(CreateCaptionButton(views::kWindowControlMinimizeIcon,
+  AddButton(CreateCaptionButton(views::kWindowControlMinimizeIcon,
                                 IDS_APP_ACCNAME_MINIMIZE,
                                 AssistantButtonId::kMinimize, this));
 
   // Close.
-  AddChildView(CreateCaptionButton(views::kWindowControlCloseIcon,
+  AddButton(CreateCaptionButton(views::kWindowControlCloseIcon,
                                 IDS_APP_ACCNAME_CLOSE,
                                 AssistantButtonId::kClose, this));
 }
 
+void CaptionBar::AddButton(AssistantButton* button) {
+  buttons_.push_back(button);
+  AddChildView(button);
+}
+
 void CaptionBar::HandleButton(AssistantButtonId id) {
-  if (!GetViewByID(static_cast<int>(id))->GetVisible())
+  if (!GetButtonWithId(id)->GetVisible())
     return;
 
   // If the delegate returns |true| it has handled the event and wishes to
@@ -159,4 +164,12 @@ void CaptionBar::HandleButton(AssistantButtonId id) {
   }
 }
 
+AssistantButton* CaptionBar::GetButtonWithId(AssistantButtonId id) {
+  for (auto* button : buttons_) {
+    if (button->GetAssistantButtonId() == id)
+      return button;
+  }
+  return nullptr;
+}
+
 }  // namespace ash

ash/assistant/ui/caption_bar.h

@@ -7,6 +7,7 @@
 
 #include <memory>
 
+#include "ash/assistant/ui/base/assistant_button_listener.h"
 #include "base/component_export.h"
 #include "base/macros.h"
 #include "ui/events/event_observer.h"
@@ -19,12 +20,12 @@ class EventMonitor;
 
 namespace ash {
 
-enum class AssistantButtonId;
+class AssistantButton;
 
 // CaptionBarDelegate ----------------------------------------------------------
 
 // TODO(wutao): Remove this class and call methods on AssistantViewDelegate
-// derectly.
+// directly.
 class COMPONENT_EXPORT(ASSISTANT_UI) CaptionBarDelegate {
  public:
   // Invoked when the caption button identified by |id| is pressed. Return
@@ -38,7 +39,8 @@ class COMPONENT_EXPORT(ASSISTANT_UI) CaptionBarDelegate {
 // CaptionBar ------------------------------------------------------------------
 
 class COMPONENT_EXPORT(ASSISTANT_UI) CaptionBar : public views::View,
-                                                  public views::ButtonListener,
+                                                  public AssistantButtonListener
+    ,
                                                   public ui::EventObserver {
  public:
   // This is necessary to inform clang that our overload of |OnEvent|,
@@ -54,8 +56,8 @@ class COMPONENT_EXPORT(ASSISTANT_UI) CaptionBar : public views::View,
   int GetHeightForWidth(int width) const override;
   void VisibilityChanged(views::View* starting_from, bool visible) override;
 
-  // views::ButtonListener:
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
+  // AssistantButtonListener:
+  void OnButtonPressed(AssistantButtonId button_id) override;
 
   // ui::EventObserver:
   void OnEvent(const ui::Event& event) override;
@@ -67,11 +69,14 @@ class COMPONENT_EXPORT(ASSISTANT_UI) CaptionBar : public views::View,
 
  private:
   void InitLayout();
+  void AddButton(AssistantButton* button);
   void HandleButton(AssistantButtonId id);
+  AssistantButton* GetButtonWithId(AssistantButtonId id);
 
   CaptionBarDelegate* delegate_ = nullptr;
 
   std::unique_ptr<views::EventMonitor> event_monitor_;
+  std::vector<AssistantButton*> buttons_;
 
   DISALLOW_COPY_AND_ASSIGN(CaptionBar);
 };

ash/assistant/ui/dialog_plate/dialog_plate.cc

@@ -86,8 +86,9 @@ int DialogPlate::GetHeightForWidth(int width) const {
   return kPreferredHeightDip;
 }
 
-void DialogPlate::ButtonPressed(views::Button* sender, const ui::Event& event) {
-  OnButtonPressed(static_cast<AssistantButtonId>(sender->GetID()));
+void DialogPlate::OnButtonPressed(AssistantButtonId button_id) {
+  delegate_->OnDialogPlateButtonPressed(button_id);
+  textfield_->SetText(base::string16());
 }
 
 bool DialogPlate::HandleKeyEvent(views::Textfield* textfield,
@@ -382,11 +383,6 @@ void DialogPlate::InitVoiceLayoutContainer() {
   input_modality_layout_container_->AddChildView(voice_layout_container_);
 }
 
-void DialogPlate::OnButtonPressed(AssistantButtonId id) {
-  delegate_->OnDialogPlateButtonPressed(id);
-  textfield_->SetText(base::string16());
-}
-
 void DialogPlate::OnAnimationStarted(
     const ui::CallbackLayerAnimationObserver& observer) {
   keyboard_layout_container_->set_can_process_events_within_subtree(false);

ash/assistant/ui/dialog_plate/dialog_plate.h

@@ -11,6 +11,7 @@
 #include "ash/assistant/model/assistant_interaction_model_observer.h"
 #include "ash/assistant/model/assistant_query_history.h"
 #include "ash/assistant/model/assistant_ui_model_observer.h"
+#include "ash/assistant/ui/base/assistant_button_listener.h"
 #include "base/component_export.h"
 #include "base/macros.h"
 #include "ui/views/controls/button/button.h"
@@ -27,7 +28,6 @@ class ImageButton;
 
 namespace ash {
 
-enum class AssistantButtonId;
 class AssistantViewDelegate;
 class MicView;
 
@@ -43,7 +43,7 @@ class COMPONENT_EXPORT(ASSISTANT_UI) DialogPlate
       public views::TextfieldController,
       public AssistantInteractionModelObserver,
       public AssistantUiModelObserver,
-      public views::ButtonListener {
+      public AssistantButtonListener {
  public:
   explicit DialogPlate(AssistantViewDelegate* delegate);
   ~DialogPlate() override;
@@ -54,8 +54,8 @@ class COMPONENT_EXPORT(ASSISTANT_UI) DialogPlate
   int GetHeightForWidth(int width) const override;
   void RequestFocus() override;
 
-  // ButtonListener:
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
+  // AssistantButtonListener:
+  void OnButtonPressed(AssistantButtonId button_id) override;
 
   // views::TextfieldController:
   bool HandleKeyEvent(views::Textfield* sender,
@@ -80,8 +80,6 @@ class COMPONENT_EXPORT(ASSISTANT_UI) DialogPlate
   void InitKeyboardLayoutContainer();
   void InitVoiceLayoutContainer();
 
-  void OnButtonPressed(AssistantButtonId id);
-
   void OnAnimationStarted(const ui::CallbackLayerAnimationObserver& observer);
   bool OnAnimationEnded(const ui::CallbackLayerAnimationObserver& observer);
 

ash/assistant/ui/dialog_plate/mic_view.cc

@@ -26,7 +26,7 @@ constexpr int kPreferredSizeDip = 32;
 
 }  // namespace
 
-MicView::MicView(views::ButtonListener* listener,
+MicView::MicView(AssistantButtonListener* listener,
                  AssistantViewDelegate* delegate,
                  AssistantButtonId button_id)
     : AssistantButton(listener, button_id), delegate_(delegate) {

ash/assistant/ui/dialog_plate/mic_view.h

@@ -12,7 +12,6 @@
 
 namespace ash {
 
-enum class AssistantButtonId;
 class AssistantViewDelegate;
 class LogoView;
 
@@ -22,7 +21,7 @@ class COMPONENT_EXPORT(ASSISTANT_UI) MicView
     : public AssistantButton,
       public AssistantInteractionModelObserver {
  public:
-  MicView(views::ButtonListener* listener,
+  MicView(AssistantButtonListener* listener,
           AssistantViewDelegate* delegate,
           AssistantButtonId button_id);
   ~MicView() override;

ash/public/cpp/test/assistant_test_api.h

@@ -67,6 +67,14 @@ class ASH_EXPORT AssistantTestApi {
   // Can only be used after the Assistant UI has been shown at least once.
   virtual views::View* greeting_label() = 0;
 
+  // Returns the button to enable voice mode.
+  // Can only be used after the Assistant UI has been shown at least once.
+  virtual views::View* voice_input_toggle() = 0;
+
+  // Returns the button to enable text mode.
+  // Can only be used after the Assistant UI has been shown at least once.
+  virtual views::View* keyboard_input_toggle() = 0;
+
   // Returns the window containing the Assistant UI.
   // Note that this window is shared for all components of the |AppList|.
   virtual aura::Window* window() = 0;