From 01d2b7eaf54a67e9cbcb1dbb563deb1229e5960e Mon Sep 17 00:00:00 2001 From: "pneubeck@chromium.org" Date: Wed, 5 Jun 2013 21:04:12 +0000 Subject: [PATCH] Let NetworkConfigurationUpdater wait for PolicyService::IsInitializationComplete. This fixes a race between policy initialization and the NetworkConfigurationUpdater applying an empty user policy. This change is on purpose made as local as possible to be mergeable to R28. In a follow-up change, the observer will be integrated into the NetworkConfigurationUpdater. BUG=236094, 242668, 246753 Review URL: https://chromiumcodereview.appspot.com/16439007 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@204337 0039d316-1c4b-4281-b951-d872f2087c98 --- .../policy/profile_policy_connector.cc | 63 ++++++++++++++++++- .../browser/policy/profile_policy_connector.h | 4 ++ 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/chrome/browser/policy/profile_policy_connector.cc b/chrome/browser/policy/profile_policy_connector.cc index bd2f289d6c151a..f7e0e5790ea5ef 100644 --- a/chrome/browser/policy/profile_policy_connector.cc +++ b/chrome/browser/policy/profile_policy_connector.cc @@ -18,6 +18,7 @@ #if defined(OS_CHROMEOS) #include "base/bind.h" +#include "base/message_loop.h" #include "base/prefs/pref_service.h" #include "chrome/browser/chromeos/login/user.h" #include "chrome/browser/chromeos/login/user_manager.h" @@ -36,6 +37,56 @@ namespace policy { +#if defined(OS_CHROMEOS) +class ProfilePolicyConnector::PolicyInitializationObserver + : public PolicyService::Observer { + public: + PolicyInitializationObserver(const base::Closure& closure, + PolicyService* policy_service) + : closure_(closure), + policy_service_(policy_service), + weak_ptr_factory_(this) { + if (policy_service_->IsInitializationComplete(POLICY_DOMAIN_CHROME)) + OnPolicyServiceInitialized(POLICY_DOMAIN_CHROME); + else + policy_service_->AddObserver(POLICY_DOMAIN_CHROME, this); + } + + virtual ~PolicyInitializationObserver() { + policy_service_->RemoveObserver(POLICY_DOMAIN_CHROME, this); + } + + // PolicyService::Observer overrides. + virtual void OnPolicyUpdated(const PolicyNamespace& ns, + const PolicyMap& previous, + const PolicyMap& current) OVERRIDE { + // Ignore policy changes. + } + + virtual void OnPolicyServiceInitialized(PolicyDomain domain) OVERRIDE { + if (domain != POLICY_DOMAIN_CHROME) + return; + policy_service_->RemoveObserver(POLICY_DOMAIN_CHROME, this); + // Delay one cycle, so that the policies are propagated to the device policy + // service before calling the closure. + base::MessageLoop::current()->PostTask( + FROM_HERE, base::Bind(&PolicyInitializationObserver::RunClosure, + weak_ptr_factory_.GetWeakPtr())); + } + + void RunClosure() { + closure_.Run(); + } + + private: + base::Closure closure_; + PolicyService* policy_service_; + base::WeakPtrFactory weak_ptr_factory_; + + DISALLOW_COPY_AND_ASSIGN(PolicyInitializationObserver); +}; +#endif + ProfilePolicyConnector::ProfilePolicyConnector(Profile* profile) : profile_(profile), #if defined(OS_CHROMEOS) @@ -123,8 +174,10 @@ void ProfilePolicyConnector::InitForTesting(scoped_ptr service) { void ProfilePolicyConnector::Shutdown() { #if defined(OS_CHROMEOS) - if (is_primary_user_) + if (is_primary_user_) { g_browser_process->browser_policy_connector()->SetUserPolicyDelegate(NULL); + user_policy_init_observer_.reset(); + } if (device_local_account_policy_provider_) device_local_account_policy_provider_->Shutdown(); #endif @@ -169,7 +222,13 @@ void ProfilePolicyConnector::InitializeNetworkConfigurationUpdater( g_browser_process->browser_policy_connector(); NetworkConfigurationUpdater* network_updater = connector->GetNetworkConfigurationUpdater(); - network_updater->OnUserPolicyInitialized(is_managed, hashed_username); + + user_policy_init_observer_.reset(new PolicyInitializationObserver( + base::Bind(&NetworkConfigurationUpdater::OnUserPolicyInitialized, + base::Unretained(network_updater), + is_managed, + hashed_username), + policy_service())); } #endif diff --git a/chrome/browser/policy/profile_policy_connector.h b/chrome/browser/policy/profile_policy_connector.h index 0669c7d04e9f23..ce71726cca9ba9 100644 --- a/chrome/browser/policy/profile_policy_connector.h +++ b/chrome/browser/policy/profile_policy_connector.h @@ -62,6 +62,8 @@ class ProfilePolicyConnector : public BrowserContextKeyedService { #if defined(ENABLE_CONFIGURATION_POLICY) #if defined(OS_CHROMEOS) + class PolicyInitializationObserver; + void InitializeDeviceLocalAccountPolicyProvider(const std::string& username); // Callback for CryptohomeClient::GetSanitizedUsername() that initializes the @@ -84,6 +86,8 @@ class ProfilePolicyConnector : public BrowserContextKeyedService { scoped_ptr device_local_account_policy_provider_; + + scoped_ptr user_policy_init_observer_; #endif #if defined(ENABLE_MANAGED_USERS) && defined(ENABLE_CONFIGURATION_POLICY)