Signed settings refactoring: Proper caching and more tests.

chrome/browser/policy/device_policy_cache.cc

 // Copyright (c) 2011 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_policy_cache.h"
 
 #include <limits>
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/values.h"
+#include "chrome/browser/chromeos/cros_settings.h"
 #include "chrome/browser/chromeos/dbus/dbus_thread_manager.h"
 #include "chrome/browser/chromeos/dbus/update_engine_client.h"
 #include "chrome/browser/chromeos/login/ownership_service.h"
-#include "chrome/browser/chromeos/user_cros_settings_provider.h"
+#include "chrome/browser/chromeos/login/signed_settings_helper.h"
 #include "chrome/browser/policy/cloud_policy_data_store.h"
 #include "chrome/browser/policy/enterprise_install_attributes.h"
 #include "chrome/browser/policy/enterprise_metrics.h"
 #include "chrome/browser/policy/policy_map.h"
 #include "chrome/browser/policy/proto/device_management_backend.pb.h"
 #include "chrome/browser/policy/proto/device_management_constants.h"
 #include "chrome/browser/policy/proto/device_management_local.pb.h"
 #include "policy/configuration_policy_type.h"
 
 namespace {
 
 // Stores policy, updates the owner key if required and reports the status
 // through a callback.
-class StorePolicyOperation : public chromeos::SignedSettingsHelper::Callback,
-                             public chromeos::OwnerManager::KeyUpdateDelegate {
+class StorePolicyOperation : public chromeos::OwnerManager::KeyUpdateDelegate {
  public:
   typedef base::Callback<void(chromeos::SignedSettings::ReturnCode)> Callback;
 
   StorePolicyOperation(chromeos::SignedSettingsHelper* signed_settings_helper,
                        const em::PolicyFetchResponse& policy,
                        const Callback& callback)
       : signed_settings_helper_(signed_settings_helper),
         policy_(policy),
-        callback_(callback) {
-    signed_settings_helper_->StartStorePolicyOp(policy, this);
+        callback_(callback),
+        weak_ptr_factory_(this) {
+    signed_settings_helper_->StartStorePolicyOp(
+        policy,
+        base::Bind(&StorePolicyOperation::OnStorePolicyCompleted,
+                   weak_ptr_factory_.GetWeakPtr()));
   }
   virtual ~StorePolicyOperation() {
-    signed_settings_helper_->CancelCallback(this);
   }
 
-  // SignedSettingsHelper implementation:
-  virtual void OnStorePolicyCompleted(
-      chromeos::SignedSettings::ReturnCode code) OVERRIDE {
+  void OnStorePolicyCompleted(chromeos::SignedSettings::ReturnCode code) {
     if (code != chromeos::SignedSettings::SUCCESS) {
       callback_.Run(code);
       delete this;
       return;
     }
 
     if (policy_.has_new_public_key()) {
       // The session manager has successfully done a key rotation. Replace the
       // owner key also in chrome.
       const std::string& new_key = policy_.new_public_key();
       const std::vector<uint8> new_key_data(new_key.c_str(),
                                             new_key.c_str() + new_key.size());
       chromeos::OwnershipService::GetSharedInstance()->StartUpdateOwnerKey(
           new_key_data, this);
       return;
     } else {
-      UpdateUserCrosSettings();
+      chromeos::CrosSettings::Get()->ReloadProviders();
       callback_.Run(chromeos::SignedSettings::SUCCESS);
       delete this;
       return;
     }
   }
 
   // OwnerManager::KeyUpdateDelegate implementation:
   virtual void OnKeyUpdated() OVERRIDE {
-    UpdateUserCrosSettings();
+    chromeos::CrosSettings::Get()->ReloadProviders();
     callback_.Run(chromeos::SignedSettings::SUCCESS);
     delete this;
   }
 
  private:
-  void UpdateUserCrosSettings() {
-    // TODO(mnissler): Find a better way. This is a hack that updates the
-    // UserCrosSettingsProvider's cache, since it is unable to notice we've
-    // updated policy information.
-    chromeos::UserCrosSettingsProvider().Reload();
-  }
 
   chromeos::SignedSettingsHelper* signed_settings_helper_;
   em::PolicyFetchResponse policy_;
   Callback callback_;
 
+  base::WeakPtrFactory<StorePolicyOperation> weak_ptr_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(StorePolicyOperation);
 };
 
 // Decodes a protobuf integer to an IntegerValue. The caller assumes ownership
 // of the return Value*. Returns NULL in case the input value is out of bounds.
 Value* DecodeIntegerValue(google::protobuf::int64 value) {
   if (value < std::numeric_limits<int>::min() ||
       value > std::numeric_limits<int>::max()) {
     LOG(WARNING) << "Integer value " << value
                  << " out of numeric limits, ignoring.";
@@ skipping 20 matching lines @@
     CloudPolicyDataStore* data_store,
     EnterpriseInstallAttributes* install_attributes,
     chromeos::SignedSettingsHelper* signed_settings_helper)
     : data_store_(data_store),
       install_attributes_(install_attributes),
       signed_settings_helper_(signed_settings_helper),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
 }
 
 DevicePolicyCache::~DevicePolicyCache() {
-  signed_settings_helper_->CancelCallback(this);
 }
 
 void DevicePolicyCache::Load() {
-  signed_settings_helper_->StartRetrievePolicyOp(this);
+  signed_settings_helper_->StartRetrievePolicyOp(
+      base::Bind(&DevicePolicyCache::OnRetrievePolicyCompleted,
+                 weak_ptr_factory_.GetWeakPtr()));
 }
 
 void DevicePolicyCache::SetPolicy(const em::PolicyFetchResponse& policy) {
   DCHECK(IsReady());
 
   // Make sure we have an enterprise device.
   std::string registration_user(install_attributes_->GetRegistrationUser());
   if (registration_user.empty()) {
     LOG(WARNING) << "Refusing to accept policy on non-enterprise device.";
     UMA_HISTOGRAM_ENUMERATION(kMetricPolicy,
@@ skipping 99 matching lines @@
     } else {
       UMA_HISTOGRAM_ENUMERATION(kMetricPolicy, kMetricPolicyFetchOtherFailed,
                                 kMetricPolicySize);
       InformNotifier(CloudPolicySubsystem::LOCAL_ERROR,
                      CloudPolicySubsystem::POLICY_LOCAL_ERROR);
     }
     return;
   }
   UMA_HISTOGRAM_ENUMERATION(kMetricPolicy, kMetricPolicyStoreSucceeded,
                             kMetricPolicySize);
-  signed_settings_helper_->StartRetrievePolicyOp(this);
+  signed_settings_helper_->StartRetrievePolicyOp(
+      base::Bind(&DevicePolicyCache::OnRetrievePolicyCompleted,
+                 weak_ptr_factory_.GetWeakPtr()));
 }
 
 void DevicePolicyCache::InstallInitialPolicy(
     chromeos::SignedSettings::ReturnCode code,
     const em::PolicyFetchResponse& policy,
     std::string* device_token) {
   if (code == chromeos::SignedSettings::NOT_FOUND ||
       code == chromeos::SignedSettings::KEY_UNAVAILABLE ||
       !policy.has_policy_data()) {
     InformNotifier(CloudPolicySubsystem::UNENROLLED,
@@ skipping 85 matching lines @@
   if (policy.has_open_network_configuration() &&
       policy.open_network_configuration().has_open_network_configuration()) {
     std::string config(
         policy.open_network_configuration().open_network_configuration());
     mandatory->Set(kPolicyDeviceOpenNetworkConfiguration,
                    Value::CreateStringValue(config));
   }
 }
 
 }  // namespace policy