Refactored the CloudPolicyProvider so that it becomes initialized once and stays initialized.

chrome/browser/policy/cloud_policy_provider_unittest.cc

Index: chrome/browser/policy/cloud_policy_provider_unittest.cc
diff --git a/chrome/browser/policy/cloud_policy_provider_unittest.cc b/chrome/browser/policy/cloud_policy_provider_unittest.cc
index cd0f1b95492788c1a362e2aa4a3246e360dd9cc1..8a96ec35947d50ce0e9ecffebde24d0fd0110dd2 100644
--- a/chrome/browser/policy/cloud_policy_provider_unittest.cc
+++ b/chrome/browser/policy/cloud_policy_provider_unittest.cc
@@ -8,58 +8,46 @@
 #include "base/values.h"
 #include "chrome/browser/policy/browser_policy_connector.h"
 #include "chrome/browser/policy/cloud_policy_cache_base.h"
-#include "chrome/browser/policy/cloud_policy_provider_impl.h"
-#include "chrome/browser/policy/configuration_policy_provider.h"
+#include "chrome/browser/policy/cloud_policy_provider.h"
 #include "chrome/browser/policy/mock_configuration_policy_provider.h"
+#include "content/test/test_browser_thread.h"
 #include "policy/policy_constants.h"
 #include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
 
 using testing::Mock;
