Remove the delay for invalidations from cloud policy refresh scheduler.

Through UMA metrics we have discovered that it can take longer than expected for the invalidation service to become ready after sign in. The strategy of delaying the refresh scheduler to wait for invalidations to be ready is not useful. This change removes the delay and allows the refresh scheduler to fetch immediately on sign in.

BUG=355884

Review URL: https://codereview.chromium.org/210293006

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

3 files changed, 12 insertions, 112 deletions

diff --git a/components/policy/core/common/cloud/cloud_policy_refresh_scheduler.cc b/components/policy/core/common/cloud/cloud_policy_refresh_scheduler.cc
index 9bc4a32..ea4d53d 100644
--- a/components/policy/core/common/cloud/cloud_policy_refresh_scheduler.cc
+++ b/components/policy/core/common/cloud/cloud_policy_refresh_scheduler.cc
@@ -22,10 +22,6 @@ namespace {
 // The maximum rate at which to refresh policies.
 const size_t kMaxRefreshesPerHour = 5;
 
-// The maximum time to wait for the invalidations service to become available
-// before starting to issue requests.
-const int kWaitForInvalidationsTimeoutSeconds = 5;
-
 }  // namespace
 
 #if defined(OS_ANDROID) || defined(OS_IOS)
@@ -90,13 +86,7 @@ CloudPolicyRefreshScheduler::CloudPolicyRefreshScheduler(
   net::NetworkChangeNotifier::AddIPAddressObserver(this);
 
   UpdateLastRefreshFromPolicy();
-
-  // Give some time for the invalidation service to become available before the
-  // first refresh if there is already policy present.
-  if (store->has_policy())
-    WaitForInvalidationService();
-  else
-    ScheduleRefresh();
+  ScheduleRefresh();
 }
 
 CloudPolicyRefreshScheduler::~CloudPolicyRefreshScheduler() {
@@ -112,10 +102,6 @@ void CloudPolicyRefreshScheduler::SetRefreshDelay(int64 refresh_delay) {
 }
 
 void CloudPolicyRefreshScheduler::RefreshSoon() {
-  // An external consumer needs a policy update now (e.g. a new extension, or
-  // the InvalidationService received a policy invalidation), so don't wait
-  // before fetching anymore.
-  wait_for_invalidations_timeout_callback_.Cancel();
   rate_limiter_.PostRequest();
 }
 
@@ -129,21 +115,14 @@ void CloudPolicyRefreshScheduler::SetInvalidationServiceAvailability(
   }
 
   if (is_available == invalidations_available_) {
-    // No change in state. If we're currently WaitingForInvalidationService
-    // then the timeout task will eventually execute and trigger a reschedule;
-    // let the InvalidationService keep retrying until that happens.
+    // No change in state.
     return;
   }
 
-  wait_for_invalidations_timeout_callback_.Cancel();
   invalidations_available_ = is_available;
 
-  // Schedule a refresh since the refresh delay has been updated; however, allow
-  // some time for the invalidation service to update. If it is now online, the
-  // wait allows pending invalidations to be delivered. If it is now offline,
-  // then the wait allows for the service to recover from transient failure
-  // before falling back on the polling behavior.
-  WaitForInvalidationService();
+  // Schedule a refresh since the refresh delay has been updated.
+  ScheduleRefresh();
 }
 
 void CloudPolicyRefreshScheduler::OnPolicyFetched(CloudPolicyClient* client) {
@@ -264,11 +243,6 @@ void CloudPolicyRefreshScheduler::ScheduleRefresh() {
     return;
   }
 
-  // Don't schedule anything yet if we're still waiting for the invalidations
-  // service.
-  if (WaitingForInvalidationService())
-    return;
-
   // If policy invalidations are available then periodic updates are done at
   // a much lower rate; otherwise use the |refresh_delay_ms_| value.
   int64 refresh_delay_ms =
@@ -344,27 +318,4 @@ void CloudPolicyRefreshScheduler::RefreshAfter(int delta_ms) {
   task_runner_->PostDelayedTask(FROM_HERE, refresh_callback_.callback(), delay);
 }
 
-void CloudPolicyRefreshScheduler::WaitForInvalidationService() {
-  DCHECK(!WaitingForInvalidationService());
-  wait_for_invalidations_timeout_callback_.Reset(
-      base::Bind(
-          &CloudPolicyRefreshScheduler::OnWaitForInvalidationServiceTimeout,
-          base::Unretained(this)));
-  base::TimeDelta delay =
-      base::TimeDelta::FromSeconds(kWaitForInvalidationsTimeoutSeconds);
-  task_runner_->PostDelayedTask(
-      FROM_HERE,
-      wait_for_invalidations_timeout_callback_.callback(),
-      delay);
-}
-
-void CloudPolicyRefreshScheduler::OnWaitForInvalidationServiceTimeout() {
-  wait_for_invalidations_timeout_callback_.Cancel();
-  ScheduleRefresh();
-}
-
-bool CloudPolicyRefreshScheduler::WaitingForInvalidationService() const {
-  return !wait_for_invalidations_timeout_callback_.IsCancelled();
-}
-
 }  // namespace policy
