Break CloudPolicyRefreshScheduler's dependency on PrefService.

This is in preparation for refreshing public account policy, which
doesn't have a PrefService at its disposal. CloudPolicyManager now takes
care of extracting the pref value and passing it on to
CloudPolicyRefreshScheduler.

BUG=chromium:152937
TEST=unit tests


Review URL: https://chromiumcodereview.appspot.com/11299305

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

chrome/browser/policy/cloud_policy_manager.cc

@@ -11,6 +11,7 @@
 #include "chrome/browser/policy/cloud_policy_service.h"
 #include "chrome/browser/policy/policy_bundle.h"
 #include "chrome/browser/policy/policy_map.h"
+#include "chrome/browser/prefs/pref_service.h"
 
 namespace policy {
 
@@ -73,6 +74,7 @@ void CloudPolicyManager::InitializeService(
 }
 
 void CloudPolicyManager::ShutdownService() {
+  refresh_delay_.reset();
   refresh_scheduler_.reset();
   service_.reset();
   client_.reset();
@@ -82,10 +84,15 @@ void CloudPolicyManager::StartRefreshScheduler(
     PrefService* local_state,
     const std::string& refresh_rate_pref) {
   if (!refresh_scheduler_.get()) {
+    refresh_delay_.reset(new IntegerPrefMember());
+    refresh_delay_->Init(refresh_rate_pref.c_str(), local_state,
+                         base::Bind(&CloudPolicyManager::UpdateRefreshDelay,
+                                    base::Unretained(this)));
     refresh_scheduler_.reset(
         new CloudPolicyRefreshScheduler(
-            client_.get(), store_, local_state, refresh_rate_pref,
+            client_.get(), store_,
             MessageLoop::current()->message_loop_proxy()));
+    UpdateRefreshDelay();
   }
 }
 
@@ -103,4 +110,8 @@ void CloudPolicyManager::OnRefreshComplete() {
   CheckAndPublishPolicy();
 }
 
+void CloudPolicyManager::UpdateRefreshDelay() {
+  refresh_scheduler_->SetRefreshDelay(refresh_delay_->GetValue());
+}
+
 }  // namespace policy

chrome/browser/policy/cloud_policy_manager.h

@@ -10,6 +10,7 @@
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/memory/scoped_ptr.h"
+#include "chrome/browser/api/prefs/pref_member.h"
 #include "chrome/browser/policy/cloud_policy_store.h"
 #include "chrome/browser/policy/configuration_policy_provider.h"
 
@@ -76,6 +77,9 @@ class CloudPolicyManager : public ConfigurationPolicyProvider,
   // Completion handler for policy refresh operations.
   void OnRefreshComplete();
 
+  // Updates the refresh scheduler on refresh delay changes.
+  void UpdateRefreshDelay();
+
   CloudPolicyStore* store_;
   scoped_ptr<CloudPolicyClient> client_;
   scoped_ptr<CloudPolicyService> service_;
@@ -85,6 +89,9 @@ class CloudPolicyManager : public ConfigurationPolicyProvider,
   // policy update notifications are deferred until after it completes.
   bool waiting_for_policy_refresh_;
 
+  // Keeps track of the refresh delay pref.
+  scoped_ptr<IntegerPrefMember> refresh_delay_;
+
   DISALLOW_COPY_AND_ASSIGN(CloudPolicyManager);
 };
 

chrome/browser/policy/cloud_policy_refresh_scheduler.cc

@@ -8,12 +8,13 @@
 
 #include "base/task_runner.h"
 #include "chrome/browser/policy/cloud_policy_constants.h"
