Skip to content

Commit

Permalink
Let NetworkConfigurationUpdater wait for PolicyService::IsInitializat…
Browse files Browse the repository at this point in the history
…ionComplete.

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
  • Loading branch information
pneubeck@chromium.org committed Jun 5, 2013
1 parent 92f2146 commit 01d2b7e
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 2 deletions.
63 changes: 61 additions & 2 deletions chrome/browser/policy/profile_policy_connector.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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<PolicyInitializationObserver> weak_ptr_factory_;

DISALLOW_COPY_AND_ASSIGN(PolicyInitializationObserver);
};
#endif

ProfilePolicyConnector::ProfilePolicyConnector(Profile* profile)
: profile_(profile),
#if defined(OS_CHROMEOS)
Expand Down Expand Up @@ -123,8 +174,10 @@ void ProfilePolicyConnector::InitForTesting(scoped_ptr<PolicyService> 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
Expand Down Expand Up @@ -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

Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/policy/profile_policy_connector.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -84,6 +86,8 @@ class ProfilePolicyConnector : public BrowserContextKeyedService {

scoped_ptr<DeviceLocalAccountPolicyProvider>
device_local_account_policy_provider_;

scoped_ptr<PolicyInitializationObserver> user_policy_init_observer_;
#endif

#if defined(ENABLE_MANAGED_USERS) && defined(ENABLE_CONFIGURATION_POLICY)
Expand Down

0 comments on commit 01d2b7e

Please sign in to comment.