diff --git a/components/policy/core/common/cloud/cloud_policy_refresh_scheduler.h b/components/policy/core/common/cloud/cloud_policy_refresh_scheduler.h
index afba9b3..a0e4090 100644
--- a/components/policy/core/common/cloud/cloud_policy_refresh_scheduler.h
+++ b/components/policy/core/common/cloud/cloud_policy_refresh_scheduler.h
@@ -103,16 +103,6 @@ class POLICY_EXPORT CloudPolicyRefreshScheduler
   // relative to |last_refresh_|.
   void RefreshAfter(int delta_ms);
 
-  // Sets the |wait_for_invalidations_timeout_callback_| and schedules it.
-  void WaitForInvalidationService();
-
-  // Callback for |wait_for_invalidations_timeout_callback_|.
-  void OnWaitForInvalidationServiceTimeout();
-
-  // Returns true if the refresh scheduler is currently waiting for the
-  // availability of the invalidations service.
-  bool WaitingForInvalidationService() const;
-
   CloudPolicyClient* client_;
   CloudPolicyStore* store_;
 
@@ -138,12 +128,6 @@ class POLICY_EXPORT CloudPolicyRefreshScheduler
   // of policy updates.
   bool invalidations_available_;
 
-  // The refresh scheduler waits some seconds for the invalidations service
-  // before starting to issue refresh requests. If the invalidations service
-  // doesn't become available during this time then the refresh scheduler will
-  // use the polling refresh rate.
-  base::CancelableClosure wait_for_invalidations_timeout_callback_;
-
   // Used to measure how long it took for the invalidations service to report
   // its initial status.
   base::Time creation_time_;
diff --git a/components/policy/core/common/cloud/cloud_policy_refresh_scheduler_unittest.cc b/components/policy/core/common/cloud/cloud_policy_refresh_scheduler_unittest.cc
index 1750992..63149fc 100644
--- a/components/policy/core/common/cloud/cloud_policy_refresh_scheduler_unittest.cc
+++ b/components/policy/core/common/cloud/cloud_policy_refresh_scheduler_unittest.cc
@@ -57,11 +57,6 @@ class CloudPolicyRefreshSchedulerTest : public testing::Test {
     CloudPolicyRefreshScheduler* scheduler =
         new CloudPolicyRefreshScheduler(&client_, &store_, task_runner_);
     scheduler->SetRefreshDelay(kPolicyRefreshRate);
-    // If the store has policy, run the wait-for-invalidations timeout task.
-    if (store_.has_policy()) {
-      EXPECT_EQ(1u, task_runner_->GetPendingTasks().size());
-      task_runner_->RunPendingTasks();
-    }
     return scheduler;
   }
 
@@ -219,22 +214,13 @@ TEST_F(CloudPolicyRefreshSchedulerTest, InvalidationsAvailable) {
       new CloudPolicyRefreshScheduler(&client_, &store_, task_runner_));
   scheduler->SetRefreshDelay(kPolicyRefreshRate);
 
