Consolidate data storage and notifications in the cloud policy subsystem

chrome/browser/policy/cloud_policy_controller.cc

Index: chrome/browser/policy/cloud_policy_controller.cc
diff --git a/chrome/browser/policy/cloud_policy_controller.cc b/chrome/browser/policy/cloud_policy_controller.cc
index 8c3ba08c9d3518cbfb1c9ea1518b376c636c6176..77984af457b27930fe5282a62336c84b4a519807 100644
--- a/chrome/browser/policy/cloud_policy_controller.cc
+++ b/chrome/browser/policy/cloud_policy_controller.cc
@@ -17,6 +17,7 @@
 #include "chrome/browser/policy/device_management_backend.h"

[pastarmovj, 2011/07/06 12:11:57]

What does the IWYU say about including the same header both in the .h and the
.cc file?

[gfeher, 2011/07/06 15:14:20]

Do it only at one place...

 #include "chrome/browser/policy/device_management_service.h"
 #include "chrome/browser/policy/proto/device_management_constants.h"
+#include "chrome/common/guid.h"
 
 // Domain names that are known not to be managed.
 // We don't register the device when such a user logs in.
@@ -66,19 +67,18 @@ CloudPolicyController::CloudPolicyController(
     DeviceManagementService* service,
     CloudPolicyCacheBase* cache,
     DeviceTokenFetcher* token_fetcher,
-    CloudPolicyIdentityStrategy* identity_strategy,
+    CloudPolicyData* data,
     PolicyNotifier* notifier) {
   Initialize(service,
              cache,
              token_fetcher,
-             identity_strategy,
+             data,
              notifier,
              new DelayedWorkScheduler);
 }
 
 CloudPolicyController::~CloudPolicyController() {
-  token_fetcher_->RemoveObserver(this);
-  identity_strategy_->RemoveObserver(this);
+  data_->RemoveObserver(this);
   scheduler_->CancelDelayedWork();
 }
 
@@ -153,37 +153,43 @@ void CloudPolicyController::OnError(DeviceManagementBackend::ErrorCode code) {
   }
 }
 
-void CloudPolicyController::OnDeviceTokenAvailable() {
-  identity_strategy_->OnDeviceTokenAvailable(token_fetcher_->GetDeviceToken());
-}
-
 void CloudPolicyController::OnDeviceTokenChanged() {
-  if (identity_strategy_->GetDeviceToken().empty())
+  if (data_->device_token().empty())
     SetState(STATE_TOKEN_UNAVAILABLE);
   else
     SetState(STATE_TOKEN_VALID);
 }
 
 void CloudPolicyController::OnCredentialsChanged() {
-  notifier_->Inform(CloudPolicySubsystem::UNENROLLED,
-                    CloudPolicySubsystem::NO_DETAILS,
-                    PolicyNotifier::POLICY_CONTROLLER);
-  effective_policy_refresh_error_delay_ms_ =
-      kPolicyRefreshErrorDelayInMilliseconds;
-  SetState(STATE_TOKEN_UNAVAILABLE);
+  // This notification is only interesting if we don't have a device token.
+  // If we already have a device token, that must be matching the current

[Joao da Silva, 2011/07/06 16:45:14]

Can that be DCHECK'd?

[gfeher, 2011/07/07 13:51:00]

I think not. The user name cannot be extracted from the token on the client
side, I believe only the server is able to do that.

+  // user, because (1) we always recreate the policy subsystem after user
+  // login (2) tokens are cached per user.
+  if (data_->device_token().empty()) {
+    notifier_->Inform(CloudPolicySubsystem::UNENROLLED,
+                      CloudPolicySubsystem::NO_DETAILS,
+                      PolicyNotifier::POLICY_CONTROLLER);
+    effective_policy_refresh_error_delay_ms_ =
+        kPolicyRefreshErrorDelayInMilliseconds;
+    SetState(STATE_TOKEN_UNAVAILABLE);
+  }
+}
+
+void CloudPolicyController::OnPolicyDataGoingAway() {
+  NOTREACHED();
 }
 
 CloudPolicyController::CloudPolicyController(
     DeviceManagementService* service,
     CloudPolicyCacheBase* cache,
     DeviceTokenFetcher* token_fetcher,
-    CloudPolicyIdentityStrategy* identity_strategy,
+    CloudPolicyData* data,
     PolicyNotifier* notifier,
     DelayedWorkScheduler* scheduler) {
   Initialize(service,
              cache,
              token_fetcher,
-             identity_strategy,
+             data,
              notifier,
              scheduler);
 }
