Make proxy settings one atomic dictionary in the PrefStores such that modifications are atomic.

chrome/browser/policy/configuration_policy_pref_store_unittest.cc

Index: chrome/browser/policy/configuration_policy_pref_store_unittest.cc
diff --git a/chrome/browser/policy/configuration_policy_pref_store_unittest.cc b/chrome/browser/policy/configuration_policy_pref_store_unittest.cc
index b948712f517604a0332badbdd7a694a1de8bc28d..cde676dad5d5cf7177a5f4e28d960bde43efbe42 100644
--- a/chrome/browser/policy/configuration_policy_pref_store_unittest.cc
+++ b/chrome/browser/policy/configuration_policy_pref_store_unittest.cc
@@ -238,33 +238,32 @@ class ConfigurationPolicyPrefStoreProxyTest : public testing::Test {
       const std::string& expected_proxy_bypass_list,
       const ProxyPrefs::ProxyMode& expected_proxy_mode) {
     Value* value = NULL;
-
+    ASSERT_EQ(PrefStore::READ_OK,
+              store.GetValue(prefs::kProxy, &value));
+    ASSERT_EQ(Value::TYPE_DICTIONARY, value->GetType());
+    ProxyPrefsDictionary dict(static_cast<DictionaryValue*>(value));
+    std::string s;
     if (expected_proxy_server.empty()) {
-      EXPECT_EQ(PrefStore::READ_USE_DEFAULT,
-                store.GetValue(prefs::kProxyServer, NULL));
+      EXPECT_FALSE(dict.GetProxyServer(&s));
     } else {
-      EXPECT_EQ(PrefStore::READ_OK,
-                store.GetValue(prefs::kProxyServer, &value));
-      EXPECT_TRUE(StringValue(expected_proxy_server).Equals(value));
+      ASSERT_TRUE(dict.GetProxyServer(&s));
+      EXPECT_EQ(expected_proxy_server, s);
     }
     if (expected_proxy_pac_url.empty()) {
-      EXPECT_EQ(PrefStore::READ_USE_DEFAULT,
-                store.GetValue(prefs::kProxyPacUrl, NULL));
+      EXPECT_FALSE(dict.GetPacUrl(&s));
     } else {
-      EXPECT_EQ(PrefStore::READ_OK,
-                store.GetValue(prefs::kProxyPacUrl, &value));
-      EXPECT_TRUE(StringValue(expected_proxy_pac_url).Equals(value));
+      ASSERT_TRUE(dict.GetPacUrl(&s));
+      EXPECT_EQ(expected_proxy_pac_url, s);
     }
     if (expected_proxy_bypass_list.empty()) {
-      EXPECT_EQ(PrefStore::READ_USE_DEFAULT,
-                store.GetValue(prefs::kProxyBypassList, NULL));
+      EXPECT_FALSE(dict.GetBypassList(&s));
     } else {
-      EXPECT_EQ(PrefStore::READ_OK,
-                store.GetValue(prefs::kProxyBypassList, &value));
-      EXPECT_TRUE(StringValue(expected_proxy_bypass_list).Equals(value));
+      ASSERT_TRUE(dict.GetBypassList(&s));
+      EXPECT_EQ(expected_proxy_bypass_list, s);
     }
-    EXPECT_EQ(PrefStore::READ_OK, store.GetValue(prefs::kProxyMode, &value));
-    EXPECT_TRUE(FundamentalValue(expected_proxy_mode).Equals(value));
+    ProxyPrefs::ProxyMode mode;
+    ASSERT_TRUE(dict.GetMode(&mode));
+    EXPECT_EQ(expected_proxy_mode, mode);
   }
 };
 
@@ -409,8 +408,9 @@ TEST_F(ConfigurationPolicyPrefStoreProxyTest, ProxyInvalid) {
 
     scoped_refptr<ConfigurationPolicyPrefStore> store(
         new ConfigurationPolicyPrefStore(&provider));
-    EXPECT_EQ(PrefStore::READ_NO_VALUE,
-              store->GetValue(prefs::kProxyMode, NULL));
+    Value* value = NULL;
+    ASSERT_EQ(PrefStore::READ_NO_VALUE,

[danno, 2011/02/04 10:32:13]

Why is this an assert? Asserts are for fatal errors that should abort test
execution.

[battre, 2011/02/07 14:13:47]

Done.

+              store->GetValue(prefs::kProxy, &value));
   }
 }