Migrate ArcService to BrowserContextKeyedService part 30.

This CL migrates ArcNotificationManager.

BUG=672829
TEST=Ran try. Ran on DUT.

Change-Id: I4b7283246b9d808fe61a97e20061a4b87c8c0321
Reviewed-on: https://chromium-review.googlesource.com/574890Reviewed-by: Paweł Hajdan Jr. <phajdan.jr@chromium.org>
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487526}

chrome/browser/chromeos/arc/arc_service_launcher.cc

@@ -113,6 +113,8 @@ void ArcServiceLauncher::MaybeSetProfile(Profile* profile) {
 
   arc_session_manager_->SetProfile(profile);
   arc_service_manager_->set_browser_context(profile);
+  arc_service_manager_->set_account_id(
+      multi_user_util::GetAccountIdFromProfile(profile));
 }
 
 void ArcServiceLauncher::OnPrimaryUserProfilePrepared(Profile* profile) {
@@ -146,6 +148,7 @@ void ArcServiceLauncher::OnPrimaryUserProfilePrepared(Profile* profile) {
   ArcIntentHelperBridge::GetForBrowserContext(profile);
   ArcMetricsService::GetForBrowserContext(profile);
   ArcNetHostImpl::GetForBrowserContext(profile);
+  ArcNotificationManager::GetForBrowserContext(profile);
   ArcObbMounterBridge::GetForBrowserContext(profile);
   ArcPolicyBridge::GetForBrowserContext(profile);
   ArcPowerBridge::GetForBrowserContext(profile);
@@ -165,9 +168,6 @@ void ArcServiceLauncher::OnPrimaryUserProfilePrepared(Profile* profile) {
   arc_service_manager_->AddService(
       base::MakeUnique<ArcFileSystemOperationRunner>(
           arc_service_manager_->arc_bridge_service(), profile));
-  arc_service_manager_->AddService(base::MakeUnique<ArcNotificationManager>(
-      arc_service_manager_->arc_bridge_service(),
-      multi_user_util::GetAccountIdFromProfile(profile)));
 
   // Kiosk apps should be run only for kiosk sessions.
   if (user_manager::UserManager::Get()->IsLoggedInAsArcKioskApp()) {

components/arc/BUILD.gn

@@ -122,6 +122,7 @@ static_library("arc_base") {
     "//base",
     "//chromeos",
     "//components/keyed_service/content",
+    "//components/signin/core/account_id",
     "//components/user_manager",
     "//mojo/edk/system",
   ]

components/arc/arc_service_manager.h

@@ -16,6 +16,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/threading/thread_checker.h"
 #include "components/arc/arc_service.h"
+#include "components/signin/core/account_id/account_id.h"
 
 namespace content {
 class BrowserContext;
@@ -78,6 +79,16 @@ class ArcServiceManager {
     browser_context_ = browser_context;
   }
 
+  // Returns the current AccountID which ARC is allowed.
+  // This is workaround to split the dependency from chrome/.
+  // TODO(hidehiko): Remove this when we move IsArcAllowedForProfile() to
+  // components/arc.
+  const AccountId& account_id() const { return account_id_; }
+
+  // TODO(hidehiko): Remove this when we move IsArcAllowedForProfile() to
+  // components/arc.
+  void set_account_id(const AccountId& account_id) { account_id_ = account_id; }
+
   // |arc_bridge_service| can only be accessed on the thread that this
   // class was created on.
   ArcBridgeService* arc_bridge_service();
@@ -137,6 +148,11 @@ class ArcServiceManager {
   // components/arc. See browser_context() for details.
   content::BrowserContext* browser_context_ = nullptr;
 
+  // This holds the AccountId corresponding to the |browser_context_|.
+  // TODO(hidehiko): Remove this when we move IsArcAllowedForProfile() to
+  // components/arc. See browser_context() for details.
+  AccountId account_id_;
+
   DISALLOW_COPY_AND_ASSIGN(ArcServiceManager);
 };
 

ui/arc/BUILD.gn

@@ -32,6 +32,7 @@ static_library("arc") {
     "//base",
     "//components/arc:arc_base",
     "//components/exo",
+    "//components/keyed_service/content:content",
     "//components/signin/core/account_id",
     "//mojo/common:common_base",
     "//skia",

ui/arc/notification/DEPS

 include_rules = [
   "+components/exo",
+  "+components/keyed_service",
   "+components/signin/core/account_id",
   "+third_party/skia",
   "+ui/accessibility",

ui/arc/notification/arc_notification_manager.cc

@@ -10,39 +10,85 @@
 #include "ash/shell.h"
 #include "ash/system/toast/toast_manager.h"
 #include "base/memory/ptr_util.h"
+#include "base/memory/singleton.h"
 #include "base/stl_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/arc/arc_bridge_service.h"
+#include "components/arc/arc_browser_context_keyed_service_factory_base.h"
 #include "ui/arc/notification/arc_notification_item_impl.h"
 
 namespace arc {
+namespace {
+
+// Singleton factory for ArcNotificationManager.
+class ArcNotificationManagerFactory
+    : public internal::ArcBrowserContextKeyedServiceFactoryBase<
+          ArcNotificationManager,
+          ArcNotificationManagerFactory> {
+ public:
+  // Factory name used by ArcBrowserContextKeyedServiceFactoryBase.
+  static constexpr const char* kName = "ArcNotificationManagerFactory";
+
+  static ArcNotificationManagerFactory* GetInstance() {
+    return base::Singleton<ArcNotificationManagerFactory>::get();
+  }
+
+ private:
+  friend base::DefaultSingletonTraits<ArcNotificationManagerFactory>;
+  ArcNotificationManagerFactory() = default;
+  ~ArcNotificationManagerFactory() override = default;
+};
+
+}  // namespace
+
+// static
+ArcNotificationManager* ArcNotificationManager::GetForBrowserContext(
+    content::BrowserContext* context) {
+  return ArcNotificationManagerFactory::GetForBrowserContext(context);
+}
+
+// static
+std::unique_ptr<ArcNotificationManager>
+ArcNotificationManager::CreateForTesting(
+    ArcBridgeService* bridge_service,
+    const AccountId& main_profile_id,
+    message_center::MessageCenter* message_center) {
+  // MakeUnique cannot be used because the used ctor is private.
+  return base::WrapUnique(new ArcNotificationManager(
+      bridge_service, main_profile_id, message_center));
+}
 
-ArcNotificationManager::ArcNotificationManager(ArcBridgeService* bridge_service,
-                                               const AccountId& main_profile_id)
+ArcNotificationManager::ArcNotificationManager(content::BrowserContext* context,
+                                               ArcBridgeService* bridge_service)
     : ArcNotificationManager(bridge_service,
-                             main_profile_id,
+                             ArcServiceManager::Get()->account_id(),
                              message_center::MessageCenter::Get()) {}
 
 ArcNotificationManager::ArcNotificationManager(
     ArcBridgeService* bridge_service,
     const AccountId& main_profile_id,
     message_center::MessageCenter* message_center)
-    : ArcService(bridge_service),
+    : arc_bridge_service_(bridge_service),
       main_profile_id_(main_profile_id),
       message_center_(message_center),
       binding_(this) {
-  arc_bridge_service()->notifications()->AddObserver(this);
+  arc_bridge_service_->notifications()->AddObserver(this);
 }
 
 ArcNotificationManager::~ArcNotificationManager() {
-  arc_bridge_service()->notifications()->RemoveObserver(this);
+  // TODO(hidehiko): Currently, the lifetime of ArcBridgeService and
+  // BrowserContextKeyedService is not nested.
+  // If ArcServiceManager::Get() returns nullptr, it is already destructed,
+  // so do not touch it.
+  if (ArcServiceManager::Get())
+    arc_bridge_service_->notifications()->RemoveObserver(this);
 }
 
 void ArcNotificationManager::OnInstanceReady() {
   DCHECK(!ready_);
 
   auto* notifications_instance =
-      ARC_GET_INSTANCE_FOR_METHOD(arc_bridge_service()->notifications(), Init);
+      ARC_GET_INSTANCE_FOR_METHOD(arc_bridge_service_->notifications(), Init);
   DCHECK(notifications_instance);
 
   mojom::NotificationsHostPtr host_proxy;
@@ -107,7 +153,7 @@ void ArcNotificationManager::SendNotificationRemovedFromChrome(
   items_.erase(it);
 
   auto* notifications_instance = ARC_GET_INSTANCE_FOR_METHOD(
-      arc_bridge_service()->notifications(), SendNotificationEventToAndroid);
+      arc_bridge_service_->notifications(), SendNotificationEventToAndroid);
 
   // On shutdown, the ARC channel may quit earlier than notifications.
   if (!notifications_instance) {
@@ -129,7 +175,7 @@ void ArcNotificationManager::SendNotificationClickedOnChrome(
   }
 
   auto* notifications_instance = ARC_GET_INSTANCE_FOR_METHOD(
-      arc_bridge_service()->notifications(), SendNotificationEventToAndroid);
+      arc_bridge_service_->notifications(), SendNotificationEventToAndroid);
 
   // On shutdown, the ARC channel may quit earlier than notifications.
   if (!notifications_instance) {
@@ -152,7 +198,7 @@ void ArcNotificationManager::SendNotificationButtonClickedOnChrome(
   }
 
   auto* notifications_instance = ARC_GET_INSTANCE_FOR_METHOD(
-      arc_bridge_service()->notifications(), SendNotificationEventToAndroid);
+      arc_bridge_service_->notifications(), SendNotificationEventToAndroid);
 
   // On shutdown, the ARC channel may quit earlier than notifications.
   if (!notifications_instance) {
@@ -195,7 +241,7 @@ void ArcNotificationManager::CreateNotificationWindow(const std::string& key) {
   }
 
   auto* notifications_instance = ARC_GET_INSTANCE_FOR_METHOD(
-      arc_bridge_service()->notifications(), CreateNotificationWindow);
+      arc_bridge_service_->notifications(), CreateNotificationWindow);
   if (!notifications_instance)
     return;
 
@@ -210,7 +256,7 @@ void ArcNotificationManager::CloseNotificationWindow(const std::string& key) {
   }
 
   auto* notifications_instance = ARC_GET_INSTANCE_FOR_METHOD(
-      arc_bridge_service()->notifications(), CloseNotificationWindow);
+      arc_bridge_service_->notifications(), CloseNotificationWindow);
   if (!notifications_instance)
     return;
 
@@ -225,7 +271,7 @@ void ArcNotificationManager::OpenNotificationSettings(const std::string& key) {
   }
 
   auto* notifications_instance = ARC_GET_INSTANCE_FOR_METHOD(
-      arc_bridge_service()->notifications(), OpenNotificationSettings);
+      arc_bridge_service_->notifications(), OpenNotificationSettings);
 
   // On shutdown, the ARC channel may quit earlier than notifications.
   if (!notifications_instance)
@@ -236,7 +282,7 @@ void ArcNotificationManager::OpenNotificationSettings(const std::string& key) {
 
 bool ArcNotificationManager::IsOpeningSettingsSupported() const {
   const auto* notifications_instance = ARC_GET_INSTANCE_FOR_METHOD(
-      arc_bridge_service()->notifications(), OpenNotificationSettings);
+      arc_bridge_service_->notifications(), OpenNotificationSettings);
   return notifications_instance != nullptr;
 }
 
@@ -249,7 +295,7 @@ void ArcNotificationManager::SendNotificationToggleExpansionOnChrome(
   }
 
   auto* notifications_instance = ARC_GET_INSTANCE_FOR_METHOD(
-      arc_bridge_service()->notifications(), SendNotificationEventToAndroid);
+      arc_bridge_service_->notifications(), SendNotificationEventToAndroid);
 
   // On shutdown, the ARC channel may quit earlier than notifications.
   if (!notifications_instance) {

ui/arc/notification/arc_notification_manager.h

@@ -9,29 +9,42 @@
 #include <string>
 #include <unordered_map>
 
-#include "components/arc/arc_service.h"
 #include "components/arc/common/notifications.mojom.h"
 #include "components/arc/instance_holder.h"
+#include "components/keyed_service/core/keyed_service.h"
 #include "components/signin/core/account_id/account_id.h"
 #include "mojo/public/cpp/bindings/binding.h"
 #include "ui/message_center/message_center.h"
 
+namespace content {
+class BrowserContext;
+}  // namespace content
+
 namespace arc {
 
 class ArcBridgeService;
 class ArcNotificationItem;
 
 class ArcNotificationManager
-    : public ArcService,
+    : public KeyedService,
       public InstanceHolder<mojom::NotificationsInstance>::Observer,
       public mojom::NotificationsHost {
  public:
-  ArcNotificationManager(ArcBridgeService* bridge_service,
-                         const AccountId& main_profile_id);
-
-  ArcNotificationManager(ArcBridgeService* bridge_service,
-                         const AccountId& main_profile_id,
-                         message_center::MessageCenter* message_center);
+  // Returns singleton instance for the given BrowserContext,
+  // or nullptr if the browser |context| is not allowed to use ARC.
+  static ArcNotificationManager* GetForBrowserContext(
+      content::BrowserContext* context);
+
+  // Returns a created instance for testing.
+  static std::unique_ptr<ArcNotificationManager> CreateForTesting(
+      ArcBridgeService* bridge_service,
+      const AccountId& main_profile_id,
+      message_center::MessageCenter* message_center);
+
+  // TODO(hidehiko): Make ctor private to enforce all service users should
+  // use GetForBrowserContext().
+  ArcNotificationManager(content::BrowserContext* context,
+                         ArcBridgeService* bridge_service);
 
   ~ArcNotificationManager() override;
 
@@ -57,6 +70,11 @@ class ArcNotificationManager
   void SendNotificationToggleExpansionOnChrome(const std::string& key);
 
  private:
+  ArcNotificationManager(ArcBridgeService* bridge_service,
+                         const AccountId& main_profile_id,
+                         message_center::MessageCenter* message_center);
+
+  ArcBridgeService* const arc_bridge_service_;  // Owned by ArcServiceManager.
   const AccountId main_profile_id_;
   message_center::MessageCenter* const message_center_;
 

ui/arc/notification/arc_notification_manager_unittest.cc

@@ -119,7 +119,7 @@ class ArcNotificationManagerTest : public testing::Test {
     service_ = base::MakeUnique<ArcBridgeService>();
     message_center_ = base::MakeUnique<MockMessageCenter>();
 
-    arc_notification_manager_ = base::MakeUnique<ArcNotificationManager>(
+    arc_notification_manager_ = ArcNotificationManager::CreateForTesting(
         service_.get(), EmptyAccountId(), message_center_.get());
 
     NotificationsObserver observer;