cleanup how ConfigurationPolicyProviders are set

It's expected that the providers are available when the service is
committed, so it make it explicit.

BUG=none
TEST=none

Change-Id: Ibc2f9be6ecec9261e1a2b9ebef18b16557882398
Reviewed-on: https://chromium-review.googlesource.com/890722
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533126}

android_webview/browser/aw_browser_policy_connector.cc

@@ -62,14 +62,17 @@ std::unique_ptr<policy::ConfigurationPolicyHandlerList> BuildHandlerList(
 }  // namespace
 
 AwBrowserPolicyConnector::AwBrowserPolicyConnector()
-   : BrowserPolicyConnectorBase(base::Bind(&BuildHandlerList)) {
+    : BrowserPolicyConnectorBase(base::Bind(&BuildHandlerList)) {}
+
+AwBrowserPolicyConnector::~AwBrowserPolicyConnector() = default;
+
+std::vector<std::unique_ptr<policy::ConfigurationPolicyProvider>>
+AwBrowserPolicyConnector::CreatePolicyProviders() {
   std::vector<std::unique_ptr<policy::ConfigurationPolicyProvider>> providers;
   providers.push_back(
       std::make_unique<policy::android::AndroidCombinedPolicyProvider>(
           GetSchemaRegistry()));
-  SetPolicyProviders(std::move(providers));
+  return providers;
 }
 
-AwBrowserPolicyConnector::~AwBrowserPolicyConnector() {}
-
 }  // namespace android_webview

android_webview/browser/aw_browser_policy_connector.h

@@ -13,11 +13,16 @@ namespace android_webview {
 // Sets up and keeps the browser-global policy objects such as the PolicyService
 // and the platform-specific PolicyProvider.
 class AwBrowserPolicyConnector : public policy::BrowserPolicyConnectorBase {
-public:
+ public:
   AwBrowserPolicyConnector();
   ~AwBrowserPolicyConnector() override;
 
-private:
+ protected:
+  // policy::BrowserPolicyConnectorBase:
+  std::vector<std::unique_ptr<policy::ConfigurationPolicyProvider>>
+  CreatePolicyProviders() override;
+
+ private:
   DISALLOW_COPY_AND_ASSIGN(AwBrowserPolicyConnector);
 };
 

chrome/browser/browser_process_impl.cc

@@ -653,7 +653,6 @@ BrowserProcessImpl::browser_policy_connector() {
     DCHECK(!browser_policy_connector_);
     browser_policy_connector_ = platform_part_->CreateBrowserPolicyConnector();
     created_browser_policy_connector_ = true;
-    browser_policy_connector_->InitPolicyProviders();
   }
   return browser_policy_connector_.get();
 }

chrome/browser/chromeos/policy/browser_policy_connector_chromeos.cc

@@ -355,11 +355,13 @@ void BrowserPolicyConnectorChromeOS::OnDeviceCloudPolicyManagerDisconnected() {
   RestartDeviceCloudPolicyInitializer();
 }
 
