[CrOS PhoneHub] Add NotificationManager class

This class exposes the notifications which have been synced from the
phone and provides APIs to dismiss notifications and send inline
replies. The functionality to add notifications to this class and to
implement dismissal/reply are not yet implemented.

This CL also simplifies some logic in FakePhoneHubManager to avoid
unnecessary unique_ptrs.

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

chromeos/components/phonehub/BUILD.gn

@@ -26,6 +26,10 @@ static_library("phonehub") {
     "notification_access_manager_impl.h",
     "notification_access_setup_operation.cc",
     "notification_access_setup_operation.h",
+    "notification_manager.cc",
+    "notification_manager.h",
+    "notification_manager_impl.cc",
+    "notification_manager_impl.h",
     "phone_hub_manager.h",
     "phone_hub_manager_impl.cc",
     "phone_hub_manager_impl.h",
@@ -63,6 +67,8 @@ static_library("test_support") {
     "fake_feature_status_provider.h",
     "fake_notification_access_manager.cc",
     "fake_notification_access_manager.h",
+    "fake_notification_manager.cc",
+    "fake_notification_manager.h",
     "fake_phone_hub_manager.cc",
     "fake_phone_hub_manager.h",
     "fake_tether_controller.cc",
@@ -84,6 +90,7 @@ source_set("unit_tests") {
     "feature_status_provider_impl_unittest.cc",
     "mutable_phone_model_unittest.cc",
     "notification_access_manager_impl_unittest.cc",
+    "notification_manager_impl_unittest.cc",
     "phone_status_model_unittest.cc",
     "tether_controller_impl_unittest.cc",
   ]

chromeos/components/phonehub/fake_notification_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_manager.h"
+
+#include "base/check.h"
+
+namespace chromeos {
+namespace phonehub {
+
+FakeNotificationManager::InlineReplyMetadata::InlineReplyMetadata(
+    int64_t notification_id,
+    const base::string16& inline_reply_text)
+    : notification_id(notification_id), inline_reply_text(inline_reply_text) {}
+
+FakeNotificationManager::InlineReplyMetadata::~InlineReplyMetadata() = default;
+
+FakeNotificationManager::FakeNotificationManager() = default;
+
+FakeNotificationManager::~FakeNotificationManager() = default;
+
+void FakeNotificationManager::SetNotification(
+    const Notification& notification) {
+  SetNotifications(base::flat_set<Notification>{notification});
+}
+
+void FakeNotificationManager::SetNotifications(
+    const base::flat_set<Notification>& notifications) {
+  base::flat_set<int64_t> added_ids;
+  base::flat_set<int64_t> updated_ids;
+
+  for (const Notification& notification : notifications) {
+    int64_t id = notification.id();
+    auto it = id_to_notification_map_.find(id);
+
+    if (it == id_to_notification_map_.end()) {
+      id_to_notification_map_.emplace(id, notification);
+      added_ids.emplace(id);
+      continue;
+    }
+
+    it->second = notification;
+    updated_ids.emplace(id);
+  }
+
+  NotifyNotificationsAdded(added_ids);
+  NotifyNotificationsUpdated(updated_ids);
+}
+
+void FakeNotificationManager::RemoveNotification(int64_t id) {
+  RemoveNotifications(base::flat_set<int64_t>{id});
+}
+
+void FakeNotificationManager::RemoveNotifications(
+    const base::flat_set<int64_t>& ids) {
+  for (int64_t id : ids) {
+    auto it = id_to_notification_map_.find(id);
+    DCHECK(it != id_to_notification_map_.end());
+    id_to_notification_map_.erase(it);
+  }
+
+  NotifyNotificationsRemoved(ids);
+}
+
+const Notification* FakeNotificationManager::GetNotification(
+    int64_t notification_id) const {
+  auto it = id_to_notification_map_.find(notification_id);
+  if (it == id_to_notification_map_.end())
+    return nullptr;
+  return &it->second;
+}
+
+void FakeNotificationManager::DismissNotification(int64_t notification_id) {
+  DCHECK(base::Contains(id_to_notification_map_, notification_id));
+  dismissed_notification_ids_.push_back(notification_id);
+  NotifyNotificationsRemoved(base::flat_set<int64_t>{notification_id});
+}
+
+void FakeNotificationManager::SendInlineReply(
+    int64_t notification_id,
+    const base::string16& inline_reply_text) {
+  DCHECK(base::Contains(id_to_notification_map_, notification_id));
+  inline_replies_.emplace_back(notification_id, inline_reply_text);
+}
+
+}  // namespace phonehub
+}  // namespace chromeos

chromeos/components/phonehub/fake_notification_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_MANAGER_H_
+#define CHROMEOS_COMPONENTS_PHONEHUB_FAKE_NOTIFICATION_MANAGER_H_
+
+#include <unordered_map>
+#include <vector>
+
+#include "base/containers/flat_set.h"
+#include "chromeos/components/phonehub/notification.h"
+#include "chromeos/components/phonehub/notification_manager.h"
+
+namespace chromeos {
+namespace phonehub {
+
+class FakeNotificationManager : public NotificationManager {
+ public:
+  FakeNotificationManager();
+  ~FakeNotificationManager() override;
+
+  void SetNotification(const Notification& notification);
+  void SetNotifications(const base::flat_set<Notification>& notifications);
+
+  void RemoveNotification(int64_t id);
+  void RemoveNotifications(const base::flat_set<int64_t>& ids);
+
+  const std::vector<int64_t>& dismissed_notification_ids() const {
+    return dismissed_notification_ids_;
+  }
+
+  struct InlineReplyMetadata {
+    InlineReplyMetadata(int64_t notification_id,
+                        const base::string16& inline_reply_text);
+    ~InlineReplyMetadata();
+
+    int64_t notification_id;
+    base::string16 inline_reply_text;
+  };
+
+  const std::vector<InlineReplyMetadata>& inline_replies() const {
+    return inline_replies_;
+  }
+
+ private:
+  // NotificationManager:
+  const Notification* GetNotification(int64_t notification_id) const override;
+  void DismissNotification(int64_t notification_id) override;
+  void SendInlineReply(int64_t notification_id,
+                       const base::string16& inline_reply_text) override;
+
+  std::unordered_map<int64_t, Notification> id_to_notification_map_;
+  std::vector<int64_t> dismissed_notification_ids_;
+  std::vector<InlineReplyMetadata> inline_replies_;
+};
+
+}  // namespace phonehub
+}  // namespace chromeos
+
+#endif  // CHROMEOS_COMPONENTS_PHONEHUB_FAKE_NOTIFICATION_MANAGER_H_

chromeos/components/phonehub/fake_phone_hub_manager.cc

@@ -4,38 +4,31 @@
 
 #include "chromeos/components/phonehub/fake_phone_hub_manager.h"
 
-#include "chromeos/components/phonehub/fake_feature_status_provider.h"
-#include "chromeos/components/phonehub/fake_notification_access_manager.h"
-#include "chromeos/components/phonehub/fake_tether_controller.h"
-#include "chromeos/components/phonehub/mutable_phone_model.h"
-
 namespace chromeos {
 namespace phonehub {
 
-FakePhoneHubManager::FakePhoneHubManager()
-    : fake_feature_status_provider_(
-          std::make_unique<FakeFeatureStatusProvider>()),
-      fake_notification_access_manager_(
-          std::make_unique<FakeNotificationAccessManager>()),
-      mutable_phone_model_(std::make_unique<MutablePhoneModel>()),
-      fake_tether_controller_(std::make_unique<FakeTetherController>()) {}
+FakePhoneHubManager::FakePhoneHubManager() = default;
 
 FakePhoneHubManager::~FakePhoneHubManager() = default;
 
 FeatureStatusProvider* FakePhoneHubManager::GetFeatureStatusProvider() {
-  return fake_feature_status_provider_.get();
+  return &fake_feature_status_provider_;
 }
 
 NotificationAccessManager* FakePhoneHubManager::GetNotificationAccessManager() {
-  return fake_notification_access_manager_.get();
+  return &fake_notification_access_manager_;
+}
+
+NotificationManager* FakePhoneHubManager::GetNotificationManager() {
+  return &fake_notification_manager_;
 }
 
 PhoneModel* FakePhoneHubManager::GetPhoneModel() {
-  return mutable_phone_model_.get();
+  return &mutable_phone_model_;
 }
 
 TetherController* FakePhoneHubManager::GetTetherController() {
-  return fake_tether_controller_.get();
+  return &fake_tether_controller_;
 }
 
 }  // namespace phonehub

chromeos/components/phonehub/fake_phone_hub_manager.h

@@ -7,17 +7,16 @@
 
 #include <memory>
 
+#include "chromeos/components/phonehub/fake_feature_status_provider.h"
+#include "chromeos/components/phonehub/fake_notification_access_manager.h"
+#include "chromeos/components/phonehub/fake_notification_manager.h"
+#include "chromeos/components/phonehub/fake_tether_controller.h"
+#include "chromeos/components/phonehub/mutable_phone_model.h"
 #include "chromeos/components/phonehub/phone_hub_manager.h"
 
 namespace chromeos {
-
 namespace phonehub {
 
-class FakeFeatureStatusProvider;
-class FakeNotificationAccessManager;
-class MutablePhoneModel;
-class FakeTetherController;
-
 // This class initializes fake versions of the core business logic of Phone Hub.
 class FakePhoneHubManager : public PhoneHubManager {
  public:
@@ -25,33 +24,36 @@ class FakePhoneHubManager : public PhoneHubManager {
   ~FakePhoneHubManager() override;
 
   FakeFeatureStatusProvider* fake_feature_status_provider() {
-    return fake_feature_status_provider_.get();
+    return &fake_feature_status_provider_;
   }
 
   FakeNotificationAccessManager* fake_notification_access_manager() {
-    return fake_notification_access_manager_.get();
+    return &fake_notification_access_manager_;
   }
 
-  MutablePhoneModel* mutable_phone_model() {
-    return mutable_phone_model_.get();
+  FakeNotificationManager* fake_notification_manager() {
+    return &fake_notification_manager_;
   }
 
+  MutablePhoneModel* mutable_phone_model() { return &mutable_phone_model_; }
+
   FakeTetherController* fake_tether_controller() {
-    return fake_tether_controller_.get();
+    return &fake_tether_controller_;
   }
 
  private:
   // PhoneHubManager:
   FeatureStatusProvider* GetFeatureStatusProvider() override;
   NotificationAccessManager* GetNotificationAccessManager() override;
+  NotificationManager* GetNotificationManager() override;
   PhoneModel* GetPhoneModel() override;
   TetherController* GetTetherController() override;
 
-  std::unique_ptr<FakeFeatureStatusProvider> fake_feature_status_provider_;
-  std::unique_ptr<FakeNotificationAccessManager>
-      fake_notification_access_manager_;
-  std::unique_ptr<MutablePhoneModel> mutable_phone_model_;
-  std::unique_ptr<FakeTetherController> fake_tether_controller_;
+  FakeFeatureStatusProvider fake_feature_status_provider_;
+  FakeNotificationAccessManager fake_notification_access_manager_;
+  FakeNotificationManager fake_notification_manager_;
+  MutablePhoneModel mutable_phone_model_;
+  FakeTetherController fake_tether_controller_;
 };
 
 }  // namespace phonehub

chromeos/components/phonehub/notification_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_manager.h"
+
+namespace chromeos {
+namespace phonehub {
+
+NotificationManager::NotificationManager() = default;
+
+NotificationManager::~NotificationManager() = default;
+
+void NotificationManager::AddObserver(Observer* observer) {
+  observer_list_.AddObserver(observer);
+}
+
+void NotificationManager::RemoveObserver(Observer* observer) {
+  observer_list_.RemoveObserver(observer);
+}
+
+void NotificationManager::NotifyNotificationsAdded(
+    const base::flat_set<int64_t>& notification_ids) {
+  for (auto& observer : observer_list_)
+    observer.OnNotificationsAdded(notification_ids);
+}
+
+void NotificationManager::NotifyNotificationsUpdated(
+    const base::flat_set<int64_t>& notification_ids) {
+  for (auto& observer : observer_list_)
+    observer.OnNotificationsUpdated(notification_ids);
+}
+
+void NotificationManager::NotifyNotificationsRemoved(
+    const base::flat_set<int64_t>& notification_ids) {
+  for (auto& observer : observer_list_)
+    observer.OnNotificationsRemoved(notification_ids);
+}
+
+}  // namespace phonehub
+}  // namespace chromeos

chromeos/components/phonehub/notification_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_MANAGER_H_
+#define CHROMEOS_COMPONENTS_PHONEHUB_NOTIFICATION_MANAGER_H_
+
+#include <stdint.h>
+
+#include "base/containers/flat_set.h"
+#include "base/observer_list.h"
+#include "base/observer_list_types.h"
+
+namespace chromeos {
+namespace phonehub {
+
+class Notification;
+
+// Tracks notifications which have been synced from a connected phone during a
+// Phone Hub session. Clients can access notifications via GetNotification() and
+// can be notified when the state of notifications changes by registering as
+// observers.
+//
+// This class also provides functionality for interacting with notifications;
+// namely, clients can dismiss notifications or send inline replies.
+class NotificationManager {
+ public:
+  class Observer : public base::CheckedObserver {
+   public:
+    ~Observer() override = default;
+
+    virtual void OnNotificationsAdded(
+        const base::flat_set<int64_t>& notification_ids) {}
+    virtual void OnNotificationsUpdated(
+        const base::flat_set<int64_t>& notification_ids) {}
+    virtual void OnNotificationsRemoved(
+        const base::flat_set<int64_t>& notification_ids) {}
+  };
+
+  NotificationManager(const NotificationManager&) = delete;
+  NotificationManager& operator=(const NotificationManager&) = delete;
+  virtual ~NotificationManager();
+
+  // Returns null if no notification exists with the given ID. Pointers returned
+  // by this function should not be cached, since the underlying Notification
+  // object may be deleted by a future update.
+  virtual const Notification* GetNotification(
+      int64_t notification_id) const = 0;
+
+  // Dismisses the notification with the given ID; if no notification exists
+  // with this ID, this function is a no-op.
+  virtual void DismissNotification(int64_t notification_id) = 0;
+
+  // Sends an inline reply for the notificaiton with the given ID; if no
+  // notification exists with this ID, this function is a no-op.
+  virtual void SendInlineReply(int64_t notification_id,
+                               const base::string16& inline_reply_text) = 0;
+
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
+ protected:
+  NotificationManager();
+
+  void NotifyNotificationsAdded(
+      const base::flat_set<int64_t>& notification_ids);
+  void NotifyNotificationsUpdated(
+      const base::flat_set<int64_t>& notification_ids);
+  void NotifyNotificationsRemoved(
+      const base::flat_set<int64_t>& notification_ids);
+
+ private:
+  base::ObserverList<Observer> observer_list_;
+};
+
+}  // namespace phonehub
+}  // namespace chromeos
+
+#endif  // CHROMEOS_COMPONENTS_PHONEHUB_NOTIFICATION_MANAGER_H_

chromeos/components/phonehub/notification_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_manager_impl.h"
+
+#include "chromeos/components/multidevice/logging/logging.h"
+
+namespace chromeos {
+namespace phonehub {
+
+NotificationManagerImpl::NotificationManagerImpl() = default;
+
+NotificationManagerImpl::~NotificationManagerImpl() = default;
+
+const Notification* NotificationManagerImpl::GetNotification(
+    int64_t notification_id) const {
+  return nullptr;
+}
+
+void NotificationManagerImpl::DismissNotification(int64_t notification_id) {
+  PA_LOG(INFO) << "Dismissing notification with ID " << notification_id << ".";
+}
+
+void NotificationManagerImpl::SendInlineReply(
+    int64_t notification_id,
+    const base::string16& inline_reply_text) {
+  PA_LOG(INFO) << "Sending inline reply for notification with ID "
+               << notification_id << ".";
+}
+
+}  // namespace phonehub
+}  // namespace chromeos

chromeos/components/phonehub/notification_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_MANAGER_IMPL_H_
+#define CHROMEOS_COMPONENTS_PHONEHUB_NOTIFICATION_MANAGER_IMPL_H_
+
+#include "chromeos/components/phonehub/notification_manager.h"
+
+namespace chromeos {
+namespace phonehub {
+
+// TODO(https://crbug.com/1106937): Add real implementation.
+class NotificationManagerImpl : public NotificationManager {
+ public:
+  NotificationManagerImpl();
+  ~NotificationManagerImpl() override;
+
+ private:
+  // NotificationManager:
+  const Notification* GetNotification(int64_t notification_id) const override;
+  void DismissNotification(int64_t notification_id) override;
+  void SendInlineReply(int64_t notification_id,
+                       const base::string16& inline_reply_text) override;
+};
+
+}  // namespace phonehub
+}  // namespace chromeos
+
+#endif  // CHROMEOS_COMPONENTS_PHONEHUB_NOTIFICATION_MANAGER_IMPL_H_

chromeos/components/phonehub/notification_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_manager_impl.h"
+
+#include <memory>
+
+#include "base/containers/flat_map.h"
+#include "base/optional.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chromeos {
+namespace phonehub {
+namespace {
+
+enum class NotificationState { kAdded, kUpdated, kRemoved };
+
+class FakeObserver : public NotificationManager::Observer {
+ public:
+  FakeObserver() = default;
+  ~FakeObserver() override = default;
+
+  base::Optional<NotificationState> GetState(int64_t notification_id) const {
+    const auto it = id_to_state_map_.find(notification_id);
+    if (it == id_to_state_map_.end())
+      return base::nullopt;
+    return it->second;
+  }
+
+ private:
+  // NotificationManager::Observer:
+  void OnNotificationsAdded(
+      const base::flat_set<int64_t>& notification_ids) override {
+    for (int64_t id : notification_ids)
+      id_to_state_map_[id] = NotificationState::kAdded;
+  }
+
+  void OnNotificationsUpdated(
+      const base::flat_set<int64_t>& notification_ids) override {
+    for (int64_t id : notification_ids)
+      id_to_state_map_[id] = NotificationState::kUpdated;
+  }
+
+  void OnNotificationsRemoved(
+      const base::flat_set<int64_t>& notification_ids) override {
+    for (int64_t id : notification_ids)
+      id_to_state_map_[id] = NotificationState::kRemoved;
+  }
+
+  base::flat_map<int64_t, NotificationState> id_to_state_map_;
+};
+
+}  // namespace
+
+class NotificationManagerImplTest : public testing::Test {
+ protected:
+  NotificationManagerImplTest() = default;
+  NotificationManagerImplTest(const NotificationManagerImplTest&) = delete;
+  NotificationManagerImplTest& operator=(const NotificationManagerImplTest&) =
+      delete;
+  ~NotificationManagerImplTest() override = default;
+
+  // testing::Test:
+  void SetUp() override {
+    manager_ = std::make_unique<NotificationManagerImpl>();
+    manager_->AddObserver(&fake_observer_);
+  }
+
+  void TearDown() override { manager_->RemoveObserver(&fake_observer_); }
+
+  NotificationManager& manager() { return *manager_; }
+
+ private:
+  FakeObserver fake_observer_;
+  std::unique_ptr<NotificationManager> manager_;
+};
+
+// TODO(khorimoto): Remove this test once we have real functionality to test.
+TEST_F(NotificationManagerImplTest, Initialize) {
+  EXPECT_FALSE(manager().GetNotification(/*notification_id=*/0));
+}
+
+}  // namespace phonehub
+}  // namespace chromeos

chromeos/components/phonehub/phone_hub_manager.h

@@ -10,6 +10,7 @@ namespace phonehub {
 
 class FeatureStatusProvider;
 class NotificationAccessManager;
+class NotificationManager;
 class PhoneModel;
 class TetherController;
 
@@ -22,12 +23,11 @@ class PhoneHubManager {
   PhoneHubManager(const PhoneHubManager&) = delete;
   PhoneHubManager& operator=(const PhoneHubManager&) = delete;
 
+  // Getters for sub-elements.
   virtual FeatureStatusProvider* GetFeatureStatusProvider() = 0;
-
   virtual NotificationAccessManager* GetNotificationAccessManager() = 0;
-
+  virtual NotificationManager* GetNotificationManager() = 0;
   virtual PhoneModel* GetPhoneModel() = 0;
-
   virtual TetherController* GetTetherController() = 0;
 
  protected:

chromeos/components/phonehub/phone_hub_manager_impl.cc

@@ -7,6 +7,7 @@
 #include "chromeos/components/phonehub/feature_status_provider_impl.h"
 #include "chromeos/components/phonehub/mutable_phone_model.h"
 #include "chromeos/components/phonehub/notification_access_manager_impl.h"
+#include "chromeos/components/phonehub/notification_manager_impl.h"
 #include "chromeos/components/phonehub/tether_controller_impl.h"
 
 namespace chromeos {
@@ -21,6 +22,7 @@ PhoneHubManagerImpl::PhoneHubManagerImpl(
           multidevice_setup_client)),
       notification_access_manager_(
           std::make_unique<NotificationAccessManagerImpl>(pref_service)),
+      notification_manager_(std::make_unique<NotificationManagerImpl>()),
       phone_model_(std::make_unique<MutablePhoneModel>()),
       tether_controller_(
           std::make_unique<TetherControllerImpl>(multidevice_setup_client)) {}
@@ -35,6 +37,10 @@ NotificationAccessManager* PhoneHubManagerImpl::GetNotificationAccessManager() {
   return notification_access_manager_.get();
 }
 
+NotificationManager* PhoneHubManagerImpl::GetNotificationManager() {
+  return notification_manager_.get();
+}
+
 PhoneModel* PhoneHubManagerImpl::GetPhoneModel() {
   return phone_model_.get();
 }

chromeos/components/phonehub/phone_hub_manager_impl.h

@@ -36,6 +36,7 @@ class PhoneHubManagerImpl : public PhoneHubManager, public KeyedService {
   // PhoneHubManager:
   FeatureStatusProvider* GetFeatureStatusProvider() override;
   NotificationAccessManager* GetNotificationAccessManager() override;
+  NotificationManager* GetNotificationManager() override;
   PhoneModel* GetPhoneModel() override;
   TetherController* GetTetherController() override;
 
@@ -45,6 +46,7 @@ class PhoneHubManagerImpl : public PhoneHubManager, public KeyedService {
 
   std::unique_ptr<FeatureStatusProvider> feature_status_provider_;
   std::unique_ptr<NotificationAccessManager> notification_access_manager_;
+  std::unique_ptr<NotificationManager> notification_manager_;
   std::unique_ptr<PhoneModel> phone_model_;
   std::unique_ptr<TetherController> tether_controller_;
 };