Pass Profile* into ExtensionManagement() ctor.

This refactoring is needed to access Profile
within ExtensionManagemnt class in
https://crrev.com/c/1268938

Bug: none
Test: update unittests
Change-Id: Icdd53213a270d68e535d9846c5cc60dc19a1fb9e
Reviewed-on: https://chromium-review.googlesource.com/c/1275889Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Commit-Queue: Sergey Poromov <poromov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598908}

chrome/browser/extensions/extension_management.cc

@@ -41,11 +41,13 @@
 
 namespace extensions {
 
-ExtensionManagement::ExtensionManagement(PrefService* pref_service,
-                                         bool is_signin_profile)
-    : pref_service_(pref_service), is_signin_profile_(is_signin_profile) {
+ExtensionManagement::ExtensionManagement(Profile* profile)
+    : profile_(profile), pref_service_(profile_->GetPrefs()) {
   TRACE_EVENT0("browser,startup",
                "ExtensionManagement::ExtensionManagement::ctor");
+#if defined(OS_CHROMEOS)
+  is_signin_profile_ = chromeos::ProfileHelper::IsSigninProfile(profile);
+#endif
   pref_change_registrar_.Init(pref_service_);
   base::Closure pref_change_callback = base::Bind(
       &ExtensionManagement::OnExtensionPrefChanged, base::Unretained(this));
@@ -544,12 +546,7 @@ KeyedService* ExtensionManagementFactory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
   TRACE_EVENT0("browser,startup",
                "ExtensionManagementFactory::BuildServiceInstanceFor");
-  Profile* profile = Profile::FromBrowserContext(context);
-  bool is_signin_profile = false;
-#if defined(OS_CHROMEOS)
-  is_signin_profile = chromeos::ProfileHelper::IsSigninProfile(profile);
-#endif
-  return new ExtensionManagement(profile->GetPrefs(), is_signin_profile);
+  return new ExtensionManagement(Profile::FromBrowserContext(context));
 }
 
 content::BrowserContext* ExtensionManagementFactory::GetBrowserContextToUse(

chrome/browser/extensions/extension_management.h

@@ -24,6 +24,7 @@
 
 class GURL;
 class PrefService;
+class Profile;
 
 namespace content {
 class BrowserContext;
@@ -70,7 +71,7 @@ class ExtensionManagement : public KeyedService {
     INSTALLATION_RECOMMENDED,
   };
 
-  ExtensionManagement(PrefService* pref_service, bool is_signin_profile);
+  explicit ExtensionManagement(Profile* profile);
   ~ExtensionManagement() override;
 
   // KeyedService implementations:
@@ -224,6 +225,7 @@ class ExtensionManagement : public KeyedService {
   // Extension settings applicable to all extensions.
   std::unique_ptr<internal::GlobalSettings> global_settings_;
 
+  Profile* const profile_ = nullptr;
   PrefService* pref_service_ = nullptr;
   bool is_signin_profile_ = false;
 

chrome/browser/extensions/extension_management_unittest.cc

@@ -15,8 +15,9 @@
 #include "chrome/browser/extensions/extension_management_test_util.h"
 #include "chrome/browser/extensions/external_policy_loader.h"
 #include "chrome/browser/extensions/standard_management_policy_provider.h"
-#include "components/prefs/pref_registry_simple.h"
-#include "components/prefs/testing_pref_service.h"
+#include "chrome/test/base/testing_profile.h"
+#include "components/sync_preferences/testing_pref_service_syncable.h"
+#include "content/public/test/test_browser_thread_bundle.h"
 #include "extensions/browser/pref_names.h"
 #include "extensions/common/manifest.h"
 #include "extensions/common/manifest_constants.h"
@@ -92,7 +93,9 @@ const char kExampleDictNoCustomError[] =
 
 class ExtensionManagementServiceTest : public testing::Test {
  public:
-  typedef ExtensionManagementPrefUpdater<TestingPrefServiceSimple> PrefUpdater;
+  typedef ExtensionManagementPrefUpdater<
+      sync_preferences::TestingPrefServiceSyncable>
+      PrefUpdater;
 
   ExtensionManagementServiceTest() {}
   ~ExtensionManagementServiceTest() override {}
@@ -102,18 +105,9 @@ class ExtensionManagementServiceTest : public testing::Test {
 
   void InitPrefService() {
     extension_management_.reset();
-    pref_service_.reset(new TestingPrefServiceSimple());
-    pref_service_->registry()->RegisterListPref(
-        pref_names::kAllowedInstallSites);
-    pref_service_->registry()->RegisterListPref(pref_names::kAllowedTypes);
-    pref_service_->registry()->RegisterListPref(pref_names::kInstallDenyList);
-    pref_service_->registry()->RegisterListPref(pref_names::kInstallAllowList);
-    pref_service_->registry()->RegisterDictionaryPref(
-        pref_names::kInstallForceList);
-    pref_service_->registry()->RegisterDictionaryPref(
-        pref_names::kExtensionManagement);
-    extension_management_.reset(
-        new ExtensionManagement(pref_service_.get(), false));
+    profile_.reset(new TestingProfile());
+    pref_service_ = profile_->GetTestingPrefService();
+    extension_management_.reset(new ExtensionManagement(profile_.get()));
   }
 
   void SetPref(bool managed,
@@ -223,7 +217,10 @@ class ExtensionManagementServiceTest : public testing::Test {
   }
 
  protected:
-  std::unique_ptr<TestingPrefServiceSimple> pref_service_;
+  content::TestBrowserThreadBundle test_browser_thread_bundle_;
+
+  std::unique_ptr<TestingProfile> profile_;
+  sync_preferences::TestingPrefServiceSyncable* pref_service_;
   std::unique_ptr<ExtensionManagement> extension_management_;
 
  private:
@@ -588,7 +585,7 @@ TEST_F(ExtensionManagementServiceTest, kMinimumVersionRequired) {
   EXPECT_TRUE(CheckMinimumVersion(kTargetExtension, "9999.0"));
 
   {
-    PrefUpdater pref(pref_service_.get());
+    PrefUpdater pref(pref_service_);
     pref.SetMinimumVersionRequired(kTargetExtension, "3.0");
   }
 
@@ -613,7 +610,7 @@ TEST_F(ExtensionManagementServiceTest, NewInstallSources) {
 
   // Set the new dictionary preference.
   {
-    PrefUpdater updater(pref_service_.get());
+    PrefUpdater updater(pref_service_);
     updater.ClearInstallSources();
   }
   // Verifies that the new one overrides the legacy ones.
@@ -623,7 +620,7 @@ TEST_F(ExtensionManagementServiceTest, NewInstallSources) {
 
   // Updates the new dictionary preference.
   {
-    PrefUpdater updater(pref_service_.get());
+    PrefUpdater updater(pref_service_);
     updater.AddInstallSource("https://corp.mycompany.com/*");
   }
   EXPECT_TRUE(ReadGlobalSettings()->has_restricted_install_sources);
@@ -644,7 +641,7 @@ TEST_F(ExtensionManagementServiceTest, NewAllowedTypes) {
 
   // Set the new dictionary preference.
   {
-    PrefUpdater updater(pref_service_.get());
+    PrefUpdater updater(pref_service_);
     updater.ClearAllowedTypes();
   }
   // Verifies that the new one overrides the legacy ones.
@@ -653,7 +650,7 @@ TEST_F(ExtensionManagementServiceTest, NewAllowedTypes) {
 
   // Updates the new dictionary preference.
   {
-    PrefUpdater updater(pref_service_.get());
+    PrefUpdater updater(pref_service_);
     updater.AddAllowedType("theme");
   }
   EXPECT_TRUE(ReadGlobalSettings()->has_restricted_allowed_types);
@@ -666,7 +663,7 @@ TEST_F(ExtensionManagementServiceTest, NewAllowedTypes) {
 TEST_F(ExtensionManagementServiceTest, NewInstallBlacklist) {
   // Set the new dictionary preference.
   {
-    PrefUpdater updater(pref_service_.get());
+    PrefUpdater updater(pref_service_);
     updater.SetBlacklistedByDefault(false);  // Allowed by default.
     updater.SetIndividualExtensionInstallationAllowed(kTargetExtension, false);
     updater.ClearPerExtensionSettings(kTargetExtension2);
@@ -704,7 +701,7 @@ TEST_F(ExtensionManagementServiceTest, NewInstallBlacklist) {
 TEST_F(ExtensionManagementServiceTest, NewInstallWhitelist) {
   // Set the new dictionary preference.
   {
-    PrefUpdater updater(pref_service_.get());
+    PrefUpdater updater(pref_service_);
     updater.SetBlacklistedByDefault(true);  // Disallowed by default.
     updater.SetIndividualExtensionInstallationAllowed(kTargetExtension, true);
     updater.ClearPerExtensionSettings(kTargetExtension2);
@@ -748,7 +745,7 @@ TEST_F(ExtensionManagementServiceTest, NewInstallForcelist) {
 
   // Set the new dictionary preference.
   {
-    PrefUpdater updater(pref_service_.get());
+    PrefUpdater updater(pref_service_);
     updater.SetIndividualExtensionAutoInstalled(
         kTargetExtension, kExampleUpdateUrl, true);
   }
@@ -782,7 +779,7 @@ TEST_F(ExtensionManagementServiceTest, IsInstallationExplicitlyAllowed) {
 
   {
     // Set BlacklistedByDefault() to false.
-    PrefUpdater pref(pref_service_.get());
+    PrefUpdater pref(pref_service_);
     pref.SetBlacklistedByDefault(false);
   }
 

chrome/browser/extensions/permissions_based_management_policy_provider_unittest.cc

@@ -16,8 +16,9 @@
 #include "chrome/browser/extensions/extension_management.h"
 #include "chrome/browser/extensions/extension_management_test_util.h"
 #include "chrome/common/extensions/permissions/chrome_api_permissions.h"
-#include "components/prefs/pref_registry_simple.h"
-#include "components/prefs/testing_pref_service.h"
+#include "chrome/test/base/testing_profile.h"
+#include "components/sync_preferences/testing_pref_service_syncable.h"
+#include "content/public/test/test_browser_thread_bundle.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/manifest.h"
 #include "extensions/common/manifest_constants.h"
@@ -28,17 +29,17 @@ namespace extensions {
 
 class PermissionsBasedManagementPolicyProviderTest : public testing::Test {
  public:
-  typedef ExtensionManagementPrefUpdater<TestingPrefServiceSimple> PrefUpdater;
+  typedef ExtensionManagementPrefUpdater<
+      sync_preferences::TestingPrefServiceSyncable>
+      PrefUpdater;
 
   PermissionsBasedManagementPolicyProviderTest()
-      : pref_service_(new TestingPrefServiceSimple()),
-        settings_(new ExtensionManagement(pref_service_.get(), false)),
+      : profile_(new TestingProfile()),
+        pref_service_(profile_->GetTestingPrefService()),
+        settings_(new ExtensionManagement(profile_.get())),
         provider_(settings_.get()) {}
 
-  void SetUp() override {
-    pref_service_->registry()->RegisterDictionaryPref(
-        pref_names::kExtensionManagement);
-  }
+  void SetUp() override {}
 
   void TearDown() override {}
 
@@ -79,7 +80,10 @@ class PermissionsBasedManagementPolicyProviderTest : public testing::Test {
   }
 
  protected:
-  std::unique_ptr<TestingPrefServiceSimple> pref_service_;
+  content::TestBrowserThreadBundle test_browser_thread_bundle_;
+
+  std::unique_ptr<TestingProfile> profile_;
+  sync_preferences::TestingPrefServiceSyncable* pref_service_;
   std::unique_ptr<ExtensionManagement> settings_;
 
   PermissionsBasedManagementPolicyProvider provider_;
@@ -110,7 +114,7 @@ TEST_F(PermissionsBasedManagementPolicyProviderTest, APIPermissions) {
 
   // Blocks kProxy by default. The test extension should still be allowed.
   {
-    PrefUpdater pref(pref_service_.get());
+    PrefUpdater pref(pref_service_);
     pref.AddBlockedPermission("*",
                               GetAPIPermissionName(APIPermission::kProxy));
   }
@@ -120,7 +124,7 @@ TEST_F(PermissionsBasedManagementPolicyProviderTest, APIPermissions) {
 
   // Blocks kCookie this time. The test extension should not be allowed now.
   {
-    PrefUpdater pref(pref_service_.get());
+    PrefUpdater pref(pref_service_);
     pref.AddBlockedPermission("*",
                               GetAPIPermissionName(APIPermission::kCookie));
   }
@@ -130,7 +134,7 @@ TEST_F(PermissionsBasedManagementPolicyProviderTest, APIPermissions) {
 
   // Explictly allows kCookie for test extension. It should be allowed again.
   {
-    PrefUpdater pref(pref_service_.get());
+    PrefUpdater pref(pref_service_);
     pref.AddAllowedPermission(extension->id(),
                               GetAPIPermissionName(APIPermission::kCookie));
   }
@@ -140,7 +144,7 @@ TEST_F(PermissionsBasedManagementPolicyProviderTest, APIPermissions) {
 
   // Explictly blocks kCookie for test extension. It should still be allowed.
   {
-    PrefUpdater pref(pref_service_.get());
+    PrefUpdater pref(pref_service_);
     pref.AddBlockedPermission(extension->id(),
                               GetAPIPermissionName(APIPermission::kCookie));
   }
@@ -150,7 +154,7 @@ TEST_F(PermissionsBasedManagementPolicyProviderTest, APIPermissions) {
 
   // Any extension specific definition overrides all defaults, even if blank.
   {
-    PrefUpdater pref(pref_service_.get());
+    PrefUpdater pref(pref_service_);
     pref.UnsetBlockedPermissions(extension->id());
     pref.UnsetAllowedPermissions(extension->id());
     pref.ClearBlockedPermissions("*");
@@ -163,7 +167,7 @@ TEST_F(PermissionsBasedManagementPolicyProviderTest, APIPermissions) {
 
   // Blocks kDownloads by default. It should be blocked.
   {
-    PrefUpdater pref(pref_service_.get());
+    PrefUpdater pref(pref_service_);
     pref.UnsetPerExtensionSettings(extension->id());
     pref.UnsetPerExtensionSettings(extension->id());
     pref.ClearBlockedPermissions("*");
@@ -181,7 +185,7 @@ TEST_F(PermissionsBasedManagementPolicyProviderTest, APIPermissions) {
   const std::string blocked_install_message =
       "Visit https://example.com/exception";
   {
-    PrefUpdater pref(pref_service_.get());
+    PrefUpdater pref(pref_service_);
     pref.UnsetPerExtensionSettings(extension->id());
     pref.UnsetPerExtensionSettings(extension->id());
     pref.SetBlockedInstallMessage(extension->id(), blocked_install_message);

chrome/browser/extensions/standard_management_policy_provider_unittest.cc

@@ -11,9 +11,8 @@
 #include "base/values.h"
 #include "chrome/browser/extensions/blacklist.h"
 #include "chrome/browser/extensions/extension_management.h"
-#include "chrome/browser/extensions/test_extension_prefs.h"
+#include "chrome/test/base/testing_profile.h"
 #include "content/public/test/test_browser_thread_bundle.h"
-#include "extensions/browser/extension_prefs.h"
 #include "extensions/common/extension_builder.h"
 #include "extensions/common/manifest_constants.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -23,22 +22,17 @@ namespace extensions {
 class StandardManagementPolicyProviderTest : public testing::Test {
  public:
   StandardManagementPolicyProviderTest()
-      : prefs_(base::ThreadTaskRunnerHandle::Get()),
-        settings_(new ExtensionManagement(prefs()->pref_service(), false)),
+      : settings_(new ExtensionManagement(&profile_)),
         provider_(settings_.get()) {}
 
  protected:
-  ExtensionPrefs* prefs() {
-    return prefs_.prefs();
-  }
-
   scoped_refptr<const Extension> CreateExtension(Manifest::Location location) {
     return ExtensionBuilder("test").SetLocation(location).Build();
   }
 
   content::TestBrowserThreadBundle test_browser_thread_bundle_;
 
-  TestExtensionPrefs prefs_;
+  TestingProfile profile_;
   std::unique_ptr<ExtensionManagement> settings_;
 
   StandardManagementPolicyProvider provider_;