[Policy] Run ExtensionPolicyMigrators on user cloud policies

Previously, ExtensionPolicyMigrators only ran on platform policies, and
machine-level user cloud policies. This leaves out user cloud policies,
and CrOS-specific policy providers.

This CL makes the list of ExtensionPolicyMigrators static, instead of
each provider keeping a separate list. This ensures no provider is left
out of the policy migrator logic.

TESTED=checked that the LBS extension picks up `BrowserSwitcherEnabled`
    when it's set through user cloud policy

Bug: 1043775, 1030607
Change-Id: Ib257d2f18e3a106cbcd04d957b84a09b005a4680
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2013485Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Commit-Queue: Nicolas Ouellet-Payeur <nicolaso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#734461}

chrome/browser/browser_switcher/browser_switcher_policy_migrator.cc

@@ -51,8 +51,6 @@ void BrowserSwitcherPolicyMigrator::Migrate(policy::PolicyBundle* bundle) {
       policy::POLICY_DOMAIN_EXTENSIONS, kLBSExtensionId));
   policy::PolicyMap& chrome_map =
       bundle->Get(policy::PolicyNamespace(policy::POLICY_DOMAIN_CHROME, ""));
-  if (extension_map.empty())
-    return;
 
   const auto* entry = chrome_map.Get("BrowserSwitcherEnabled");
   if (!entry || !entry->value || !entry->value->GetBool())

chrome/browser/policy/chrome_browser_policy_connector.cc

