[CrOS PhoneHub] Add NotificationAccessManager

This class tracks whether notification access has been granted by the
user on their phone. Because notification access is a sensitive
permission, this requires the user completing a special setup flow and
is not included with the rest of Phone Hub opt-in.

Note that currently, this class defaults to "false" (i.e., has not yet
granted access). A future CL will set this value based on whether the
phone has sent a message indicating that access has been granted.

Bug: 1106937
Change-Id: Ice4968ef684b96f090ee566013aa39dc8224e4f4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2353709
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Reviewed-by: Josh Nohle <nohle@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797757}

chrome/browser/chromeos/phonehub/phone_hub_manager_factory.cc

@@ -8,10 +8,12 @@
 #include "chrome/browser/chromeos/multidevice_setup/multidevice_setup_client_factory.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chromeos/components/phonehub/notification_access_manager_impl.h"
 #include "chromeos/components/phonehub/phone_hub_manager.h"
 #include "chromeos/constants/chromeos_features.h"
 #include "chromeos/services/multidevice_setup/public/cpp/prefs.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
+#include "components/pref_registry/pref_registry_syncable.h"
 
 namespace chromeos {
 namespace phonehub {
@@ -73,6 +75,7 @@ KeyedService* PhoneHubManagerFactory::BuildServiceInstanceFor(
     return nullptr;
 
   return new PhoneHubManager(
+      profile->GetPrefs(),
       device_sync::DeviceSyncClientFactory::GetForProfile(profile),
       multidevice_setup::MultiDeviceSetupClientFactory::GetForProfile(profile));
 }
@@ -81,5 +84,10 @@ bool PhoneHubManagerFactory::ServiceIsCreatedWithBrowserContext() const {
   return true;
 }
 
+void PhoneHubManagerFactory::RegisterProfilePrefs(
+    user_prefs::PrefRegistrySyncable* registry) {
+  NotificationAccessManagerImpl::RegisterPrefs(registry);
+}
+
 }  // namespace phonehub
 }  // namespace chromeos

chrome/browser/chromeos/phonehub/phone_hub_manager_factory.h

@@ -36,6 +36,8 @@ class PhoneHubManagerFactory : public BrowserContextKeyedServiceFactory {
   KeyedService* BuildServiceInstanceFor(
       content::BrowserContext* context) const override;
   bool ServiceIsCreatedWithBrowserContext() const override;
+  void RegisterProfilePrefs(
+      user_prefs::PrefRegistrySyncable* registry) override;
 };
 
 }  // namespace phonehub

chromeos/components/phonehub/BUILD.gn

@@ -14,8 +14,14 @@ static_library("phonehub") {
     "feature_status_provider.h",
     "feature_status_provider_impl.cc",
     "feature_status_provider_impl.h",
+    "notification_access_manager.cc",
+    "notification_access_manager.h",
+    "notification_access_manager_impl.cc",
+    "notification_access_manager_impl.h",
     "phone_hub_manager.cc",
     "phone_hub_manager.h",
+    "pref_names.cc",
+    "pref_names.h",
   ]
 
   deps = [
@@ -25,6 +31,7 @@ static_library("phonehub") {
     "//chromeos/services/device_sync/public/cpp",
     "//chromeos/services/multidevice_setup/public/cpp",
     "//components/keyed_service/core",
+    "//components/prefs",
     "//device/bluetooth",
   ]
 }
