Reland "Phone Hub: Modify Phone Hub tray to show nudge."

This is a reland of commit cfaa812836

Change from original CL: There is now a check to ensure timer does exist before calling to stop it.

Original change's description:
> Phone Hub: Modify Phone Hub tray to show nudge.
>
> Modify Phone Hub tray methods to display Phone Hub nudge if nudge flag is on. Also, hide the nudge when Phone Hub icon is clicked.
>
> UI:
> (Recording of nudge being hidden after clicking on Phone Hub icon)
> https://drive.google.com/file/d/1P9yVCb-aRB7IXY2gqHhorPFXbojnpckz/view?usp=sharing
> https://screenshot.googleplex.com/6DLVFX739n9Us6a
>
> Change-Id: I81ea423963ce395521e749a96b0ac7db33474020
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4163684
> Commit-Queue: Jennifer Serrano <jennserrano@google.com>
> Reviewed-by: Jon Mann <jonmann@chromium.org>
> Reviewed-by: Alex Newcomer <newcomer@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1098164}

Fixed: b/275587468

Change-Id: Ic673e04f3035a58b658806a67f3e31752270787d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4374702
Commit-Queue: Jennifer Serrano <jennserrano@google.com>
Reviewed-by: Jon Mann <jonmann@chromium.org>
Reviewed-by: Pu Shi <pushi@google.com>
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1123196}

ash/system/phonehub/phone_hub_nudge_controller.cc

@@ -8,12 +8,15 @@
 
 namespace ash {
 
-PhoneHubNudgeController::PhoneHubNudgeController(std::u16string nudge_content)
-    : nudge_content_(nudge_content) {}
+PhoneHubNudgeController::PhoneHubNudgeController() = default;
 PhoneHubNudgeController::~PhoneHubNudgeController() = default;
 
 std::unique_ptr<SystemNudge> PhoneHubNudgeController::CreateSystemNudge() {
+  SetNudgeContent();
   return std::make_unique<PhoneHubNudge>(nudge_content_);
 }
 
+void PhoneHubNudgeController::SetNudgeContent() {
+  nudge_content_ = u"";
+}
 }  // namespace ash

ash/system/phonehub/phone_hub_nudge_controller.h

