Fix style issues with ConfigDirPolicyProvider.

BUG=none
TEST=none

Review URL: http://codereview.chromium.org/3331001

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@59086 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 27 insertions, 12 deletions

diff --git a/chrome/browser/policy/config_dir_policy_provider.cc b/chrome/browser/policy/config_dir_policy_provider.cc
index 7f1a738..1890deb 100644
--- a/chrome/browser/policy/config_dir_policy_provider.cc
+++ b/chrome/browser/policy/config_dir_policy_provider.cc
@@ -8,8 +8,11 @@
 
 #include "base/file_util.h"
 #include "base/logging.h"
+#include "base/message_loop.h"
+#include "base/task.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
+#include "chrome/browser/chrome_thread.h"
 #include "chrome/common/json_value_serializer.h"
 
 namespace {
@@ -222,7 +225,7 @@ void PolicyDirWatcher::InitWatcher(
     return;
   }
 
-  if (!Watch(loader->config_dir(), loader.get()))
+  if (!watcher_.Watch(loader->config_dir(), loader.get()))
     loader->OnError();
 
   // There might have been changes to the directory in the time between
diff --git a/chrome/browser/policy/config_dir_policy_provider.h b/chrome/browser/policy/config_dir_policy_provider.h
index c310d99..cb4e1a1 100644
--- a/chrome/browser/policy/config_dir_policy_provider.h
+++ b/chrome/browser/policy/config_dir_policy_provider.h
@@ -11,10 +11,12 @@
 #include "base/lock.h"
 #include "base/ref_counted.h"
 #include "base/scoped_ptr.h"
+#include "base/time.h"
 #include "base/weak_ptr.h"
 #include "chrome/browser/file_path_watcher.h"
 #include "chrome/browser/policy/configuration_policy_provider.h"
 
+class CancelableTask;
 class ConfigDirPolicyProvider;
 class DictionaryValue;
 class MessageLoop;
@@ -24,10 +26,13 @@ class MessageLoop;
 // currently effective policy dictionary and updates it as appropriate.
 class PolicyDirLoader : public FilePathWatcher::Delegate {
  public:
-  // Create a new loader that'll load its data from |config_dir|.
+  // Creates a new loader that'll load its data from |config_dir|. The
+  // parameters |settle_interval_seconds| and |reload_interval_minutes| specify
+  // the time to wait before reading the directory contents after a change and
+  // the period for checking |config_dir| for changes, respectively.
   PolicyDirLoader(base::WeakPtr<ConfigDirPolicyProvider> provider,
                   const FilePath& config_dir,
-                  int settle_interval_second,
+                  int settle_interval_seconds,
                   int reload_interval_minutes);
 
   // Stops any pending reload tasks.
@@ -37,8 +42,8 @@ class PolicyDirLoader : public FilePathWatcher::Delegate {
   // called on the file thread.
   void Reload();
 
-  // Get the current dictionary value object. Ownership of the returned value is
-  // transferred to the caller.
+  // Gets the current dictionary value object. Ownership of the returned value
+  // is transferred to the caller.
   DictionaryValue* GetPolicy();
 
   const FilePath& config_dir() { return config_dir_; }
@@ -48,16 +53,17 @@ class PolicyDirLoader : public FilePathWatcher::Delegate {
   void OnError();
 
  private:
-  // Load the policy information. Ownership of the return value is transferred
+  // Loads the policy information. Ownership of the return value is transferred
   // to the caller.
   DictionaryValue* Load();
 
-  // Check the directory modification time to see whether reading the
+  // Checks the directory modification time to see whether reading the
   // configuration directory is safe. If not, returns false and the delay until
   // it is considered safe to reload in |delay|.
   bool IsSafeToReloadPolicy(const base::Time& now, base::TimeDelta* delay);
 
-  // Post a reload task. Must be called on the file thread.
+  // Schedules a reload task to run when |delay| expires. Must be called on the
+  // file thread.
   void ScheduleReloadTask(const base::TimeDelta& delay);
 
   // Notifies the policy provider to send out a policy changed notification.
@@ -107,19 +113,25 @@ class PolicyDirLoader : public FilePathWatcher::Delegate {
 
 // Wraps a FilePathWatcher for the configuration directory and takes care of
 // initializing the watcher object on the file thread.
-class PolicyDirWatcher : public FilePathWatcher,
-                         public base::RefCountedThreadSafe<PolicyDirWatcher> {
+class PolicyDirWatcher : public base::RefCountedThreadSafe<PolicyDirWatcher> {
  public:
   PolicyDirWatcher() {}
 
-  // Run initialization. This is in a separate method since we need to post a
+  // Runs initialization. This is in a separate method since we need to post a
   // task (which cannot be done from the constructor).
   void Init(PolicyDirLoader* loader);
 
  private:
+  // PolicyDirWatcher objects should only be deleted by RefCountedThreadSafe.
+  friend class base::RefCountedThreadSafe<PolicyDirWatcher>;
+  ~PolicyDirWatcher() {}
+
   // Actually sets up the watch with the FilePathWatcher code.
   void InitWatcher(const scoped_refptr<PolicyDirLoader>& loader);
 
+  // Wrapped watcher that takes care of the actual watching.
+  FilePathWatcher watcher_;
+
   DISALLOW_COPY_AND_ASSIGN(PolicyDirWatcher);
 };
 
diff --git a/chrome/browser/policy/config_dir_policy_provider_unittest.cc b/chrome/browser/policy/config_dir_policy_provider_unittest.cc
index c7a2e94..4cb0171 100644
--- a/chrome/browser/policy/config_dir_policy_provider_unittest.cc
+++ b/chrome/browser/policy/config_dir_policy_provider_unittest.cc
@@ -8,8 +8,8 @@
 #include "chrome/browser/policy/config_dir_policy_provider.h"
 #include "chrome/browser/policy/mock_configuration_policy_store.h"
 #include "chrome/common/json_value_serializer.h"
-#include "testing/gtest/include/gtest/gtest.h"
 #include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
 
 using testing::Mock;