PART 5 Of the signed settings refactoring. List of all changes follow beneath:

Read from the bottom to the top:
* Add new unit tests to cover what was removed before.
* Rename SignedSettingsTempStorage to SignedSettingsCache.
* Revitalize existing tests for SignedSettings[Helper].
* Add the needed infrastucture to support enrollment as well.
* Remove the second cache in OwnershipService it is obsolete.
* Remove the prop ops completely.
* Remove direct prop op from the proxy stuff.
* Serialize policy changes correctly and map side effects of policies.
    Mainly make sure we never serialize dirty policy.
    Don't reload if policy is serialized fine.
    Clear local state registration.
* Clean up redundand SS ops and make proper callbacks for the helper
    Move the temp storage finalization to where it belongs.
* Make the temp storage be the cache and use policy ops.
* Make DeviceSettingsProvider work with the protobuf blob directly.
* Merged DeviceSettingsProvider and UserCrosSettingsTrust.
* Rename UserCrosSettingsProvider to DeviceSettingsProvider.
* Extract the SignedSettingsMigrationHelper in its own file.

BUG=chromium-os:14054
TEST=unit_tests:SignedSettings*,*CrosSettings*,suite_Smoke:login_OwnershipApi

Review URL: http://codereview.chromium.org/8727037

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

4 files changed, 36 insertions, 34 deletions

diff --git a/chrome/browser/policy/device_policy_cache.cc b/chrome/browser/policy/device_policy_cache.cc
index f96a722..75a6890 100644
--- a/chrome/browser/policy/device_policy_cache.cc
+++ b/chrome/browser/policy/device_policy_cache.cc
@@ -15,10 +15,11 @@
 #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"
@@ -32,8 +33,7 @@ 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;
 
@@ -42,16 +42,17 @@ class StorePolicyOperation : public chromeos::SignedSettingsHelper::Callback,
                        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;
@@ -68,7 +69,7 @@ class StorePolicyOperation : public chromeos::SignedSettingsHelper::Callback,
           new_key_data, this);
       return;
     } else {
-      UpdateUserCrosSettings();
+      chromeos::CrosSettings::Get()->ReloadProviders();
       callback_.Run(chromeos::SignedSettings::SUCCESS);
       delete this;
       return;
@@ -77,23 +78,19 @@ class StorePolicyOperation : public chromeos::SignedSettingsHelper::Callback,
 
   // 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);
 };
 
