diff options
| author | danno@chromium.org <danno@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-12-23 09:34:35 +0000 |
|---|---|---|
| committer | danno@chromium.org <danno@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-12-23 09:34:35 +0000 |
| commit | 88616f47602e8a2a16c65ca0a59444e0ce550772 (patch) | |
| tree | 285cad010b0aee7f2001fe419191cd1ee41a00ee | |
| parent | 5e0750503a3c9743e1245f1b4c540124a9722ea0 (diff) | |
| download | chromium_src-88616f47602e8a2a16c65ca0a59444e0ce550772.zip chromium_src-88616f47602e8a2a16c65ca0a59444e0ce550772.tar.gz chromium_src-88616f47602e8a2a16c65ca0a59444e0ce550772.tar.bz2 | |
Refactor the windows policy provider to use AsynchronousPolicyProvider.
BUG=66453,65094
TEST=*Policy.*
Review URL: http://codereview.chromium.org/6091002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@70040 0039d316-1c4b-4281-b951-d872f2087c98
14 files changed, 526 insertions, 485 deletions
diff --git a/chrome/browser/policy/asynchronous_policy_loader.cc b/chrome/browser/policy/asynchronous_policy_loader.cc index e78c9aa..cf67066 100644 --- a/chrome/browser/policy/asynchronous_policy_loader.cc +++ b/chrome/browser/policy/asynchronous_policy_loader.cc @@ -6,21 +6,43 @@ #include "base/message_loop.h" #include "base/task.h" +#include "chrome/browser/browser_thread.h" namespace policy { AsynchronousPolicyLoader::AsynchronousPolicyLoader( - AsynchronousPolicyProvider::Delegate* delegate) + AsynchronousPolicyProvider::Delegate* delegate, + int reload_interval_minutes) : delegate_(delegate), provider_(NULL), - origin_loop_(MessageLoop::current()) {} + reload_task_(NULL), + reload_interval_(base::TimeDelta::FromMinutes(reload_interval_minutes)), + origin_loop_(MessageLoop::current()), + stopped_(false) {} void AsynchronousPolicyLoader::Init() { policy_.reset(delegate_->Load()); + // Initialization can happen early when the file thread is not yet available, + // but the subclass of the loader must do some of their initialization on the + // file thread. Posting to the file thread directly before it is initialized + // will cause the task to be forgotten. Instead, post a task to the ui thread + // to delay the remainder of initialization until threading is fully + // initialized. + BrowserThread::PostTask( + BrowserThread::UI, FROM_HERE, + NewRunnableMethod( + this, + &AsynchronousPolicyLoader::InitAfterFileThreadAvailable)); } void AsynchronousPolicyLoader::Stop() { - delegate_.reset(); + if (!stopped_) { + stopped_ = true; + delegate_.reset(); + BrowserThread::PostTask( + BrowserThread::FILE, FROM_HERE, + NewRunnableMethod(this, &AsynchronousPolicyLoader::StopOnFileThread)); + } } void AsynchronousPolicyLoader::SetProvider( @@ -57,6 +79,54 @@ void AsynchronousPolicyLoader::Reload() { } } +void AsynchronousPolicyLoader::CancelReloadTask() { + if (reload_task_) { + // Only check the thread if there's still a reload task. During + // destruction of unit tests, the message loop destruction can + // call this method when the file thread is no longer around, + // but in that case reload_task_ is NULL. + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); + reload_task_->Cancel(); + reload_task_ = NULL; + } +} + +void AsynchronousPolicyLoader::ScheduleReloadTask( + const base::TimeDelta& delay) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); + + CancelReloadTask(); + + reload_task_ = + NewRunnableMethod(this, &AsynchronousPolicyLoader::ReloadFromTask); + BrowserThread::PostDelayedTask(BrowserThread::FILE, FROM_HERE, reload_task_, + delay.InMilliseconds()); +} + +void AsynchronousPolicyLoader::ScheduleFallbackReloadTask() { + // As a safeguard in case that the load delegate failed to timely notice a + // change in policy, schedule a reload task that'll make us recheck after a + // reasonable interval. + ScheduleReloadTask(reload_interval_); +} + +void AsynchronousPolicyLoader::ReloadFromTask() { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); + + // Drop the reference to the reload task, since the task might be the only + // referrer that keeps us alive, so we should not Cancel() it. + reload_task_ = NULL; + + Reload(); +} + +void AsynchronousPolicyLoader::InitOnFileThread() { +} + +void AsynchronousPolicyLoader::StopOnFileThread() { + CancelReloadTask(); +} + void AsynchronousPolicyLoader::PostUpdatePolicyTask( DictionaryValue* new_policy) { origin_loop_->PostTask(FROM_HERE, new UpdatePolicyTask(this, new_policy)); @@ -75,4 +145,12 @@ void AsynchronousPolicyLoader::UpdatePolicy(DictionaryValue* new_policy_raw) { } } +void AsynchronousPolicyLoader::InitAfterFileThreadAvailable() { + if (!stopped_) { + BrowserThread::PostTask( + BrowserThread::FILE, FROM_HERE, + NewRunnableMethod(this, &AsynchronousPolicyLoader::InitOnFileThread)); + } +} + } // namespace policy diff --git a/chrome/browser/policy/asynchronous_policy_loader.h b/chrome/browser/policy/asynchronous_policy_loader.h index d88cdff..8a5838f 100644 --- a/chrome/browser/policy/asynchronous_policy_loader.h +++ b/chrome/browser/policy/asynchronous_policy_loader.h @@ -21,7 +21,8 @@ class AsynchronousPolicyLoader : public base::RefCountedThreadSafe<AsynchronousPolicyLoader> { public: explicit AsynchronousPolicyLoader( - AsynchronousPolicyProvider::Delegate* delegate); + AsynchronousPolicyProvider::Delegate* delegate, + int reload_interval_minutes); // Triggers initial policy load. virtual void Init(); @@ -55,13 +56,32 @@ class AsynchronousPolicyLoader return delegate_.get(); } - AsynchronousPolicyProvider* provider() { - return provider_; - } + // Performs start operations that must be performed on the file thread. + virtual void InitOnFileThread(); + + // Performs stop operations that must be performed on the file thread. + virtual void StopOnFileThread(); + + // Schedules a reload task to run when |delay| expires. Must be called on the + // file thread. + void ScheduleReloadTask(const base::TimeDelta& delay); + + // Schedules a reload task to run after the number of minutes specified + // in |reload_interval_minutes_|. Must be called on the file thread. + void ScheduleFallbackReloadTask(); + + void CancelReloadTask(); + + // Invoked from the reload task on the file thread. + void ReloadFromTask(); private: friend class AsynchronousPolicyLoaderTest; + // Finishes loader initialization after the threading system has been fully + // intialized. + void InitAfterFileThreadAvailable(); + // Replaces the existing policy to value map with a new one, sending // notification to the provider if there is a policy change. Must be called on // |origin_loop_| so that it's safe to call back into the provider, which is @@ -78,11 +98,21 @@ class AsynchronousPolicyLoader // called the constructor. See |origin_loop_| below. AsynchronousPolicyProvider* provider_; + // The reload task. Access only on the file thread. Holds a reference to the + // currently posted task, so we can cancel and repost it if necessary. + CancelableTask* reload_task_; + + // The interval at which a policy reload will be triggered as a fallback. + const base::TimeDelta reload_interval_; + // The message loop on which this object was constructed. Recorded so that // it's possible to call back into the non thread safe provider to fire the // notification. MessageLoop* origin_loop_; + // True if Stop has been called. + bool stopped_; + DISALLOW_COPY_AND_ASSIGN(AsynchronousPolicyLoader); }; diff --git a/chrome/browser/policy/asynchronous_policy_loader_unittest.cc b/chrome/browser/policy/asynchronous_policy_loader_unittest.cc index 4995431..1fc1ee5 100644 --- a/chrome/browser/policy/asynchronous_policy_loader_unittest.cc +++ b/chrome/browser/policy/asynchronous_policy_loader_unittest.cc @@ -53,7 +53,7 @@ TEST_F(AsynchronousPolicyLoaderTest, InitialLoad) { DictionaryValue* template_dict(new DictionaryValue()); EXPECT_CALL(*delegate_, Load()).WillOnce(Return(template_dict)); scoped_refptr<AsynchronousPolicyLoader> loader = - new AsynchronousPolicyLoader(delegate_.release()); + new AsynchronousPolicyLoader(delegate_.release(), 10); loader->Init(); const DictionaryValue* loaded_dict(loader->policy()); EXPECT_TRUE(loaded_dict->Equals(template_dict)); @@ -68,7 +68,7 @@ TEST_F(AsynchronousPolicyLoaderTest, InitialLoadWithFallback) { EXPECT_CALL(*delegate_, Load()).WillOnce( CreateSequencedTestDictionary(&dictionary_number)); scoped_refptr<AsynchronousPolicyLoader> loader = - new AsynchronousPolicyLoader(delegate_.release()); + new AsynchronousPolicyLoader(delegate_.release(), 10); loader->Init(); loop_.RunAllPending(); loader->Reload(); @@ -86,7 +86,7 @@ TEST_F(AsynchronousPolicyLoaderTest, Stop) { ON_CALL(*delegate_, Load()).WillByDefault(CreateTestDictionary()); EXPECT_CALL(*delegate_, Load()).Times(1); scoped_refptr<AsynchronousPolicyLoader> loader = - new AsynchronousPolicyLoader(delegate_.release()); + new AsynchronousPolicyLoader(delegate_.release(), 10); loader->Init(); loop_.RunAllPending(); loader->Stop(); @@ -118,7 +118,7 @@ TEST_F(AsynchronousPolicyLoaderTest, ProviderNotificationOnPolicyChange) { EXPECT_CALL(*delegate_, Load()).WillOnce( CreateSequencedTestDictionary(&dictionary_number_1)); scoped_refptr<AsynchronousPolicyLoader> loader = - new AsynchronousPolicyLoader(delegate_.release()); + new AsynchronousPolicyLoader(delegate_.release(), 10); AsynchronousPolicyProvider provider(NULL, loader); loop_.RunAllPending(); loader->Reload(); diff --git a/chrome/browser/policy/asynchronous_policy_provider_unittest.cc b/chrome/browser/policy/asynchronous_policy_provider_unittest.cc index 64306eb..c6aee70 100644 --- a/chrome/browser/policy/asynchronous_policy_provider_unittest.cc +++ b/chrome/browser/policy/asynchronous_policy_provider_unittest.cc @@ -17,6 +17,7 @@ using ::testing::Return; namespace policy { +// Creating the provider should provide initial policy. TEST_F(AsynchronousPolicyTestBase, Provide) { InSequence s; DictionaryValue* policies = new DictionaryValue(); @@ -25,10 +26,12 @@ TEST_F(AsynchronousPolicyTestBase, Provide) { EXPECT_CALL(*store_, Apply(policy::kPolicySyncDisabled, _)).Times(1); AsynchronousPolicyProvider provider( ConfigurationPolicyPrefStore::GetChromePolicyDefinitionList(), - new AsynchronousPolicyLoader(delegate_.release())); + new AsynchronousPolicyLoader(delegate_.release(), 10)); provider.Provide(store_.get()); } + +// Trigger a refresh manually and ensure that policy gets reloaded. TEST_F(AsynchronousPolicyTestBase, ProvideAfterRefresh) { InSequence s; DictionaryValue* original_policies = new DictionaryValue(); @@ -39,8 +42,8 @@ TEST_F(AsynchronousPolicyTestBase, ProvideAfterRefresh) { policy::key::kJavascriptEnabled, true); EXPECT_CALL(*delegate_, Load()).WillOnce(Return(refresh_policies)); - AsynchronousPolicyLoader* loader = new AsynchronousPolicyLoader( - delegate_.release()); + AsynchronousPolicyLoader* loader = + new AsynchronousPolicyLoader(delegate_.release(), 10); AsynchronousPolicyProvider provider( ConfigurationPolicyPrefStore::GetChromePolicyDefinitionList(), loader); diff --git a/chrome/browser/policy/configuration_policy_loader_win.cc b/chrome/browser/policy/configuration_policy_loader_win.cc new file mode 100644 index 0000000..68ccf80 --- /dev/null +++ b/chrome/browser/policy/configuration_policy_loader_win.cc @@ -0,0 +1,90 @@ +// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/policy/configuration_policy_loader_win.h" + +#include <userenv.h> + +#include "chrome/browser/browser_thread.h" + +namespace policy { + +ConfigurationPolicyLoaderWin::ConfigurationPolicyLoaderWin( + AsynchronousPolicyProvider::Delegate* delegate, + int reload_interval_minutes) + : AsynchronousPolicyLoader(delegate, reload_interval_minutes), + user_policy_changed_event_(false, false), + machine_policy_changed_event_(false, false), + user_policy_watcher_failed_(false), + machine_policy_watcher_failed_(false) { + if (!RegisterGPNotification(user_policy_changed_event_.handle(), false)) { + PLOG(WARNING) << "Failed to register user group policy notification"; + user_policy_watcher_failed_ = true; + } + if (!RegisterGPNotification(machine_policy_changed_event_.handle(), true)) { + PLOG(WARNING) << "Failed to register machine group policy notification."; + machine_policy_watcher_failed_ = true; + } +} + +void ConfigurationPolicyLoaderWin::InitOnFileThread() { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); + AsynchronousPolicyLoader::InitOnFileThread(); + MessageLoop::current()->AddDestructionObserver(this); + SetupWatches(); +} + +void ConfigurationPolicyLoaderWin::StopOnFileThread() { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); + MessageLoop::current()->RemoveDestructionObserver(this); + user_policy_watcher_.StopWatching(); + machine_policy_watcher_.StopWatching(); + AsynchronousPolicyLoader::StopOnFileThread(); +} + +void ConfigurationPolicyLoaderWin::SetupWatches() { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); + CancelReloadTask(); + + if (!user_policy_watcher_failed_ && + !user_policy_watcher_.GetWatchedObject() && + !user_policy_watcher_.StartWatching( + user_policy_changed_event_.handle(), this)) { + LOG(WARNING) << "Failed to start watch for user policy change event"; + user_policy_watcher_failed_ = true; + } + if (!machine_policy_watcher_failed_ && + !machine_policy_watcher_.GetWatchedObject() && + !machine_policy_watcher_.StartWatching( + machine_policy_changed_event_.handle(), this)) { + LOG(WARNING) << "Failed to start watch for machine policy change event"; + machine_policy_watcher_failed_ = true; + } + + if (user_policy_watcher_failed_ || machine_policy_watcher_failed_) + ScheduleFallbackReloadTask(); +} + +void ConfigurationPolicyLoaderWin::Reload() { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); + AsynchronousPolicyLoader::Reload(); + SetupWatches(); +} + +void ConfigurationPolicyLoaderWin::OnObjectSignaled(HANDLE object) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); + DCHECK(object == user_policy_changed_event_.handle() || + object == machine_policy_changed_event_.handle()) + << "unexpected object signaled policy reload, obj = " + << std::showbase << std::hex << object; + Reload(); +} + +void ConfigurationPolicyLoaderWin:: + WillDestroyCurrentMessageLoop() { + CancelReloadTask(); + MessageLoop::current()->RemoveDestructionObserver(this); +} + +} // namespace policy diff --git a/chrome/browser/policy/configuration_policy_loader_win.h b/chrome/browser/policy/configuration_policy_loader_win.h new file mode 100644 index 0000000..a898afc --- /dev/null +++ b/chrome/browser/policy/configuration_policy_loader_win.h @@ -0,0 +1,57 @@ +// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_BROWSER_POLICY_CONFIGURATION_POLICY_LOADER_WIN_H_ +#define CHROME_BROWSER_POLICY_CONFIGURATION_POLICY_LOADER_WIN_H_ +#pragma once + +#include "base/object_watcher.h" +#include "base/waitable_event.h" +#include "chrome/browser/policy/asynchronous_policy_loader.h" + +namespace policy { + +// Keeps watch on Windows Group Policy notification event to trigger a policy +// reload when Group Policy changes. +class ConfigurationPolicyLoaderWin + : public AsynchronousPolicyLoader, + public base::ObjectWatcher::Delegate, + public MessageLoop::DestructionObserver { + public: + ConfigurationPolicyLoaderWin( + AsynchronousPolicyProvider::Delegate* delegate, + int reload_interval_minutes); + virtual ~ConfigurationPolicyLoaderWin() {} + + protected: + // AsynchronousPolicyLoader overrides: + virtual void InitOnFileThread(); + virtual void StopOnFileThread(); + + private: + // Updates the watchers and schedules the reload task if appropriate. + void SetupWatches(); + + // Post a reload notification and update the watch machinery. + void Reload(); + + // ObjectWatcher::Delegate overrides: + virtual void OnObjectSignaled(HANDLE object); + + // MessageLoop::DestructionObserver overrides: + virtual void WillDestroyCurrentMessageLoop(); + + base::WaitableEvent user_policy_changed_event_; + base::WaitableEvent machine_policy_changed_event_; + base::ObjectWatcher user_policy_watcher_; + base::ObjectWatcher machine_policy_watcher_; + bool user_policy_watcher_failed_; + bool machine_policy_watcher_failed_; + + DISALLOW_COPY_AND_ASSIGN(ConfigurationPolicyLoaderWin); +}; + +} // namespace policy + +#endif // CHROME_BROWSER_POLICY_CONFIGURATION_POLICY_LOADER_WIN_H_ diff --git a/chrome/browser/policy/configuration_policy_provider_delegate_win.cc b/chrome/browser/policy/configuration_policy_provider_delegate_win.cc new file mode 100644 index 0000000..68947de --- /dev/null +++ b/chrome/browser/policy/configuration_policy_provider_delegate_win.cc @@ -0,0 +1,160 @@ +// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/policy/configuration_policy_provider_delegate_win.h" + +#include "base/string_number_conversions.h" +#include "base/utf_string_conversions.h" +#include "base/win/registry.h" +#include "chrome/common/policy_constants.h" + +using base::win::RegKey; + +namespace { + +bool ReadRegistryStringValue(RegKey* key, const string16& name, + string16* result) { + DWORD value_size = 0; + DWORD key_type = 0; + scoped_array<uint8> buffer; + + if (!key->ReadValue(name.c_str(), 0, &value_size, &key_type)) + return false; + if (key_type != REG_SZ) + return false; + + // According to the Microsoft documentation, the string + // buffer may not be explicitly 0-terminated. Allocate a + // slightly larger buffer and pre-fill to zeros to guarantee + // the 0-termination. + buffer.reset(new uint8[value_size + 2]); + memset(buffer.get(), 0, value_size + 2); + key->ReadValue(name.c_str(), buffer.get(), &value_size, NULL); + result->assign(reinterpret_cast<const wchar_t*>(buffer.get())); + return true; +} + +} // namespace + +namespace policy { + +ConfigurationPolicyProviderDelegateWin::ConfigurationPolicyProviderDelegateWin( + const ConfigurationPolicyProvider::PolicyDefinitionList* + policy_definition_list) + : policy_definition_list_(policy_definition_list) { +} + +DictionaryValue* ConfigurationPolicyProviderDelegateWin::Load() { + DictionaryValue* result = new DictionaryValue(); + const ConfigurationPolicyProvider::PolicyDefinitionList::Entry* current; + for (current = policy_definition_list_->begin; + current != policy_definition_list_->end; + ++current) { + const string16 name(ASCIIToUTF16(current->name)); + switch (current->value_type) { + case Value::TYPE_STRING: { + string16 string_value; + if (GetRegistryPolicyString(name, &string_value)) { + result->SetString(current->name, string_value); + } + break; + } + case Value::TYPE_LIST: { + scoped_ptr<ListValue> list_value(new ListValue); + if (GetRegistryPolicyStringList(name, list_value.get())) + result->Set(current->name, list_value.release()); + break; + } + case Value::TYPE_BOOLEAN: { + bool bool_value; + if (GetRegistryPolicyBoolean(name, &bool_value)) { + result->SetBoolean(current->name, bool_value); + } + break; + } + case Value::TYPE_INTEGER: { + uint32 int_value; + if (GetRegistryPolicyInteger(name, &int_value)) { + result->SetInteger(current->name, int_value); + } + break; + } + default: + NOTREACHED(); + } + } + return result; +} + +bool ConfigurationPolicyProviderDelegateWin::GetRegistryPolicyString( + const string16& name, string16* result) const { + string16 path = string16(kRegistrySubKey); + RegKey policy_key; + // First try the global policy. + if (policy_key.Open(HKEY_LOCAL_MACHINE, path.c_str(), KEY_READ)) { + if (ReadRegistryStringValue(&policy_key, name, result)) + return true; + policy_key.Close(); + } + // Fall back on user-specific policy. + if (!policy_key.Open(HKEY_CURRENT_USER, path.c_str(), KEY_READ)) + return false; + return ReadRegistryStringValue(&policy_key, name, result); +} + +bool ConfigurationPolicyProviderDelegateWin::GetRegistryPolicyStringList( + const string16& key, ListValue* result) const { + string16 path = string16(kRegistrySubKey); + path += ASCIIToUTF16("\\") + key; + RegKey policy_key; + if (!policy_key.Open(HKEY_LOCAL_MACHINE, path.c_str(), KEY_READ)) { + policy_key.Close(); + // Fall back on user-specific policy. + if (!policy_key.Open(HKEY_CURRENT_USER, path.c_str(), KEY_READ)) + return false; + } + string16 policy_string; + int index = 0; + while (ReadRegistryStringValue(&policy_key, base::IntToString16(++index), + &policy_string)) { + result->Append(Value::CreateStringValue(policy_string)); + } + return true; +} + +bool ConfigurationPolicyProviderDelegateWin::GetRegistryPolicyBoolean( + const string16& value_name, bool* result) const { + DWORD value; + RegKey hkcu_policy_key(HKEY_LOCAL_MACHINE, kRegistrySubKey, KEY_READ); + if (hkcu_policy_key.ReadValueDW(value_name.c_str(), &value)) { + *result = value != 0; + return true; + } + + RegKey hklm_policy_key(HKEY_CURRENT_USER, kRegistrySubKey, KEY_READ); + if (hklm_policy_key.ReadValueDW(value_name.c_str(), &value)) { + *result = value != 0; + return true; + } + return false; +} + +bool ConfigurationPolicyProviderDelegateWin::GetRegistryPolicyInteger( + const string16& value_name, uint32* result) const { + DWORD value; + RegKey hkcu_policy_key(HKEY_LOCAL_MACHINE, kRegistrySubKey, KEY_READ); + if (hkcu_policy_key.ReadValueDW(value_name.c_str(), &value)) { + *result = value; + return true; + } + + RegKey hklm_policy_key(HKEY_CURRENT_USER, kRegistrySubKey, KEY_READ); + if (hklm_policy_key.ReadValueDW(value_name.c_str(), &value)) { + *result = value; + return true; + } + return false; +} + +} // namespace policy diff --git a/chrome/browser/policy/configuration_policy_provider_delegate_win.h b/chrome/browser/policy/configuration_policy_provider_delegate_win.h new file mode 100644 index 0000000..63d36b2 --- /dev/null +++ b/chrome/browser/policy/configuration_policy_provider_delegate_win.h @@ -0,0 +1,47 @@ +// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_BROWSER_POLICY_CONFIGURATION_POLICY_PROVIDER_DELEGATE_WIN_H_ +#define CHROME_BROWSER_POLICY_CONFIGURATION_POLICY_PROVIDER_DELEGATE_WIN_H_ +#pragma once + +#include "chrome/browser/policy/asynchronous_policy_provider.h" + +namespace policy { + +class ConfigurationPolicyProviderDelegateWin + : public AsynchronousPolicyProvider::Delegate { + public: + ConfigurationPolicyProviderDelegateWin( + const ConfigurationPolicyProvider::PolicyDefinitionList* + policy_definition_list); + virtual ~ConfigurationPolicyProviderDelegateWin() {} + + // AsynchronousPolicyProvider::Delegate overrides: + virtual DictionaryValue* Load(); + + private: + // Methods to perform type-specific policy lookups in the registry. + // HKLM is checked first, then HKCU. + + // Reads a string registry value |name| at the specified |key| and puts the + // resulting string in |result|. + bool GetRegistryPolicyString(const string16& name, string16* result) const; + // Gets a list value contained under |key| one level below the policy root. + bool GetRegistryPolicyStringList(const string16& key, + ListValue* result) const; + bool GetRegistryPolicyBoolean(const string16& value_name, + bool* result) const; + bool GetRegistryPolicyInteger(const string16& value_name, + uint32* result) const; + + const ConfigurationPolicyProvider::PolicyDefinitionList* + policy_definition_list_; + + DISALLOW_COPY_AND_ASSIGN(ConfigurationPolicyProviderDelegateWin); +}; + +} // namespace policy + +#endif // CHROME_BROWSER_POLICY_CONFIGURATION_POLICY_PROVIDER_DELEGATE_WIN_H_ diff --git a/chrome/browser/policy/configuration_policy_provider_win.cc b/chrome/browser/policy/configuration_policy_provider_win.cc index 084f130..df336d3 100644 --- a/chrome/browser/policy/configuration_policy_provider_win.cc +++ b/chrome/browser/policy/configuration_policy_provider_win.cc @@ -4,290 +4,22 @@ #include "chrome/browser/policy/configuration_policy_provider_win.h" -#include <userenv.h> - -#include <algorithm> - -#include "base/logging.h" -#include "base/object_watcher.h" -#include "base/scoped_ptr.h" -#include "base/string_number_conversions.h" -#include "base/string_piece.h" -#include "base/string_util.h" -#include "base/sys_string_conversions.h" -#include "base/thread_restrictions.h" -#include "base/utf_string_conversions.h" -#include "base/values.h" -#include "base/win/registry.h" -#include "chrome/common/policy_constants.h" - -using base::win::RegKey; +#include "chrome/browser/policy/asynchronous_policy_provider.h" +#include "chrome/browser/policy/configuration_policy_loader_win.h" +#include "chrome/browser/policy/configuration_policy_provider_delegate_win.h" namespace policy { -namespace { - -bool ReadRegistryStringValue(RegKey* key, const string16& name, - string16* result) { - DWORD value_size = 0; - DWORD key_type = 0; - scoped_array<uint8> buffer; - - if (!key->ReadValue(name.c_str(), 0, &value_size, &key_type)) - return false; - if (key_type != REG_SZ) - return false; - - // According to the Microsoft documentation, the string - // buffer may not be explicitly 0-terminated. Allocate a - // slightly larger buffer and pre-fill to zeros to guarantee - // the 0-termination. - buffer.reset(new uint8[value_size + 2]); - memset(buffer.get(), 0, value_size + 2); - key->ReadValue(name.c_str(), buffer.get(), &value_size, NULL); - result->assign(reinterpret_cast<const wchar_t*>(buffer.get())); - return true; -} - -} // namespace - // Period at which to run the reload task in case the group policy change // watchers fail. const int kReloadIntervalMinutes = 15; -ConfigurationPolicyProviderWin::GroupPolicyChangeWatcher:: - GroupPolicyChangeWatcher( - base::WeakPtr<ConfigurationPolicyProviderWin> provider, - int reload_interval_minutes) - : provider_(provider), - user_policy_changed_event_(false, false), - machine_policy_changed_event_(false, false), - user_policy_watcher_failed_(false), - machine_policy_watcher_failed_(false), - reload_interval_minutes_(reload_interval_minutes), - reload_task_(NULL) { - if (!RegisterGPNotification(user_policy_changed_event_.handle(), false)) { - PLOG(WARNING) << "Failed to register user group policy notification"; - user_policy_watcher_failed_ = true; - } - if (!RegisterGPNotification(machine_policy_changed_event_.handle(), true)) { - PLOG(WARNING) << "Failed to register machine group policy notification."; - machine_policy_watcher_failed_ = true; - } -} - -ConfigurationPolicyProviderWin::GroupPolicyChangeWatcher:: - ~GroupPolicyChangeWatcher() { - if (MessageLoop::current()) - MessageLoop::current()->RemoveDestructionObserver(this); - DCHECK(!reload_task_); -} - -void ConfigurationPolicyProviderWin::GroupPolicyChangeWatcher::Start() { - MessageLoop::current()->AddDestructionObserver(this); - SetupWatches(); -} - -void ConfigurationPolicyProviderWin::GroupPolicyChangeWatcher::Stop() { - user_policy_watcher_.StopWatching(); - machine_policy_watcher_.StopWatching(); - if (reload_task_) { - reload_task_->Cancel(); - reload_task_ = NULL; - } -} - -void ConfigurationPolicyProviderWin::GroupPolicyChangeWatcher::SetupWatches() { - if (reload_task_) { - reload_task_->Cancel(); - reload_task_ = NULL; - } - - if (!user_policy_watcher_failed_) { - if (!user_policy_watcher_.GetWatchedObject() && - !user_policy_watcher_.StartWatching( - user_policy_changed_event_.handle(), this)) { - LOG(WARNING) << "Failed to start watch for user policy change event"; - user_policy_watcher_failed_ = true; - } - } - if (!machine_policy_watcher_failed_) { - if (!machine_policy_watcher_.GetWatchedObject() && - !machine_policy_watcher_.StartWatching( - machine_policy_changed_event_.handle(), this)) { - LOG(WARNING) << "Failed to start watch for machine policy change event"; - machine_policy_watcher_failed_ = true; - } - } - - if (user_policy_watcher_failed_ || machine_policy_watcher_failed_) { - reload_task_ = - NewRunnableMethod(this, &GroupPolicyChangeWatcher::ReloadFromTask); - int64 delay = - base::TimeDelta::FromMinutes(reload_interval_minutes_).InMilliseconds(); - MessageLoop::current()->PostDelayedTask(FROM_HERE, reload_task_, delay); - } -} - -void ConfigurationPolicyProviderWin::GroupPolicyChangeWatcher::Reload() { - if (provider_.get()) - provider_->NotifyStoreOfPolicyChange(); - SetupWatches(); -} - -void ConfigurationPolicyProviderWin::GroupPolicyChangeWatcher:: - ReloadFromTask() { - // Make sure to not call Cancel() on the task, since it might hold the last - // reference that keeps this object alive. - reload_task_ = NULL; - Reload(); -} - -void ConfigurationPolicyProviderWin::GroupPolicyChangeWatcher:: - OnObjectSignaled(HANDLE object) { - DCHECK(object == user_policy_changed_event_.handle() || - object == machine_policy_changed_event_.handle()) - << "unexpected object signaled policy reload, obj = " - << std::showbase << std::hex << object; - Reload(); -} - -void ConfigurationPolicyProviderWin::GroupPolicyChangeWatcher:: - WillDestroyCurrentMessageLoop() { - reload_task_ = NULL; - MessageLoop::current()->RemoveDestructionObserver(this); -} - ConfigurationPolicyProviderWin::ConfigurationPolicyProviderWin( const PolicyDefinitionList* policy_list) - : ConfigurationPolicyProvider(policy_list) { - watcher_ = new GroupPolicyChangeWatcher(this->AsWeakPtr(), - kReloadIntervalMinutes); - watcher_->Start(); -} - -ConfigurationPolicyProviderWin::~ConfigurationPolicyProviderWin() { - watcher_->Stop(); -} - -bool ConfigurationPolicyProviderWin::GetRegistryPolicyString( - const string16& name, string16* result) const { - string16 path = string16(kRegistrySubKey); - RegKey policy_key; - // First try the global policy. - if (policy_key.Open(HKEY_LOCAL_MACHINE, path.c_str(), KEY_READ)) { - if (ReadRegistryStringValue(&policy_key, name, result)) - return true; - policy_key.Close(); - } - // Fall back on user-specific policy. - if (!policy_key.Open(HKEY_CURRENT_USER, path.c_str(), KEY_READ)) - return false; - return ReadRegistryStringValue(&policy_key, name, result); -} - -bool ConfigurationPolicyProviderWin::GetRegistryPolicyStringList( - const string16& key, ListValue* result) const { - string16 path = string16(kRegistrySubKey); - path += ASCIIToUTF16("\\") + key; - RegKey policy_key; - if (!policy_key.Open(HKEY_LOCAL_MACHINE, path.c_str(), KEY_READ)) { - policy_key.Close(); - // Fall back on user-specific policy. - if (!policy_key.Open(HKEY_CURRENT_USER, path.c_str(), KEY_READ)) - return false; - } - string16 policy_string; - int index = 0; - while (ReadRegistryStringValue(&policy_key, base::IntToString16(++index), - &policy_string)) { - result->Append(Value::CreateStringValue(policy_string)); - } - return true; -} - -bool ConfigurationPolicyProviderWin::GetRegistryPolicyBoolean( - const string16& value_name, bool* result) const { - DWORD value; - RegKey hkcu_policy_key(HKEY_LOCAL_MACHINE, kRegistrySubKey, KEY_READ); - if (hkcu_policy_key.ReadValueDW(value_name.c_str(), &value)) { - *result = value != 0; - return true; - } - - RegKey hklm_policy_key(HKEY_CURRENT_USER, kRegistrySubKey, KEY_READ); - if (hklm_policy_key.ReadValueDW(value_name.c_str(), &value)) { - *result = value != 0; - return true; - } - return false; -} - -bool ConfigurationPolicyProviderWin::GetRegistryPolicyInteger( - const string16& value_name, uint32* result) const { - DWORD value; - RegKey hkcu_policy_key(HKEY_LOCAL_MACHINE, kRegistrySubKey, KEY_READ); - if (hkcu_policy_key.ReadValueDW(value_name.c_str(), &value)) { - *result = value; - return true; - } - - RegKey hklm_policy_key(HKEY_CURRENT_USER, kRegistrySubKey, KEY_READ); - if (hklm_policy_key.ReadValueDW(value_name.c_str(), &value)) { - *result = value; - return true; - } - return false; -} - -bool ConfigurationPolicyProviderWin::Provide( - ConfigurationPolicyStoreInterface* store) { - // This function calls GetRegistryPolicy* which hit up the registry. Those - // are I/O functions not allowed to be called on the main thread. - // http://crbug.com/66453 - base::ThreadRestrictions::ScopedAllowIO allow_io; - const PolicyDefinitionList* policy_list(policy_definition_list()); - for (const PolicyDefinitionList::Entry* current = policy_list->begin; - current != policy_list->end; ++current) { - std::wstring name = UTF8ToWide(current->name); - switch (current->value_type) { - case Value::TYPE_STRING: { - std::wstring string_value; - if (GetRegistryPolicyString(name.c_str(), &string_value)) { - store->Apply(current->policy_type, - Value::CreateStringValue(string_value)); - } - break; - } - case Value::TYPE_LIST: { - scoped_ptr<ListValue> list_value(new ListValue); - if (GetRegistryPolicyStringList(name.c_str(), list_value.get())) - store->Apply(current->policy_type, list_value.release()); - break; - } - case Value::TYPE_BOOLEAN: { - bool bool_value; - if (GetRegistryPolicyBoolean(name.c_str(), &bool_value)) { - store->Apply(current->policy_type, - Value::CreateBooleanValue(bool_value)); - } - break; - } - case Value::TYPE_INTEGER: { - uint32 int_value; - if (GetRegistryPolicyInteger(name.c_str(), &int_value)) { - store->Apply(current->policy_type, - Value::CreateIntegerValue(int_value)); - } - break; - } - default: - NOTREACHED(); - return false; - } - } - - return true; -} + : AsynchronousPolicyProvider( + policy_list, + new ConfigurationPolicyLoaderWin( + new ConfigurationPolicyProviderDelegateWin(policy_list), + kReloadIntervalMinutes)) {} } // namespace policy diff --git a/chrome/browser/policy/configuration_policy_provider_win.h b/chrome/browser/policy/configuration_policy_provider_win.h index c2d5509..31e4a2b 100644 --- a/chrome/browser/policy/configuration_policy_provider_win.h +++ b/chrome/browser/policy/configuration_policy_provider_win.h @@ -6,19 +6,7 @@ #define CHROME_BROWSER_POLICY_CONFIGURATION_POLICY_PROVIDER_WIN_H_ #pragma once -#include "base/object_watcher.h" -#include "base/ref_counted.h" -#include "base/scoped_ptr.h" -#include "base/waitable_event.h" -#include "base/weak_ptr.h" -#include "chrome/browser/policy/configuration_policy_store_interface.h" -#include "chrome/browser/policy/configuration_policy_provider.h" - -namespace base { -namespace win { -class RegKey; -} // namespace win -} // namespace base +#include "chrome/browser/policy/asynchronous_policy_provider.h" namespace policy { @@ -28,89 +16,14 @@ namespace policy { // On a managed machine in a domain, this portion of the registry is // periodically updated by the Windows Group Policy machinery to contain // the latest version of the policy set by administrators. -class ConfigurationPolicyProviderWin - : public ConfigurationPolicyProvider, - public base::SupportsWeakPtr<ConfigurationPolicyProviderWin> { +class ConfigurationPolicyProviderWin : public AsynchronousPolicyProvider { public: - // Keeps watch on Windows Group Policy notification event to trigger a policy - // reload when Group Policy changes. This class is reference counted to - // facilitate timer-based reloads through the message loop. It is not safe to - // access GroupPolicyChangeWatcher concurrently from multiple threads. - class GroupPolicyChangeWatcher - : public base::ObjectWatcher::Delegate, - public MessageLoop::DestructionObserver, - public base::RefCountedThreadSafe<GroupPolicyChangeWatcher> { - public: - GroupPolicyChangeWatcher( - base::WeakPtr<ConfigurationPolicyProviderWin> provider, - int reload_interval_minutes); - virtual ~GroupPolicyChangeWatcher(); - - // Start watching. - void Start(); - - // Stop any pending watch activity in order to allow for timely shutdown. - void Stop(); - - private: - // Updates the watchers and schedules the reload task if appropriate. - void SetupWatches(); - - // Post a reload notification and update the watch machinery. - void Reload(); - - // Called for timer-based refresh from the message loop. - void ReloadFromTask(); - - // ObjectWatcher::Delegate implementation: - virtual void OnObjectSignaled(HANDLE object); - - // MessageLoop::DestructionObserver implementation: - virtual void WillDestroyCurrentMessageLoop(); - - base::WeakPtr<ConfigurationPolicyProviderWin> provider_; - base::WaitableEvent user_policy_changed_event_; - base::WaitableEvent machine_policy_changed_event_; - base::ObjectWatcher user_policy_watcher_; - base::ObjectWatcher machine_policy_watcher_; - bool user_policy_watcher_failed_; - bool machine_policy_watcher_failed_; - - // Period to schedule the reload task at. - int reload_interval_minutes_; - - // A reference to a delayed task for timer-based reloading. - CancelableTask* reload_task_; - }; - explicit ConfigurationPolicyProviderWin( const PolicyDefinitionList* policy_list); - virtual ~ConfigurationPolicyProviderWin(); - - // ConfigurationPolicyProvider method overrides: - virtual bool Provide(ConfigurationPolicyStoreInterface* store); - - protected: - // The sub key path for Chromium's Group Policy information in the - // Windows registry. - static const wchar_t kPolicyRegistrySubKey[]; + virtual ~ConfigurationPolicyProviderWin() {} private: - scoped_refptr<GroupPolicyChangeWatcher> watcher_; - - // Methods to perform type-specific policy lookups in the registry. - // HKLM is checked first, then HKCU. - - // Reads a string registry value |name| at the specified |key| and puts the - // resulting string in |result|. - bool GetRegistryPolicyString(const string16& name, string16* result) const; - // Gets a list value contained under |key| one level below the policy root. - bool GetRegistryPolicyStringList(const string16& key, - ListValue* result) const; - bool GetRegistryPolicyBoolean(const string16& value_name, - bool* result) const; - bool GetRegistryPolicyInteger(const string16& value_name, - uint32* result) const; + DISALLOW_COPY_AND_ASSIGN(ConfigurationPolicyProviderWin); }; } // namespace policy diff --git a/chrome/browser/policy/configuration_policy_provider_win_unittest.cc b/chrome/browser/policy/configuration_policy_provider_win_unittest.cc index 39c4dce..78c6f7b 100644 --- a/chrome/browser/policy/configuration_policy_provider_win_unittest.cc +++ b/chrome/browser/policy/configuration_policy_provider_win_unittest.cc @@ -3,15 +3,17 @@ // found in the LICENSE file. #include <gtest/gtest.h> - #include <windows.h> +#include "base/message_loop.h" #include "base/scoped_ptr.h" #include "base/stl_util-inl.h" #include "base/string_number_conversions.h" #include "base/string_piece.h" #include "base/utf_string_conversions.h" #include "base/win/registry.h" +#include "chrome/browser/browser_thread.h" +#include "chrome/browser/policy/asynchronous_policy_loader.h" #include "chrome/browser/policy/configuration_policy_pref_store.h" #include "chrome/browser/policy/configuration_policy_provider_win.h" #include "chrome/browser/policy/mock_configuration_policy_store.h" @@ -151,13 +153,16 @@ class ConfigurationPolicyProviderWinTest scoped_ptr<MockConfigurationPolicyStore> store_; scoped_ptr<ConfigurationPolicyProviderWin> provider_; - private: // A message loop must be declared and instantiated for these tests, // because Windows policy provider create WaitableEvents and // ObjectWatchers that require the tests to have a MessageLoop associated // with the thread executing the tests. MessageLoop loop_; + private: + BrowserThread ui_thread_; + BrowserThread file_thread_; + // Keys are created for the lifetime of a test to contain // the sandboxed HKCU and HKLM hives, respectively. RegKey temp_hkcu_hive_key_; @@ -165,7 +170,9 @@ class ConfigurationPolicyProviderWinTest }; ConfigurationPolicyProviderWinTest::ConfigurationPolicyProviderWinTest() - : temp_hklm_hive_key_(HKEY_CURRENT_USER, kUnitTestMachineOverrideSubKey, + : ui_thread_(BrowserThread::UI, &loop_), + file_thread_(BrowserThread::FILE, &loop_), + temp_hklm_hive_key_(HKEY_CURRENT_USER, kUnitTestMachineOverrideSubKey, KEY_READ), temp_hkcu_hive_key_(HKEY_CURRENT_USER, kUnitTestUserOverrideSubKey, KEY_READ) { @@ -194,6 +201,7 @@ void ConfigurationPolicyProviderWinTest::SetUp() { void ConfigurationPolicyProviderWinTest::TearDown() { DeactivateOverrides(); DeleteRegistrySandbox(); + loop_.RunAllPending(); } void ConfigurationPolicyProviderWinTest::ActivateOverrides() { @@ -299,6 +307,8 @@ TEST_P(ConfigurationPolicyProviderWinTest, InvalidValue) { WriteInvalidValue(HKEY_CURRENT_USER, GetParam().policy_name(), GetParam().hkcu_value()); + provider_->loader()->Reload(); + loop_.RunAllPending(); provider_->Provide(store_.get()); EXPECT_TRUE(store_->policy_map().empty()); } @@ -307,6 +317,8 @@ TEST_P(ConfigurationPolicyProviderWinTest, HKLM) { WriteValue(HKEY_LOCAL_MACHINE, GetParam().policy_name(), GetParam().hklm_value()); + provider_->loader()->Reload(); + loop_.RunAllPending(); provider_->Provide(store_.get()); const Value* value = store_->Get(GetParam().type()); ASSERT_TRUE(value); @@ -317,6 +329,8 @@ TEST_P(ConfigurationPolicyProviderWinTest, HKCU) { WriteValue(HKEY_CURRENT_USER, GetParam().policy_name(), GetParam().hkcu_value()); + provider_->loader()->Reload(); + loop_.RunAllPending(); provider_->Provide(store_.get()); const Value* value = store_->Get(GetParam().type()); ASSERT_TRUE(value); @@ -330,6 +344,8 @@ TEST_P(ConfigurationPolicyProviderWinTest, HKLMOverHKCU) { WriteValue(HKEY_CURRENT_USER, GetParam().policy_name(), GetParam().hkcu_value()); + provider_->loader()->Reload(); + loop_.RunAllPending(); provider_->Provide(store_.get()); const Value* value = store_->Get(GetParam().type()); ASSERT_TRUE(value); diff --git a/chrome/browser/policy/file_based_policy_loader.cc b/chrome/browser/policy/file_based_policy_loader.cc index 5d49f98..4da6192 100644 --- a/chrome/browser/policy/file_based_policy_loader.cc +++ b/chrome/browser/policy/file_based_policy_loader.cc @@ -21,13 +21,14 @@ namespace policy { FileBasedPolicyLoader::FileBasedPolicyLoader( FileBasedPolicyProvider::ProviderDelegate* provider_delegate) - : AsynchronousPolicyLoader(provider_delegate), + : AsynchronousPolicyLoader(provider_delegate, + kReloadIntervalMinutes), config_file_path_(provider_delegate->config_file_path()), - reload_task_(NULL), - reload_interval_(base::TimeDelta::FromMinutes(kReloadIntervalMinutes)), settle_interval_(base::TimeDelta::FromSeconds(kSettleIntervalSeconds)) { } +FileBasedPolicyLoader::~FileBasedPolicyLoader() {} + class FileBasedPolicyWatcherDelegate : public FilePathWatcher::Delegate { public: explicit FileBasedPolicyWatcherDelegate( @@ -49,27 +50,6 @@ class FileBasedPolicyWatcherDelegate : public FilePathWatcher::Delegate { DISALLOW_COPY_AND_ASSIGN(FileBasedPolicyWatcherDelegate); }; -void FileBasedPolicyLoader::Init() { - AsynchronousPolicyLoader::Init(); - - // Initialization can happen early when the file thread is not yet available, - // but the watcher's initialization must be done on the file thread. Posting - // to a to the file directly before the file thread is initialized will cause - // the task to be forgotten. Instead, post a task to the ui thread to delay - // the remainder of initialization until threading is fully initialized. - BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, - NewRunnableMethod(this, - &FileBasedPolicyLoader::InitAfterFileThreadAvailable)); -} - -void FileBasedPolicyLoader::Stop() { - AsynchronousPolicyLoader::Stop(); - BrowserThread::PostTask( - BrowserThread::FILE, FROM_HERE, - NewRunnableMethod(this, &FileBasedPolicyLoader::StopOnFileThread)); -} - void FileBasedPolicyLoader::OnFilePathChanged( const FilePath& path) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); @@ -81,8 +61,6 @@ void FileBasedPolicyLoader::OnError() { << " failed."; } -FileBasedPolicyLoader::~FileBasedPolicyLoader() {} - void FileBasedPolicyLoader::Reload() { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); @@ -111,14 +89,6 @@ void FileBasedPolicyLoader::Reload() { ScheduleFallbackReloadTask(); } -void FileBasedPolicyLoader::InitAfterFileThreadAvailable() { - if (provider()) { - BrowserThread::PostTask( - BrowserThread::FILE, FROM_HERE, - NewRunnableMethod(this, &FileBasedPolicyLoader::InitOnFileThread)); - } -} - void FileBasedPolicyLoader::InitOnFileThread() { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); watcher_.reset(new FilePathWatcher); @@ -138,40 +108,7 @@ void FileBasedPolicyLoader::InitOnFileThread() { void FileBasedPolicyLoader::StopOnFileThread() { watcher_.reset(); - if (reload_task_) { - reload_task_->Cancel(); - reload_task_ = NULL; - } -} - -void FileBasedPolicyLoader::ScheduleReloadTask( - const base::TimeDelta& delay) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); - - if (reload_task_) - reload_task_->Cancel(); - - reload_task_ = - NewRunnableMethod(this, &FileBasedPolicyLoader::ReloadFromTask); - BrowserThread::PostDelayedTask(BrowserThread::FILE, FROM_HERE, reload_task_, - delay.InMilliseconds()); -} - -void FileBasedPolicyLoader::ScheduleFallbackReloadTask() { - // As a safeguard in case that the load delegate failed to timely notice a - // change in policy, schedule a reload task that'll make us recheck after a - // reasonable interval. - ScheduleReloadTask(reload_interval_); -} - -void FileBasedPolicyLoader::ReloadFromTask() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); - - // Drop the reference to the reload task, since the task might be the only - // referer that keeps us alive, so we should not Cancel() it. - reload_task_ = NULL; - - Reload(); + AsynchronousPolicyLoader::StopOnFileThread(); } bool FileBasedPolicyLoader::IsSafeToReloadPolicy( diff --git a/chrome/browser/policy/file_based_policy_loader.h b/chrome/browser/policy/file_based_policy_loader.h index f718e8d..97c3198 100644 --- a/chrome/browser/policy/file_based_policy_loader.h +++ b/chrome/browser/policy/file_based_policy_loader.h @@ -21,9 +21,7 @@ class FileBasedPolicyLoader : public AsynchronousPolicyLoader { FileBasedPolicyLoader( FileBasedPolicyProvider::ProviderDelegate* provider_delegate); - // AsynchronousPolicyLoader implementation: - virtual void Init(); - virtual void Stop(); + // AsynchronousPolicyLoader overrides: virtual void Reload(); void OnFilePathChanged(const FilePath& path); @@ -37,30 +35,14 @@ class FileBasedPolicyLoader : public AsynchronousPolicyLoader { const FilePath& config_file_path() { return config_file_path_; } - private: - // Finishes initialization after the threading system has been fully - // intiialized. - void InitAfterFileThreadAvailable(); - - // Creates the file path watcher, configures it to watch |config_file_path_| - // and schedules the fallback reload task. Must be called on the file thread. - void InitOnFileThread(); - - // Cancels file path watch notification and destroys the watcher. - // Must be called on file thread. - void StopOnFileThread(); - - // Schedules a reload task to run when |delay| expires. Must be called on the - // file thread. - void ScheduleReloadTask(const base::TimeDelta& delay); + // AsynchronousPolicyLoader overrides: - // Schedules a reload task to run after the number of minutes specified - // in |reload_interval_minutes_|. Must be called on the file thread. - void ScheduleFallbackReloadTask(); - - // Invoked from the reload task on the file thread. - void ReloadFromTask(); + // Creates the file path watcher and configures it to watch + // |config_file_path_|. Must be called on the file thread. + virtual void InitOnFileThread(); + virtual void StopOnFileThread(); + private: // Checks whether policy information is safe to read. If not, returns false // and then delays until it is considered safe to reload in |delay|. // Must be called on the file thread. @@ -76,14 +58,6 @@ class FileBasedPolicyLoader : public AsynchronousPolicyLoader { // loader and keeps it alive. scoped_ptr<FilePathWatcher> watcher_; - // The reload task. Access only on the file thread. Holds a reference to the - // currently posted task, so we can cancel and repost it if necessary. - CancelableTask* reload_task_; - - // The interval that a policy reload will be triggered as a fallback even if - // the delegate doesn't indicate that one is needed. - const base::TimeDelta reload_interval_; - // Settle interval. const base::TimeDelta settle_interval_; diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index d403c46..153568d 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -1977,12 +1977,16 @@ 'browser/policy/asynchronous_policy_provider.h', 'browser/policy/config_dir_policy_provider.cc', 'browser/policy/config_dir_policy_provider.h', + 'browser/policy/configuration_policy_loader_win.cc', + 'browser/policy/configuration_policy_loader_win.h', 'browser/policy/configuration_policy_pref_store.cc', 'browser/policy/configuration_policy_pref_store.h', 'browser/policy/configuration_policy_provider.cc', 'browser/policy/configuration_policy_provider.cc', 'browser/policy/configuration_policy_provider.h', 'browser/policy/configuration_policy_provider.h', + 'browser/policy/configuration_policy_provider_delegate_win.cc', + 'browser/policy/configuration_policy_provider_delegate_win.h', 'browser/policy/configuration_policy_provider_mac.cc', 'browser/policy/configuration_policy_provider_mac.h', 'browser/policy/configuration_policy_provider_win.cc', |