Revert "Remove AssistantState::VISIBLE"

This reverts commit 5072afda.

Reason for revert: Breaks long press animation

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}

TBR=xiyuan@chromium.org,sammc@chromium.org,xiaohuic@chromium.org,updowndota@chromium.org,wutao@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: b/140823590
Change-Id: I4e5114bc64f459b0854433405027df3490d4b8a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1829492Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Xiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#700905}

ash/assistant/assistant_ui_controller.cc

@@ -435,6 +435,11 @@ 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,6 +11,8 @@ 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 kAssistantVisibleAlpha = 255;    // 100% alpha
-constexpr uint8_t kAssistantInvisibleAlpha = 138;  // 54% alpha
+constexpr uint8_t kVoiceInteractionRunningAlpha = 255;     // 100% alpha
+constexpr uint8_t kVoiceInteractionNotRunningAlpha = 138;  // 54% alpha
 
 }  // namespace
 
@@ -93,7 +93,7 @@ void HomeButton::ButtonPressed(views::Button* sender,
   OnPressed(show_source, event.time_stamp());
 }
 
-void HomeButton::OnAssistantAvailabilityChanged() {
+void HomeButton::OnVoiceInteractionAvailabilityChanged() {
   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_.IsAssistantAvailable()) {
+  if (controller_.IsVoiceInteractionAvailable()) {
     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_.IsAssistantAvailable()) {
+    if (controller_.IsVoiceInteractionAvailable()) {
       // active: 100% alpha, inactive: 54% alpha
-      fg_flags.setAlpha(controller_.IsAssistantVisible()
-                            ? kAssistantVisibleAlpha
-                            : kAssistantInvisibleAlpha);
+      fg_flags.setAlpha(controller_.IsVoiceInteractionRunning()
+                            ? kVoiceInteractionRunningAlpha
+                            : kVoiceInteractionNotRunningAlpha);
     }
 
     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_.IsAssistantAvailable()) {
+    if (controller_.IsVoiceInteractionAvailable()) {
       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 OnAssistantAvailabilityChanged();
+  void OnVoiceInteractionAvailabilityChanged();
 
   // True if the app list is shown for the display containing this button.
   bool IsShowingAppList() const;

ash/shelf/home_button_controller.cc

@@ -20,6 +20,7 @@
 #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"
@@ -29,6 +30,8 @@
 namespace ash {
 namespace {
 
+constexpr int kVoiceInteractionAnimationDelayMs = 200;
+
 // Returns true if the button should appear activatable.
 bool CanActivate() {
   return Shell::Get()->tablet_mode_controller()->InTabletMode() ||
@@ -46,11 +49,11 @@ HomeButtonController::HomeButtonController(HomeButton* button)
   shell->tablet_mode_controller()->AddObserver(this);
   AssistantState::Get()->AddObserver(this);
 
-  // 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.
+  // 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.
   if (shell->session_controller()->IsActiveUserSessionStarted())
-    InitializeAssistantOverlay();
+    InitializeVoiceInteractionOverlay();
 }
 
 HomeButtonController::~HomeButtonController() {
@@ -71,8 +74,9 @@ bool HomeButtonController::MaybeHandleGestureEvent(ui::GestureEvent* event) {
   switch (event->type()) {
     case ui::ET_GESTURE_TAP:
     case ui::ET_GESTURE_TAP_CANCEL:
-      if (IsAssistantAvailable()) {
+      if (IsVoiceInteractionAvailable()) {
         assistant_overlay_->EndAnimation();
+        assistant_animation_delay_timer_->Stop();
       }
 
       if (CanActivate())
@@ -81,13 +85,23 @@ 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()) {
+        assistant_animation_delay_timer_->Start(
+            FROM_HERE,
+            base::TimeDelta::FromMilliseconds(
+                kVoiceInteractionAnimationDelayMs),
+            base::BindOnce(
+                &HomeButtonController::StartVoiceInteractionAnimation,
+                base::Unretained(this)));
+      }
+
       if (CanActivate())
         button_->AnimateInkDrop(views::InkDropState::ACTION_PENDING, event);
 
       return false;
     case ui::ET_GESTURE_LONG_PRESS:
-      // Only consume the long press event if the Assistant is available.
-      if (!IsAssistantAvailable())
+      // Only consume the long press event if voice interaction is available.
+      if (!IsVoiceInteractionAvailable())
         return false;
 
       base::RecordAction(base::UserMetricsAction(
@@ -100,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 the Assistant is available.
-      if (!IsAssistantAvailable())
+      // Only consume the long tap event if voice interaction is available.
+      if (!IsVoiceInteractionAvailable())
         return false;
 
       // This event happens after the user long presses and lifts the finger.
@@ -116,7 +130,7 @@ bool HomeButtonController::MaybeHandleGestureEvent(ui::GestureEvent* event) {
   }
 }
 
-bool HomeButtonController::IsAssistantAvailable() {
+bool HomeButtonController::IsVoiceInteractionAvailable() {
   AssistantStateBase* state = AssistantState::Get();
   bool settings_enabled = state->settings_enabled().value_or(false);
   bool feature_allowed =
@@ -125,12 +139,11 @@ bool HomeButtonController::IsAssistantAvailable() {
   return assistant_overlay_ && feature_allowed && settings_enabled;
 }
 
-bool HomeButtonController::IsAssistantVisible() {
-  return Shell::Get()
-             ->assistant_controller()
-             ->ui_controller()
-             ->model()
-             ->visibility() == AssistantVisibility::kVisible;
+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;
 }
 
 void HomeButtonController::OnAppListVisibilityChanged(bool shown,
@@ -145,12 +158,12 @@ void HomeButtonController::OnAppListVisibilityChanged(bool shown,
 
 void HomeButtonController::OnActiveUserSessionChanged(
     const AccountId& account_id) {
-  button_->OnAssistantAvailabilityChanged();
-  // Initialize the Assistant overlay when primary user session becomes
+  button_->OnVoiceInteractionAvailabilityChanged();
+  // Initialize voice interaction overlay when primary user session becomes
   // active.
   if (Shell::Get()->session_controller()->IsUserPrimary() &&
       !assistant_overlay_) {
-    InitializeAssistantOverlay();
+    InitializeVoiceInteractionOverlay();
   }
 }
 
@@ -160,14 +173,30 @@ void HomeButtonController::OnTabletModeStarted() {
 
 void HomeButtonController::OnAssistantStatusChanged(
     mojom::AssistantState state) {
-  button_->OnAssistantAvailabilityChanged();
+  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;
+  }
 }
 
 void HomeButtonController::OnAssistantSettingsEnabled(bool enabled) {
-  button_->OnAssistantAvailabilityChanged();
+  button_->OnVoiceInteractionAvailabilityChanged();
 }
 
-void HomeButtonController::StartAssistantAnimation() {
+void HomeButtonController::StartVoiceInteractionAnimation() {
   assistant_overlay_->StartAnimation(false);
 }
 
@@ -188,10 +217,11 @@ void HomeButtonController::OnAppListDismissed() {
       ->UpdateShelfVisibility();
 }
 
-void HomeButtonController::InitializeAssistantOverlay() {
+void HomeButtonController::InitializeVoiceInteractionOverlay() {
   assistant_overlay_ = new AssistantOverlay(button_);
   button_->AddChildView(assistant_overlay_);
   assistant_overlay_->SetVisible(false);
+  assistant_animation_delay_timer_ = std::make_unique<base::OneShotTimer>();
 }
 
 }  // namespace ash

ash/shelf/home_button_controller.h

@@ -13,6 +13,10 @@
 #include "ash/session/session_observer.h"
 #include "base/macros.h"
 
+namespace base {
+class OneShotTimer;
+}  // namespace base
+
 namespace ui {
 class GestureEvent;
 }  // namespace ui
@@ -33,16 +37,16 @@ class HomeButtonController : public AppListControllerObserver,
   explicit HomeButtonController(HomeButton* button);
   ~HomeButtonController() override;
 
-  // 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.
+  // 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.
   bool MaybeHandleGestureEvent(ui::GestureEvent* event);
 
-  // Whether the Assistant is available via long-press.
-  bool IsAssistantAvailable();
+  // Whether voice interaction is available via long-press.
+  bool IsVoiceInteractionAvailable();
 
-  // Whether the Assistant UI currently showing.
-  bool IsAssistantVisible();
+  // Whether voice interaction is currently running.
+  bool IsVoiceInteractionRunning();
 
   bool is_showing_app_list() const { return is_showing_app_list_; }
 
@@ -63,10 +67,10 @@ class HomeButtonController : public AppListControllerObserver,
   void OnAppListShown();
   void OnAppListDismissed();
 
-  void StartAssistantAnimation();
+  void StartVoiceInteractionAnimation();
 
-  // Initialize the Assistant overlay.
-  void InitializeAssistantOverlay();
+  // Initialize the voice interaction overlay.
+  void InitializeVoiceInteractionOverlay();
 
   // 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.
@@ -75,9 +79,11 @@ class HomeButtonController : public AppListControllerObserver,
   // The button that owns this controller.
   HomeButton* const button_;
 
-  // Owned by the button's view hierarchy. Null if the Assistant is not
+  // Owned by the button's view hierarchy. Null if voice interaction 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::READY);
+  assistant_state()->NotifyStatusChanged(mojom::AssistantState::VISIBLE);
   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::READY);
+  assistant_state()->NotifyStatusChanged(mojom::AssistantState::VISIBLE);
 
   // 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,7 +71,8 @@ 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::READY,
+                                           mojom::AssistantState::VISIBLE};
   const mojom::AssistantAllowedState kAllowedStates[] = {
       mojom::AssistantAllowedState::ALLOWED,
       mojom::AssistantAllowedState::DISALLOWED_BY_POLICY,
@@ -143,6 +144,7 @@ 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);
@@ -173,6 +175,7 @@ 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.