@@ -192,7 +198,7 @@ void CloudPolicyController::Initialize(
     DeviceManagementService* service,
     CloudPolicyCacheBase* cache,
     DeviceTokenFetcher* token_fetcher,
-    CloudPolicyIdentityStrategy* identity_strategy,
+    CloudPolicyData* data,
     PolicyNotifier* notifier,
     DelayedWorkScheduler* scheduler) {
   DCHECK(cache);
@@ -200,33 +206,29 @@ void CloudPolicyController::Initialize(
   service_ = service;
   cache_ = cache;
   token_fetcher_ = token_fetcher;
-  identity_strategy_ = identity_strategy;
+  data_ = data;
   notifier_ = notifier;
   state_ = STATE_TOKEN_UNAVAILABLE;
   policy_refresh_rate_ms_ = kPolicyRefreshRateInMilliseconds;
   effective_policy_refresh_error_delay_ms_ =
       kPolicyRefreshErrorDelayInMilliseconds;
   scheduler_.reset(scheduler);
-  token_fetcher_->AddObserver(this);
-  identity_strategy_->AddObserver(this);
-  if (!identity_strategy_->GetDeviceToken().empty())
+  data_->AddObserver(this);
+  if (!data_->device_token().empty())
     SetState(STATE_TOKEN_VALID);
   else
     SetState(STATE_TOKEN_UNAVAILABLE);
 }
 
 void CloudPolicyController::FetchToken() {
-  std::string username;
-  std::string auth_token;
-  std::string device_id = identity_strategy_->GetDeviceID();
-  std::string machine_id = identity_strategy_->GetMachineID();
-  std::string machine_model = identity_strategy_->GetMachineModel();
-  em::DeviceRegisterRequest_Type policy_type =
-      identity_strategy_->GetPolicyRegisterType();
-  if (identity_strategy_->GetCredentials(&username, &auth_token)) {
-    if (CanBeInManagedDomain(username)) {
-      token_fetcher_->FetchToken(auth_token, device_id, policy_type,
-                                 machine_id, machine_model);
+  if (data_->token_cache_loaded() &&
+      !data_->user_name().empty() &&
+      !data_->gaia_token().empty()) {
+    if (CanBeInManagedDomain(data_->user_name())) {
+      // Generate a new random device id. (It'll only be kept if registration
+      // succeeds.)
+      data_->set_device_id(guid::GenerateGUID());
+      token_fetcher_->FetchToken();
     } else {
       SetState(STATE_TOKEN_UNMANAGED);
     }
@@ -235,11 +237,11 @@ void CloudPolicyController::FetchToken() {
 
 void CloudPolicyController::SendPolicyRequest() {
   backend_.reset(service_->CreateBackend());
-  DCHECK(!identity_strategy_->GetDeviceToken().empty());
+  DCHECK(!data_->device_token().empty());
   em::DevicePolicyRequest policy_request;
   em::PolicyFetchRequest* fetch_request = policy_request.add_request();
   fetch_request->set_signature_type(em::PolicyFetchRequest::SHA1_RSA);
-  fetch_request->set_policy_type(identity_strategy_->GetPolicyType());
+  fetch_request->set_policy_type(data_->policy_type());
   if (!cache_->is_unmanaged() &&
       !cache_->last_policy_refresh_time().is_null()) {
     base::TimeDelta timestamp =
@@ -250,8 +252,8 @@ void CloudPolicyController::SendPolicyRequest() {
   if (cache_->GetPublicKeyVersion(&key_version))
     fetch_request->set_public_key_version(key_version);
 
-  backend_->ProcessPolicyRequest(identity_strategy_->GetDeviceToken(),
-                                 identity_strategy_->GetDeviceID(),
+  backend_->ProcessPolicyRequest(data_->device_token(),
+                                 data_->device_id(),
                                  policy_request, this);
 }