-void BrowserPolicyConnectorChromeOS::BuildPolicyProviders(
-    std::vector<std::unique_ptr<ConfigurationPolicyProvider>>* providers) {
+std::vector<std::unique_ptr<policy::ConfigurationPolicyProvider>>
+BrowserPolicyConnectorChromeOS::CreatePolicyProviders() {
+  auto providers = ChromeBrowserPolicyConnector::CreatePolicyProviders();
   for (auto& provider_ptr : providers_for_init_)
-    providers->push_back(std::move(provider_ptr));
+    providers.push_back(std::move(provider_ptr));
   providers_for_init_.clear();
+  return providers;
 }
 
 void BrowserPolicyConnectorChromeOS::SetTimezoneIfPolicyAvailable() {

chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h

@@ -180,9 +180,8 @@ class BrowserPolicyConnectorChromeOS
 
  protected:
   // ChromeBrowserPolicyConnector:
-  void BuildPolicyProviders(
-      std::vector<std::unique_ptr<ConfigurationPolicyProvider>>* providers)
-      override;
+  std::vector<std::unique_ptr<policy::ConfigurationPolicyProvider>>
+  CreatePolicyProviders() override;
 
  private:
   // Set the timezone as soon as the policies are available.

chrome/browser/chromeos/policy/cloud_external_data_policy_observer_unittest.cc

@@ -334,8 +334,7 @@ void CloudExternalDataPolicyObserverTest::LogInAsDeviceLocalAccount(
   providers.push_back(device_local_account_policy_provider_.get());
   TestingProfile::Builder builder;
   std::unique_ptr<PolicyServiceImpl> policy_service =
-      std::make_unique<PolicyServiceImpl>();
-  policy_service->SetProviders(providers);
+      std::make_unique<PolicyServiceImpl>(std::move(providers));
   builder.SetPolicyService(std::move(policy_service));
   builder.SetPath(chromeos::ProfileHelper::Get()->GetProfilePathByUserIdHash(
       chromeos::ProfileHelper::GetUserIdHashByUserIdForTesting(
@@ -370,8 +369,7 @@ void CloudExternalDataPolicyObserverTest::LogInAsRegularUser() {
   providers.push_back(&user_policy_provider_);
   TestingProfile::Builder builder;
   std::unique_ptr<PolicyServiceImpl> policy_service =
-      std::make_unique<PolicyServiceImpl>();
-  policy_service->SetProviders(providers);
+      std::make_unique<PolicyServiceImpl>(std::move(providers));
   builder.SetPolicyService(std::move(policy_service));
   builder.SetPath(chromeos::ProfileHelper::Get()->GetProfilePathByUserIdHash(
       chromeos::ProfileHelper::GetUserIdHashByUserIdForTesting(

chrome/browser/chromeos/policy/network_configuration_updater_unittest.cc

@@ -220,8 +220,7 @@ class NetworkConfigurationUpdaterTest : public testing::Test {
     provider_.Init();
     PolicyServiceImpl::Providers providers;
     providers.push_back(&provider_);
-    policy_service_ = std::make_unique<PolicyServiceImpl>();
-    policy_service_->SetProviders(providers);
+    policy_service_ = std::make_unique<PolicyServiceImpl>(std::move(providers));
 
     std::unique_ptr<base::DictionaryValue> fake_toplevel_onc =
         chromeos::onc::ReadDictionaryFromJson(kFakeONC);

chrome/browser/policy/chrome_browser_policy_connector.cc

@@ -71,18 +71,6 @@ void ChromeBrowserPolicyConnector::OnResourceBundleCreated() {
   BrowserPolicyConnectorBase::OnResourceBundleCreated();
 }
 
-void ChromeBrowserPolicyConnector::InitPolicyProviders() {
-  std::vector<std::unique_ptr<ConfigurationPolicyProvider>> providers;
-  std::unique_ptr<ConfigurationPolicyProvider> platform_provider =
-      CreatePlatformProvider();
-  if (platform_provider) {
-    platform_provider_ = platform_provider.get();
-    providers.push_back(std::move(platform_provider));
-  }
-  BuildPolicyProviders(&providers);
-  SetPolicyProviders(std::move(providers));
-}
-
 void ChromeBrowserPolicyConnector::Init(
     PrefService* local_state,
     scoped_refptr<net::URLRequestContextGetter> request_context) {
@@ -104,6 +92,19 @@ ChromeBrowserPolicyConnector::GetPlatformProvider() {
   return provider ? provider : platform_provider_;
 }
 
+std::vector<std::unique_ptr<policy::ConfigurationPolicyProvider>>
+ChromeBrowserPolicyConnector::CreatePolicyProviders() {
+  auto providers = BrowserPolicyConnector::CreatePolicyProviders();
+  std::unique_ptr<ConfigurationPolicyProvider> platform_provider =
+      CreatePlatformProvider();
+  if (platform_provider) {
+    platform_provider_ = platform_provider.get();
+    // PlatformProvider should be before all other providers (highest priority).
+    providers.insert(providers.begin(), std::move(platform_provider));
+  }
+  return providers;
+}
+
 std::unique_ptr<ConfigurationPolicyProvider>
 ChromeBrowserPolicyConnector::CreatePlatformProvider() {
 #if defined(OS_WIN)
@@ -140,7 +141,4 @@ ChromeBrowserPolicyConnector::CreatePlatformProvider() {
 #endif
 }
 
-void ChromeBrowserPolicyConnector::BuildPolicyProviders(
-    std::vector<std::unique_ptr<ConfigurationPolicyProvider>>* providers) {}
-
 }  // namespace policy

chrome/browser/policy/chrome_browser_policy_connector.h

@@ -42,9 +42,6 @@ class ChromeBrowserPolicyConnector : public BrowserPolicyConnector {
   // class to notify observers.
   void OnResourceBundleCreated();
 
-  // TODO(sky): remove. Temporary until resolve ordering.
-  void InitPolicyProviders();
-
   void Init(
       PrefService* local_state,
       scoped_refptr<net::URLRequestContextGetter> request_context) override;
@@ -52,12 +49,9 @@ class ChromeBrowserPolicyConnector : public BrowserPolicyConnector {
   ConfigurationPolicyProvider* GetPlatformProvider();
 
  protected:
-  // Called from Init() to build the list of ConfigurationPolicyProviders that
-  // is supplied to SetPolicyProviders(). This implementation does nothing
-  // and is provided for subclasses. NOTE: |providers| may already contain
-  // some providers, generally subclasses should append.
-  virtual void BuildPolicyProviders(
-      std::vector<std::unique_ptr<ConfigurationPolicyProvider>>* providers);
+  // BrowserPolicyConnector:
+  std::vector<std::unique_ptr<policy::ConfigurationPolicyProvider>>
+  CreatePolicyProviders() override;
 
  private:
   std::unique_ptr<ConfigurationPolicyProvider> CreatePlatformProvider();

chrome/browser/policy/profile_policy_connector.cc

@@ -104,10 +104,7 @@ void ProfilePolicyConnector::Init(
   }
 #endif
 
-  std::unique_ptr<PolicyServiceImpl> policy_service =
-      std::make_unique<PolicyServiceImpl>();
-  policy_service->SetProviders(policy_providers_);
-  policy_service_ = std::move(policy_service);
+  policy_service_ = std::make_unique<PolicyServiceImpl>(policy_providers_);
 
 #if defined(OS_CHROMEOS)
   if (is_primary_user_) {

chrome/browser/policy/profile_policy_connector_factory.cc

@@ -154,8 +154,7 @@ ProfilePolicyConnectorFactory::CreateForBrowserContextInternal(
     providers.push_back(test_providers_.front());
     test_providers_.pop_front();
     std::unique_ptr<PolicyServiceImpl> service =
-        std::make_unique<PolicyServiceImpl>();
-    service->SetProviders(providers);
+        std::make_unique<PolicyServiceImpl>(std::move(providers));
     connector->InitForTesting(std::move(service));
   }
 

chrome/browser/prefs/proxy_policy_unittest.cc

@@ -98,8 +98,7 @@ class ProxyPolicyTest : public testing::Test {
 
     PolicyServiceImpl::Providers providers;
     providers.push_back(&provider_);
-    policy_service_ = std::make_unique<PolicyServiceImpl>();
-    policy_service_->SetProviders(providers);
+    policy_service_ = std::make_unique<PolicyServiceImpl>(std::move(providers));
     provider_.Init();
   }
 

chrome/test/base/testing_profile.cc

@@ -800,8 +800,7 @@ void TestingProfile::CreateProfilePolicyConnector() {
   if (!policy_service_) {
     std::vector<policy::ConfigurationPolicyProvider*> providers;
     std::unique_ptr<policy::PolicyServiceImpl> policy_service =
-        std::make_unique<policy::PolicyServiceImpl>();
-    policy_service->SetProviders(providers);
+        std::make_unique<policy::PolicyServiceImpl>(std::move(providers));
     policy_service_ = std::move(policy_service);
   }
   profile_policy_connector_.reset(new policy::ProfilePolicyConnector());

components/policy/core/browser/browser_policy_connector_base.cc

@@ -26,8 +26,7 @@ ConfigurationPolicyProvider* g_testing_provider = nullptr;
 }  // namespace
 
 BrowserPolicyConnectorBase::BrowserPolicyConnectorBase(
-    const HandlerListFactory& handler_list_factory)
-    : is_initialized_(false) {
+    const HandlerListFactory& handler_list_factory) {
   // GetPolicyService() must be ready after the constructor is done.
   // The connector is created very early during startup, when the browser
   // threads aren't running yet; initialize components that need local_state,
@@ -56,10 +55,8 @@ void BrowserPolicyConnectorBase::Shutdown() {
   is_initialized_ = false;
   if (g_testing_provider)
     g_testing_provider->Shutdown();
-  if (policy_providers_) {
-    for (const auto& provider : *policy_providers_)
+  for (const auto& provider : policy_providers_)
     provider->Shutdown();
-  }
   // Drop g_testing_provider so that tests executed with --single_process can
   // call SetPolicyProviderForTesting() again. It is still owned by the test.
   g_testing_provider = nullptr;
@@ -75,12 +72,23 @@ CombinedSchemaRegistry* BrowserPolicyConnectorBase::GetSchemaRegistry() {
 }
 
 PolicyService* BrowserPolicyConnectorBase::GetPolicyService() {
-  if (!policy_service_) {
+  if (policy_service_)
+    return policy_service_.get();
+
+  DCHECK(!is_initialized_);
+  is_initialized_ = true;
+
+  policy_providers_ = CreatePolicyProviders();
+
+  if (g_testing_provider)
+    g_testing_provider->Init(GetSchemaRegistry());
+
+  for (const auto& provider : policy_providers_)
+    provider->Init(GetSchemaRegistry());
+
   g_created_policy_service = true;
-    policy_service_ = std::make_unique<PolicyServiceImpl>();
-    if (policy_providers_ || g_testing_provider)
-      policy_service_->SetProviders(GetProvidersForPolicyService());
-  }
+  policy_service_ =
+      std::make_unique<PolicyServiceImpl>(GetProvidersForPolicyService());
   return policy_service_.get();
 }
 
@@ -111,32 +119,6 @@ BrowserPolicyConnectorBase::GetPolicyProviderForTesting() {
   return g_testing_provider;
 }
 
-void BrowserPolicyConnectorBase::SetPolicyProviders(
-    std::vector<std::unique_ptr<ConfigurationPolicyProvider>> providers) {
-  // SetPolicyProviders() should only called once.
-  DCHECK(!is_initialized_);
-  policy_providers_ = std::move(providers);
-
-  if (g_testing_provider)
-    g_testing_provider->Init(GetSchemaRegistry());
-
-  for (const auto& provider : *policy_providers_)
-    provider->Init(GetSchemaRegistry());
-
-  is_initialized_ = true;
-
-  if (policy_service_) {
-    if (!policy_service_->has_providers()) {
-      policy_service_->SetProviders(GetProvidersForPolicyService());
-    } else {
-      // GetPolicyService() triggers calling SetProviders() if
-      // |g_testing_provider| has been set. That's the only way that should
-      // result in ending up in this branch.
-      DCHECK(g_testing_provider);
-    }
-  }
-}
-
 std::vector<ConfigurationPolicyProvider*>
 BrowserPolicyConnectorBase::GetProvidersForPolicyService() {
   std::vector<ConfigurationPolicyProvider*> providers;
@@ -144,12 +126,17 @@ BrowserPolicyConnectorBase::GetProvidersForPolicyService() {
     providers.push_back(g_testing_provider);
     return providers;
   }
-  providers.reserve(policy_providers_->size());
-  for (const auto& policy : *policy_providers_)
+  providers.reserve(policy_providers_.size());
+  for (const auto& policy : policy_providers_)
     providers.push_back(policy.get());
   return providers;
 }
 
+std::vector<std::unique_ptr<ConfigurationPolicyProvider>>
+BrowserPolicyConnectorBase::CreatePolicyProviders() {
+  return {};
+}
+
 void BrowserPolicyConnectorBase::OnResourceBundleCreated() {
   std::vector<base::OnceClosure> resource_bundle_callbacks;
   std::swap(resource_bundle_callbacks, resource_bundle_callbacks_);

components/policy/core/browser/browser_policy_connector_base.h

@@ -10,7 +10,6 @@
 
 #include "base/callback_forward.h"
 #include "base/macros.h"
-#include "base/optional.h"
 #include "components/policy/core/browser/configuration_policy_handler_list.h"
 #include "components/policy/core/common/schema.h"
 #include "components/policy/core/common/schema_registry.h"
@@ -73,10 +72,11 @@ class POLICY_EXPORT BrowserPolicyConnectorBase {
   explicit BrowserPolicyConnectorBase(
       const HandlerListFactory& handler_list_factory);
 
-  // Sets the set of providers, in decreasing order of priority. May only be
-  // called once.
-  void SetPolicyProviders(
-      std::vector<std::unique_ptr<ConfigurationPolicyProvider>> providers);
+  // Called from GetPolicyService() to create the set of
+  // ConfigurationPolicyProviders that are used, in decreasing order of
+  // priority.
+  virtual std::vector<std::unique_ptr<ConfigurationPolicyProvider>>
+  CreatePolicyProviders();
 
   // Must be called when ui::ResourceBundle has been loaded, results in running
   // any callbacks scheduled in NotifyWhenResourceBundleReady().
@@ -88,8 +88,10 @@ class POLICY_EXPORT BrowserPolicyConnectorBase {
   // called.
   std::vector<ConfigurationPolicyProvider*> GetProvidersForPolicyService();
 
-  // Whether SetPolicyProviders() but not Shutdown() has been invoked.
-  bool is_initialized_;
+  // Set to true when the PolicyService has been created, and false in
+  // Shutdown(). Once created the PolicyService is destroyed in the destructor,
+  // not Shutdown().
+  bool is_initialized_ = false;
 
   // Used to convert policies to preferences. The providers declared below
   // may trigger policy updates during shutdown, which will result in
@@ -105,8 +107,7 @@ class POLICY_EXPORT BrowserPolicyConnectorBase {
   CombinedSchemaRegistry schema_registry_;
 
   // The browser-global policy providers, in decreasing order of priority.
-  base::Optional<std::vector<std::unique_ptr<ConfigurationPolicyProvider>>>
-      policy_providers_;
+  std::vector<std::unique_ptr<ConfigurationPolicyProvider>> policy_providers_;
 
   // Must be deleted before all the policy providers.
   std::unique_ptr<PolicyServiceImpl> policy_service_;

components/policy/core/browser/configuration_policy_pref_store_test.cc

@@ -30,8 +30,7 @@ ConfigurationPolicyPrefStoreTest::ConfigurationPolicyPrefStoreTest()
       .WillRepeatedly(Return(false));
   provider_.Init();
   providers_.push_back(&provider_);
-  policy_service_ = std::make_unique<PolicyServiceImpl>();
-  policy_service_->SetProviders(providers_);
+  policy_service_ = std::make_unique<PolicyServiceImpl>(providers_);
   store_ = new ConfigurationPolicyPrefStore(
       nullptr, policy_service_.get(), &handler_list_, POLICY_LEVEL_MANDATORY);
 }

components/policy/core/browser/proxy_policy_handler_unittest.cc

@@ -32,8 +32,7 @@ class ProxyPolicyHandlerTest
     // preprocessor. The previous store must be nulled out first so that it
     // removes itself from the service's observer list.
     store_ = nullptr;
-    policy_service_ = std::make_unique<PolicyServiceImpl>();
-    policy_service_->SetProviders(providers_);
+    policy_service_ = std::make_unique<PolicyServiceImpl>(providers_);
     store_ = new ConfigurationPolicyPrefStore(
         nullptr, policy_service_.get(), &handler_list_, POLICY_LEVEL_MANDATORY);
   }

components/policy/core/common/policy_service_impl.cc

@@ -72,25 +72,12 @@ void RemapProxyPolicies(PolicyMap* policies) {
 
 }  // namespace
 
-PolicyServiceImpl::PolicyServiceImpl() : update_task_ptr_factory_(this) {
-  for (int domain = 0; domain < POLICY_DOMAIN_SIZE; ++domain)
-    initialization_complete_[domain] = false;
-}
-
-PolicyServiceImpl::~PolicyServiceImpl() {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  if (providers_) {
-    for (auto* provider : *providers_)
-      provider->RemoveObserver(this);
-  }
-}
-
-void PolicyServiceImpl::SetProviders(Providers providers) {
-  DCHECK(!providers_);
+PolicyServiceImpl::PolicyServiceImpl(Providers providers)
+    : update_task_ptr_factory_(this) {
   providers_ = std::move(providers);
   for (int domain = 0; domain < POLICY_DOMAIN_SIZE; ++domain)
     initialization_complete_[domain] = true;
-  for (auto* provider : *providers_) {
+  for (auto* provider : providers_) {
     provider->AddObserver(this);
     for (int domain = 0; domain < POLICY_DOMAIN_SIZE; ++domain) {
       initialization_complete_[domain] &=
@@ -102,6 +89,12 @@ void PolicyServiceImpl::SetProviders(Providers providers) {
   MergeAndTriggerUpdates();
 }
 
+PolicyServiceImpl::~PolicyServiceImpl() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  for (auto* provider : providers_)
+    provider->RemoveObserver(this);
+}
+
 void PolicyServiceImpl::AddObserver(PolicyDomain domain,
                                     PolicyService::Observer* observer) {
   DCHECK(thread_checker_.CalledOnValidThread());
@@ -143,7 +136,7 @@ void PolicyServiceImpl::RefreshPolicies(const base::Closure& callback) {
   if (!callback.is_null())
     refresh_callbacks_.push_back(callback);
 
-  if (!providers_ || providers_->empty()) {
+  if (providers_.empty()) {
     // Refresh is immediately complete if there are no providers. See the note
     // on OnUpdatePolicy() about why this is a posted task.
     update_task_ptr_factory_.InvalidateWeakPtrs();
@@ -153,15 +146,15 @@ void PolicyServiceImpl::RefreshPolicies(const base::Closure& callback) {
   } else {
     // Some providers might invoke OnUpdatePolicy synchronously while handling
     // RefreshPolicies. Mark all as pending before refreshing.
-    for (auto* provider : *providers_)
+    for (auto* provider : providers_)
       refresh_pending_.insert(provider);
-    for (auto* provider : *providers_)
+    for (auto* provider : providers_)
       provider->RefreshPolicies();
   }
 }
 
 void PolicyServiceImpl::OnUpdatePolicy(ConfigurationPolicyProvider* provider) {
-  DCHECK_EQ(1, std::count(providers_->begin(), providers_->end(), provider));
+  DCHECK_EQ(1, std::count(providers_.begin(), providers_.end(), provider));
   refresh_pending_.erase(provider);
 
   // Note: a policy change may trigger further policy changes in some providers.
@@ -194,14 +187,12 @@ void PolicyServiceImpl::MergeAndTriggerUpdates() {
   // Merge from each provider in their order of priority.
   const PolicyNamespace chrome_namespace(POLICY_DOMAIN_CHROME, std::string());
   PolicyBundle bundle;
-  if (providers_) {
-    for (auto* provider : *providers_) {
+  for (auto* provider : providers_) {
     PolicyBundle provided_bundle;
     provided_bundle.CopyFrom(provider->policies());
     RemapProxyPolicies(&provided_bundle.Get(chrome_namespace));
     bundle.MergeFrom(provided_bundle);
   }
-  }
 
   // Swap first, so that observers that call GetPolicies() see the current
   // values.
@@ -247,9 +238,6 @@ void PolicyServiceImpl::MergeAndTriggerUpdates() {
 void PolicyServiceImpl::CheckInitializationComplete() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  if (!providers_)
-    return;
-
   // Check if all the providers just became initialized for each domain; if so,
   // notify that domain's observers.
   for (int domain = 0; domain < POLICY_DOMAIN_SIZE; ++domain) {
@@ -259,7 +247,7 @@ void PolicyServiceImpl::CheckInitializationComplete() {
     PolicyDomain policy_domain = static_cast<PolicyDomain>(domain);
 
     bool all_complete = true;
-    for (auto* provider : *providers_) {
+    for (auto* provider : providers_) {
       if (!provider->IsInitializationComplete(policy_domain)) {
         all_complete = false;
         break;

components/policy/core/common/policy_service_impl.h

@@ -15,7 +15,6 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
-#include "base/optional.h"
 #include "base/threading/thread_checker.h"
 #include "components/policy/core/common/configuration_policy_provider.h"
 #include "components/policy/core/common/policy_bundle.h"
@@ -32,18 +31,11 @@ class POLICY_EXPORT PolicyServiceImpl
  public:
   using Providers = std::vector<ConfigurationPolicyProvider*>;
 
-  // Creates a new PolicyServiceImpl, it is expected SetProviders() is called
-  // once to complete initialization.
-  PolicyServiceImpl();
-
+  // Creates a new PolicyServiceImpl with the list of
+  // ConfigurationPolicyProviders, in order of decreasing priority.
+  explicit PolicyServiceImpl(Providers providers);
   ~PolicyServiceImpl() override;
 
-  // Sets the providers; see description of constructor for details.
-  void SetProviders(Providers providers);
-
-  // Returns true if SetProviders() was called.
-  bool has_providers() const { return providers_.has_value(); }
-
   // PolicyService overrides:
   void AddObserver(PolicyDomain domain,
                    PolicyService::Observer* observer) override;
@@ -76,8 +68,8 @@ class POLICY_EXPORT PolicyServiceImpl
   // Invokes all the refresh callbacks if there are no more refreshes pending.
   void CheckRefreshComplete();
 
-  // The providers set via SetProviders(), in order of decreasing priority.
-  base::Optional<Providers> providers_;
+  // The providers, in order of decreasing priority.
+  Providers providers_;
 
   // Maps each policy namespace to its current policies.
   PolicyBundle policy_bundle_;

components/policy/core/common/policy_service_impl_unittest.cc

@@ -120,8 +120,7 @@ class PolicyServiceTest : public testing::Test {
     providers.push_back(&provider0_);
     providers.push_back(&provider1_);
     providers.push_back(&provider2_);
-    policy_service_ = std::make_unique<PolicyServiceImpl>();
-    policy_service_->SetProviders(providers);
+    policy_service_ = std::make_unique<PolicyServiceImpl>(std::move(providers));
   }
 
   void TearDown() override {
@@ -561,8 +560,7 @@ TEST_F(PolicyServiceTest, IsInitializationComplete) {
   providers.push_back(&provider0_);
   providers.push_back(&provider1_);
   providers.push_back(&provider2_);
-  policy_service_ = std::make_unique<PolicyServiceImpl>();
-  policy_service_->SetProviders(providers);
+  policy_service_ = std::make_unique<PolicyServiceImpl>(std::move(providers));
   EXPECT_FALSE(policy_service_->IsInitializationComplete(POLICY_DOMAIN_CHROME));
   EXPECT_FALSE(
       policy_service_->IsInitializationComplete(POLICY_DOMAIN_EXTENSIONS));

remoting/host/policy_watcher.cc

@@ -376,8 +376,7 @@ std::unique_ptr<PolicyWatcher> PolicyWatcher::CreateFromPolicyLoader(
   policy::PolicyServiceImpl::Providers providers;
   providers.push_back(policy_provider.get());
   std::unique_ptr<policy::PolicyServiceImpl> policy_service =
-      std::make_unique<policy::PolicyServiceImpl>();
-  policy_service->SetProviders(providers);
+      std::make_unique<policy::PolicyServiceImpl>(std::move(providers));
 
   policy::PolicyService* borrowed_policy_service = policy_service.get();
   return base::WrapUnique(new PolicyWatcher(