Refresh policies from DM server periodically

chrome/browser/policy/device_management_policy_provider.cc

 // 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/device_management_policy_provider.h"
 
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/path_service.h"
+#include "base/rand_util.h"
 #include "base/task.h"
-#include "base/time.h"
 #include "chrome/browser/browser_thread.h"
 #include "chrome/browser/policy/device_management_backend.h"
 #include "chrome/browser/policy/device_management_backend_impl.h"
 #include "chrome/browser/policy/device_management_policy_cache.h"
-#include "chrome/browser/policy/device_token_fetcher.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/notification_service.h"
 #include "chrome/common/notification_type.h"
 
 namespace {
 
 const char kChromePolicyScope[] = "cros/device";
 const char kChromeDevicePolicySettingKey[] = "chrome-policy";
-const int64 kPolicyRefreshRateInMinutes = 3 * 60;  // 3 hours
+const int64 kPolicyRefreshRateInMilliseconds = 3 * 60 * 60 * 1000;  // 3 hours
+const int64 kPolicyRefreshMaxEarlierInMilliseconds = 20 * 60 * 1000;  // 20 mins
+// These are the base values for delays before retrying after an error. They
+// will be doubled each time they are used.
+const int64 kPolicyRefreshErrorDelayInMilliseconds = 3 * 1000;  // 3 seconds
+const int64 kDeviceTokenRefreshErrorDelayInMilliseconds = 3 * 1000;
 
 }  // namespace
 
 namespace policy {
 
 // Ensures that the portion of the policy provider implementation that requires
 // the IOThread is deferred until the IOThread is fully initialized. The policy
 // provider posts this task on the UI thread during its constructor, thereby
 // guaranteeing that the code won't get executed until after the UI and IO
 // threads are fully constructed.
 class DeviceManagementPolicyProvider::InitializeAfterIOThreadExistsTask
     : public Task {
  public:
   explicit InitializeAfterIOThreadExistsTask(
       base::WeakPtr<DeviceManagementPolicyProvider> provider)
       : provider_(provider) {
   }
 
   // Task implementation:
   virtual void Run() {
     DeviceManagementPolicyProvider* provider = provider_.get();
     if (provider)
       provider->InitializeAfterIOThreadExists();
   }
 
  private:
   base::WeakPtr<DeviceManagementPolicyProvider> provider_;
 };
 
+class DeviceManagementPolicyProvider::RefreshTask : public Task {
+ public:
+  explicit RefreshTask(base::WeakPtr<DeviceManagementPolicyProvider> provider)
+      : provider_(provider) {}
+
+  // Task implementation:
+  virtual void Run() {
+    DeviceManagementPolicyProvider* provider = provider_.get();
+    if (provider)
+      provider->RefreshTaskExecute();
+  }
+
+ private:
+  base::WeakPtr<DeviceManagementPolicyProvider> provider_;
+};
+
 DeviceManagementPolicyProvider::DeviceManagementPolicyProvider(
     const ConfigurationPolicyProvider::PolicyDefinitionList* policy_list,
     DeviceManagementBackend* backend,
     TokenService* token_service,
     const FilePath& storage_dir)
     : ConfigurationPolicyProvider(policy_list),
       backend_(backend),
       token_service_(token_service),
       storage_dir_(GetOrCreateDeviceManagementDir(storage_dir)),
-      policy_request_pending_(false) {
+      policy_request_pending_(false),
+      refresh_task_pending_(false),
+      policy_refresh_rate_ms_(kPolicyRefreshRateInMilliseconds),
+      policy_refresh_max_earlier_ms_(kPolicyRefreshMaxEarlierInMilliseconds),
+      policy_refresh_error_delay_ms_(kPolicyRefreshErrorDelayInMilliseconds),
+      token_fetch_error_delay_ms_(kDeviceTokenRefreshErrorDelayInMilliseconds) {
   Initialize();
 }
 
 DeviceManagementPolicyProvider::~DeviceManagementPolicyProvider() {}
 
 bool DeviceManagementPolicyProvider::Provide(
     ConfigurationPolicyStoreInterface* policy_store) {
   scoped_ptr<DictionaryValue> policies(cache_->GetPolicy());
   DecodePolicyValueTree(policies.get(), policy_store);
   return true;
 }
 
-void DeviceManagementPolicyProvider::Observe(
-    NotificationType type,
-    const NotificationSource& source,
-    const NotificationDetails& details) {
-  if (type == NotificationType::DEVICE_TOKEN_AVAILABLE) {
-    if (token_fetcher_.get() == Source<DeviceTokenFetcher>(source).ptr() &&
-        !policy_request_pending_ &&
-        IsPolicyStale()) {
-      SendPolicyRequest();
-    }
-  } else {
-    NOTREACHED();
-  }
-}
-
 void DeviceManagementPolicyProvider::HandlePolicyResponse(
     const em::DevicePolicyResponse& response) {
-  cache_->SetPolicy(response);
-  NotifyStoreOfPolicyChange();
+  if (cache_->SetPolicy(response))
+    NotifyStoreOfPolicyChange();
   policy_request_pending_ = false;
+  // Reset the error delay since policy fetching succeeded this time.
+  policy_refresh_error_delay_ms_ = kPolicyRefreshErrorDelayInMilliseconds;
+  ScheduleRefreshTask(GetRefreshTaskDelay(
+      policy_refresh_rate_ms_, policy_refresh_max_earlier_ms_));
 }
 
 void DeviceManagementPolicyProvider::OnError(
     DeviceManagementBackend::ErrorCode code) {
-  LOG(WARNING) << "could not provide policy from the device manager (error = "
+  LOG(WARNING) << "Could not provide policy from the device manager (error = "
                << code << ")";

[danno, 2010/11/22 13:47:50]

you may want to add a "retrying in x millis ...." to the log

[Jakob Kummerow (corp), 2010/11/22 16:56:08]

Done.

   policy_request_pending_ = false;
-  // TODO(danno): do something sensible in the error case, perhaps retry later?
+  policy_refresh_error_delay_ms_ *= 2;
+  if (policy_refresh_rate_ms_ &&
+      policy_refresh_rate_ms_ < policy_refresh_error_delay_ms_) {
+    policy_refresh_error_delay_ms_ = policy_refresh_rate_ms_;
+  }
+  ScheduleRefreshTask(policy_refresh_error_delay_ms_);
+}
+
+void DeviceManagementPolicyProvider::OnTokenSuccess() {
+  if (policy_request_pending_)
+    return;
+  SendPolicyRequest();
+}
+
+void DeviceManagementPolicyProvider::OnTokenError() {
+  LOG(WARNING) << "Could not retrieve device token.";
+  token_fetch_error_delay_ms_ *= 2;
+  if (token_fetch_error_delay_ms_ > policy_refresh_rate_ms_)
+    token_fetch_error_delay_ms_ = policy_refresh_rate_ms_;
+  ScheduleRefreshTask(token_fetch_error_delay_ms_);

[danno, 2010/11/22 13:47:50]

Perhaps a second, token-specific task to address my concern below?

[Jakob Kummerow (corp), 2010/11/22 16:56:08]

See my answer below.

 }
 
 void DeviceManagementPolicyProvider::Shutdown() {
   token_service_ = NULL;
   if (token_fetcher_)
     token_fetcher_->Shutdown();
 }
 
 DeviceManagementBackend* DeviceManagementPolicyProvider::GetBackend() {
   if (!backend_.get()) {
     backend_.reset(new DeviceManagementBackendImpl(
         GetDeviceManagementURL()));
   }
   return backend_.get();
 }
 
 void DeviceManagementPolicyProvider::Initialize() {
-  registrar_.Add(this,
-                 NotificationType::DEVICE_TOKEN_AVAILABLE,
-                 NotificationService::AllSources());
-
   const FilePath policy_path = storage_dir_.Append(
       FILE_PATH_LITERAL("Policy"));
   cache_.reset(new DeviceManagementPolicyCache(policy_path));
   cache_->LoadPolicyFromFile();
 
   // Defer initialization that requires the IOThread until after the IOThread
   // has been initialized.
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
                           new InitializeAfterIOThreadExistsTask(AsWeakPtr()));
 }
 
 void DeviceManagementPolicyProvider::InitializeAfterIOThreadExists() {
   const FilePath token_path = storage_dir_.Append(
       FILE_PATH_LITERAL("Token"));
   if (token_service_) {
     token_fetcher_ =
         new DeviceTokenFetcher(GetBackend(), token_service_, token_path);
+    token_fetcher_->AddObserver(this);

[danno, 2010/11/22 13:47:50]

What ensures that the observer gets removed? You probably need a registrar
object or the like.

[Jakob Kummerow (corp), 2010/11/22 16:56:08]

Done. 
Added a registrar mechanism. 

     token_fetcher_->StartFetching();
   }
 }
 
 void DeviceManagementPolicyProvider::SendPolicyRequest() {
   if (!policy_request_pending_) {
+    policy_request_pending_ = true;
     em::DevicePolicyRequest policy_request;
     policy_request.set_policy_scope(kChromePolicyScope);
     em::DevicePolicySettingRequest* setting =
         policy_request.add_setting_request();
     setting->set_key(kChromeDevicePolicySettingKey);
     GetBackend()->ProcessPolicyRequest(token_fetcher_->GetDeviceToken(),
                                        policy_request,
                                        this);
-    policy_request_pending_ = true;
+  } else {
+    LOG(WARNING) << "Not sending another PolicyRequest.";
   }
 }
 
-bool DeviceManagementPolicyProvider::IsPolicyStale() const {
-  base::Time now(base::Time::NowFromSystemTime());
-  base::Time last_policy_refresh_time =
-      cache_->last_policy_refresh_time();
-  base::Time policy_expiration_time =
-      last_policy_refresh_time + base::TimeDelta::FromMinutes(
-          kPolicyRefreshRateInMinutes);
-  return (now > policy_expiration_time);
+void DeviceManagementPolicyProvider::RefreshTaskExecute() {

[danno, 2010/11/22 13:47:50]

Combining both the token request and the policy request into one seems to not
appropriately split responsibilities, and in any case the name becomes
misleading. Refresh==get token if I don't have one and refresh policy. I think I
would like this better if this routine was a little less of a big hammer that
both faults in the token and sends policy requests, e.g. I see only the token
fetching code as an appropriate response to OnTokenError(), not policy fetching
code.

[Jakob Kummerow (corp), 2010/11/22 16:56:08]

I don't see what's wrong with a task that assesses the overall situation and
then does whatever needs to be done. 
Also, splitting this into two separate tasks would mean to perform the same
functionality with almost twice as much code. 

Discussed offline: will keep it as it is. 

+  refresh_task_pending_ = false;

[danno, 2010/11/22 13:47:50]

DCHECK(refresh_task_pending_) before?

[Jakob Kummerow (corp), 2010/11/22 16:56:08]

Done.

+  // If there is no valid device token, the token_fetcher_ apparently failed,
+  // so it must be restarted.
+  if (!token_fetcher_->IsTokenValid()) {
+    if (token_fetcher_->IsTokenPending()) {
+      NOTREACHED();
+      return;
+    }
+    token_fetcher_->Restart();
+    return;
+  }
+  // If there is a device token, just refresh policies.
+  SendPolicyRequest();
+}
+
+void DeviceManagementPolicyProvider::ScheduleRefreshTask(
+    int64 delay_in_milliseconds) {
+  // This check is simply a safeguard, the situation currently cannot happen.
+  if (refresh_task_pending_) {
+    NOTREACHED();
+    return;
+  }
+  refresh_task_pending_ = true;
+  BrowserThread::PostDelayedTask(BrowserThread::UI, FROM_HERE,
+                                 new RefreshTask(AsWeakPtr()),
+                                 delay_in_milliseconds);
 }
 
 // static
+int64 DeviceManagementPolicyProvider::GetRefreshTaskDelay(
+    int64 policy_refresh_rate_ms,
+    int64 policy_refresh_max_random_earlier_ms) {
+  if (policy_refresh_max_random_earlier_ms)
+    policy_refresh_rate_ms -=
+        base::RandGenerator(policy_refresh_max_random_earlier_ms);
+  return policy_refresh_rate_ms;
+}
+
+// static
 std::string DeviceManagementPolicyProvider::GetDeviceManagementURL() {
   return CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
       switches::kDeviceManagementUrl);
 }
 
 // static
 FilePath DeviceManagementPolicyProvider::GetOrCreateDeviceManagementDir(
     const FilePath& user_data_dir) {
   const FilePath device_management_dir = user_data_dir.Append(
       FILE_PATH_LITERAL("Device Management"));
   if (!file_util::DirectoryExists(device_management_dir)) {
     if (!file_util::CreateDirectory(device_management_dir))
       NOTREACHED();
   }
   return device_management_dir;
 }
 
 }  // namespace policy