[CrOS PhoneHub] Fix notification set up UI issue.

Previously if user revokes the notification access they granted earlier
from the phone, notification set up UI cannot show up again unless you
close and reopen the bubble. This CL fixes this by observing the change
of notification access and update the view's visibility accordingly.

BUG=1141610, 1106937
TEST=unittested.

Change-Id: Ife8f19e7e5b8c2ec9ea12a7cc9e0e2cc739af72d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2508398
Commit-Queue: Meilin Wang <meilinw@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Tim Song <tengs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823377}

ash/system/phonehub/notification_opt_in_view.cc

@@ -59,6 +59,13 @@ NotificationOptInView::NotificationOptInView(
       notification_access_manager_(notification_access_manager) {
   SetID(PhoneHubViewID::kNotificationOptInView);
   InitLayout();
+
+  DCHECK(notification_access_manager_);
+  access_manager_observer_.Add(notification_access_manager_);
+
+  // Checks and updates its visibility upon creation.
+  UpdateVisibility();
+
   LogNotificationOptInEvent(InterstitialScreenEvent::kShown);
 }
 
@@ -79,6 +86,10 @@ void NotificationOptInView::DismissButtonPressed() {
   notification_access_manager_->DismissSetupRequiredUi();
 }
 
+void NotificationOptInView::OnNotificationAccessChanged() {
+  UpdateVisibility();
+}
+
 void NotificationOptInView::InitLayout() {
   SetPaintToLayer();
   layer()->SetFillsBoundsOpaquely(false);
@@ -142,6 +153,14 @@ void NotificationOptInView::InitLayout() {
           /*paint_background=*/true));
 }
 
+void NotificationOptInView::UpdateVisibility() {
+  DCHECK(notification_access_manager_);
+  const bool should_show =
+      !notification_access_manager_->HasAccessBeenGranted() &&
+      !notification_access_manager_->HasNotificationSetupUiBeenDismissed();
+  SetVisible(should_show);
+}
+
 BEGIN_METADATA(NotificationOptInView, views::View)
 END_METADATA
 

ash/system/phonehub/notification_opt_in_view.h

@@ -7,6 +7,8 @@
 
 #include "ash/ash_export.h"
 #include "ash/system/phonehub/interstitial_view_button.h"
+#include "base/scoped_observer.h"
+#include "chromeos/components/phonehub/notification_access_manager.h"
 #include "ui/views/controls/button/button.h"
 #include "ui/views/metadata/metadata_header_macros.h"
 #include "ui/views/view.h"
@@ -27,7 +29,9 @@ class TrayBubbleView;
 
 // An additional entry point shown on the Phone Hub bubble for the user to grant
 // access or opt out for notifications from the phone.
-class ASH_EXPORT NotificationOptInView : public views::View {
+class ASH_EXPORT NotificationOptInView
+    : public views::View,
+      public chromeos::phonehub::NotificationAccessManager::Observer {
  public:
   METADATA_HEADER(NotificationOptInView);
 
@@ -38,15 +42,21 @@ class ASH_EXPORT NotificationOptInView : public views::View {
   NotificationOptInView& operator=(const NotificationOptInView&) = delete;
   ~NotificationOptInView() override;
 
+  // chromeos::phonehub::NotificationAccessManager::Observer:
+  void OnNotificationAccessChanged() override;
+
   views::View* set_up_button_for_testing() { return set_up_button_; }
   views::View* dismiss_button_for_testing() { return dismiss_button_; }
-
  private:
   void InitLayout();
 
   void SetUpButtonPressed();
   void DismissButtonPressed();
 
+  // Calculates whether this view should be visible and updates its visibility
+  // accordingly.
+  void UpdateVisibility();
+
   // Main components of this view. Owned by view hierarchy.
   views::Label* text_label_ = nullptr;
   InterstitialViewButton* set_up_button_ = nullptr;
@@ -54,6 +64,10 @@ class ASH_EXPORT NotificationOptInView : public views::View {
 
   TrayBubbleView* bubble_view_ = nullptr;
   chromeos::phonehub::NotificationAccessManager* notification_access_manager_;
+
+  ScopedObserver<chromeos::phonehub::NotificationAccessManager,
+                 chromeos::phonehub::NotificationAccessManager::Observer>
+      access_manager_observer_{this};
 };
 
 }  // namespace ash

ash/system/phonehub/phone_connected_view.cc

@@ -41,15 +41,8 @@ PhoneConnectedView::PhoneConnectedView(
       gfx::Insets(0, kBubbleHorizontalSidePaddingDip)));
   layout->SetDefaultFlex(1);
 
-  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()));
-  }
+  AddChildView(std::make_unique<NotificationOptInView>(
+      bubble_view, phone_hub_manager->GetNotificationAccessManager()));
 
   setup_layered_view(
       AddChildView(std::make_unique<QuickActionsView>(phone_hub_manager)));

ash/system/phonehub/phone_hub_tray_unittest.cc

@@ -218,7 +218,8 @@ TEST_F(PhoneHubTrayTest, HideNotificationOptInViewWhenAccessHasBeenGranted) {
 
   ClickTrayButton();
 
-  EXPECT_FALSE(notification_opt_in_view());
+  EXPECT_TRUE(notification_opt_in_view());
+  EXPECT_FALSE(notification_opt_in_view()->GetVisible());
 }
 
 TEST_F(PhoneHubTrayTest, StartNotificationSetUpFlow) {
@@ -239,6 +240,18 @@ TEST_F(PhoneHubTrayTest, StartNotificationSetUpFlow) {
       });
 
   ClickOnAndWait(notification_opt_in_view()->set_up_button_for_testing());
+
+  // Simulate that notification access has been granted.
+  GetNotificationAccessManager()->SetHasAccessBeenGrantedInternal(true);
+
+  // This view should be dismissed.
+  EXPECT_FALSE(notification_opt_in_view()->GetVisible());
+
+  // Simulate that notification access has been revoked by the phone.
+  GetNotificationAccessManager()->SetHasAccessBeenGrantedInternal(false);
+
+  // This view should show up again.
+  EXPECT_TRUE(notification_opt_in_view()->GetVisible());
 }
 
 TEST_F(PhoneHubTrayTest, HideTrayItemOnUiStateChange) {