-  // The scheduler is currently waiting for the invalidations service to
-  // initialize.
-  EXPECT_EQ(1u, task_runner_->GetPendingTasks().size());
-
-  // Signal that invalidations are available. The scheduler is currently
-  // waiting for any pending invalidations to be received.
-  scheduler->SetInvalidationServiceAvailability(true);
+  // The scheduler has scheduled refreshes at the initial refresh rate.
   EXPECT_EQ(2u, task_runner_->GetPendingTasks().size());
 
-  // Run the invalidation service timeout task.
-  EXPECT_CALL(client_, FetchPolicy()).Times(0);
-  task_runner_->RunPendingTasks();
-  Mock::VerifyAndClearExpectations(&client_);
+  // Signal that invalidations are available.
+  scheduler->SetInvalidationServiceAvailability(true);
+  EXPECT_EQ(3u, task_runner_->GetPendingTasks().size());
 
-  // The initial refresh is scheduled.
-  EXPECT_EQ(1u, task_runner_->GetPendingTasks().size());
   CheckInitialRefresh(true);
 
   EXPECT_CALL(client_, FetchPolicy()).Times(1);
@@ -255,22 +241,13 @@ TEST_F(CloudPolicyRefreshSchedulerTest, InvalidationsNotAvailable) {
       new CloudPolicyRefreshScheduler(&client_, &store_, task_runner_));
   scheduler->SetRefreshDelay(kPolicyRefreshRate);
 
-  // The scheduler is currently waiting for the invalidations service to
-  // initialize.
-  EXPECT_EQ(1u, task_runner_->GetPendingTasks().size());
-
-  // Signal that invalidations are not available. The scheduler will keep
-  // waiting for us.
+  // Signal that invalidations are not available. The scheduler will not
+  // schedule refreshes since the available state is not changed.
   for (int i = 0; i < 10; ++i) {
     scheduler->SetInvalidationServiceAvailability(false);
-    EXPECT_EQ(1u, task_runner_->GetPendingTasks().size());
+    EXPECT_EQ(2u, task_runner_->GetPendingTasks().size());
   }
 
-  // Run the timeout task.
-  EXPECT_CALL(client_, FetchPolicy()).Times(0);
-  task_runner_->RunPendingTasks();
-  Mock::VerifyAndClearExpectations(&client_);
-
   // This scheduled the initial refresh.
   CheckInitialRefresh(false);
 
@@ -301,21 +278,13 @@ TEST_F(CloudPolicyRefreshSchedulerTest, InvalidationsOffAndOn) {
   client_.NotifyPolicyFetched();
 
   // The next refresh has been scheduled using a lower refresh rate.
-  // Flush that task.
   CheckTiming(CloudPolicyRefreshScheduler::kWithInvalidationsRefreshDelayMs);
-  EXPECT_CALL(client_, FetchPolicy()).Times(1);
-  task_runner_->RunPendingTasks();
-  Mock::VerifyAndClearExpectations(&client_);
 
   // If the service goes down and comes back up before the timeout then a
   // refresh is rescheduled at the lower rate again; after executing all
   // pending tasks only 1 fetch is performed.
-  EXPECT_CALL(client_, FetchPolicy()).Times(0);
   scheduler->SetInvalidationServiceAvailability(false);
   scheduler->SetInvalidationServiceAvailability(true);
-  // Run the invalidation service timeout task.
-  task_runner_->RunPendingTasks();
-  Mock::VerifyAndClearExpectations(&client_);
   // The next refresh has been scheduled using a lower refresh rate.
   EXPECT_CALL(client_, FetchPolicy()).Times(1);
   CheckTiming(CloudPolicyRefreshScheduler::kWithInvalidationsRefreshDelayMs);
@@ -343,12 +312,8 @@ TEST_F(CloudPolicyRefreshSchedulerTest, InvalidationsDisconnected) {
   Mock::VerifyAndClearExpectations(&client_);
 
   // If the service goes down then the refresh scheduler falls back on the
-  // default polling rate after a timeout.
-  EXPECT_CALL(client_, FetchPolicy()).Times(0);
+  // default polling rate.
   scheduler->SetInvalidationServiceAvailability(false);
-  task_runner_->RunPendingTasks();
-  Mock::VerifyAndClearExpectations(&client_);
-  // The next refresh has been scheduled at the normal rate.
   CheckTiming(kPolicyRefreshRate);
 }