@@ -60,13 +60,6 @@ namespace policy {
 
 namespace {
 
-void AddMigrators(ConfigurationPolicyProvider* provider) {
-#if defined(OS_WIN)
-  provider->AddMigrator(
-      std::make_unique<browser_switcher::BrowserSwitcherPolicyMigrator>());
-#endif
-}
-
 bool ProviderHasPolicies(const ConfigurationPolicyProvider* provider) {
   if (!provider)
     return false;
@@ -142,11 +135,18 @@ ChromeBrowserPolicyConnector::GetPlatformProvider() {
 
 std::vector<std::unique_ptr<policy::ConfigurationPolicyProvider>>
 ChromeBrowserPolicyConnector::CreatePolicyProviders() {
+  // Assign ExtensionPolicyMigrators before any policy providers are created.
+#if defined(OS_WIN)
+  std::vector<std::unique_ptr<ExtensionPolicyMigrator>> migrators;
+  migrators.push_back(
+      std::make_unique<browser_switcher::BrowserSwitcherPolicyMigrator>());
+  ConfigurationPolicyProvider::SetMigrators(std::move(migrators));
+#endif
+
   auto providers = BrowserPolicyConnector::CreatePolicyProviders();
   std::unique_ptr<ConfigurationPolicyProvider> platform_provider =
       CreatePlatformProvider();
   if (platform_provider) {
-    AddMigrators(platform_provider.get());
     platform_provider_ = platform_provider.get();
     // PlatformProvider should be before all other providers (highest priority).
     providers.insert(providers.begin(), std::move(platform_provider));
@@ -158,7 +158,6 @@ ChromeBrowserPolicyConnector::CreatePolicyProviders() {
           ChromeBrowserCloudManagementController::CreatePolicyManager(
               platform_provider_);
   if (machine_level_user_cloud_policy_manager) {
-    AddMigrators(machine_level_user_cloud_policy_manager.get());
     machine_level_user_cloud_policy_manager_ =
         machine_level_user_cloud_policy_manager.get();
     providers.push_back(std::move(machine_level_user_cloud_policy_manager));

components/policy/core/common/configuration_policy_provider.cc

@@ -5,12 +5,21 @@
 #include "components/policy/core/common/configuration_policy_provider.h"
 
 #include "base/callback.h"
+#include "base/lazy_instance.h"
 #include "components/policy/core/common/extension_policy_migrator.h"
 #include "components/policy/core/common/external_data_fetcher.h"
 #include "components/policy/core/common/policy_map.h"
 
 namespace policy {
 
+namespace {
+
+// static
+base::LazyInstance<std::vector<std::unique_ptr<ExtensionPolicyMigrator>>>::Leaky
+    g_migrators = LAZY_INSTANCE_INITIALIZER;
+
+}  // namespace
+
 ConfigurationPolicyProvider::Observer::~Observer() = default;
 
 ConfigurationPolicyProvider::ConfigurationPolicyProvider()
@@ -20,6 +29,12 @@ ConfigurationPolicyProvider::~ConfigurationPolicyProvider() {
   DCHECK(!initialized_);
 }
 
+// static
+void ConfigurationPolicyProvider::SetMigrators(
+    std::vector<std::unique_ptr<ExtensionPolicyMigrator>> migrators) {
+  g_migrators.Get() = std::move(migrators);
+}
+
 void ConfigurationPolicyProvider::Init(SchemaRegistry* registry) {
   schema_registry_ = registry;
   schema_registry_->AddObserver(this);
@@ -41,16 +56,10 @@ bool ConfigurationPolicyProvider::IsInitializationComplete(
   return true;
 }
 
-void ConfigurationPolicyProvider::AddMigrator(
-    std::unique_ptr<ExtensionPolicyMigrator> migrator) {
-  DCHECK(migrator);
-  migrators_.push_back(std::move(migrator));
-}
-
 void ConfigurationPolicyProvider::UpdatePolicy(
     std::unique_ptr<PolicyBundle> bundle) {
   if (bundle) {
-    for (const auto& migrator : migrators_)
+    for (const auto& migrator : g_migrators.Get())
       migrator->Migrate(bundle.get());
     policy_bundle_.Swap(bundle.get());
   } else {

components/policy/core/common/configuration_policy_provider.h

@@ -39,6 +39,9 @@ class POLICY_EXPORT ConfigurationPolicyProvider
   // to post to the FILE thread, for example.
   ~ConfigurationPolicyProvider() override;
 
+  static void SetMigrators(
+      std::vector<std::unique_ptr<ExtensionPolicyMigrator>> migrators);
+
   // Invoked as soon as the main message loops are spinning. Policy providers
   // are created early during startup to provide the initial policies; the
   // Init() call allows them to perform initialization tasks that require
@@ -73,10 +76,6 @@ class POLICY_EXPORT ConfigurationPolicyProvider
   virtual void AddObserver(Observer* observer);
   virtual void RemoveObserver(Observer* observer);
 
-  // Adds an ExtensionPolicyMigrator to be run before OnUpdatePolicy() is
-  // called.
-  void AddMigrator(std::unique_ptr<ExtensionPolicyMigrator> migrator);
-
   // SchemaRegistry::Observer:
   void OnSchemaRegistryUpdated(bool has_new_schemas) override;
   void OnSchemaRegistryReady() override;
@@ -103,8 +102,6 @@ class POLICY_EXPORT ConfigurationPolicyProvider
 
   base::ObserverList<Observer, true>::Unchecked observer_list_;
 
-  std::vector<std::unique_ptr<ExtensionPolicyMigrator>> migrators_;
-
   DISALLOW_COPY_AND_ASSIGN(ConfigurationPolicyProvider);
 };
 

components/policy/core/common/configuration_policy_provider_test.cc

@@ -137,6 +137,7 @@ void PolicyTestBase::SetUp() {
 
 void PolicyTestBase::TearDown() {
   task_environment_.RunUntilIdle();
+  ConfigurationPolicyProvider::SetMigrators({});
 }
 
 bool PolicyTestBase::RegisterSchema(const PolicyNamespace& ns,
@@ -350,9 +351,11 @@ class MockPolicyMigrator : public ExtensionPolicyMigrator {
 };
 
 TEST_P(ConfigurationPolicyProviderTest, AddMigrator) {
-  MockPolicyMigrator* migrator = new MockPolicyMigrator;
+  auto migrator = std::make_unique<MockPolicyMigrator>();
   EXPECT_CALL(*migrator, Migrate(_));
-  provider_->AddMigrator(std::unique_ptr<ExtensionPolicyMigrator>(migrator));
+  std::vector<std::unique_ptr<ExtensionPolicyMigrator>> migrators;
+  migrators.emplace_back(std::move(migrator));
+  ConfigurationPolicyProvider::SetMigrators(std::move(migrators));
   provider_->RefreshPolicies();
   task_environment_.RunUntilIdle();
 }