[CrOS PhoneHub] Persist hiding of notification setup banner on dismiss.

Note that OnShouldShowSetupRequiredUiDismissed() was added for
the debug UI control which will be added in a later CL.

Bug: 1106937
Change-Id: I0b4915827ec690d3f639177d1189fac74da0220f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2488613
Commit-Queue: Regan Hsu <hsuregan@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819460}

ash/system/phonehub/notification_opt_in_view.cc

@@ -17,6 +17,7 @@
 #include "ash/system/unified/rounded_label_button.h"
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chromeos/components/phonehub/notification_access_manager.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/geometry/insets.h"
 #include "ui/views/border.h"
@@ -55,8 +56,11 @@ constexpr char kMultideviceSettingsUrl[] =
 
 }  // namespace
 
-NotificationOptInView::NotificationOptInView(TrayBubbleView* bubble_view)
-    : bubble_view_(bubble_view) {
+NotificationOptInView::NotificationOptInView(
+    TrayBubbleView* bubble_view,
+    chromeos::phonehub::NotificationAccessManager* notification_access_manager)
+    : bubble_view_(bubble_view),
+      notification_access_manager_(notification_access_manager) {
   SetID(PhoneHubViewID::kNotificationOptInView);
   InitLayout();
   LogInterstitialScreenEvent(InterstitialScreen::kNotificationOptIn,
@@ -74,6 +78,7 @@ void NotificationOptInView::ButtonPressed(views::Button* sender,
                                  InterstitialScreenEvent::kDismiss);
       SetVisible(false);
       bubble_view_->UpdateBubble();
+      notification_access_manager_->DismissSetupRequiredUi();
       break;
     case kSetUpButtonTag:
       // Opens the notification set up dialog in settings to start the opt in

ash/system/phonehub/notification_opt_in_view.h

@@ -16,6 +16,12 @@ namespace views {
 class Label;
 }  // namespace views
 
+namespace chromeos {
+namespace phonehub {
+class NotificationAccessManager;
+}  // namespace phonehub
+}  // namespace chromeos
+
 namespace ash {
 
 class TrayBubbleView;
@@ -27,7 +33,9 @@ class ASH_EXPORT NotificationOptInView : public views::View,
  public:
   METADATA_HEADER(NotificationOptInView);
 
-  explicit NotificationOptInView(TrayBubbleView* bubble_view);
+  NotificationOptInView(TrayBubbleView* bubble_view,
+                        chromeos::phonehub::NotificationAccessManager*
+                            notification_access_manager);
   NotificationOptInView(const NotificationOptInView&) = delete;
   NotificationOptInView& operator=(const NotificationOptInView&) = delete;
   ~NotificationOptInView() override;
@@ -47,6 +55,7 @@ class ASH_EXPORT NotificationOptInView : public views::View,
   InterstitialViewButton* dismiss_button_ = nullptr;
 
   TrayBubbleView* bubble_view_ = nullptr;
+  chromeos::phonehub::NotificationAccessManager* notification_access_manager_;
 };
 
 }  // namespace ash

ash/system/phonehub/phone_connected_view.cc

@@ -45,11 +45,14 @@ PhoneConnectedView::PhoneConnectedView(
 
   AddSeparator();
 
-  // TODO(meilinw): handle the case when the user has dismissed this opt in
-  // view once, we shouldn't show it again.
-  if (!phone_hub_manager->GetNotificationAccessManager()
-           ->HasAccessBeenGranted()) {
-    AddChildView(std::make_unique<NotificationOptInView>(bubble_view));
+  chromeos::phonehub::NotificationAccessManager* access_manager =
+      phone_hub_manager->GetNotificationAccessManager();
+  bool should_show_notification_setup_ui =
+      !access_manager->HasAccessBeenGranted() &&
+      !access_manager->HasNotificationSetupUiBeenDismissed();
+  if (should_show_notification_setup_ui) {
+    AddChildView(std::make_unique<NotificationOptInView>(
+        bubble_view, phone_hub_manager->GetNotificationAccessManager()));
   }
 
   setup_layered_view(

chromeos/components/phonehub/fake_notification_access_manager.cc

@@ -26,6 +26,19 @@ bool FakeNotificationAccessManager::HasAccessBeenGranted() const {
   return has_access_been_granted_;
 }
 
+bool FakeNotificationAccessManager::HasNotificationSetupUiBeenDismissed()
+    const {
+  return has_notification_setup_ui_been_dismissed_;
+}
+
+void FakeNotificationAccessManager::DismissSetupRequiredUi() {
+  has_notification_setup_ui_been_dismissed_ = true;
+}
+
+void FakeNotificationAccessManager::ResetHasNotificationSetupUiBeenDismissed() {
+  has_notification_setup_ui_been_dismissed_ = false;
+}
+
 void FakeNotificationAccessManager::SetNotificationSetupOperationStatus(
     NotificationAccessSetupOperation::Status new_status) {
   if (new_status ==

chromeos/components/phonehub/fake_notification_access_manager.h

@@ -23,9 +23,14 @@ class FakeNotificationAccessManager : public NotificationAccessManager {
 
   // NotificationAccessManager:
   bool HasAccessBeenGranted() const override;
+  bool HasNotificationSetupUiBeenDismissed() const override;
+  void DismissSetupRequiredUi() override;
+
+  void ResetHasNotificationSetupUiBeenDismissed();
 
  private:
   bool has_access_been_granted_;
+  bool has_notification_setup_ui_been_dismissed_ = false;
 };
 
 }  // namespace phonehub

chromeos/components/phonehub/notification_access_manager.h

@@ -41,6 +41,11 @@ class NotificationAccessManager {
 
   virtual bool HasAccessBeenGranted() const = 0;
 
+  virtual bool HasNotificationSetupUiBeenDismissed() const = 0;
+
+  // Disables the ability to show the banner within the PhoneHub UI.
+  virtual void DismissSetupRequiredUi() = 0;
+
   // Starts an attempt to enable the notification access. |delegate| will be
   // updated with the status of the flow as long as the operation object
   // returned by this function remains instantiated.

chromeos/components/phonehub/notification_access_manager_impl.cc

@@ -18,6 +18,7 @@ namespace phonehub {
 void NotificationAccessManagerImpl::RegisterPrefs(
     PrefRegistrySimple* registry) {
   registry->RegisterBooleanPref(prefs::kNotificationAccessGranted, false);
+  registry->RegisterBooleanPref(prefs::kHasDismissedSetupRequiredUi, false);
 }
 
 NotificationAccessManagerImpl::NotificationAccessManagerImpl(
@@ -40,6 +41,15 @@ NotificationAccessManagerImpl::~NotificationAccessManagerImpl() {
   feature_status_provider_->RemoveObserver(this);
 }
 
+bool NotificationAccessManagerImpl::HasNotificationSetupUiBeenDismissed()
+    const {
+  return pref_service_->GetBoolean(prefs::kHasDismissedSetupRequiredUi);
+}
+
+void NotificationAccessManagerImpl::DismissSetupRequiredUi() {
+  pref_service_->SetBoolean(prefs::kHasDismissedSetupRequiredUi, true);
+}
+
 bool NotificationAccessManagerImpl::HasAccessBeenGranted() const {
   return pref_service_->GetBoolean(prefs::kNotificationAccessGranted);
 }

chromeos/components/phonehub/notification_access_manager_impl.h

@@ -43,6 +43,9 @@ class NotificationAccessManagerImpl : public NotificationAccessManager,
   void SetHasAccessBeenGrantedInternal(bool has_access_been_granted) override;
   void OnSetupRequested() override;
 
+  bool HasNotificationSetupUiBeenDismissed() const override;
+  void DismissSetupRequiredUi() override;
+
   // FeatureStatusProvider::Observer:
   void OnFeatureStatusChanged() override;
 

chromeos/components/phonehub/notification_access_manager_impl_unittest.cc

@@ -93,6 +93,12 @@ class NotificationAccessManagerImplTest : public testing::Test {
     EXPECT_EQ(expected_value, manager_->HasAccessBeenGranted());
   }
 
+  bool HasNotificationSetupUiBeenDismissed() {
+    return manager_->HasNotificationSetupUiBeenDismissed();
+  }
+
+  void DismissSetupRequiredUi() { manager_->DismissSetupRequiredUi(); }
+
   std::unique_ptr<NotificationAccessSetupOperation> StartSetupOperation() {
     return manager_->AttemptNotificationSetup(&fake_delegate_);
   }
@@ -134,6 +140,30 @@ class NotificationAccessManagerImplTest : public testing::Test {
   std::unique_ptr<NotificationAccessManager> manager_;
 };
 
+TEST_F(NotificationAccessManagerImplTest, ShouldShowSetupRequiredUi) {
+  // Notification setup is not dismissed initially even when access has been
+  // granted.
+  Initialize(/*initial_has_access_been_granted=*/true);
+  EXPECT_FALSE(HasNotificationSetupUiBeenDismissed());
+
+  // Notification setup is not dismissed initially when access has not been
+  // granted.
+  Initialize(/*initial_has_access_been_granted=*/false);
+  EXPECT_FALSE(HasNotificationSetupUiBeenDismissed());
+
+  // Simlulate dismissal of UI.
+  DismissSetupRequiredUi();
+  EXPECT_TRUE(HasNotificationSetupUiBeenDismissed());
+
+  // Dismissal value is persisted on initialization with access not granted.
+  Initialize(/*initial_has_access_been_granted=*/false);
+  EXPECT_TRUE(HasNotificationSetupUiBeenDismissed());
+
+  // Dismissal value is persisted on initialization with access granted.
+  Initialize(/*initial_has_access_been_granted=*/true);
+  EXPECT_TRUE(HasNotificationSetupUiBeenDismissed());
+}
+
 TEST_F(NotificationAccessManagerImplTest, InitiallyGranted) {
   Initialize(/*initial_has_access_been_granted=*/true);
   VerifyNotificationAccessGrantedState(/*expected_value=*/true);

chromeos/components/phonehub/pref_names.cc

@@ -21,6 +21,11 @@ const char kHasDismissedUiAfterCompletingOnboarding[] =
 const char kIsAwaitingVerifiedHost[] =
     "cros.phonehub.is_awaiting_verified_host";
 
+// Whether the Notification access setup banner in the PhoneHub UI has
+// been dismissed.
+const char kHasDismissedSetupRequiredUi[] =
+    "cros.phonehub.has_dismissed_setup_required_ui";
+
 }  // namespace prefs
 }  // namespace phonehub
 }  // namespace chromeos

chromeos/components/phonehub/pref_names.h

@@ -12,6 +12,7 @@ namespace prefs {
 extern const char kNotificationAccessGranted[];
 extern const char kHasDismissedUiAfterCompletingOnboarding[];
 extern const char kIsAwaitingVerifiedHost[];
+extern const char kHasDismissedSetupRequiredUi[];
 
 }  // namespace prefs
 }  // namespace phonehub