Migrate ArcService to BrowserContextKeyedService part 19.

This CL migrates ArcSettingsService.

BUG=672829
TEST=Ran try.

Change-Id: I5f75acf2d1ca539921eb6038be1399ef7c115c63
Reviewed-on: https://chromium-review.googlesource.com/572885
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Yusuke Sato (in China Mon-Thurs, may be offline) <yusukes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487368}

chrome/browser/chromeos/arc/arc_service_launcher.cc

@@ -96,8 +96,6 @@ void ArcServiceLauncher::Initialize() {
   // List in lexicographical order.
   arc_service_manager_->AddService(
       base::MakeUnique<ArcIntentHelperBridge>(arc_bridge_service));
-  arc_service_manager_->AddService(
-      base::MakeUnique<ArcSettingsService>(arc_bridge_service));
   if (chromeos::switches::IsVoiceInteractionEnabled()) {
     arc_service_manager_->AddService(
         base::MakeUnique<ArcVoiceInteractionFrameworkService>(
@@ -177,6 +175,7 @@ void ArcServiceLauncher::OnPrimaryUserProfilePrepared(Profile* profile) {
   ArcPrintService::GetForBrowserContext(profile);
   ArcProcessService::GetForBrowserContext(profile);
   ArcProvisionNotificationService::GetForBrowserContext(profile);
+  ArcSettingsService::GetForBrowserContext(profile);
   ArcTracingBridge::GetForBrowserContext(profile);
   ArcTtsService::GetForBrowserContext(profile);
   ArcUserSessionService::GetForBrowserContext(profile);

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

@@ -9,11 +9,12 @@
 #include "base/command_line.h"
 #include "base/gtest_prod_util.h"
 #include "base/json/json_writer.h"
+#include "base/memory/singleton.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/arc/arc_session_manager.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
-#include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
 #include "chromeos/chromeos_switches.h"
 #include "chromeos/network/network_handler.h"
@@ -25,6 +26,7 @@
 #include "chromeos/settings/cros_settings_names.h"
 #include "chromeos/settings/timezone_settings.h"
 #include "components/arc/arc_bridge_service.h"
+#include "components/arc/arc_browser_context_keyed_service_factory_base.h"
 #include "components/arc/intent_helper/font_size_util.h"
 #include "components/onc/onc_pref_names.h"
 #include "components/prefs/pref_change_registrar.h"
@@ -67,20 +69,31 @@ bool GetHttpProxyServer(const ProxyConfigDictionary* proxy_config_dict,
   return !host->empty() && *port;
 }
 
-PrefService* GetPrefs() {
-  return ProfileManager::GetActiveUserProfile()->GetPrefs();
-}
-
-// Returns whether kProxy pref proxy config is applied.
-bool IsPrefProxyConfigApplied() {
-  net::ProxyConfig config;
-  return PrefProxyConfigTrackerImpl::PrefPrecedes(
-      PrefProxyConfigTrackerImpl::ReadPrefConfig(GetPrefs(), &config));
-}
-
 }  // namespace
 
 namespace arc {
+namespace {
+
+// Singleton factory for ArcSettingsService.
+class ArcSettingsServiceFactory
+    : public internal::ArcBrowserContextKeyedServiceFactoryBase<
+          ArcSettingsService,
+          ArcSettingsServiceFactory> {
+ public:
+  // Factory name used by ArcBrowserContextKeyedServiceFactoryBase.
+  static constexpr const char* kName = "ArcSettingsServiceFactory";
+
+  static ArcSettingsServiceFactory* GetInstance() {
+    return base::Singleton<ArcSettingsServiceFactory>::get();
+  }
+
+ private:
+  friend base::DefaultSingletonTraits<ArcSettingsServiceFactory>;
+  ArcSettingsServiceFactory() = default;
+  ~ArcSettingsServiceFactory() override = default;
+};
+
+}  // namespace
 
 // Listens to changes for select Chrome settings (prefs) that Android cares
 // about and sends the new values to Android to keep the state in sync.
@@ -90,7 +103,8 @@ class ArcSettingsServiceImpl
       public ArcSessionManager::Observer,
       public chromeos::NetworkStateHandlerObserver {
  public:
-  explicit ArcSettingsServiceImpl(ArcBridgeService* arc_bridge_service);
+  ArcSettingsServiceImpl(content::BrowserContext* context,
+                         ArcBridgeService* arc_bridge_service);
   ~ArcSettingsServiceImpl() override;
 
   // Called when a Chrome pref we have registered an observer for has changed.
@@ -111,6 +125,13 @@ class ArcSettingsServiceImpl
   void DefaultNetworkChanged(const chromeos::NetworkState* network) override;
 
  private:
+  PrefService* GetPrefs() const {
+    return Profile::FromBrowserContext(context_)->GetPrefs();
+  }
+
+  // Returns whether kProxy pref proxy config is applied.
+  bool IsPrefProxyConfigApplied() const;
+
   // Registers to observe changes for Chrome settings we care about.
   void StartObservingSettingsChanges();
 
@@ -168,12 +189,14 @@ class ArcSettingsServiceImpl
   void SendSettingsBroadcast(const std::string& action,
                              const base::DictionaryValue& extras) const;
 
+  content::BrowserContext* const context_;
+  ArcBridgeService* const arc_bridge_service_;  // Owned by ArcServiceManager.
+
   // Manages pref observation registration.
   PrefChangeRegistrar registrar_;
 
   std::unique_ptr<chromeos::CrosSettings::ObserverSubscription>
       reporting_consent_subscription_;
-  ArcBridgeService* const arc_bridge_service_;
 
   scoped_refptr<device::BluetoothAdapter> bluetooth_adapter_;
 
@@ -184,8 +207,11 @@ class ArcSettingsServiceImpl
 };
 
 ArcSettingsServiceImpl::ArcSettingsServiceImpl(
+    content::BrowserContext* context,
     ArcBridgeService* arc_bridge_service)
-    : arc_bridge_service_(arc_bridge_service), weak_factory_(this) {
+    : context_(context),
+      arc_bridge_service_(arc_bridge_service),
+      weak_factory_(this) {
   StartObservingSettingsChanges();
   SyncRuntimeSettings();
   DCHECK(ArcSessionManager::Get());
@@ -270,6 +296,12 @@ void ArcSettingsServiceImpl::DefaultNetworkChanged(
     SyncProxySettings();
 }
 
+bool ArcSettingsServiceImpl::IsPrefProxyConfigApplied() const {
+  net::ProxyConfig config;
+  return PrefProxyConfigTrackerImpl::PrefPrecedes(
+      PrefProxyConfigTrackerImpl::ReadPrefConfig(GetPrefs(), &config));
+}
+
 void ArcSettingsServiceImpl::StartObservingSettingsChanges() {
   registrar_.Init(GetPrefs());
 
@@ -609,17 +641,30 @@ void ArcSettingsServiceImpl::SendSettingsBroadcast(
                           extras_json);
 }
 
-ArcSettingsService::ArcSettingsService(ArcBridgeService* bridge_service)
-    : ArcService(bridge_service) {
-  arc_bridge_service()->intent_helper()->AddObserver(this);
+// static
+ArcSettingsService* ArcSettingsService::GetForBrowserContext(
+    content::BrowserContext* context) {
+  return ArcSettingsServiceFactory::GetForBrowserContext(context);
+}
+
+ArcSettingsService::ArcSettingsService(content::BrowserContext* context,
+                                       ArcBridgeService* bridge_service)
+    : context_(context), arc_bridge_service_(bridge_service) {
+  arc_bridge_service_->intent_helper()->AddObserver(this);
 }
 
 ArcSettingsService::~ArcSettingsService() {
-  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 ArcSettingsService::OnInstanceReady() {
-  impl_.reset(new ArcSettingsServiceImpl(arc_bridge_service()));
+  impl_ =
+      base::MakeUnique<ArcSettingsServiceImpl>(context_, arc_bridge_service_);
 }
 
 void ArcSettingsService::OnInstanceClosed() {

chrome/browser/chromeos/arc/intent_helper/arc_settings_service.h

@@ -8,9 +8,13 @@
 #include <memory>
 
 #include "base/macros.h"
-#include "components/arc/arc_service.h"
 #include "components/arc/common/intent_helper.mojom.h"
 #include "components/arc/instance_holder.h"
+#include "components/keyed_service/core/keyed_service.h"
+
+namespace content {
+class BrowserContext;
+}  // namespace content
 
 namespace arc {
 
@@ -18,10 +22,16 @@ class ArcBridgeService;
 class ArcSettingsServiceImpl;
 
 class ArcSettingsService
-    : public ArcService,
+    : public KeyedService,
       public InstanceHolder<mojom::IntentHelperInstance>::Observer {
  public:
-  explicit ArcSettingsService(ArcBridgeService* bridge_service);
+  // Returns singleton instance for the given BrowserContext,
+  // or nullptr if the browser |context| is not allowed to use ARC.
+  static ArcSettingsService* GetForBrowserContext(
+      content::BrowserContext* context);
+
+  ArcSettingsService(content::BrowserContext* context,
+                     ArcBridgeService* bridge_service);
   ~ArcSettingsService() override;
 
   // InstanceHolder<mojom::IntentHelperInstance>::Observer
@@ -29,6 +39,8 @@ class ArcSettingsService
   void OnInstanceClosed() override;
 
  private:
+  content::BrowserContext* const context_;
+  ArcBridgeService* const arc_bridge_service_;  // Owned by ArcServiceManager.
   std::unique_ptr<ArcSettingsServiceImpl> impl_;
 
   DISALLOW_COPY_AND_ASSIGN(ArcSettingsService);

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

@@ -28,6 +28,7 @@
 #include "chromeos/network/proxy/proxy_config_handler.h"
 #include "components/arc/arc_bridge_service.h"
 #include "components/arc/arc_service_manager.h"
+#include "components/arc/arc_util.h"
 #include "components/arc/test/fake_intent_helper_instance.h"
 #include "components/policy/core/browser/browser_policy_connector.h"
 #include "components/policy/core/common/mock_configuration_policy_provider.h"
@@ -223,6 +224,10 @@ class ArcSettingsServiceTest : public InProcessBrowserTest {
   // InProcessBrowserTest:
   ~ArcSettingsServiceTest() override = default;
 
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    arc::SetArcAvailableCommandLineForTesting(command_line);
+  }
+
   void SetUpInProcessBrowserTestFixture() override {
     EXPECT_CALL(provider_, IsInitializationComplete(_))
         .WillRepeatedly(Return(true));