@@ -8,14 +8,16 @@
 #include "ash/system/tray/system_nudge_controller.h"
 
 namespace ash {
-
+// This class controls showing a nudge when a user is eligible for Phone Hub.
 class ASH_EXPORT PhoneHubNudgeController : public SystemNudgeController {
  public:
-  explicit PhoneHubNudgeController(std::u16string nudge_content_);
+  PhoneHubNudgeController();
   PhoneHubNudgeController(const PhoneHubNudgeController&) = delete;
   PhoneHubNudgeController& operator=(const PhoneHubNudgeController&) = delete;
   ~PhoneHubNudgeController() override;
 
+  void SetNudgeContent();
+
  protected:
   // SystemNudgeController: Creates PhoneHubNudge
   std::unique_ptr<SystemNudge> CreateSystemNudge() override;

ash/system/phonehub/phone_hub_nudge_controller_unittest.cc

@@ -17,14 +17,13 @@ class PhoneHubNudgeControllerTest : public AshTestBase {
   // AshTestBase:
   void SetUp() override {
     AshTestBase::SetUp();
-    controller_ = std::make_unique<PhoneHubNudgeController>(nudge_content_);
+    controller_ = std::make_unique<PhoneHubNudgeController>();
   }
 
   PhoneHubNudgeController* GetController() { return controller_.get(); }
 
  private:
   std::unique_ptr<PhoneHubNudgeController> controller_;
-  std::u16string nudge_content_;
 };
 
 TEST_F(PhoneHubNudgeControllerTest, PhoneHubNudgeControllerExists) {

ash/system/phonehub/phone_hub_tray.cc

@@ -60,11 +60,23 @@ constexpr int kIconSpacing = 12;
 constexpr auto kBubblePadding =
     gfx::Insets::TLBR(0, 0, kBubbleBottomPaddingDip, 0);
 
+bool IsInUserSession() {
+  SessionControllerImpl* session_controller =
+      Shell::Get()->session_controller();
+  return session_controller->GetSessionState() ==
+             session_manager::SessionState::ACTIVE &&
+         !session_controller->IsRunningInAppMode();
+}
+
 }  // namespace
 
 PhoneHubTray::PhoneHubTray(Shelf* shelf)
     : TrayBackgroundView(shelf, TrayBackgroundViewCatalogName::kPhoneHub),
-      ui_controller_(new PhoneHubUiController()) {
+      ui_controller_(new PhoneHubUiController()),
+      phone_hub_nudge_controller_(
+          features::IsPhoneHubNudgeEnabled()
+              ? std::make_unique<PhoneHubNudgeController>()
+              : nullptr) {
   // By default, if the individual buttons did not handle the event consider it
   // as a phone hub icon event.
   SetPressedCallback(base::BindRepeating(&PhoneHubTray::PhoneHubIconActivated,
@@ -385,7 +397,14 @@ void PhoneHubTray::CloseBubble() {
 void PhoneHubTray::UpdateVisibility() {
   DCHECK(ui_controller_.get());
   auto ui_state = ui_controller_->ui_state();
-  SetVisiblePreferred(ui_state != PhoneHubUiController::UiState::kHidden);
+  SetVisiblePreferred(ui_state != PhoneHubUiController::UiState::kHidden &&
+                      IsInUserSession());
+  if (features::IsPhoneHubNudgeEnabled() && IsInUserSession()) {
+    if (ui_state == PhoneHubUiController::UiState::kOnboardingWithoutPhone) {
+      phone_hub_nudge_controller_->ShowNudge();
+      // TODO (b/266853434): Animation of icon.
+    }
+  }
 }
 
 void PhoneHubTray::UpdateHeaderVisibility() {
@@ -413,8 +432,11 @@ void PhoneHubTray::PhoneHubIconActivated(const ui::Event& event) {
   // Simply toggle between visible/invisibvle
   if (bubble_ && bubble_->bubble_view()->GetVisible()) {
     CloseBubble();
-  } else {
-    ShowBubble();
+    return;
+  }
+  ShowBubble();
+  if (features::IsPhoneHubNudgeEnabled()) {
+    phone_hub_nudge_controller_->HideNudge();
   }
 }
 

ash/system/phonehub/phone_hub_tray.h

@@ -10,6 +10,7 @@
 #include "ash/session/session_controller_impl.h"
 #include "ash/system/phonehub/onboarding_view.h"
 #include "ash/system/phonehub/phone_hub_content_view.h"
+#include "ash/system/phonehub/phone_hub_nudge_controller.h"
 #include "ash/system/phonehub/phone_hub_ui_controller.h"
 #include "ash/system/phonehub/phone_status_view.h"
 #include "ash/system/status_area_widget.h"
@@ -112,6 +113,10 @@ class ASH_EXPORT PhoneHubTray : public TrayBackgroundView,
     return ui_controller_.get();
   }
 
+  PhoneHubNudgeController* phone_hub_nudge_controller_for_testing() {
+    return phone_hub_nudge_controller_.get();
+  }
+
  private:
   FRIEND_TEST_ALL_PREFIXES(PhoneHubTrayTest, SafeAccessToHeaderView);
 
@@ -163,6 +168,9 @@ class ASH_EXPORT PhoneHubTray : public TrayBackgroundView,
   // PhoneHub state.
   std::unique_ptr<PhoneHubUiController> ui_controller_;
 
+  // Controls the behavior of a nudge shown to eligible users.
+  std::unique_ptr<PhoneHubNudgeController> phone_hub_nudge_controller_;
+
   // The bubble that appears after clicking the tray button.
   std::unique_ptr<TrayBubbleWrapper> bubble_;
 

ash/system/phonehub/phone_hub_tray_unittest.cc

@@ -61,7 +61,8 @@ class PhoneHubTrayTest : public AshTestBase {
     feature_list_.InitWithFeatures(
         /*enabled_features=*/{features::kPhoneHub,
                               features::kPhoneHubCameraRoll,
-                              features::kEcheLauncher, features::kEcheSWA},
+                              features::kEcheLauncher, features::kEcheSWA,
+                              features::kPhoneHubNudge},
         /*disabled_features=*/{});
     auto delegate = std::make_unique<MockNewWindowDelegate>();
     new_window_delegate_ = delegate.get();
@@ -791,4 +792,35 @@ TEST_F(PhoneHubTrayTest, MultiDisplay) {
   EXPECT_TRUE(secondary_phone_hub_tray->GetVisible());
 }
 
+TEST_F(PhoneHubTrayTest, ShowNudge) {
+  // Simulate kOnboardingWithoutPhone state.
+  GetFeatureStatusProvider()->SetStatus(
+      phonehub::FeatureStatus::kEligiblePhoneButNotSetUp);
+  GetOnboardingUiTracker()->SetShouldShowOnboardingUi(true);
+  GetSessionControllerClient()->SetSessionState(
+      session_manager::SessionState::ACTIVE);
+
+  PhoneHubNudgeController* nudge_controller =
+      phone_hub_tray_->phone_hub_nudge_controller_for_testing();
+  SystemNudge* nudge = nudge_controller->GetSystemNudgeForTesting();
+  EXPECT_TRUE(nudge);
+}
+
+TEST_F(PhoneHubTrayTest, HideNudge) {
+  GetFeatureStatusProvider()->SetStatus(
+      phonehub::FeatureStatus::kEligiblePhoneButNotSetUp);
+  GetOnboardingUiTracker()->SetShouldShowOnboardingUi(true);
+  GetSessionControllerClient()->SetSessionState(
+      session_manager::SessionState::ACTIVE);
+
+  PhoneHubNudgeController* nudge_controller =
+      phone_hub_tray_->phone_hub_nudge_controller_for_testing();
+  SystemNudge* nudge = nudge_controller->GetSystemNudgeForTesting();
+  EXPECT_TRUE(nudge);
+
+  ClickTrayButton();
+  nudge = nudge_controller->GetSystemNudgeForTesting();
+  EXPECT_FALSE(nudge);
+}
+
 }  // namespace ash

ash/system/tray/system_nudge_controller.cc

@@ -109,7 +109,9 @@ void SystemNudgeController::RecordNudgeAction(NudgeCatalogName catalog_name) {
 
 void SystemNudgeController::ShowNudge() {
   if (nudge_ && !nudge_->widget()->IsClosed()) {
-    hide_nudge_timer_.AbandonAndStop();
+    if (hide_nudge_timer_) {
+      hide_nudge_timer_->AbandonAndStop();
+    }
     nudge_->Close();
   }
 
@@ -119,10 +121,11 @@ void SystemNudgeController::ShowNudge() {
   StartFadeAnimation(/*show=*/true);
   RecordNudgeShown(nudge_->catalog_name());
 
+  hide_nudge_timer_ = std::make_unique<base::OneShotTimer>();
   // Start a timer to close the nudge after a set amount of time.
-  hide_nudge_timer_.Start(FROM_HERE, kNudgeShowTime,
-                          base::BindOnce(&SystemNudgeController::HideNudge,
-                                         weak_ptr_factory_.GetWeakPtr()));
+  hide_nudge_timer_->Start(FROM_HERE, kNudgeShowTime,
+                           base::BindOnce(&SystemNudgeController::HideNudge,
+                                          weak_ptr_factory_.GetWeakPtr()));
 }
 
 void SystemNudgeController::ForceCloseAnimatingNudge() {
@@ -130,7 +133,7 @@ void SystemNudgeController::ForceCloseAnimatingNudge() {
 }
 
 void SystemNudgeController::FireHideNudgeTimerForTesting() {
-  hide_nudge_timer_.FireNow();
+  hide_nudge_timer_->FireNow();
 }
 
 void SystemNudgeController::ResetNudgeRegistryForTesting() {
@@ -150,6 +153,7 @@ SystemNudgeController::GetNudgeRegistry() {
 }
 
 void SystemNudgeController::StartFadeAnimation(bool show) {
+  hide_nudge_timer_.reset();
   // Clean any pending animation observer.
   hide_nudge_animation_observer_.reset();
 
@@ -159,6 +163,9 @@ void SystemNudgeController::StartFadeAnimation(bool show) {
     return;
 
   ui::Layer* layer = nudge_->widget()->GetLayer();
+  if (layer->GetAnimator()->is_animating()) {
+    return;
+  }
   gfx::Rect widget_bounds = layer->bounds();
 
   gfx::Transform scaled_nudge_transform;

ash/system/tray/system_nudge_controller.h

@@ -50,15 +50,15 @@ class ASH_EXPORT SystemNudgeController {
   // shown.
   void ResetNudgeRegistryForTesting();
 
+  // Hides the nudge widget.
+  void HideNudge();
+
  protected:
   // Concrete subclasses must implement this method to return a
   // SystemNudge that creates a label and specifies an icon specific
   // to the nudge.
   virtual std::unique_ptr<SystemNudge> CreateSystemNudge() = 0;
 
-  // Hides the nudge widget.
-  void HideNudge();
-
  private:
   // Returns the registry which keeps track of when a nudge was last shown.
   static std::vector<std::pair<NudgeCatalogName, base::TimeTicks>>&
@@ -75,7 +75,7 @@ class ASH_EXPORT SystemNudgeController {
   std::unique_ptr<SystemNudge> nudge_;
 
   // Timer to hide the nudge.
-  base::OneShotTimer hide_nudge_timer_;
+  std::unique_ptr<base::OneShotTimer> hide_nudge_timer_;
 
   std::unique_ptr<ui::ImplicitAnimationObserver> hide_nudge_animation_observer_;