@@ -35,6 +42,8 @@ static_library("test_support") {
   sources = [
     "fake_feature_status_provider.cc",
     "fake_feature_status_provider.h",
+    "fake_notification_access_manager.cc",
+    "fake_notification_access_manager.h",
   ]
 
   public_deps = [ ":phonehub" ]
@@ -45,7 +54,10 @@ static_library("test_support") {
 source_set("unit_tests") {
   testonly = true
 
-  sources = [ "feature_status_provider_impl_unittest.cc" ]
+  sources = [
+    "feature_status_provider_impl_unittest.cc",
+    "notification_access_manager_impl_unittest.cc",
+  ]
 
   deps = [
     ":phonehub",
@@ -58,6 +70,7 @@ source_set("unit_tests") {
     "//chromeos/services/device_sync/public/cpp:test_support",
     "//chromeos/services/multidevice_setup/public/cpp",
     "//chromeos/services/multidevice_setup/public/cpp:test_support",
+    "//components/prefs:test_support",
     "//device/bluetooth:mocks",
     "//testing/gtest",
   ]

chromeos/components/phonehub/DEPS

@@ -3,5 +3,6 @@ include_rules = [
   "+chromeos/services/device_sync/public/cpp",
   "+chromeos/services/multidevice_setup/public/cpp",
   "+components/keyed_service/core/keyed_service.h",
+  "+components/prefs",
   "+device/bluetooth",
 ]

chromeos/components/phonehub/fake_notification_access_manager.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chromeos/components/phonehub/fake_notification_access_manager.h"
+
+namespace chromeos {
+namespace phonehub {
+
+FakeNotificationAccessManager::FakeNotificationAccessManager(
+    bool has_access_been_granted)
+    : has_access_been_granted_(has_access_been_granted) {}
+
+FakeNotificationAccessManager::~FakeNotificationAccessManager() = default;
+
+void FakeNotificationAccessManager::SetHasAccessBeenGranted(
+    bool has_access_been_granted) {
+  if (has_access_been_granted_ == has_access_been_granted)
+    return;
+
+  has_access_been_granted_ = has_access_been_granted;
+  NotifyNotificationAccessChanged();
+}
+
+bool FakeNotificationAccessManager::HasAccessBeenGranted() const {
+  return has_access_been_granted_;
+}
+
+}  // namespace phonehub
+}  // namespace chromeos

chromeos/components/phonehub/fake_notification_access_manager.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMEOS_COMPONENTS_PHONEHUB_FAKE_NOTIFICATION_ACCESS_MANAGER_H_
+#define CHROMEOS_COMPONENTS_PHONEHUB_FAKE_NOTIFICATION_ACCESS_MANAGER_H_
+
+#include "chromeos/components/phonehub/notification_access_manager.h"
+
+namespace chromeos {
+namespace phonehub {
+
+class FakeNotificationAccessManager : public NotificationAccessManager {
+ public:
+  explicit FakeNotificationAccessManager(bool has_access_been_granted = false);
+  ~FakeNotificationAccessManager() override;
+
+  void SetHasAccessBeenGranted(bool has_access_been_granted);
+
+  // NotificationAccessManager:
+  bool HasAccessBeenGranted() const override;
+
+ private:
+  bool has_access_been_granted_;
+};
+
+}  // namespace phonehub
+}  // namespace chromeos
+
+#endif  // CHROMEOS_COMPONENTS_PHONEHUB_FAKE_NOTIFICATION_ACCESS_MANAGER_H_

chromeos/components/phonehub/notification_access_manager.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chromeos/components/phonehub/notification_access_manager.h"
+
+namespace chromeos {
+namespace phonehub {
+
+NotificationAccessManager::NotificationAccessManager() = default;
+
+NotificationAccessManager::~NotificationAccessManager() = default;
+
+void NotificationAccessManager::AddObserver(Observer* observer) {
+  observer_list_.AddObserver(observer);
+}
+
+void NotificationAccessManager::RemoveObserver(Observer* observer) {
+  observer_list_.RemoveObserver(observer);
+}
+
+void NotificationAccessManager::NotifyNotificationAccessChanged() {
+  for (auto& observer : observer_list_)
+    observer.OnNotificationAccessChanged();
+}
+
+}  // namespace phonehub
+}  // namespace chromeos

chromeos/components/phonehub/notification_access_manager.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMEOS_COMPONENTS_PHONEHUB_NOTIFICATION_ACCESS_MANAGER_H_
+#define CHROMEOS_COMPONENTS_PHONEHUB_NOTIFICATION_ACCESS_MANAGER_H_
+
+#include "base/observer_list.h"
+#include "base/observer_list_types.h"
+
+namespace chromeos {
+namespace phonehub {
+
+// Tracks the status of whether the user has enabled notification access on
+// their phone. While Phone Hub can be enabled via Chrome OS, access to
+// notifications requires that the user grant access via Android settings. If a
+// Phone Hub connection to the phone has never succeeded, we assume that access
+// has not yet been granted. If there is no active Phone Hub connection, we
+// assume that the last access value seen is the current value.
+class NotificationAccessManager {
+ public:
+  class Observer : public base::CheckedObserver {
+   public:
+    ~Observer() override = default;
+
+    // Called when notification access has changed; use HasAccessBeenGranted()
+    // for the new status.
+    virtual void OnNotificationAccessChanged() = 0;
+  };
+
+  NotificationAccessManager(const NotificationAccessManager&) = delete;
+  NotificationAccessManager& operator=(const NotificationAccessManager&) =
+      delete;
+  virtual ~NotificationAccessManager();
+
+  virtual bool HasAccessBeenGranted() const = 0;
+
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
+ protected:
+  NotificationAccessManager();
+
+  void NotifyNotificationAccessChanged();
+
+ private:
+  base::ObserverList<Observer> observer_list_;
+};
+
+}  // namespace phonehub
+}  // namespace chromeos
+
+#endif  // CHROMEOS_COMPONENTS_PHONEHUB_NOTIFICATION_ACCESS_MANAGER_H_

chromeos/components/phonehub/notification_access_manager_impl.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chromeos/components/phonehub/notification_access_manager_impl.h"
+
+#include "chromeos/components/phonehub/pref_names.h"
+#include "components/prefs/pref_registry_simple.h"
+#include "components/prefs/pref_service.h"
+
+namespace chromeos {
+namespace phonehub {
+
+// static
+void NotificationAccessManagerImpl::RegisterPrefs(
+    PrefRegistrySimple* registry) {
+  registry->RegisterBooleanPref(prefs::kNotificationAccessGranted, false);
+}
+
+NotificationAccessManagerImpl::NotificationAccessManagerImpl(
+    PrefService* pref_service)
+    : pref_service_(pref_service) {}
+
+NotificationAccessManagerImpl::~NotificationAccessManagerImpl() = default;
+
+bool NotificationAccessManagerImpl::HasAccessBeenGranted() const {
+  return pref_service_->GetBoolean(prefs::kNotificationAccessGranted);
+}
+
+}  // namespace phonehub
+}  // namespace chromeos

chromeos/components/phonehub/notification_access_manager_impl.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMEOS_COMPONENTS_PHONEHUB_NOTIFICATION_ACCESS_MANAGER_IMPL_H_
+#define CHROMEOS_COMPONENTS_PHONEHUB_NOTIFICATION_ACCESS_MANAGER_IMPL_H_
+
+#include "chromeos/components/phonehub/notification_access_manager.h"
+
+class PrefRegistrySimple;
+class PrefService;
+
+namespace chromeos {
+namespace phonehub {
+
+// Implements NotificationAccessManager by persisting the last-known
+// notification access value to user prefs.
+// TODO(khorimoto): Currently HasAccessBeenGranted() always returns false. Have
+// it return true once the phone has sent a message indicating that it has
+// granted access.
+class NotificationAccessManagerImpl : public NotificationAccessManager {
+ public:
+  static void RegisterPrefs(PrefRegistrySimple* registry);
+
+  explicit NotificationAccessManagerImpl(PrefService* pref_service);
+  ~NotificationAccessManagerImpl() override;
+
+ private:
+  // NotificationAccessManager:
+  bool HasAccessBeenGranted() const override;
+
+  PrefService* pref_service_;
+};
+
+}  // namespace phonehub
+}  // namespace chromeos
+
+#endif  // CHROMEOS_COMPONENTS_PHONEHUB_NOTIFICATION_ACCESS_MANAGER_IMPL_H_

chromeos/components/phonehub/notification_access_manager_impl_unittest.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chromeos/components/phonehub/notification_access_manager_impl.h"
+
+#include <memory>
+
+#include "chromeos/components/phonehub/pref_names.h"
+#include "components/prefs/testing_pref_service.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chromeos {
+namespace phonehub {
+namespace {
+
+class FakeObserver : public NotificationAccessManager::Observer {
+ public:
+  FakeObserver() = default;
+  ~FakeObserver() override = default;
+
+  size_t num_calls() const { return num_calls_; }
+
+  // NotificationAccessManager::Observer:
+  void OnNotificationAccessChanged() override { ++num_calls_; }
+
+ private:
+  size_t num_calls_ = 0;
+};
+
+}  // namespace
+
+class NotificationAccessManagerImplTest : public testing::Test {
+ protected:
+  NotificationAccessManagerImplTest() = default;
+  NotificationAccessManagerImplTest(const NotificationAccessManagerImplTest&) =
+      delete;
+  NotificationAccessManagerImplTest& operator=(
+      const NotificationAccessManagerImplTest&) = delete;
+  ~NotificationAccessManagerImplTest() override = default;
+
+  // testing::Test:
+  void SetUp() override {
+    NotificationAccessManagerImpl::RegisterPrefs(pref_service_.registry());
+  }
+
+  void Initialize(bool initial_has_access_been_granted) {
+    pref_service_.SetBoolean(prefs::kNotificationAccessGranted,
+                             initial_has_access_been_granted);
+    manager_ = std::make_unique<NotificationAccessManagerImpl>(&pref_service_);
+  }
+
+  bool GetHasAccessBeenGranted() { return manager_->HasAccessBeenGranted(); }
+
+  size_t GetNumObserverCalls() const { return fake_observer_.num_calls(); }
+
+ private:
+  TestingPrefServiceSimple pref_service_;
+
+  FakeObserver fake_observer_;
+  std::unique_ptr<NotificationAccessManager> manager_;
+};
+
+TEST_F(NotificationAccessManagerImplTest, InitiallyGranted) {
+  Initialize(/*initial_has_access_been_granted=*/true);
+  EXPECT_TRUE(GetHasAccessBeenGranted());
+}
+
+TEST_F(NotificationAccessManagerImplTest, InitiallyNotGranted) {
+  Initialize(/*initial_has_access_been_granted=*/false);
+  EXPECT_FALSE(GetHasAccessBeenGranted());
+}
+
+}  // namespace phonehub
+}  // namespace chromeos

chromeos/components/phonehub/phone_hub_manager.cc

@@ -7,6 +7,7 @@
 #include "base/callback.h"
 #include "base/no_destructor.h"
 #include "chromeos/components/phonehub/feature_status_provider_impl.h"
+#include "chromeos/components/phonehub/notification_access_manager_impl.h"
 
 namespace chromeos {
 namespace phonehub {
@@ -20,11 +21,14 @@ PhoneHubManager* PhoneHubManager::Get() {
 }
 
 PhoneHubManager::PhoneHubManager(
+    PrefService* pref_service,
     device_sync::DeviceSyncClient* device_sync_client,
     multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client)
     : feature_status_provider_(std::make_unique<FeatureStatusProviderImpl>(
           device_sync_client,
-          multidevice_setup_client)) {
+          multidevice_setup_client)),
+      notification_access_manager_(
+          std::make_unique<NotificationAccessManagerImpl>(pref_service)) {
   DCHECK(!g_instance);
   g_instance = this;
 }
@@ -35,6 +39,7 @@ void PhoneHubManager::Shutdown() {
   DCHECK(g_instance);
   g_instance = nullptr;
 
+  notification_access_manager_.reset();
   feature_status_provider_.reset();
 }
 

chromeos/components/phonehub/phone_hub_manager.h

@@ -10,6 +10,8 @@
 #include "base/callback_forward.h"
 #include "components/keyed_service/core/keyed_service.h"
 
+class PrefService;
+
 namespace chromeos {
 
 namespace device_sync {
@@ -23,6 +25,7 @@ class MultiDeviceSetupClient;
 namespace phonehub {
 
 class FeatureStatusProvider;
+class NotificationAccessManager;
 
 // Implements the core logic of the Phone Hub feature and exposes interfaces via
 // its public API. Implemented as a KeyedService which is keyed by the primary
@@ -37,6 +40,7 @@ class PhoneHubManager : public KeyedService {
   static PhoneHubManager* Get();
 
   PhoneHubManager(
+      PrefService* pref_service,
       device_sync::DeviceSyncClient* device_sync_client,
       multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client);
   PhoneHubManager(const PhoneHubManager&) = delete;
@@ -47,11 +51,16 @@ class PhoneHubManager : public KeyedService {
     return feature_status_provider_.get();
   }
 
+  NotificationAccessManager* notification_access_manager() {
+    return notification_access_manager_.get();
+  }
+
  private:
   // KeyedService:
   void Shutdown() override;
 
   std::unique_ptr<FeatureStatusProvider> feature_status_provider_;
+  std::unique_ptr<NotificationAccessManager> notification_access_manager_;
 };
 
 }  // namespace phonehub

chromeos/components/phonehub/pref_names.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chromeos/components/phonehub/pref_names.h"
+
+namespace chromeos {
+namespace phonehub {
+namespace prefs {
+
+// Whether notification access had been granted by the user on their phone.
+const char kNotificationAccessGranted[] =
+    "cros.phonehub.notification_access_granted";
+
+}  // namespace prefs
+}  // namespace phonehub
+}  // namespace chromeos

chromeos/components/phonehub/pref_names.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMEOS_COMPONENTS_PHONEHUB_PREF_NAMES_H_
+#define CHROMEOS_COMPONENTS_PHONEHUB_PREF_NAMES_H_
+
+namespace chromeos {
+namespace phonehub {
+namespace prefs {
+
+extern const char kNotificationAccessGranted[];
+
+}  // namespace prefs
+}  // namespace phonehub
+}  // namespace chromeos
+
+#endif  // CHROMEOS_COMPONENTS_PHONEHUB_PREF_NAMES_H_