Reland "Remove AssistantState::VISIBLE"

This is a reland of 5072afda

Original change's description:
> Remove AssistantState::VISIBLE
> 
> Remove the VISIBLE state and use AssistantVisibility instead.
> Also remove obsolete handling for AGSA animations.
> 
> Bug: b/140823590
> Test: Run existing tests
> Change-Id: Id64c5e536dfe6d3e10c9b003c135e76c96a9fee1
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1802819
> Commit-Queue: Yue Li <updowndota@chromium.org>
> Reviewed-by: Sam McNally <sammc@chromium.org>
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Reviewed-by: Tao Wu <wutao@chromium.org>
> Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#696936}

Bug: b/140823590
Change-Id: Ia161398488e6d920548ade8a1eb37a0a99cc3403
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1832704
Commit-Queue: Yue Li <updowndota@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702095}

ash/assistant/assistant_ui_controller.cc

@@ -435,11 +435,6 @@ void AssistantUiController::OnUiVisibilityChanged(
     AssistantVisibility old_visibility,
     base::Optional<AssistantEntryPoint> entry_point,
     base::Optional<AssistantExitPoint> exit_point) {
-  AssistantState::Get()->NotifyStatusChanged(
-      new_visibility == AssistantVisibility::kVisible
-          ? mojom::AssistantState::VISIBLE
-          : mojom::AssistantState::READY);
-
   switch (new_visibility) {
     case AssistantVisibility::kClosed:
       // When the UI is closed, we stop the auto close timer as it may be

ash/public/mojom/assistant_state_controller.mojom

@@ -11,8 +11,6 @@ enum AssistantState {
   NOT_READY = 0,
   // The Assistant service is ready.
   READY,
-  // The Assistant UI is showing.
-  VISIBLE
 };
 
 enum AssistantAllowedState {

ash/shelf/home_button.cc

@@ -28,8 +28,8 @@
 namespace ash {
 namespace {
 
-constexpr uint8_t kVoiceInteractionRunningAlpha = 255;     // 100% alpha
-constexpr uint8_t kVoiceInteractionNotRunningAlpha = 138;  // 54% alpha
+constexpr uint8_t kAssistantVisibleAlpha = 255;    // 100% alpha
+constexpr uint8_t kAssistantInvisibleAlpha = 138;  // 54% alpha
 
 }  // namespace
 
@@ -93,7 +93,7 @@ void HomeButton::ButtonPressed(views::Button* sender,
   OnPressed(show_source, event.time_stamp());
 }
 
-void HomeButton::OnVoiceInteractionAvailabilityChanged() {
+void HomeButton::OnAssistantAvailabilityChanged() {
   SchedulePaint();
 }
 
@@ -125,7 +125,7 @@ void HomeButton::PaintButtonContents(gfx::Canvas* canvas) {
   // factors.
   float ring_outer_radius_dp = 7.f;
   float ring_thickness_dp = 1.5f;
-  if (controller_.IsVoiceInteractionAvailable()) {
+  if (controller_.IsAssistantAvailable()) {
     ring_outer_radius_dp = 8.f;
     ring_thickness_dp = 1.f;
   }
@@ -138,11 +138,11 @@ void HomeButton::PaintButtonContents(gfx::Canvas* canvas) {
     fg_flags.setStyle(cc::PaintFlags::kStroke_Style);
     fg_flags.setColor(ShelfConfig::Get()->shelf_icon_color());
 
-    if (controller_.IsVoiceInteractionAvailable()) {
+    if (controller_.IsAssistantAvailable()) {
       // active: 100% alpha, inactive: 54% alpha
-      fg_flags.setAlpha(controller_.IsVoiceInteractionRunning()
-                            ? kVoiceInteractionRunningAlpha
-                            : kVoiceInteractionNotRunningAlpha);
+      fg_flags.setAlpha(controller_.IsAssistantVisible()
+                            ? kAssistantVisibleAlpha
+                            : kAssistantInvisibleAlpha);
     }
 
     const float thickness = std::ceil(ring_thickness_dp * dsf);
@@ -151,7 +151,7 @@ void HomeButton::PaintButtonContents(gfx::Canvas* canvas) {
     // Make sure the center of the circle lands on pixel centers.
     canvas->DrawCircle(circle_center, radius, fg_flags);
 
-    if (controller_.IsVoiceInteractionAvailable()) {
+    if (controller_.IsAssistantAvailable()) {
       fg_flags.setAlpha(255);
       const float kCircleRadiusDp = 5.f;
       fg_flags.setStyle(cc::PaintFlags::kFill_Style);

ash/shelf/home_button.h

@@ -49,7 +49,7 @@ class ASH_EXPORT HomeButton : public ShelfControlButton,
 
   // Called when the availability of a long-press gesture may have changed, e.g.
   // when Assistant becomes enabled.
-  void OnVoiceInteractionAvailabilityChanged();
+  void OnAssistantAvailabilityChanged();
 
   // True if the app list is shown for the display containing this button.
   bool IsShowingAppList() const;

ash/shelf/home_button_controller.cc

@@ -20,7 +20,6 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/user_metrics.h"
 #include "base/metrics/user_metrics_action.h"
-#include "base/timer/timer.h"
 #include "chromeos/constants/chromeos_features.h"
 #include "components/account_id/account_id.h"
 #include "ui/display/screen.h"
@@ -30,7 +29,8 @@
 namespace ash {
 namespace {
 
-constexpr int kVoiceInteractionAnimationDelayMs = 200;
+constexpr base::TimeDelta kAssistantAnimationDelay =
+    base::TimeDelta::FromMilliseconds(200);
 
 // Returns true if the button should appear activatable.
 bool CanActivate() {
@@ -47,13 +47,14 @@ HomeButtonController::HomeButtonController(HomeButton* button)
   shell->app_list_controller()->AddObserver(this);
   shell->session_controller()->AddObserver(this);
   shell->tablet_mode_controller()->AddObserver(this);
+  shell->assistant_controller()->ui_controller()->AddModelObserver(this);
   AssistantState::Get()->AddObserver(this);
 
-  // Initialize voice interaction overlay and sync the flags if active user
-  // session has already started. This could happen when an external monitor
-  // is plugged in.
+  // Initialize the Assistant overlay and sync the flags if active user session
+  // has already started. This could happen when an external monitor is plugged
+  // in.
   if (shell->session_controller()->IsActiveUserSessionStarted())
-    InitializeVoiceInteractionOverlay();
+    InitializeAssistantOverlay();
 }
 
 HomeButtonController::~HomeButtonController() {
@@ -61,6 +62,8 @@ HomeButtonController::~HomeButtonController() {
 
   // AppListController and TabletModeController are destroyed early when Shell
   // is being destroyed, so they may not exist.
+  if (shell->assistant_controller())
+    shell->assistant_controller()->ui_controller()->RemoveModelObserver(this);
   if (shell->app_list_controller())
     shell->app_list_controller()->RemoveObserver(this);
   if (shell->tablet_mode_controller())
@@ -74,7 +77,7 @@ bool HomeButtonController::MaybeHandleGestureEvent(ui::GestureEvent* event) {
   switch (event->type()) {
     case ui::ET_GESTURE_TAP:
     case ui::ET_GESTURE_TAP_CANCEL:
-      if (IsVoiceInteractionAvailable()) {
+      if (IsAssistantAvailable()) {
         assistant_overlay_->EndAnimation();
         assistant_animation_delay_timer_->Stop();
       }
@@ -85,14 +88,11 @@ bool HomeButtonController::MaybeHandleGestureEvent(ui::GestureEvent* event) {
       // After animating the ripple, let the button handle the event.
       return false;
     case ui::ET_GESTURE_TAP_DOWN:
-      if (IsVoiceInteractionAvailable()) {
+      if (IsAssistantAvailable()) {
         assistant_animation_delay_timer_->Start(
-            FROM_HERE,
-            base::TimeDelta::FromMilliseconds(
-                kVoiceInteractionAnimationDelayMs),
-            base::BindOnce(
-                &HomeButtonController::StartVoiceInteractionAnimation,
-                base::Unretained(this)));
+            FROM_HERE, kAssistantAnimationDelay,
+            base::BindOnce(&HomeButtonController::StartAssistantAnimation,
+                           base::Unretained(this)));
       }
 
       if (CanActivate())
@@ -100,8 +100,8 @@ bool HomeButtonController::MaybeHandleGestureEvent(ui::GestureEvent* event) {
 
       return false;
     case ui::ET_GESTURE_LONG_PRESS:
-      // Only consume the long press event if voice interaction is available.
-      if (!IsVoiceInteractionAvailable())
+      // Only consume the long press event if the Assistant is available.
+      if (!IsAssistantAvailable())
         return false;
 
       base::RecordAction(base::UserMetricsAction(
@@ -114,8 +114,8 @@ bool HomeButtonController::MaybeHandleGestureEvent(ui::GestureEvent* event) {
           AssistantEntryPoint::kLongPressLauncher);
       return true;
     case ui::ET_GESTURE_LONG_TAP:
-      // Only consume the long tap event if voice interaction is available.
-      if (!IsVoiceInteractionAvailable())
+      // Only consume the long tap event if the Assistant is available.
+      if (!IsAssistantAvailable())
         return false;
 
       // This event happens after the user long presses and lifts the finger.
@@ -130,7 +130,7 @@ bool HomeButtonController::MaybeHandleGestureEvent(ui::GestureEvent* event) {
   }
 }
 
-bool HomeButtonController::IsVoiceInteractionAvailable() {
+bool HomeButtonController::IsAssistantAvailable() {
   AssistantStateBase* state = AssistantState::Get();
   bool settings_enabled = state->settings_enabled().value_or(false);
   bool feature_allowed =
@@ -139,11 +139,12 @@ bool HomeButtonController::IsVoiceInteractionAvailable() {
   return assistant_overlay_ && feature_allowed && settings_enabled;
 }
 
-bool HomeButtonController::IsVoiceInteractionRunning() {
-  // TODO(b/140823590): Update the method name/description and use Assistant
-  // visibility state instead.
-  return AssistantState::Get()->assistant_state() ==
-         mojom::AssistantState::VISIBLE;
+bool HomeButtonController::IsAssistantVisible() {
+  return Shell::Get()
+             ->assistant_controller()
+             ->ui_controller()
+             ->model()
+             ->visibility() == AssistantVisibility::kVisible;
 }
 
 void HomeButtonController::OnAppListVisibilityChanged(bool shown,
@@ -158,12 +159,12 @@ void HomeButtonController::OnAppListVisibilityChanged(bool shown,
 
 void HomeButtonController::OnActiveUserSessionChanged(
     const AccountId& account_id) {
-  button_->OnVoiceInteractionAvailabilityChanged();
-  // Initialize voice interaction overlay when primary user session becomes
+  button_->OnAssistantAvailabilityChanged();
+  // Initialize the Assistant overlay when primary user session becomes
   // active.
   if (Shell::Get()->session_controller()->IsUserPrimary() &&
       !assistant_overlay_) {
-    InitializeVoiceInteractionOverlay();
+    InitializeAssistantOverlay();
   }
 }
 
@@ -173,30 +174,22 @@ void HomeButtonController::OnTabletModeStarted() {
 
 void HomeButtonController::OnAssistantStatusChanged(
     mojom::AssistantState state) {
-  button_->OnVoiceInteractionAvailabilityChanged();
-
-  if (!assistant_overlay_)
-    return;
-
-  switch (state) {
-    case mojom::AssistantState::READY:
-      UMA_HISTOGRAM_TIMES(
-          "VoiceInteraction.OpenDuration",
-          base::TimeTicks::Now() - voice_interaction_start_timestamp_);
-      break;
-    case mojom::AssistantState::NOT_READY:
-      break;
-    case mojom::AssistantState::VISIBLE:
-      voice_interaction_start_timestamp_ = base::TimeTicks::Now();
-      break;
-  }
+  button_->OnAssistantAvailabilityChanged();
 }
 
 void HomeButtonController::OnAssistantSettingsEnabled(bool enabled) {
-  button_->OnVoiceInteractionAvailabilityChanged();
+  button_->OnAssistantAvailabilityChanged();
+}
+
+void HomeButtonController::OnUiVisibilityChanged(
+    AssistantVisibility new_visibility,
+    AssistantVisibility old_visibility,
+    base::Optional<AssistantEntryPoint> entry_point,
+    base::Optional<AssistantExitPoint> exit_point) {
+  button_->OnAssistantAvailabilityChanged();
 }
 
-void HomeButtonController::StartVoiceInteractionAnimation() {
+void HomeButtonController::StartAssistantAnimation() {
   assistant_overlay_->StartAnimation(false);
 }
 
@@ -217,7 +210,7 @@ void HomeButtonController::OnAppListDismissed() {
       ->UpdateShelfVisibility();
 }
 
-void HomeButtonController::InitializeVoiceInteractionOverlay() {
+void HomeButtonController::InitializeAssistantOverlay() {
   assistant_overlay_ = new AssistantOverlay(button_);
   button_->AddChildView(assistant_overlay_);
   assistant_overlay_->SetVisible(false);

ash/shelf/home_button_controller.h

@@ -7,16 +7,13 @@
 
 #include <memory>
 
+#include "ash/assistant/model/assistant_ui_model_observer.h"
 #include "ash/public/cpp/app_list/app_list_controller_observer.h"
 #include "ash/public/cpp/assistant/assistant_state.h"
 #include "ash/public/cpp/tablet_mode_observer.h"
 #include "ash/session/session_observer.h"
 #include "base/macros.h"
 
-namespace base {
-class OneShotTimer;
-}  // namespace base
-
 namespace ui {
 class GestureEvent;
 }  // namespace ui
@@ -32,21 +29,22 @@ class HomeButton;
 class HomeButtonController : public AppListControllerObserver,
                              public SessionObserver,
                              public TabletModeObserver,
-                             public AssistantStateObserver {
+                             public AssistantStateObserver,
+                             public AssistantUiModelObserver {
  public:
   explicit HomeButtonController(HomeButton* button);
   ~HomeButtonController() override;
 
-  // Maybe handles a gesture event based on the event and whether voice
-  // interaction is available. Returns true if the event is consumed; otherwise,
-  // HomeButton should pass the event along to Button to consume.
+  // Maybe handles a gesture event based on the event and whether the Assistant
+  // is available. Returns true if the event is consumed; otherwise, HomeButton
+  // should pass the event along to Button to consume.
   bool MaybeHandleGestureEvent(ui::GestureEvent* event);
 
-  // Whether voice interaction is available via long-press.
-  bool IsVoiceInteractionAvailable();
+  // Whether the Assistant is available via long-press.
+  bool IsAssistantAvailable();
 
-  // Whether voice interaction is currently running.
-  bool IsVoiceInteractionRunning();
+  // Whether the Assistant UI currently showing.
+  bool IsAssistantVisible();
 
   bool is_showing_app_list() const { return is_showing_app_list_; }
 
@@ -64,13 +62,20 @@ class HomeButtonController : public AppListControllerObserver,
   void OnAssistantStatusChanged(mojom::AssistantState state) override;
   void OnAssistantSettingsEnabled(bool enabled) override;
 
+  // AssistantUiModelObserver:
+  void OnUiVisibilityChanged(
+      AssistantVisibility new_visibility,
+      AssistantVisibility old_visibility,
+      base::Optional<AssistantEntryPoint> entry_point,
+      base::Optional<AssistantExitPoint> exit_point) override;
+
   void OnAppListShown();
   void OnAppListDismissed();
 
-  void StartVoiceInteractionAnimation();
+  void StartAssistantAnimation();
 
-  // Initialize the voice interaction overlay.
-  void InitializeVoiceInteractionOverlay();
+  // Initialize the Assistant overlay.
+  void InitializeAssistantOverlay();
 
   // True if the app list is currently showing for the button's display.
   // This is useful because other app_list_visible functions aren't per-display.
@@ -79,11 +84,10 @@ class HomeButtonController : public AppListControllerObserver,
   // The button that owns this controller.
   HomeButton* const button_;
 
-  // Owned by the button's view hierarchy. Null if voice interaction is not
+  // Owned by the button's view hierarchy. Null if the Assistant is not
   // enabled.
   AssistantOverlay* assistant_overlay_ = nullptr;
   std::unique_ptr<base::OneShotTimer> assistant_animation_delay_timer_;
-  base::TimeTicks voice_interaction_start_timestamp_;
 
   DISALLOW_COPY_AND_ASSIGN(HomeButtonController);
 };

ash/system/palette/palette_tray_unittest.cc

@@ -338,7 +338,7 @@ TEST_F(PaletteTrayTestWithAssistant, MetalayerToolViewCreated) {
 TEST_F(PaletteTrayTestWithAssistant, MetalayerToolActivatesHighlighter) {
   ui::ScopedAnimationDurationScaleMode animation_duration_mode(
       ui::ScopedAnimationDurationScaleMode::NON_ZERO_DURATION);
-  assistant_state()->NotifyStatusChanged(mojom::AssistantState::VISIBLE);
+  assistant_state()->NotifyStatusChanged(mojom::AssistantState::READY);
   prefs()->SetBoolean(chromeos::assistant::prefs::kAssistantEnabled, true);
   prefs()->SetBoolean(chromeos::assistant::prefs::kAssistantContextEnabled,
                       true);
@@ -453,7 +453,7 @@ TEST_F(PaletteTrayTestWithAssistant, StylusBarrelButtonActivatesHighlighter) {
                              false /* no highlighter on press */);
 
   // Once the service is ready, the button should start working.
-  assistant_state()->NotifyStatusChanged(mojom::AssistantState::VISIBLE);
+  assistant_state()->NotifyStatusChanged(mojom::AssistantState::READY);
 
   // Press and drag with no button, still no highlighter.
   WaitDragAndAssertMetalayer("all enabled, no button ", origin, ui::EF_NONE,

ash/system/palette/tools/metalayer_unittest.cc

@@ -71,8 +71,7 @@ class MetalayerToolTest : public AshTestBase {
 // has enabled the metalayer AND the Assistant framework is ready.
 TEST_F(MetalayerToolTest, PaletteMenuState) {
   const mojom::AssistantState kStates[] = {mojom::AssistantState::NOT_READY,
-                                           mojom::AssistantState::READY,
-                                           mojom::AssistantState::VISIBLE};
+                                           mojom::AssistantState::READY};
   const mojom::AssistantAllowedState kAllowedStates[] = {
       mojom::AssistantAllowedState::ALLOWED,
       mojom::AssistantAllowedState::DISALLOWED_BY_POLICY,
@@ -144,7 +143,6 @@ TEST_F(MetalayerToolTest, EnablingDisablingMetalayerEnablesDisablesController) {
 
 // Verifies that disabling the metalayer support disables the tool.
 TEST_F(MetalayerToolTest, MetalayerUnsupportedDisablesPaletteTool) {
-  assistant_state()->NotifyStatusChanged(mojom::AssistantState::VISIBLE);
   prefs()->SetBoolean(chromeos::assistant::prefs::kAssistantEnabled, true);
   prefs()->SetBoolean(chromeos::assistant::prefs::kAssistantContextEnabled,
                       true);
@@ -175,7 +173,6 @@ TEST_F(MetalayerToolTest, MetalayerUnsupportedDisablesPaletteTool) {
               DisableTool(PaletteToolId::METALAYER))
       .Times(0);
   assistant_state()->NotifyStatusChanged(mojom::AssistantState::READY);
-  assistant_state()->NotifyStatusChanged(mojom::AssistantState::VISIBLE);
   testing::Mock::VerifyAndClearExpectations(palette_tool_delegate_.get());
 
   // Changing the state to NOT_READY should disable the tool.