Migrate ArcService to BrowserContextKeyedService part 1.

This CL migrates ArcIntentHelperBridge.

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

Change-Id: Ic7bba1bdd95dc5c41c7789d4081f9abb3fc9ad12
Reviewed-on: https://chromium-review.googlesource.com/571384Reviewed-by: Yusuke Sato (in China Mon-Thurs, may be offline) <yusukes@chromium.org>
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487436}

chrome/browser/chromeos/arc/arc_service_launcher.cc

@@ -81,20 +81,10 @@ ArcServiceLauncher* ArcServiceLauncher::Get() {
 }
 
 void ArcServiceLauncher::Initialize() {
-  // Create ARC service manager.
   arc_service_manager_ = base::MakeUnique<ArcServiceManager>();
-
-  ArcBridgeService* arc_bridge_service =
-      arc_service_manager_->arc_bridge_service();
-
-  // Creates ArcSessionManager at first.
-  arc_session_manager_ =
-      base::MakeUnique<ArcSessionManager>(base::MakeUnique<ArcSessionRunner>(
-          base::Bind(ArcSession::Create, arc_bridge_service)));
-
-  // List in lexicographical order.
-  arc_service_manager_->AddService(
-      base::MakeUnique<ArcIntentHelperBridge>(arc_bridge_service));
+  arc_session_manager_ = base::MakeUnique<ArcSessionManager>(
+      base::MakeUnique<ArcSessionRunner>(base::Bind(
+          ArcSession::Create, arc_service_manager_->arc_bridge_service())));
 }
 
 void ArcServiceLauncher::MaybeSetProfile(Profile* profile) {
@@ -153,6 +143,7 @@ void ArcServiceLauncher::OnPrimaryUserProfilePrepared(Profile* profile) {
   ArcEnterpriseReportingService::GetForBrowserContext(profile);
   ArcFileSystemMounter::GetForBrowserContext(profile);
   ArcImeService::GetForBrowserContext(profile);
+  ArcIntentHelperBridge::GetForBrowserContext(profile);
   ArcMetricsService::GetForBrowserContext(profile);
   ArcNetHostImpl::GetForBrowserContext(profile);
   ArcObbMounterBridge::GetForBrowserContext(profile);

chrome/browser/chromeos/arc/intent_helper/arc_external_protocol_dialog.cc

@@ -366,8 +366,13 @@ void OnUrlHandlerList(int render_process_host_id,
 
   auto* instance = ARC_GET_INSTANCE_FOR_METHOD(
       arc_service_manager->arc_bridge_service()->intent_helper(), HandleUrl);
+
+  WebContents* web_contents =
+      tab_util::GetWebContentsByID(render_process_host_id, routing_id);
   auto* intent_helper_bridge =
-      ArcServiceManager::GetGlobalService<ArcIntentHelperBridge>();
+      web_contents ? ArcIntentHelperBridge::GetForBrowserContext(
+                         web_contents->GetBrowserContext())
+                   : nullptr;
   if (!instance || !intent_helper_bridge) {
     // ARC is not running anymore. Show the Chrome OS dialog.
     ShowFallbackExternalProtocolDialog(render_process_host_id, routing_id, url);

chrome/browser/chromeos/arc/intent_helper/arc_navigation_throttle.cc

@@ -178,8 +178,8 @@ ArcNavigationThrottle::HandleRequest() {
   if (!arc_service_manager)
     return content::NavigationThrottle::PROCEED;
 
-  auto* intent_helper_bridge =
-      arc_service_manager->GetService<ArcIntentHelperBridge>();
+  auto* intent_helper_bridge = ArcIntentHelperBridge::GetForBrowserContext(
+      navigation_handle()->GetWebContents()->GetBrowserContext());
   if (!intent_helper_bridge)
     return content::NavigationThrottle::PROCEED;
 
@@ -247,8 +247,8 @@ void ArcNavigationThrottle::OnAppCandidatesReceived(
   if (IsSwapElementsNeeded(handlers, &indices))
     std::swap(handlers[indices.first], handlers[indices.second]);
 
-  auto* intent_helper_bridge =
-      ArcServiceManager::GetGlobalService<ArcIntentHelperBridge>();
+  auto* intent_helper_bridge = ArcIntentHelperBridge::GetForBrowserContext(
+      navigation_handle()->GetWebContents()->GetBrowserContext());
   if (!intent_helper_bridge) {
     LOG(ERROR) << "Cannot get an instance of ArcIntentHelperBridge";
     navigation_handle()->Resume();

chrome/browser/chromeos/extensions/file_manager/event_router.cc

@@ -414,7 +414,7 @@ void EventRouter::Shutdown() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   auto* intent_helper =
-      arc::ArcServiceManager::GetGlobalService<arc::ArcIntentHelperBridge>();
+      arc::ArcIntentHelperBridge::GetForBrowserContext(profile_);
   if (intent_helper)
     intent_helper->RemoveObserver(this);
 
@@ -509,12 +509,10 @@ void EventRouter::ObserveEvents() {
 
   chromeos::system::TimezoneSettings::GetInstance()->AddObserver(this);
 
-  if (arc::IsArcAllowedForProfile(profile_)) {
-    auto* intent_helper =
-        arc::ArcServiceManager::GetGlobalService<arc::ArcIntentHelperBridge>();
-    if (intent_helper)
-      intent_helper->AddObserver(this);
-  }
+  auto* intent_helper =
+      arc::ArcIntentHelperBridge::GetForBrowserContext(profile_);
+  if (intent_helper)
+    intent_helper->AddObserver(this);
 }
 
 // File watch setup routines.

chrome/browser/chromeos/extensions/file_manager/event_router_factory.cc

@@ -9,6 +9,7 @@
 #include "chrome/browser/chromeos/file_manager/volume_manager_factory.h"
 #include "chrome/browser/profiles/incognito_helpers.h"
 #include "chrome/browser/profiles/profile.h"
+#include "components/arc/intent_helper/arc_intent_helper_bridge.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 #include "extensions/browser/extension_system_provider.h"
 #include "extensions/browser/extensions_browser_client.h"
@@ -34,6 +35,7 @@ EventRouterFactory::EventRouterFactory()
   DependsOn(
       extensions::ExtensionsBrowserClient::Get()->GetExtensionSystemFactory());
   DependsOn(VolumeManagerFactory::GetInstance());
+  DependsOn(arc::ArcIntentHelperBridge::GetFactory());
 }
 
 EventRouterFactory::~EventRouterFactory() {

chrome/browser/chromeos/file_manager/arc_file_tasks.cc

@@ -83,6 +83,7 @@ arc::mojom::ActivityNamePtr AppIdToActivityName(const std::string& id) {
 
 // Below is the sequence of thread-hopping for loading ARC file tasks.
 void OnArcHandlerList(
+    Profile* profile,
     std::unique_ptr<std::vector<FullTaskDescriptor>> result_list,
     const FindTasksCallback& callback,
     std::vector<arc::mojom::IntentHandlerInfoPtr> handlers);
@@ -95,13 +96,14 @@ void OnArcIconLoaded(
 
 // Called after the handlers from ARC is obtained. Proceeds to OnArcIconLoaded.
 void OnArcHandlerList(
+    Profile* profile,
     std::unique_ptr<std::vector<FullTaskDescriptor>> result_list,
     const FindTasksCallback& callback,
     std::vector<arc::mojom::IntentHandlerInfoPtr> handlers) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   auto* intent_helper_bridge =
-      arc::ArcServiceManager::GetGlobalService<arc::ArcIntentHelperBridge>();
+      arc::ArcIntentHelperBridge::GetForBrowserContext(profile);
   if (!intent_helper_bridge) {
     callback.Run(std::move(result_list));
     return;
@@ -194,9 +196,10 @@ void FindArcTasks(Profile* profile,
     url_with_type->mime_type = entry.mime_type;
     urls.push_back(std::move(url_with_type));
   }
+  // The callback will be invoked on UI thread, so |profile| should be alive.
   arc_intent_helper->RequestUrlListHandlerList(
-      std::move(urls),
-      base::Bind(&OnArcHandlerList, base::Passed(&result_list), callback));
+      std::move(urls), base::Bind(&OnArcHandlerList, base::Unretained(profile),
+                                  base::Passed(&result_list), callback));
 }
 
 bool ExecuteArcTask(Profile* profile,

chrome/browser/chromeos/note_taking_helper.cc

@@ -293,19 +293,19 @@ NoteTakingHelper::NoteTakingHelper()
     // IsArcPlayStoreEnabledForProfile() can return true only for the primary
     // profile.
     play_store_enabled_ |= arc::IsArcPlayStoreEnabledForProfile(profile);
+
+    // ArcIntentHelperBridge will notify us about changes to the list of
+    // available Android apps.
+    auto* intent_helper_bridge =
+        arc::ArcIntentHelperBridge::GetForBrowserContext(profile);
+    if (intent_helper_bridge)
+      intent_helper_bridge->AddObserver(this);
   }
 
   // Watch for changes of Google Play Store enabled state.
   auto* session_manager = arc::ArcSessionManager::Get();
   session_manager->AddObserver(this);
 
-  // ArcIntentHelperBridge will notify us about changes to the list of available
-  // Android apps.
-  auto* intent_helper_bridge =
-      arc::ArcServiceManager::GetGlobalService<arc::ArcIntentHelperBridge>();
-  if (intent_helper_bridge)
-    intent_helper_bridge->AddObserver(this);
-
   // If the ARC intent helper is ready, get the Android apps. Otherwise,
   // UpdateAndroidApps() will be called when ArcServiceManager calls
   // OnIntentFiltersUpdated().
@@ -321,12 +321,15 @@ NoteTakingHelper::~NoteTakingHelper() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   // ArcSessionManagerTest shuts down ARC before NoteTakingHelper.
-  auto* intent_helper_bridge =
-      arc::ArcServiceManager::GetGlobalService<arc::ArcIntentHelperBridge>();
-  if (intent_helper_bridge)
-    intent_helper_bridge->RemoveObserver(this);
   if (arc::ArcSessionManager::Get())
     arc::ArcSessionManager::Get()->RemoveObserver(this);
+  for (Profile* profile :
+       g_browser_process->profile_manager()->GetLoadedProfiles()) {
+    auto* intent_helper_bridge =
+        arc::ArcIntentHelperBridge::GetForBrowserContext(profile);
+    if (intent_helper_bridge)
+      intent_helper_bridge->RemoveObserver(this);
+  }
 }
 
 bool NoteTakingHelper::IsWhitelistedChromeApp(

chrome/browser/task_manager/providers/arc/arc_process_task.cc

@@ -81,8 +81,11 @@ ArcProcessTask::ArcProcessTask(base::ProcessId pid,
 void ArcProcessTask::StartIconLoading() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  auto* intent_helper_bridge =
-      arc::ArcServiceManager::GetGlobalService<arc::ArcIntentHelperBridge>();
+  // TaskManager is not tied to BrowserContext. Thus, we just use the
+  // BrowserContext which is tied to ARC.
+  auto* arc_service_manager = arc::ArcServiceManager::Get();
+  auto* intent_helper_bridge = arc::ArcIntentHelperBridge::GetForBrowserContext(
+      arc_service_manager->browser_context());
   arc::ArcIntentHelperBridge::GetResult result =
       arc::ArcIntentHelperBridge::GetResult::FAILED_ARC_NOT_READY;
   if (intent_helper_bridge) {
@@ -99,10 +102,8 @@ void ArcProcessTask::StartIconLoading() {
 
   if (result == arc::ArcIntentHelperBridge::GetResult::FAILED_ARC_NOT_READY) {
     // Need to retry loading the icon.
-    arc::ArcServiceManager::Get()
-        ->arc_bridge_service()
-        ->intent_helper()
-        ->AddObserver(this);
+    arc_service_manager->arc_bridge_service()->intent_helper()->AddObserver(
+        this);
   }
 }
 

components/arc/intent_helper/arc_intent_helper_bridge.cc

@@ -10,8 +10,10 @@
 #include "ash/shell.h"
 #include "ash/shell_delegate.h"
 #include "ash/wallpaper/wallpaper_controller.h"
+#include "base/memory/singleton.h"
 #include "base/memory/weak_ptr.h"
 #include "components/arc/arc_bridge_service.h"
+#include "components/arc/arc_browser_context_keyed_service_factory_base.h"
 #include "components/arc/arc_service_manager.h"
 #include "components/arc/audio/arc_audio_bridge.h"
 #include "components/arc/intent_helper/link_handler_model_impl.h"
@@ -19,30 +21,67 @@
 #include "url/gurl.h"
 
 namespace arc {
+namespace {
+
+// Singleton factory for ArcIntentHelperBridge.
+class ArcIntentHelperBridgeFactory
+    : public internal::ArcBrowserContextKeyedServiceFactoryBase<
+          ArcIntentHelperBridge,
+          ArcIntentHelperBridgeFactory> {
+ public:
+  // Factory name used by ArcBrowserContextKeyedServiceFactoryBase.
+  static constexpr const char* kName = "ArcIntentHelperBridgeFactory";
+
+  static ArcIntentHelperBridgeFactory* GetInstance() {
+    return base::Singleton<ArcIntentHelperBridgeFactory>::get();
+  }
 
-// static
-const char ArcIntentHelperBridge::kArcServiceName[] =
-    "arc::ArcIntentHelperBridge";
+ private:
+  friend struct base::DefaultSingletonTraits<ArcIntentHelperBridgeFactory>;
+
+  ArcIntentHelperBridgeFactory() = default;
+  ~ArcIntentHelperBridgeFactory() override = default;
+};
+
+}  // namespace
 
 // static
 const char ArcIntentHelperBridge::kArcIntentHelperPackageName[] =
     "org.chromium.arc.intent_helper";
 
-ArcIntentHelperBridge::ArcIntentHelperBridge(ArcBridgeService* bridge_service)
-    : ArcService(bridge_service), binding_(this) {
-  arc_bridge_service()->intent_helper()->AddObserver(this);
+// static
+ArcIntentHelperBridge* ArcIntentHelperBridge::GetForBrowserContext(
+    content::BrowserContext* context) {
+  return ArcIntentHelperBridgeFactory::GetForBrowserContext(context);
+}
+
+// static
+KeyedServiceBaseFactory* ArcIntentHelperBridge::GetFactory() {
+  return ArcIntentHelperBridgeFactory::GetInstance();
+}
+
+ArcIntentHelperBridge::ArcIntentHelperBridge(content::BrowserContext* context,
+                                             ArcBridgeService* bridge_service)
+    : context_(context), arc_bridge_service_(bridge_service), binding_(this) {
+  arc_bridge_service_->intent_helper()->AddObserver(this);
 }
 
 ArcIntentHelperBridge::~ArcIntentHelperBridge() {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  arc_bridge_service()->intent_helper()->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_->intent_helper()->RemoveObserver(this);
 }
 
 void ArcIntentHelperBridge::OnInstanceReady() {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   ash::Shell::Get()->set_link_handler_model_factory(this);
   auto* instance =
-      ARC_GET_INSTANCE_FOR_METHOD(arc_bridge_service()->intent_helper(), Init);
+      ARC_GET_INSTANCE_FOR_METHOD(arc_bridge_service_->intent_helper(), Init);
   DCHECK(instance);
   mojom::IntentHelperHostPtr host_proxy;
   binding_.Bind(mojo::MakeRequest(&host_proxy));
@@ -126,7 +165,7 @@ void ArcIntentHelperBridge::RemoveObserver(ArcIntentHelperObserver* observer) {
 std::unique_ptr<ash::LinkHandlerModel> ArcIntentHelperBridge::CreateModel(
     const GURL& url) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  auto impl = base::MakeUnique<LinkHandlerModelImpl>();
+  auto impl = base::MakeUnique<LinkHandlerModelImpl>(context_);
   if (!impl->Init(url))
     return nullptr;
   return std::move(impl);

components/arc/intent_helper/arc_intent_helper_bridge.h

@@ -13,19 +13,23 @@
 #include "base/macros.h"
 #include "base/observer_list.h"
 #include "base/threading/thread_checker.h"
-#include "components/arc/arc_service.h"
 #include "components/arc/common/intent_helper.mojom.h"
 #include "components/arc/instance_holder.h"
 #include "components/arc/intent_helper/activity_icon_loader.h"
 #include "components/arc/intent_helper/arc_intent_helper_observer.h"
+#include "components/keyed_service/core/keyed_service.h"
 #include "mojo/public/cpp/bindings/binding.h"
 
-namespace ash {
+class KeyedServiceBaseFactory;
 
+namespace ash {
 class LinkHandlerModel;
-
 }  // namespace ash
 
+namespace content {
+class BrowserContext;
+}  // namespace content
+
 namespace arc {
 
 class ArcBridgeService;
@@ -33,12 +37,21 @@ class IntentFilter;
 
 // Receives intents from ARC.
 class ArcIntentHelperBridge
-    : public ArcService,
+    : public KeyedService,
       public InstanceHolder<mojom::IntentHelperInstance>::Observer,
       public mojom::IntentHelperHost,
       public ash::LinkHandlerModelFactory {
  public:
-  explicit ArcIntentHelperBridge(ArcBridgeService* bridge_service);
+  // Returns singleton instance for the given BrowserContext,
+  // or nullptr if the browser |context| is not allowed to use ARC.
+  static ArcIntentHelperBridge* GetForBrowserContext(
+      content::BrowserContext* context);
+
+  // Returns factory for the ArcIntentHelperBridge.
+  static KeyedServiceBaseFactory* GetFactory();
+
+  ArcIntentHelperBridge(content::BrowserContext* context,
+                        ArcBridgeService* bridge_service);
   ~ArcIntentHelperBridge() override;
 
   void AddObserver(ArcIntentHelperObserver* observer);
@@ -88,14 +101,14 @@ class ArcIntentHelperBridge
   static std::vector<mojom::IntentHandlerInfoPtr> FilterOutIntentHelper(
       std::vector<mojom::IntentHandlerInfoPtr> handlers);
 
-  // For supporting ArcServiceManager::GetService<T>().
-  static const char kArcServiceName[];
-
   static const char kArcIntentHelperPackageName[];
 
  private:
   THREAD_CHECKER(thread_checker_);
 
+  content::BrowserContext* const context_;
+  ArcBridgeService* const arc_bridge_service_;  // Owned by ArcServiceManager.
+
   mojo::Binding<mojom::IntentHelperHost> binding_;
   internal::ActivityIconLoader icon_loader_;
 

components/arc/intent_helper/arc_intent_helper_bridge_unittest.cc

@@ -18,8 +18,6 @@ namespace {
 IntentFilter GetIntentFilter(const std::string& host) {
   std::vector<IntentFilter::AuthorityEntry> authorities;
   authorities.emplace_back(host, -1);
-
-  // TODO
   return IntentFilter(std::move(authorities),
                       std::vector<IntentFilter::PatternMatcher>());
 }
@@ -35,8 +33,8 @@ class ArcIntentHelperTest : public testing::Test {
  private:
   void SetUp() override {
     arc_bridge_service_ = base::MakeUnique<ArcBridgeService>();
-    instance_ =
-        base::MakeUnique<ArcIntentHelperBridge>(arc_bridge_service_.get());
+    instance_ = base::MakeUnique<ArcIntentHelperBridge>(
+        nullptr /* context */, arc_bridge_service_.get());
   }
 
   void TearDown() override {

components/arc/intent_helper/link_handler_model_impl.cc

@@ -47,7 +47,8 @@ bool GetQueryValue(const GURL& url,
 
 }  // namespace
 
-LinkHandlerModelImpl::LinkHandlerModelImpl() : weak_ptr_factory_(this) {}
+LinkHandlerModelImpl::LinkHandlerModelImpl(content::BrowserContext* context)
+    : context_(context), weak_ptr_factory_(this) {}
 
 LinkHandlerModelImpl::~LinkHandlerModelImpl() = default;
 
@@ -97,7 +98,7 @@ void LinkHandlerModelImpl::OnUrlHandlerList(
 
   bool icon_info_notified = false;
   auto* intent_helper_bridge =
-      ArcServiceManager::GetGlobalService<ArcIntentHelperBridge>();
+      ArcIntentHelperBridge::GetForBrowserContext(context_);
   if (intent_helper_bridge) {
     std::vector<ArcIntentHelperBridge::ActivityName> activities;
     for (size_t i = 0; i < handlers_.size(); ++i) {

components/arc/intent_helper/link_handler_model_impl.h

@@ -15,11 +15,15 @@
 #include "components/arc/intent_helper/arc_intent_helper_bridge.h"
 #include "url/gurl.h"
 
+namespace content {
+class BrowserContext;
+}  // namespace content
+
 namespace arc {
 
 class LinkHandlerModelImpl : public ash::LinkHandlerModel {
  public:
-  LinkHandlerModelImpl();
+  explicit LinkHandlerModelImpl(content::BrowserContext* context);
   ~LinkHandlerModelImpl() override;
 
   // ash::LinkHandlerModel overrides:
@@ -45,6 +49,8 @@ class LinkHandlerModelImpl : public ash::LinkHandlerModel {
   // Otherwise, returns the original |url| as-us.
   static GURL RewriteUrlFromQueryIfAvailable(const GURL& url);
 
+  content::BrowserContext* const context_;
+
   base::ObserverList<Observer> observer_list_;
 
   // Url handler info passed from ARC.