-using testing::_;
 
 namespace em = enterprise_management;
 
 namespace policy {
 
-namespace {
-
-// Utility function for tests.
-void SetPolicy(PolicyMap* map, const char* policy, Value* value) {
-  map->Set(policy, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, value);
-}
-
-}  // namespace
-
 class MockCloudPolicyCache : public CloudPolicyCacheBase {
  public:
   MockCloudPolicyCache() {}
   virtual ~MockCloudPolicyCache() {}
 
-  // CloudPolicyCacheBase implementation.
-  void Load() OVERRIDE {}
-  bool SetPolicy(const em::PolicyFetchResponse& policy) OVERRIDE {
-    return true;
-  }
-  bool DecodePolicyData(const em::PolicyData& policy_data,
-                        PolicyMap* policies) OVERRIDE {
+  virtual void Load() OVERRIDE {}
+
+  virtual bool SetPolicy(const em::PolicyFetchResponse& policy) OVERRIDE {
     return true;
   }
 
-  void SetUnmanaged() OVERRIDE {
+  virtual void SetUnmanaged() OVERRIDE {
     is_unmanaged_ = true;
   }
 
-  PolicyMap* mutable_policy() {
-    return &policies_;
+  virtual bool DecodePolicyData(const em::PolicyData& policy_data,
+                                PolicyMap* policies) OVERRIDE {
+    return true;
   }
 
-  void set_initialized(bool initialized) {
-    initialization_complete_ = initialized;
+  PolicyMap* mutable_policy() {
+    return &policies_;
   }
 
-  void Set(const char *name, Value* value) {
-    policies_.Set(name, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, value);
-  }
+  // Make these methods public.
+  using CloudPolicyCacheBase::SetReady;
+  using CloudPolicyCacheBase::NotifyObservers;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockCloudPolicyCache);
@@ -67,228 +55,184 @@ class MockCloudPolicyCache : public CloudPolicyCacheBase {
 
 class CloudPolicyProviderTest : public testing::Test {
  protected:
-  void CreateCloudPolicyProvider() {
-    cloud_policy_provider_.reset(
-        new CloudPolicyProviderImpl(&browser_policy_connector_,
-                                    GetChromePolicyDefinitionList(),
-                                    POLICY_LEVEL_MANDATORY));
+  CloudPolicyProviderTest()
+      : cloud_policy_provider_(&browser_policy_connector_,
+                               GetChromePolicyDefinitionList(),
+                               POLICY_LEVEL_MANDATORY) {}
+
+  void SetUp() OVERRIDE {
+    registrar_.Init(&cloud_policy_provider_, &observer_);
   }
 
-  // Appends the caches to a provider and then provides the policies to
-  // |result|.
-  void RunCachesThroughProvider(MockCloudPolicyCache caches[], int n,
-                                PolicyMap* result) {
-    CloudPolicyProviderImpl provider(
-        &browser_policy_connector_,
-        GetChromePolicyDefinitionList(),
-        POLICY_LEVEL_MANDATORY);
-    for (int i = 0; i < n; i++) {
-      provider.AppendCache(&caches[i]);
-    }
-    provider.Provide(result);
+  void AddUserCache() {
+    EXPECT_CALL(observer_, OnUpdatePolicy(&cloud_policy_provider_));
+    cloud_policy_provider_.SetUserPolicyCache(&user_policy_cache_);
+    Mock::VerifyAndClearExpectations(&observer_);
   }
 
-  void CombineTwoPolicyMaps(const PolicyMap& base,
-                            const PolicyMap& overlay,
-                            PolicyMap* out_map) {
-    MockCloudPolicyCache caches[2];
-    caches[0].mutable_policy()->CopyFrom(base);
-    caches[0].set_initialized(true);
-    caches[1].mutable_policy()->CopyFrom(overlay);
-    caches[1].set_initialized(true);
-    RunCachesThroughProvider(caches, 2, out_map);
+  void AddDeviceCache() {
+#if defined(OS_CHROMEOS)
+    EXPECT_CALL(observer_, OnUpdatePolicy(&cloud_policy_provider_));
+    cloud_policy_provider_.SetDevicePolicyCache(&device_policy_cache_);
+    Mock::VerifyAndClearExpectations(&observer_);
+#endif
   }
 
-  void FixDeprecatedPolicies(PolicyMap* policies) {
-    CloudPolicyProviderImpl::FixDeprecatedPolicies(policies);
+  void SetUserCacheReady() {
+    EXPECT_CALL(observer_, OnUpdatePolicy(&cloud_policy_provider_));
+    user_policy_cache_.SetReady();
+    Mock::VerifyAndClearExpectations(&observer_);
+    EXPECT_TRUE(user_policy_cache_.IsReady());
   }
 
-  scoped_ptr<CloudPolicyProviderImpl> cloud_policy_provider_;
+  void SetDeviceCacheReady() {
+#if defined(OS_CHROMEOS)
+    EXPECT_CALL(observer_, OnUpdatePolicy(&cloud_policy_provider_));
+    device_policy_cache_.SetReady();
+    Mock::VerifyAndClearExpectations(&observer_);
+    EXPECT_TRUE(device_policy_cache_.IsReady());
+#endif
+  }
 
- private:
   BrowserPolicyConnector browser_policy_connector_;
-};
 
-// Proxy setting distributed over multiple caches.
-TEST_F(CloudPolicyProviderTest,
-       ProxySettingDistributedOverMultipleCaches) {
-  // There are proxy_policy_count()+1 = 6 caches and they are mixed together by
-  // one instance of CloudPolicyProvider. The first cache has some policies but
-  // no proxy-related ones. The following caches have each one proxy-policy set.
-  const int n = 6;
-  MockCloudPolicyCache caches[n];
-
-  // Prepare |cache[0]| to serve some non-proxy policies.
-  caches[0].Set(key::kShowHomeButton, Value::CreateBooleanValue(true));
-  caches[0].Set(key::kIncognitoEnabled, Value::CreateBooleanValue(true));
-  caches[0].Set(key::kTranslateEnabled, Value::CreateBooleanValue(true));
-  caches[0].set_initialized(true);
-
-  // Prepare the other caches to serve one proxy-policy each.
-  caches[1].Set(key::kProxyMode, Value::CreateStringValue("cache 1"));
-  caches[1].set_initialized(true);
-  caches[2].Set(key::kProxyServerMode, Value::CreateIntegerValue(2));
-  caches[2].set_initialized(true);
-  caches[3].Set(key::kProxyServer, Value::CreateStringValue("cache 3"));
-  caches[3].set_initialized(true);
-  caches[4].Set(key::kProxyPacUrl, Value::CreateStringValue("cache 4"));
-  caches[4].set_initialized(true);
-  caches[5].Set(key::kProxyMode, Value::CreateStringValue("cache 5"));
-  caches[5].set_initialized(true);
-
-  PolicyMap policies;
-  RunCachesThroughProvider(caches, n, &policies);
-
-  // Verify expectations.
-  EXPECT_TRUE(policies.Get(key::kProxyMode) == NULL);
-  EXPECT_TRUE(policies.Get(key::kProxyServerMode) == NULL);
-  EXPECT_TRUE(policies.Get(key::kProxyServer) == NULL);
-  EXPECT_TRUE(policies.Get(key::kProxyPacUrl) == NULL);
-
-  const Value* value = policies.GetValue(key::kProxySettings);
-  ASSERT_TRUE(value != NULL);
-  ASSERT_TRUE(value->IsType(Value::TYPE_DICTIONARY));
-  const DictionaryValue* settings = static_cast<const DictionaryValue*>(value);
-  std::string mode;
-  EXPECT_TRUE(settings->GetString(key::kProxyMode, &mode));
-  EXPECT_EQ("cache 1", mode);
-
-  base::FundamentalValue expected(true);
-  EXPECT_TRUE(base::Value::Equals(&expected,
-                                  policies.GetValue(key::kShowHomeButton)));
-  EXPECT_TRUE(base::Value::Equals(&expected,
-                                  policies.GetValue(key::kIncognitoEnabled)));
-  EXPECT_TRUE(base::Value::Equals(&expected,
-                                  policies.GetValue(key::kTranslateEnabled)));
-}
-
-// Combining two PolicyMaps.
-TEST_F(CloudPolicyProviderTest, CombineTwoPolicyMapsSame) {
-  PolicyMap A, B, C;
-  SetPolicy(&A, key::kHomepageLocation,
-            Value::CreateStringValue("http://www.chromium.org"));
-  SetPolicy(&B, key::kHomepageLocation,
-            Value::CreateStringValue("http://www.google.com"));
-  SetPolicy(&A, key::kApplicationLocaleValue, Value::CreateStringValue("hu"));
-  SetPolicy(&B, key::kApplicationLocaleValue, Value::CreateStringValue("us"));
-  SetPolicy(&A, key::kDevicePolicyRefreshRate, new base::FundamentalValue(100));
-  SetPolicy(&B, key::kDevicePolicyRefreshRate, new base::FundamentalValue(200));
-  CombineTwoPolicyMaps(A, B, &C);
-  EXPECT_TRUE(A.Equals(C));
-}
+  MockCloudPolicyCache user_policy_cache_;
+#if defined(OS_CHROMEOS)
+  MockCloudPolicyCache device_policy_cache_;
+#endif
 
-TEST_F(CloudPolicyProviderTest, CombineTwoPolicyMapsEmpty) {
-  PolicyMap A, B, C;
-  CombineTwoPolicyMaps(A, B, &C);
-  EXPECT_TRUE(C.empty());
-}
+  CloudPolicyProvider cloud_policy_provider_;
+  MockConfigurationPolicyObserver observer_;
+  ConfigurationPolicyObserverRegistrar registrar_;
+};
 
-TEST_F(CloudPolicyProviderTest, CombineTwoPolicyMapsPartial) {
-  PolicyMap A, B, C;
-
-  SetPolicy(&A, key::kHomepageLocation,
-            Value::CreateStringValue("http://www.chromium.org"));
-  SetPolicy(&B, key::kHomepageLocation,
-            Value::CreateStringValue("http://www.google.com"));
-  SetPolicy(&B, key::kApplicationLocaleValue, Value::CreateStringValue("us"));
-  SetPolicy(&A, key::kDevicePolicyRefreshRate, new base::FundamentalValue(100));
-  SetPolicy(&B, key::kDevicePolicyRefreshRate, new base::FundamentalValue(200));
-  CombineTwoPolicyMaps(A, B, &C);
-
-  const Value* value;
-  std::string string_value;
-  int int_value;
-  value = C.GetValue(key::kHomepageLocation);
-  ASSERT_TRUE(NULL != value);
-  EXPECT_TRUE(value->GetAsString(&string_value));
-  EXPECT_EQ("http://www.chromium.org", string_value);
-  value = C.GetValue(key::kApplicationLocaleValue);
-  ASSERT_TRUE(NULL != value);
-  EXPECT_TRUE(value->GetAsString(&string_value));
-  EXPECT_EQ("us", string_value);
-  value = C.GetValue(key::kDevicePolicyRefreshRate);
-  ASSERT_TRUE(NULL != value);
-  EXPECT_TRUE(value->GetAsInteger(&int_value));
-  EXPECT_EQ(100, int_value);
+TEST_F(CloudPolicyProviderTest, Initialization) {
+  EXPECT_FALSE(cloud_policy_provider_.IsInitializationComplete());
+
+  // The provider only becomes initialized when it has all caches, and the
+  // caches are ready too.
+  AddDeviceCache();
+  EXPECT_FALSE(cloud_policy_provider_.IsInitializationComplete());
+  SetDeviceCacheReady();
+  EXPECT_FALSE(cloud_policy_provider_.IsInitializationComplete());
+  AddUserCache();
+  EXPECT_FALSE(user_policy_cache_.IsReady());
+  EXPECT_FALSE(cloud_policy_provider_.IsInitializationComplete());
+
+  PolicyMap policy;
+  EXPECT_TRUE(cloud_policy_provider_.Provide(&policy));
+  EXPECT_TRUE(policy.empty());
+
+  SetUserCacheReady();
+  EXPECT_TRUE(cloud_policy_provider_.IsInitializationComplete());
+  EXPECT_TRUE(cloud_policy_provider_.Provide(&policy));
+  EXPECT_TRUE(policy.empty());
+
+  const std::string kUrl("http://chromium.org");
+  user_policy_cache_.mutable_policy()->Set(key::kHomepageLocation,
+                                           POLICY_LEVEL_MANDATORY,
+                                           POLICY_SCOPE_USER,
+                                           Value::CreateStringValue(kUrl));
+  EXPECT_CALL(observer_, OnUpdatePolicy(&cloud_policy_provider_));
+  user_policy_cache_.NotifyObservers();
+  Mock::VerifyAndClearExpectations(&observer_);
+  EXPECT_TRUE(cloud_policy_provider_.Provide(&policy));
+
+  PolicyMap expected;
+  expected.Set(key::kHomepageLocation,
+               POLICY_LEVEL_MANDATORY,
+               POLICY_SCOPE_USER,
+               Value::CreateStringValue(kUrl));
+  EXPECT_TRUE(policy.Equals(expected));
 }
 
-TEST_F(CloudPolicyProviderTest, CombineTwoPolicyMapsProxies) {
-  const int a_value = 1;
-  const int b_value = -1;
-  PolicyMap A, B, C;
+TEST_F(CloudPolicyProviderTest, RefreshPolicies) {
+  // OnUpdatePolicy is called when the provider doesn't have any caches.
+  EXPECT_CALL(observer_, OnUpdatePolicy(&cloud_policy_provider_)).Times(1);
+  cloud_policy_provider_.RefreshPolicies();
+  Mock::VerifyAndClearExpectations(&observer_);
 
-  SetPolicy(&A, key::kProxyMode, Value::CreateIntegerValue(a_value));
-  SetPolicy(&B, key::kProxyServerMode, Value::CreateIntegerValue(b_value));
-  SetPolicy(&B, key::kProxyServer, Value::CreateIntegerValue(b_value));
-  SetPolicy(&B, key::kProxyPacUrl, Value::CreateIntegerValue(b_value));
-  SetPolicy(&B, key::kProxyBypassList, Value::CreateIntegerValue(b_value));
+  // OnUpdatePolicy is called when all the caches have updated.
+  AddUserCache();
 
-  CombineTwoPolicyMaps(A, B, &C);
+  EXPECT_CALL(observer_, OnUpdatePolicy(&cloud_policy_provider_)).Times(0);
+  cloud_policy_provider_.RefreshPolicies();
+  Mock::VerifyAndClearExpectations(&observer_);
 
-  FixDeprecatedPolicies(&A);
-  FixDeprecatedPolicies(&B);
-  EXPECT_TRUE(A.Equals(C));
-  EXPECT_FALSE(B.Equals(C));
-}
+  EXPECT_CALL(observer_, OnUpdatePolicy(&cloud_policy_provider_)).Times(1);
+  cloud_policy_provider_.OnCacheUpdate(&user_policy_cache_);
+  Mock::VerifyAndClearExpectations(&observer_);
 
-TEST_F(CloudPolicyProviderTest, RefreshPolicies) {
-  CreateCloudPolicyProvider();
-  MockCloudPolicyCache cache0;
-  MockCloudPolicyCache cache1;
-  MockCloudPolicyCache cache2;
-  MockConfigurationPolicyObserver observer;
-  ConfigurationPolicyObserverRegistrar registrar;
-  registrar.Init(cloud_policy_provider_.get(), &observer);
+#if defined(OS_CHROMEOS)
+  AddDeviceCache();
 
-  // OnUpdatePolicy is called when the provider doesn't have any caches.
-  EXPECT_CALL(observer, OnUpdatePolicy(cloud_policy_provider_.get())).Times(1);
-  cloud_policy_provider_->RefreshPolicies();
-  Mock::VerifyAndClearExpectations(&observer);
+  EXPECT_CALL(observer_, OnUpdatePolicy(&cloud_policy_provider_)).Times(0);
+  cloud_policy_provider_.RefreshPolicies();
+  Mock::VerifyAndClearExpectations(&observer_);
 
-  // OnUpdatePolicy is called when all the caches have updated.
-  EXPECT_CALL(observer, OnUpdatePolicy(cloud_policy_provider_.get())).Times(2);
-  cloud_policy_provider_->AppendCache(&cache0);
-  cloud_policy_provider_->AppendCache(&cache1);
-  Mock::VerifyAndClearExpectations(&observer);
-
-  EXPECT_CALL(observer, OnUpdatePolicy(cloud_policy_provider_.get())).Times(0);
-  cloud_policy_provider_->RefreshPolicies();
-  Mock::VerifyAndClearExpectations(&observer);
-
-  EXPECT_CALL(observer, OnUpdatePolicy(cloud_policy_provider_.get())).Times(0);
-  // Updating just one of the caches is not enough.
-  cloud_policy_provider_->OnCacheUpdate(&cache0);
-  Mock::VerifyAndClearExpectations(&observer);
-
-  EXPECT_CALL(observer, OnUpdatePolicy(cloud_policy_provider_.get())).Times(0);
-  // This cache wasn't available when RefreshPolicies was called, so it isn't
-  // required to fire the update.
-  cloud_policy_provider_->AppendCache(&cache2);
-  Mock::VerifyAndClearExpectations(&observer);
-
-  EXPECT_CALL(observer, OnUpdatePolicy(cloud_policy_provider_.get())).Times(1);
-  cloud_policy_provider_->OnCacheUpdate(&cache1);
-  Mock::VerifyAndClearExpectations(&observer);
-
-  EXPECT_CALL(observer, OnUpdatePolicy(cloud_policy_provider_.get())).Times(0);
-  cloud_policy_provider_->RefreshPolicies();
-  Mock::VerifyAndClearExpectations(&observer);
-
-  EXPECT_CALL(observer, OnUpdatePolicy(cloud_policy_provider_.get())).Times(0);
-  cloud_policy_provider_->OnCacheUpdate(&cache0);
-  cloud_policy_provider_->OnCacheUpdate(&cache1);
-  Mock::VerifyAndClearExpectations(&observer);
+  // Updating one of the caches is not enough, both must be updated.
+  EXPECT_CALL(observer_, OnUpdatePolicy(&cloud_policy_provider_)).Times(0);
+  cloud_policy_provider_.OnCacheUpdate(&user_policy_cache_);
+  Mock::VerifyAndClearExpectations(&observer_);
 
   // If a cache refreshes more than once, the provider should still wait for
   // the others before firing the update.
-  EXPECT_CALL(observer, OnUpdatePolicy(cloud_policy_provider_.get())).Times(0);
-  cloud_policy_provider_->OnCacheUpdate(&cache0);
-  Mock::VerifyAndClearExpectations(&observer);
-
-  // Fire updates if one of the required caches goes away while waiting.
-  EXPECT_CALL(observer, OnUpdatePolicy(cloud_policy_provider_.get())).Times(1);
-  cloud_policy_provider_->OnCacheGoingAway(&cache2);
-  Mock::VerifyAndClearExpectations(&observer);
+  EXPECT_CALL(observer_, OnUpdatePolicy(&cloud_policy_provider_)).Times(0);
+  cloud_policy_provider_.OnCacheUpdate(&user_policy_cache_);
+  Mock::VerifyAndClearExpectations(&observer_);
+
+  EXPECT_CALL(observer_, OnUpdatePolicy(&cloud_policy_provider_)).Times(1);
+  cloud_policy_provider_.OnCacheUpdate(&device_policy_cache_);
+  Mock::VerifyAndClearExpectations(&observer_);
+#endif
+}
+
+#if defined(OS_CHROMEOS)
+TEST_F(CloudPolicyProviderTest, MergeProxyPolicies) {
+  AddDeviceCache();
+  AddUserCache();
+  SetDeviceCacheReady();
+  SetUserCacheReady();
+  EXPECT_TRUE(cloud_policy_provider_.IsInitializationComplete());
+  PolicyMap policy;
+  EXPECT_TRUE(cloud_policy_provider_.Provide(&policy));
+  EXPECT_TRUE(policy.empty());
+
+  // User policy takes precedence over device policy.
+  EXPECT_CALL(observer_, OnUpdatePolicy(&cloud_policy_provider_));
+  device_policy_cache_.mutable_policy()->Set(
+      key::kProxyMode, POLICY_LEVEL_RECOMMENDED, POLICY_SCOPE_MACHINE,
+      Value::CreateStringValue("device mode"));
+  device_policy_cache_.NotifyObservers();
+  Mock::VerifyAndClearExpectations(&observer_);
+
+  EXPECT_CALL(observer_, OnUpdatePolicy(&cloud_policy_provider_));
+  user_policy_cache_.mutable_policy()->Set(
+      key::kProxyMode, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+      Value::CreateStringValue("user mode"));
+  user_policy_cache_.NotifyObservers();
+  Mock::VerifyAndClearExpectations(&observer_);
+
+  EXPECT_TRUE(cloud_policy_provider_.Provide(&policy));
+
+  // The deprecated proxy policies are converted to the new ProxySettings.
+  EXPECT_TRUE(policy.Get(key::kProxyMode) == NULL);
+  EXPECT_TRUE(policy.Get(key::kProxyServerMode) == NULL);
+  EXPECT_TRUE(policy.Get(key::kProxyServer) == NULL);
+  EXPECT_TRUE(policy.Get(key::kProxyPacUrl) == NULL);
+
+  EXPECT_EQ(1u, policy.size());
+  const PolicyMap::Entry* entry = policy.Get(key::kProxySettings);
+  ASSERT_TRUE(entry != NULL);
+  ASSERT_TRUE(entry->value != NULL);
+  EXPECT_EQ(POLICY_LEVEL_MANDATORY, entry->level);
+  EXPECT_EQ(POLICY_SCOPE_USER, entry->scope);
+  const DictionaryValue* settings;
+  ASSERT_TRUE(entry->value->GetAsDictionary(&settings));
+  std::string mode;
+  EXPECT_TRUE(settings->GetString(key::kProxyMode, &mode));
+  EXPECT_EQ("user mode", mode);
 }
+#endif
 
 }  // namespace policy