@@ -134,11 +131,12 @@ DevicePolicyCache::DevicePolicyCache(
 }
 
 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) {
@@ -258,7 +256,9 @@ void DevicePolicyCache::PolicyStoreOpCompleted(
   }
   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(
diff --git a/chrome/browser/policy/device_policy_cache.h b/chrome/browser/policy/device_policy_cache.h
index 63ed1ce..4a2fd11 100644
--- a/chrome/browser/policy/device_policy_cache.h
+++ b/chrome/browser/policy/device_policy_cache.h
@@ -8,10 +8,13 @@
 
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/chromeos/login/signed_settings.h"
-#include "chrome/browser/chromeos/login/signed_settings_helper.h"
 #include "chrome/browser/policy/cloud_policy_cache_base.h"
 #include "chrome/browser/policy/proto/chrome_device_policy.pb.h"
 
+namespace chromeos {
+class SignedSettingsHelper;
+}  // namespace chromeos
+
 namespace policy {
 
 class CloudPolicyDataStore;
@@ -22,8 +25,7 @@ namespace em = enterprise_management;
 
 // CloudPolicyCacheBase implementation that persists policy information
 // to ChromeOS' session manager (via SignedSettingsHelper).
-class DevicePolicyCache : public CloudPolicyCacheBase,
-                          public chromeos::SignedSettingsHelper::Callback {
+class DevicePolicyCache : public CloudPolicyCacheBase {
  public:
   DevicePolicyCache(CloudPolicyDataStore* data_store,
                     EnterpriseInstallAttributes* install_attributes);
@@ -34,10 +36,8 @@ class DevicePolicyCache : public CloudPolicyCacheBase,
   virtual void SetPolicy(const em::PolicyFetchResponse& policy) OVERRIDE;
   virtual void SetUnmanaged() OVERRIDE;
 
-  // SignedSettingsHelper::Callback implementation:
-  virtual void OnRetrievePolicyCompleted(
-      chromeos::SignedSettings::ReturnCode code,
-      const em::PolicyFetchResponse& policy) OVERRIDE;
+  void OnRetrievePolicyCompleted(chromeos::SignedSettings::ReturnCode code,
+                                 const em::PolicyFetchResponse& policy);
 
  private:
   friend class DevicePolicyCacheTest;
diff --git a/chrome/browser/policy/device_policy_cache_unittest.cc b/chrome/browser/policy/device_policy_cache_unittest.cc
index 675af19..f28b99e 100644
--- a/chrome/browser/policy/device_policy_cache_unittest.cc
+++ b/chrome/browser/policy/device_policy_cache_unittest.cc
@@ -8,6 +8,7 @@
 #include "chrome/browser/chromeos/login/mock_signed_settings_helper.h"
 #include "chrome/browser/policy/cloud_policy_data_store.h"
 #include "chrome/browser/policy/enterprise_install_attributes.h"
+#include "content/test/test_browser_thread.h"
 #include "policy/configuration_policy_type.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -70,7 +71,10 @@ class DevicePolicyCacheTest : public testing::Test {
  protected:
   DevicePolicyCacheTest()
       : cryptohome_(chromeos::CryptohomeLibrary::GetImpl(true)),
-        install_attributes_(cryptohome_.get()) {}
+        install_attributes_(cryptohome_.get()),
+        message_loop_(MessageLoop::TYPE_UI),
+        ui_thread_(content::BrowserThread::UI, &message_loop_),
+        file_thread_(content::BrowserThread::FILE, &message_loop_) {}
 
   virtual void SetUp() {
     data_store_.reset(CloudPolicyDataStore::CreateForUserPolicies());
@@ -80,7 +84,6 @@ class DevicePolicyCacheTest : public testing::Test {
   }
 
   virtual void TearDown() {
-    EXPECT_CALL(signed_settings_helper_, CancelCallback(_));
     cache_.reset();
   }
 
@@ -103,6 +106,10 @@ class DevicePolicyCacheTest : public testing::Test {
   chromeos::MockSignedSettingsHelper signed_settings_helper_;
   scoped_ptr<DevicePolicyCache> cache_;
 
+  MessageLoop message_loop_;
+  content::TestBrowserThread ui_thread_;
+  content::TestBrowserThread file_thread_;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(DevicePolicyCacheTest);
 };
@@ -147,7 +154,6 @@ TEST_F(DevicePolicyCacheTest, SetPolicy) {
   EXPECT_CALL(signed_settings_helper_, StartRetrievePolicyOp(_)).WillOnce(
       MockSignedSettingsHelperRetrievePolicy(SignedSettings::SUCCESS,
                                              new_policy));
-  EXPECT_CALL(signed_settings_helper_, CancelCallback(_));
   cache_->SetPolicy(new_policy);
   testing::Mock::VerifyAndClearExpectations(&signed_settings_helper_);
   base::FundamentalValue updated_expected(300);
diff --git a/chrome/browser/policy/enterprise_metrics_browsertest.cc b/chrome/browser/policy/enterprise_metrics_browsertest.cc
index 853fd8e..71118ea 100644
--- a/chrome/browser/policy/enterprise_metrics_browsertest.cc
+++ b/chrome/browser/policy/enterprise_metrics_browsertest.cc
@@ -696,9 +696,6 @@ class DevicePolicyCacheTestHelper {
     install_attributes_.ExpectUsage();
     EXPECT_CALL(mock_signed_settings_helper_, StartStorePolicyOp(_, _))
         .WillOnce(MockSignedSettingsHelperStorePolicy(code));
-    EXPECT_CALL(mock_signed_settings_helper_, CancelCallback(_))
-        .Times(1)
-        .RetiresOnSaturation();
     EXPECT_CALL(mock_signed_settings_helper_,
                 StartRetrievePolicyOp(_)).Times(expected_retrieves);
   }
@@ -714,7 +711,6 @@ class DevicePolicyCacheTestHelper {
                                install_attributes_.install_attributes(),
                                &mock_signed_settings_helper_));
     data_store_->SetupForTesting("", "id", "user", "gaia_token", false);
-    EXPECT_CALL(mock_signed_settings_helper_, CancelCallback(_)).Times(1);
   }
 
   chromeos::MockSignedSettingsHelper mock_signed_settings_helper_;