-#include "chrome/browser/prefs/pref_service.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "content/public/browser/notification_details.h"
 
 namespace policy {
 
+const int64 CloudPolicyRefreshScheduler::kDefaultRefreshDelayMs =
+    3 * 60 * 60 * 1000;  // 3 hours.
 const int64 CloudPolicyRefreshScheduler::kUnmanagedRefreshDelayMs =
     24 * 60 * 60 * 1000;  // 1 day.
 const int64 CloudPolicyRefreshScheduler::kInitialErrorRetryDelayMs =
@@ -26,22 +27,16 @@ const int64 CloudPolicyRefreshScheduler::kRefreshDelayMaxMs =
 CloudPolicyRefreshScheduler::CloudPolicyRefreshScheduler(
     CloudPolicyClient* client,
     CloudPolicyStore* store,
-    PrefService* prefs,
-    const std::string& refresh_pref,
     const scoped_refptr<base::TaskRunner>& task_runner)
     : client_(client),
       store_(store),
       task_runner_(task_runner),
-      error_retry_delay_ms_(kInitialErrorRetryDelayMs) {
+      error_retry_delay_ms_(kInitialErrorRetryDelayMs),
+      refresh_delay_ms_(kDefaultRefreshDelayMs) {
   client_->AddObserver(this);
   store_->AddObserver(this);
   net::NetworkChangeNotifier::AddIPAddressObserver(this);
 
-  refresh_delay_.Init(
-      refresh_pref.c_str(), prefs,
-      base::Bind(&CloudPolicyRefreshScheduler::ScheduleRefresh,
-                 base::Unretained(this)));
-
   UpdateLastRefreshFromPolicy();
   ScheduleRefresh();
 }
@@ -52,6 +47,12 @@ CloudPolicyRefreshScheduler::~CloudPolicyRefreshScheduler() {
   net::NetworkChangeNotifier::RemoveIPAddressObserver(this);
 }
 
+void CloudPolicyRefreshScheduler::SetRefreshDelay(int64 refresh_delay) {
+  refresh_delay_ms_ = std::min(std::max(refresh_delay, kRefreshDelayMinMs),
+                               kRefreshDelayMaxMs);
+  ScheduleRefresh();
+}
+
 void CloudPolicyRefreshScheduler::OnPolicyFetched(CloudPolicyClient* client) {
   error_retry_delay_ms_ = kInitialErrorRetryDelayMs;
 
@@ -82,7 +83,7 @@ void CloudPolicyRefreshScheduler::OnClientError(CloudPolicyClient* client) {
       (status == DM_STATUS_REQUEST_FAILED ||
        status == DM_STATUS_TEMPORARY_UNAVAILABLE)) {
     error_retry_delay_ms_ = std::min(error_retry_delay_ms_ * 2,
-                                     GetRefreshDelay());
+                                     refresh_delay_ms_);
   } else {
     error_retry_delay_ms_ = kInitialErrorRetryDelayMs;
   }
@@ -141,13 +142,13 @@ void CloudPolicyRefreshScheduler::ScheduleRefresh() {
   switch (client_->status()) {
     case DM_STATUS_SUCCESS:
       if (store_->is_managed())
-        RefreshAfter(GetRefreshDelay());
+        RefreshAfter(refresh_delay_ms_);
       else
         RefreshAfter(kUnmanagedRefreshDelayMs);
       return;
     case DM_STATUS_SERVICE_ACTIVATION_PENDING:
     case DM_STATUS_SERVICE_POLICY_NOT_FOUND:
-      RefreshAfter(GetRefreshDelay());
+      RefreshAfter(refresh_delay_ms_);
       return;
     case DM_STATUS_REQUEST_FAILED:
     case DM_STATUS_TEMPORARY_UNAVAILABLE:
@@ -202,10 +203,4 @@ void CloudPolicyRefreshScheduler::RefreshAfter(int delta_ms) {
   task_runner_->PostDelayedTask(FROM_HERE, refresh_callback_.callback(), delay);
 }
 
-int64 CloudPolicyRefreshScheduler::GetRefreshDelay() {
-  return std::min(std::max<int64>(refresh_delay_.GetValue(),
-                                  kRefreshDelayMinMs),
-                  kRefreshDelayMaxMs);
-}
-
 }  // namespace policy

chrome/browser/policy/cloud_policy_refresh_scheduler.h

@@ -5,19 +5,14 @@
 #ifndef CHROME_BROWSER_POLICY_CLOUD_POLICY_REFRESH_SCHEDULER_H_
 #define CHROME_BROWSER_POLICY_CLOUD_POLICY_REFRESH_SCHEDULER_H_
 
-#include <string>
-
 #include "base/basictypes.h"
 #include "base/cancelable_callback.h"
 #include "base/memory/ref_counted.h"
 #include "base/time.h"
-#include "chrome/browser/api/prefs/pref_member.h"
 #include "chrome/browser/policy/cloud_policy_client.h"
 #include "chrome/browser/policy/cloud_policy_store.h"
 #include "net/base/network_change_notifier.h"
 
-class PrefService;
-
 namespace base {
 class TaskRunner;
 }
@@ -32,6 +27,7 @@ class CloudPolicyRefreshScheduler
       public net::NetworkChangeNotifier::IPAddressObserver {
  public:
   // Refresh constants.
+  static const int64 kDefaultRefreshDelayMs;
   static const int64 kUnmanagedRefreshDelayMs;
   static const int64 kInitialErrorRetryDelayMs;
 
@@ -44,11 +40,12 @@ class CloudPolicyRefreshScheduler
   CloudPolicyRefreshScheduler(
       CloudPolicyClient* client,
       CloudPolicyStore* store,
-      PrefService* prefs,
-      const std::string& refresh_pref,
       const scoped_refptr<base::TaskRunner>& task_runner);
   virtual ~CloudPolicyRefreshScheduler();
 
+  // Sets the refresh delay to |refresh_delay| (subject to min/max clamping).
+  void SetRefreshDelay(int64 refresh_delay);
+
   // CloudPolicyClient::Observer:
   virtual void OnPolicyFetched(CloudPolicyClient* client) OVERRIDE;
   virtual void OnRegistrationStateChanged(CloudPolicyClient* client) OVERRIDE;
@@ -80,9 +77,6 @@ class CloudPolicyRefreshScheduler
   // relative to |last_refresh_|.
   void RefreshAfter(int delta_ms);
 
-  // Gets the refresh delay in milliseconds, clamped to the allowed bounds.
-  int64 GetRefreshDelay();
-
   CloudPolicyClient* client_;
   CloudPolicyStore* store_;
 
@@ -98,7 +92,8 @@ class CloudPolicyRefreshScheduler
   // Error retry delay in milliseconds.
   int64 error_retry_delay_ms_;
 
-  IntegerPrefMember refresh_delay_;
+  // The refresh delay.
+  int64 refresh_delay_ms_;
 
   DISALLOW_COPY_AND_ASSIGN(CloudPolicyRefreshScheduler);
 };

chrome/browser/policy/cloud_policy_refresh_scheduler_unittest.cc

@@ -12,8 +12,7 @@
 #include "chrome/browser/policy/mock_cloud_policy_store.h"
 #include "chrome/browser/policy/test_task_runner.h"
 #include "chrome/browser/prefs/browser_prefs.h"
-#include "chrome/common/pref_names.h"
-#include "chrome/test/base/testing_pref_service.h"
+#include "policy/policy_constants.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -34,10 +33,7 @@ class CloudPolicyRefreshSchedulerTest : public testing::Test {
  protected:
   CloudPolicyRefreshSchedulerTest()
       : task_runner_(new TestTaskRunner()),
-        network_change_notifier_(net::NetworkChangeNotifier::CreateMock()) {
-    chrome::RegisterLocalState(&prefs_);
-    prefs_.SetInteger(prefs::kUserPolicyRefreshRate, kPolicyRefreshRate);
-  }
+        network_change_notifier_(net::NetworkChangeNotifier::CreateMock()) {}
 
   virtual void SetUp() OVERRIDE {
     client_.SetDMToken("token");
@@ -61,9 +57,10 @@ class CloudPolicyRefreshSchedulerTest : public testing::Test {
   }
 
   CloudPolicyRefreshScheduler* CreateRefreshScheduler() {
-    return new CloudPolicyRefreshScheduler(&client_, &store_, &prefs_,
-                                         prefs::kUserPolicyRefreshRate,
-                                         task_runner_);
+    CloudPolicyRefreshScheduler* scheduler =
+        new CloudPolicyRefreshScheduler(&client_, &store_, task_runner_);
+    scheduler->SetRefreshDelay(kPolicyRefreshRate);
+    return scheduler;
   }
 
   void NotifyIPAddressChanged() {
@@ -82,7 +79,6 @@ class CloudPolicyRefreshSchedulerTest : public testing::Test {
   MessageLoop loop_;
   MockCloudPolicyClient client_;
   MockCloudPolicyStore store_;
-  TestingPrefService prefs_;
   scoped_refptr<TestTaskRunner> task_runner_;
   scoped_ptr<net::NetworkChangeNotifier> network_change_notifier_;
 
@@ -136,19 +132,19 @@ TEST_F(CloudPolicyRefreshSchedulerTest, Unregistered) {
   client_.NotifyPolicyFetched();
   client_.NotifyRegistrationStateChanged();
   client_.NotifyClientError();
+  scheduler->SetRefreshDelay(12 * 60 * 60 * 1000);
   store_.NotifyStoreLoaded();
   store_.NotifyStoreError();
-  prefs_.SetInteger(prefs::kUserPolicyRefreshRate, 12 * 60 * 60 * 1000);
 }
 
 class CloudPolicyRefreshSchedulerSteadyStateTest
     : public CloudPolicyRefreshSchedulerTest {
  protected:
   CloudPolicyRefreshSchedulerSteadyStateTest()
-      : refresh_scheduler_(&client_, &store_, &prefs_,
-                           prefs::kUserPolicyRefreshRate, task_runner_) {}
+      : refresh_scheduler_(&client_, &store_, task_runner_) {}
 
   virtual void SetUp() OVERRIDE {
+    refresh_scheduler_.SetRefreshDelay(kPolicyRefreshRate);
     CloudPolicyRefreshSchedulerTest::SetUp();
     last_refresh_ = base::Time::NowFromSystemTime();
     client_.NotifyPolicyFetched();
@@ -185,15 +181,15 @@ TEST_F(CloudPolicyRefreshSchedulerSteadyStateTest, OnStoreError) {
 
 TEST_F(CloudPolicyRefreshSchedulerSteadyStateTest, RefreshDelayChange) {
   const int delay_short_ms = 5 * 60 * 1000;
-  prefs_.SetInteger(prefs::kUserPolicyRefreshRate, delay_short_ms);
+  refresh_scheduler_.SetRefreshDelay(delay_short_ms);
   CheckTiming(CloudPolicyRefreshScheduler::kRefreshDelayMinMs);
 
   const int delay_ms = 12 * 60 * 60 * 1000;
-  prefs_.SetInteger(prefs::kUserPolicyRefreshRate, delay_ms);
+  refresh_scheduler_.SetRefreshDelay(delay_ms);
   CheckTiming(delay_ms);
 
   const int delay_long_ms = 2 * 24 * 60 * 60 * 1000;
-  prefs_.SetInteger(prefs::kUserPolicyRefreshRate, delay_long_ms);
+  refresh_scheduler_.SetRefreshDelay(delay_long_ms);
   CheckTiming(CloudPolicyRefreshScheduler::kRefreshDelayMaxMs);
 }