summaryrefslogtreecommitdiffstats
path: root/chrome/browser
diff options
context:
space:
mode:
authormnissler@chromium.org <mnissler@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-08-31 15:49:20 +0000
committermnissler@chromium.org <mnissler@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-08-31 15:49:20 +0000
commit9dd3ca659246cb1dced7e0c6b0b962dad33f9a48 (patch)
treea9e4ce71be478240749cfe39d8d08a9d34523277 /chrome/browser
parent5c7fa36cc3186a7f3da8925c90fae68b3d70a20d (diff)
downloadchromium_src-9dd3ca659246cb1dced7e0c6b0b962dad33f9a48.zip
chromium_src-9dd3ca659246cb1dced7e0c6b0b962dad33f9a48.tar.gz
chromium_src-9dd3ca659246cb1dced7e0c6b0b962dad33f9a48.tar.bz2
[win] Add fallback for periodic reload to ConfigurationPolicyProviderWin.
In-the-wild crashes indicate it's possible RegisterGPNotification fails. Handle that case gracefully, i.e. degrade to periodic group policy refresh. BUG=53250 TEST=ConfigurationPolicyProviderWinTest, no more crashes due to CHECKs in ConfigurationPolicyProviderWin::GroupPolicyChangeWatcher::GroupPolicyChangeWatcher() Review URL: http://codereview.chromium.org/3233007 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@57999 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser')
-rw-r--r--chrome/browser/policy/configuration_policy_provider_win.cc132
-rw-r--r--chrome/browser/policy/configuration_policy_provider_win.h59
2 files changed, 155 insertions, 36 deletions
diff --git a/chrome/browser/policy/configuration_policy_provider_win.cc b/chrome/browser/policy/configuration_policy_provider_win.cc
index 0ccb559..7130083 100644
--- a/chrome/browser/policy/configuration_policy_provider_win.cc
+++ b/chrome/browser/policy/configuration_policy_provider_win.cc
@@ -20,41 +20,123 @@
#include "base/values.h"
#include "chrome/common/policy_constants.h"
+namespace {
+
+// Period at which to run the reload task in case the group policy change
+// watchers fail.
+const int kReloadIntervalMinutes = 15;
+
+}
+
ConfigurationPolicyProviderWin::GroupPolicyChangeWatcher::
- GroupPolicyChangeWatcher(ConfigurationPolicyProvider* provider)
+ GroupPolicyChangeWatcher(
+ base::WeakPtr<ConfigurationPolicyProviderWin> provider,
+ int reload_interval_minutes)
: provider_(provider),
user_policy_changed_event_(false, false),
- machine_policy_changed_event_(false, false) {
- CHECK(RegisterGPNotification(user_policy_changed_event_.handle(), false));
- CHECK(RegisterGPNotification(machine_policy_changed_event_.handle(), true));
- user_policy_watcher_.StartWatching(
- user_policy_changed_event_.handle(),
- this);
- machine_policy_watcher_.StartWatching(
- machine_policy_changed_event_.handle(),
- this);
+ 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) {
- provider_->NotifyStoreOfPolicyChange();
- if (object == user_policy_changed_event_.handle()) {
- user_policy_watcher_.StartWatching(
- user_policy_changed_event_.handle(),
- this);
- } else if (object == machine_policy_changed_event_.handle()) {
- machine_policy_watcher_.StartWatching(
- machine_policy_changed_event_.handle(),
- this);
- } else {
- // This method should only be called as a result of signals
- // to the user- and machine-policy handle watchers.
- NOTREACHED();
- }
+ DCHECK(object == user_policy_changed_event_.handle() ||
+ object == machine_policy_changed_event_.handle());
+ Reload();
+}
+
+void ConfigurationPolicyProviderWin::GroupPolicyChangeWatcher::
+ WillDestroyCurrentMessageLoop() {
+ reload_task_ = NULL;
+ MessageLoop::current()->RemoveDestructionObserver(this);
}
ConfigurationPolicyProviderWin::ConfigurationPolicyProviderWin() {
- watcher_.reset(new GroupPolicyChangeWatcher(this));
+ watcher_ = new GroupPolicyChangeWatcher(this->AsWeakPtr(),
+ kReloadIntervalMinutes);
+ watcher_->Start();
+}
+
+ConfigurationPolicyProviderWin::~ConfigurationPolicyProviderWin() {
+ watcher_->Stop();
}
bool ConfigurationPolicyProviderWin::GetRegistryPolicyString(
diff --git a/chrome/browser/policy/configuration_policy_provider_win.h b/chrome/browser/policy/configuration_policy_provider_win.h
index 59a2770..ea10aea 100644
--- a/chrome/browser/policy/configuration_policy_provider_win.h
+++ b/chrome/browser/policy/configuration_policy_provider_win.h
@@ -7,8 +7,10 @@
#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.h"
#include "chrome/browser/policy/configuration_policy_provider.h"
@@ -20,27 +22,63 @@ class RegKey;
// 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 {
+class ConfigurationPolicyProviderWin
+ : public ConfigurationPolicyProvider,
+ public base::SupportsWeakPtr<ConfigurationPolicyProviderWin> {
public:
- // Keeps watch on Windows Group Policy notification event to trigger
- // a policy reload when Group Policy changes.
- class GroupPolicyChangeWatcher : public base::ObjectWatcher::Delegate {
+ // 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:
- explicit GroupPolicyChangeWatcher(ConfigurationPolicyProvider* provider);
- virtual ~GroupPolicyChangeWatcher() {}
+ GroupPolicyChangeWatcher(
+ base::WeakPtr<ConfigurationPolicyProviderWin> provider,
+ int reload_interval_minutes);
+ virtual ~GroupPolicyChangeWatcher();
- virtual void OnObjectSignaled(HANDLE object);
+ // Start watching.
+ void Start();
+
+ // Stop any pending watch activity in order to allow for timely shutdown.
+ void Stop();
private:
- ConfigurationPolicyProvider* provider_;
+ // 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_;
};
ConfigurationPolicyProviderWin();
- virtual ~ConfigurationPolicyProviderWin() { }
+ virtual ~ConfigurationPolicyProviderWin();
// ConfigurationPolicyProvider method overrides:
virtual bool Provide(ConfigurationPolicyStore* store);
@@ -51,7 +89,7 @@ class ConfigurationPolicyProviderWin : public ConfigurationPolicyProvider {
static const wchar_t kPolicyRegistrySubKey[];
private:
- scoped_ptr<GroupPolicyChangeWatcher> watcher_;
+ scoped_refptr<GroupPolicyChangeWatcher> watcher_;
// Methods to perform type-specific policy lookups in the registry.
// HKLM is checked first, then HKCU.
@@ -69,4 +107,3 @@ class ConfigurationPolicyProviderWin : public ConfigurationPolicyProvider {
};
#endif // CHROME_BROWSER_POLICY_CONFIGURATION_POLICY_PROVIDER_WIN_H_
-
generated by cgit v1.1 at 2025-03-06 13:04:24 +0000