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}
chromium · Jan 27, 2023 · cfaa812 · cfaa812
1 parent bc4bbde
commit cfaa812
Show file tree

Hide file tree

Showing 8 changed files with 91 additions and 20 deletions.
diff --git a/ash/system/phonehub/phone_hub_nudge_controller.cc b/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
diff --git a/ash/system/phonehub/phone_hub_nudge_controller.h b/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;

diff --git a/ash/system/phonehub/phone_hub_nudge_controller_unittest.cc b/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) {

diff --git a/ash/system/phonehub/phone_hub_tray.cc b/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();
   }
 }
 

diff --git a/ash/system/phonehub/phone_hub_tray.h b/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_;
 

diff --git a/ash/system/phonehub/phone_hub_tray_unittest.cc b/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
diff --git a/ash/system/tray/system_nudge_controller.cc b/ash/system/tray/system_nudge_controller.cc
@@ -109,7 +109,7 @@ void SystemNudgeController::RecordNudgeAction(NudgeCatalogName catalog_name) {
 
 void SystemNudgeController::ShowNudge() {
   if (nudge_ && !nudge_->widget()->IsClosed()) {
-    hide_nudge_timer_.AbandonAndStop();
+    hide_nudge_timer_->AbandonAndStop();
     nudge_->Close();
   }
 
@@ -119,18 +119,19 @@ 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() {
   hide_nudge_animation_observer_.reset();
 }
 
 void SystemNudgeController::FireHideNudgeTimerForTesting() {
-  hide_nudge_timer_.FireNow();
+  hide_nudge_timer_->FireNow();
 }
 
 void SystemNudgeController::ResetNudgeRegistryForTesting() {
@@ -150,6 +151,7 @@ SystemNudgeController::GetNudgeRegistry() {
 }
 
 void SystemNudgeController::StartFadeAnimation(bool show) {
+  hide_nudge_timer_.reset();
   // Clean any pending animation observer.
   hide_nudge_animation_observer_.reset();
 
@@ -159,6 +161,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;

diff --git a/ash/system/tray/system_nudge_controller.h b/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_;