Migrate ArcService to BrowserContextKeyedService part 14.

This CL migrates ArcPolicyBridge.

BUG=672829
TEST=Ran try.

Change-Id: Ie649513dede52ff28c0eb54713b1e29cc2e37857
Reviewed-on: https://chromium-review.googlesource.com/572483
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487146}

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<ArcPolicyBridge>(arc_bridge_service));
   arc_service_manager_->AddService(
       base::MakeUnique<ArcProcessService>(arc_bridge_service));
   arc_service_manager_->AddService(
@@ -182,6 +180,7 @@ void ArcServiceLauncher::OnPrimaryUserProfilePrepared(Profile* profile) {
   ArcMetricsService::GetForBrowserContext(profile);
   ArcNetHostImpl::GetForBrowserContext(profile);
   ArcObbMounterBridge::GetForBrowserContext(profile);
+  ArcPolicyBridge::GetForBrowserContext(profile);
   ArcPowerBridge::GetForBrowserContext(profile);
   ArcPrintService::GetForBrowserContext(profile);
   ArcProvisionNotificationService::GetForBrowserContext(profile);

chrome/browser/chromeos/arc/policy/arc_policy_bridge.cc

@@ -6,12 +6,14 @@
 
 #include <memory>
 #include <utility>
+#include <vector>
 
 #include "base/bind.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_string_value_serializer.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
+#include "base/memory/singleton.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
@@ -25,14 +27,13 @@
 #include "chrome/common/pref_names.h"
 #include "chromeos/network/onc/onc_utils.h"
 #include "components/arc/arc_bridge_service.h"
+#include "components/arc/arc_browser_context_keyed_service_factory_base.h"
 #include "components/onc/onc_constants.h"
 #include "components/policy/core/common/policy_map.h"
 #include "components/policy/core/common/policy_namespace.h"
 #include "components/policy/policy_constants.h"
 #include "components/prefs/pref_service.h"
 #include "components/safe_json/safe_json_parser.h"
-#include "components/user_manager/user.h"
-#include "components/user_manager/user_manager.h"
 #include "crypto/sha2.h"
 
 namespace arc {
@@ -243,12 +244,6 @@ std::string GetFilteredJSONPolicies(const policy::PolicyMap& policy_map) {
   return policy_json;
 }
 
-Profile* GetProfile() {
-  const user_manager::User* const primary_user =
-      user_manager::UserManager::Get()->GetPrimaryUser();
-  return chromeos::ProfileHelper::Get()->GetProfileByUser(primary_user);
-}
-
 void OnReportComplianceParseFailure(
     const ArcPolicyBridge::ReportComplianceCallback& callback,
     const std::string& error) {
@@ -288,27 +283,67 @@ std::string GetPoliciesHash(const std::string& json_policies) {
       base::HexEncode(hash_bits.c_str(), hash_bits.length()));
 }
 
+// Singleton factory for ArcPolicyBridge.
+class ArcPolicyBridgeFactory
+    : public internal::ArcBrowserContextKeyedServiceFactoryBase<
+          ArcPolicyBridge,
+          ArcPolicyBridgeFactory> {
+ public:
+  // Factory name used by ArcBrowserContextKeyedServiceFactoryBase.
+  static constexpr const char* kName = "ArcPolicyBridgeFactory";
+
+  static ArcPolicyBridgeFactory* GetInstance() {
+    return base::Singleton<ArcPolicyBridgeFactory>::get();
+  }
+
+ private:
+  friend base::DefaultSingletonTraits<ArcPolicyBridgeFactory>;
+
+  ArcPolicyBridgeFactory() {
+    DependsOn(policy::ProfilePolicyConnectorFactory::GetInstance());
+  }
+  ~ArcPolicyBridgeFactory() override = default;
+};
+
 }  // namespace
 
-ArcPolicyBridge::ArcPolicyBridge(ArcBridgeService* bridge_service)
-    : ArcService(bridge_service), binding_(this), weak_ptr_factory_(this) {
+// static
+ArcPolicyBridge* ArcPolicyBridge::GetForBrowserContext(
+    content::BrowserContext* context) {
+  return ArcPolicyBridgeFactory::GetForBrowserContext(context);
+}
+
+ArcPolicyBridge::ArcPolicyBridge(content::BrowserContext* context,
+                                 ArcBridgeService* bridge_service)
+    : context_(context),
+      arc_bridge_service_(bridge_service),
+      binding_(this),
+      weak_ptr_factory_(this) {
   VLOG(2) << "ArcPolicyBridge::ArcPolicyBridge";
-  arc_bridge_service()->policy()->AddObserver(this);
+  arc_bridge_service_->policy()->AddObserver(this);
 }
 
-ArcPolicyBridge::ArcPolicyBridge(ArcBridgeService* bridge_service,
+ArcPolicyBridge::ArcPolicyBridge(content::BrowserContext* context,
+                                 ArcBridgeService* bridge_service,
                                  policy::PolicyService* policy_service)
-    : ArcService(bridge_service),
+    : context_(context),
+      arc_bridge_service_(bridge_service),
       binding_(this),
       policy_service_(policy_service),
       weak_ptr_factory_(this) {
   VLOG(2) << "ArcPolicyBridge::ArcPolicyBridge(bridge_service, policy_service)";
-  arc_bridge_service()->policy()->AddObserver(this);
+  arc_bridge_service_->policy()->AddObserver(this);
 }
 
 ArcPolicyBridge::~ArcPolicyBridge() {
   VLOG(2) << "ArcPolicyBridge::~ArcPolicyBridge";
-  arc_bridge_service()->policy()->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_->policy()->RemoveObserver(this);
 }
 
 // static
@@ -330,7 +365,7 @@ void ArcPolicyBridge::OnInstanceReady() {
   initial_policies_hash_ = GetPoliciesHash(GetCurrentJSONPolicies());
 
   mojom::PolicyInstance* const policy_instance =
-      ARC_GET_INSTANCE_FOR_METHOD(arc_bridge_service()->policy(), Init);
+      ARC_GET_INSTANCE_FOR_METHOD(arc_bridge_service_->policy(), Init);
   DCHECK(policy_instance);
   mojom::PolicyHostPtr host_proxy;
   binding_.Bind(mojo::MakeRequest(&host_proxy));
@@ -363,7 +398,7 @@ void ArcPolicyBridge::OnPolicyUpdated(const policy::PolicyNamespace& ns,
                                       const policy::PolicyMap& previous,
                                       const policy::PolicyMap& current) {
   VLOG(1) << "ArcPolicyBridge::OnPolicyUpdated";
-  auto* instance = ARC_GET_INSTANCE_FOR_METHOD(arc_bridge_service()->policy(),
+  auto* instance = ARC_GET_INSTANCE_FOR_METHOD(arc_bridge_service_->policy(),
                                                OnPolicyUpdated);
   if (!instance)
     return;
@@ -380,7 +415,7 @@ void ArcPolicyBridge::OnPolicyUpdated(const policy::PolicyNamespace& ns,
 
 void ArcPolicyBridge::InitializePolicyService() {
   auto* profile_policy_connector =
-      policy::ProfilePolicyConnectorFactory::GetForBrowserContext(GetProfile());
+      policy::ProfilePolicyConnectorFactory::GetForBrowserContext(context_);
   policy_service_ = profile_policy_connector->policy_service();
   is_managed_ = profile_policy_connector->IsManaged();
 }
@@ -400,8 +435,8 @@ void ArcPolicyBridge::OnReportComplianceParseSuccess(
     std::unique_ptr<base::Value> parsed_json) {
   // Always returns "compliant".
   callback.Run(kPolicyCompliantJson);
-  GetProfile()->GetPrefs()->SetBoolean(prefs::kArcPolicyComplianceReported,
-                                       true);
+  Profile::FromBrowserContext(context_)->GetPrefs()->SetBoolean(
+      prefs::kArcPolicyComplianceReported, true);
 
   const base::DictionaryValue* dict = nullptr;
   if (parsed_json->GetAsDictionary(&dict))

chrome/browser/chromeos/arc/policy/arc_policy_bridge.h

@@ -14,11 +14,16 @@
 #include "components/arc/arc_service.h"
 #include "components/arc/common/policy.mojom.h"
 #include "components/arc/instance_holder.h"
+#include "components/keyed_service/core/keyed_service.h"
 #include "components/policy/core/common/policy_namespace.h"
 #include "components/policy/core/common/policy_service.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "mojo/public/cpp/bindings/binding.h"
 
+namespace content {
+class BrowserContext;
+}  // namespace content
+
 namespace policy {
 class PolicyMap;
 }  // namespace policy
@@ -36,13 +41,20 @@ enum ArcCertsSyncMode : int32_t {
   COPY_CA_CERTS = 1
 };
 
-class ArcPolicyBridge : public ArcService,
+class ArcPolicyBridge : public KeyedService,
                         public InstanceHolder<mojom::PolicyInstance>::Observer,
                         public mojom::PolicyHost,
                         public policy::PolicyService::Observer {
  public:
-  explicit ArcPolicyBridge(ArcBridgeService* bridge_service);
-  ArcPolicyBridge(ArcBridgeService* bridge_service,
+  // Returns singleton instance for the given BrowserContext,
+  // or nullptr if the browser |context| is not allowed to use ARC.
+  static ArcPolicyBridge* GetForBrowserContext(
+      content::BrowserContext* context);
+
+  ArcPolicyBridge(content::BrowserContext* context,
+                  ArcBridgeService* bridge_service);
+  ArcPolicyBridge(content::BrowserContext* context,
+                  ArcBridgeService* bridge_service,
                   policy::PolicyService* policy_service);
   ~ArcPolicyBridge() override;
 
@@ -77,6 +89,9 @@ class ArcPolicyBridge : public ArcService,
 
   void UpdateComplianceReportMetrics(const base::DictionaryValue* report);
 
+  content::BrowserContext* const context_;
+  ArcBridgeService* const arc_bridge_service_;  // Owned by ArcServiceManager.
+
   mojo::Binding<PolicyHost> binding_;
   policy::PolicyService* policy_service_ = nullptr;
   bool is_managed_ = false;

chrome/browser/chromeos/arc/policy/arc_policy_bridge_unittest.cc

@@ -118,10 +118,6 @@ class ArcPolicyBridgeTest : public testing::Test {
 
   void SetUp() override {
     bridge_service_ = base::MakeUnique<ArcBridgeService>();
-    policy_bridge_ = base::MakeUnique<ArcPolicyBridge>(bridge_service_.get(),
-                                                       &policy_service_);
-    policy_bridge_->OverrideIsManagedForTesting(true);
-
     EXPECT_CALL(policy_service_,
                 GetPolicies(policy::PolicyNamespace(
                     policy::POLICY_DOMAIN_CHROME, std::string())))
@@ -146,6 +142,11 @@ class ArcPolicyBridgeTest : public testing::Test {
     ASSERT_TRUE(testing_profile_manager_->SetUp());
     profile_ = testing_profile_manager_->CreateTestingProfile("user@gmail.com");
     ASSERT_TRUE(profile_);
+
+    // TODO(hidehiko): Use Singleton instance tied to BrowserContext.
+    policy_bridge_ = base::MakeUnique<ArcPolicyBridge>(
+        profile_, bridge_service_.get(), &policy_service_);
+    policy_bridge_->OverrideIsManagedForTesting(true);
   }
 
  protected: