Migrate old cookie setting to new system.

Also, do a better job of making sure we don't write useless default values to disk, and we force the in-memory copy of the default prefs to never say "DEFAULT" even if a caller provides that to SetDefaultContentSetting().

BUG=32719
TEST=unittests
Review URL: http://codereview.chromium.org/562003

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

7 files changed, 80 insertions, 41 deletions

diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index 70926bb..2728c27 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -1377,8 +1377,6 @@ void Browser::RegisterUserPrefs(PrefService* prefs) {
   prefs->RegisterStringPref(prefs::kHomePage,
                             ASCIIToWide(chrome::kChromeUINewTabURL));
   prefs->RegisterBooleanPref(prefs::kHomePageIsNewTabPage, true);
-  prefs->RegisterIntegerPref(prefs::kCookieBehavior,
-                             net::StaticCookiePolicy::ALLOW_ALL_COOKIES);
   prefs->RegisterBooleanPref(prefs::kShowHomeButton, false);
 #if defined(OS_MACOSX)
   // This really belongs in platform code, but there's no good place to
diff --git a/chrome/browser/host_content_settings_map.cc b/chrome/browser/host_content_settings_map.cc
index 0cf48a1..a31b05e 100644
--- a/chrome/browser/host_content_settings_map.cc
+++ b/chrome/browser/host_content_settings_map.cc
@@ -9,6 +9,7 @@
 #include "chrome/browser/profile.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/pref_service.h"
+#include "net/base/static_cookie_policy.h"
 
 // static
 const wchar_t*
@@ -20,13 +21,40 @@ const wchar_t*
   L"popups",
 };
 
+// static
+const ContentSetting
+    HostContentSettingsMap::kDefaultSettings[CONTENT_SETTINGS_NUM_TYPES] = {
+  CONTENT_SETTING_ALLOW,  // CONTENT_SETTINGS_TYPE_COOKIES
+  CONTENT_SETTING_ALLOW,  // CONTENT_SETTINGS_TYPE_IMAGES
+  CONTENT_SETTING_ALLOW,  // CONTENT_SETTINGS_TYPE_JAVASCRIPT
+  CONTENT_SETTING_ALLOW,  // CONTENT_SETTINGS_TYPE_PLUGINS
+  CONTENT_SETTING_BLOCK,  // CONTENT_SETTINGS_TYPE_POPUPS
+};
+
 HostContentSettingsMap::HostContentSettingsMap(Profile* profile)
     : profile_(profile),
       block_third_party_cookies_(false) {
+  PrefService* prefs = profile_->GetPrefs();
+
+  // Migrate obsolete cookie pref.
+  if (prefs->HasPrefPath(prefs::kCookieBehavior)) {
+    int cookie_behavior = prefs->GetInteger(prefs::kCookieBehavior);
+    prefs->ClearPref(prefs::kCookieBehavior);
+    if (!prefs->HasPrefPath(prefs::kDefaultContentSettings)) {
+        SetDefaultContentSetting(CONTENT_SETTINGS_TYPE_COOKIES,
+            (cookie_behavior == net::StaticCookiePolicy::BLOCK_ALL_COOKIES) ?
+                CONTENT_SETTING_BLOCK : CONTENT_SETTING_ALLOW);
+    }
+    if (!prefs->HasPrefPath(prefs::kBlockThirdPartyCookies)) {
+      SetBlockThirdPartyCookies(cookie_behavior ==
+          net::StaticCookiePolicy::BLOCK_THIRD_PARTY_COOKIES);
+    }
+  }
+
   DCHECK_EQ(arraysize(kTypeNames),
             static_cast<size_t>(CONTENT_SETTINGS_NUM_TYPES));
   const DictionaryValue* default_settings_dictionary =
-      profile_->GetPrefs()->GetDictionary(prefs::kDefaultContentSettings);
+      prefs->GetDictionary(prefs::kDefaultContentSettings);
   // Careful: The returned value could be NULL if the pref has never been set.
   if (default_settings_dictionary != NULL) {
     GetSettingsFromDictionary(default_settings_dictionary,
@@ -35,7 +63,7 @@ HostContentSettingsMap::HostContentSettingsMap(Profile* profile)
   ForceDefaultsToBeExplicit();
 
   const DictionaryValue* all_settings_dictionary =
-      profile_->GetPrefs()->GetDictionary(prefs::kPerHostContentSettings);
+      prefs->GetDictionary(prefs::kPerHostContentSettings);
   // Careful: The returned value could be NULL if the pref has never been set.
   if (all_settings_dictionary != NULL) {
     for (DictionaryValue::key_iterator i(all_settings_dictionary->begin_keys());
@@ -52,7 +80,7 @@ HostContentSettingsMap::HostContentSettingsMap(Profile* profile)
   }
 
   block_third_party_cookies_ =
-      profile_->GetPrefs()->GetBoolean(prefs::kBlockThirdPartyCookies);
+      prefs->GetBoolean(prefs::kBlockThirdPartyCookies);
 }
 
 // static
@@ -60,6 +88,10 @@ void HostContentSettingsMap::RegisterUserPrefs(PrefService* prefs) {
   prefs->RegisterDictionaryPref(prefs::kDefaultContentSettings);
   prefs->RegisterDictionaryPref(prefs::kPerHostContentSettings);
   prefs->RegisterBooleanPref(prefs::kBlockThirdPartyCookies, false);
+
+  // Obsolete prefs, for migration:
+  prefs->RegisterIntegerPref(prefs::kCookieBehavior,
+                             net::StaticCookiePolicy::ALLOW_ALL_COOKIES);
 }
 
 ContentSetting HostContentSettingsMap::GetDefaultContentSetting(
@@ -116,14 +148,24 @@ void HostContentSettingsMap::SetDefaultContentSetting(
     ContentSetting setting) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
 
+  DictionaryValue* default_settings_dictionary =
+      profile_->GetPrefs()->GetMutableDictionary(
+          prefs::kDefaultContentSettings);
+  std::wstring dictionary_path(kTypeNames[content_type]);
   {
     AutoLock auto_lock(lock_);
-    default_content_settings_.settings[content_type] = setting;
+    if ((setting == CONTENT_SETTING_DEFAULT) ||
+        (setting == kDefaultSettings[content_type])) {
+      default_content_settings_.settings[content_type] =
+          kDefaultSettings[content_type];
+      default_settings_dictionary->RemoveWithoutPathExpansion(dictionary_path,
+                                                              NULL);
+    } else {
+      default_content_settings_.settings[content_type] = setting;
+      default_settings_dictionary->SetWithoutPathExpansion(
+          dictionary_path, Value::CreateIntegerValue(setting));
+    }
   }
-
-  profile_->GetPrefs()->GetMutableDictionary(prefs::kDefaultContentSettings)->
-      SetWithoutPathExpansion(std::wstring(kTypeNames[content_type]),
-                              Value::CreateIntegerValue(setting));
 }
 
 void HostContentSettingsMap::SetContentSetting(const std::string& host,
@@ -131,7 +173,10 @@ void HostContentSettingsMap::SetContentSetting(const std::string& host,
                                                ContentSetting setting) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
 
-  bool all_default;
+  std::wstring wide_host(UTF8ToWide(host));
+  DictionaryValue* all_settings_dictionary =
+      profile_->GetPrefs()->GetMutableDictionary(
+          prefs::kPerHostContentSettings);
   {
     AutoLock auto_lock(lock_);
     if (!host_content_settings_.count(host))
@@ -139,19 +184,13 @@ void HostContentSettingsMap::SetContentSetting(const std::string& host,
     HostContentSettings::iterator i(host_content_settings_.find(host));
     ContentSettings& settings = i->second;
     settings.settings[content_type] = setting;
-    all_default = AllDefault(settings);
-    if (all_default)
+    if (AllDefault(settings)) {
       host_content_settings_.erase(i);
+      all_settings_dictionary->RemoveWithoutPathExpansion(wide_host, NULL);
+      return;
+    }
   }
 
-  std::wstring wide_host(UTF8ToWide(host));
-  DictionaryValue* all_settings_dictionary =
-      profile_->GetPrefs()->GetMutableDictionary(
-          prefs::kPerHostContentSettings);
-  if (all_default) {
-    all_settings_dictionary->RemoveWithoutPathExpansion(wide_host, NULL);
-    return;
-  }
   DictionaryValue* host_settings_dictionary;
   bool found = all_settings_dictionary->GetDictionaryWithoutPathExpansion(
       wide_host, &host_settings_dictionary);
@@ -159,6 +198,7 @@ void HostContentSettingsMap::SetContentSetting(const std::string& host,
     host_settings_dictionary = new DictionaryValue;
     all_settings_dictionary->SetWithoutPathExpansion(
         wide_host, host_settings_dictionary);
+    DCHECK_NE(setting, CONTENT_SETTING_DEFAULT);
   }
   std::wstring dictionary_path(kTypeNames[content_type]);
   if (setting == CONTENT_SETTING_DEFAULT) {
@@ -206,8 +246,11 @@ void HostContentSettingsMap::SetBlockThirdPartyCookies(bool block) {
     block_third_party_cookies_ = block;
   }
 
-  profile_->GetPrefs()->SetBoolean(prefs::kBlockThirdPartyCookies,
-                                   block_third_party_cookies_);
+  PrefService* prefs = profile_->GetPrefs();
+  if (block)
+    prefs->SetBoolean(prefs::kBlockThirdPartyCookies, true);
+  else
+    prefs->ClearPref(prefs::kBlockThirdPartyCookies);
 }
 
 void HostContentSettingsMap::ResetToDefaults() {
@@ -221,9 +264,10 @@ void HostContentSettingsMap::ResetToDefaults() {
     block_third_party_cookies_ = false;
   }
 
-  profile_->GetPrefs()->ClearPref(prefs::kDefaultContentSettings);
-  profile_->GetPrefs()->ClearPref(prefs::kPerHostContentSettings);
-  profile_->GetPrefs()->ClearPref(prefs::kBlockThirdPartyCookies);
+  PrefService* prefs = profile_->GetPrefs();
+  prefs->ClearPref(prefs::kDefaultContentSettings);
+  prefs->ClearPref(prefs::kPerHostContentSettings);
+  prefs->ClearPref(prefs::kBlockThirdPartyCookies);
 }
 
 HostContentSettingsMap::~HostContentSettingsMap() {
@@ -249,13 +293,9 @@ void HostContentSettingsMap::GetSettingsFromDictionary(
 }
 
 void HostContentSettingsMap::ForceDefaultsToBeExplicit() {
-  static const ContentSetting kDefaultSettings[CONTENT_SETTINGS_NUM_TYPES] = {
-    CONTENT_SETTING_ALLOW,  // CONTENT_SETTINGS_TYPE_COOKIES
-    CONTENT_SETTING_ALLOW,  // CONTENT_SETTINGS_TYPE_IMAGES
-    CONTENT_SETTING_ALLOW,  // CONTENT_SETTINGS_TYPE_JAVASCRIPT
-    CONTENT_SETTING_ALLOW,  // CONTENT_SETTINGS_TYPE_PLUGINS
-    CONTENT_SETTING_BLOCK,  // CONTENT_SETTINGS_TYPE_POPUPS
-  };
+  DCHECK_EQ(arraysize(kDefaultSettings),
+            static_cast<size_t>(CONTENT_SETTINGS_NUM_TYPES));
+
   for (int i = 0; i < CONTENT_SETTINGS_NUM_TYPES; ++i) {
     if (default_content_settings_.settings[i] == CONTENT_SETTING_DEFAULT)
       default_content_settings_.settings[i] = kDefaultSettings[i];
diff --git a/chrome/browser/host_content_settings_map.h b/chrome/browser/host_content_settings_map.h
index c24a412..658225a 100644
--- a/chrome/browser/host_content_settings_map.h
+++ b/chrome/browser/host_content_settings_map.h
@@ -96,7 +96,10 @@ class HostContentSettingsMap
   typedef std::map<std::string, ContentSettings> HostContentSettings;
 
   // The names of the ContentSettingsType values, for use with dictionary prefs.
-  static const wchar_t* kTypeNames[];
+  static const wchar_t* kTypeNames[CONTENT_SETTINGS_NUM_TYPES];
+
+  // The default setting for each content type.
+  static const ContentSetting kDefaultSettings[CONTENT_SETTINGS_NUM_TYPES];
 
   ~HostContentSettingsMap();
 
diff --git a/chrome/browser/net/chrome_url_request_context.cc b/chrome/browser/net/chrome_url_request_context.cc
index dee0d62..6b97030 100644
--- a/chrome/browser/net/chrome_url_request_context.cc
+++ b/chrome/browser/net/chrome_url_request_context.cc
@@ -508,7 +508,6 @@ void ChromeURLRequestContextGetter::CleanupOnUIThread() {
   if (prefs_) {
     // Unregister for pref notifications.
     prefs_->RemovePrefObserver(prefs::kAcceptLanguages, this);
-    prefs_->RemovePrefObserver(prefs::kCookieBehavior, this);
     prefs_->RemovePrefObserver(prefs::kDefaultCharset, this);
     prefs_ = NULL;
   }
@@ -566,7 +565,6 @@ void ChromeURLRequestContextGetter::RegisterPrefsObserver(Profile* profile) {
   prefs_ = profile->GetPrefs();
 
   prefs_->AddPrefObserver(prefs::kAcceptLanguages, this);
-  prefs_->AddPrefObserver(prefs::kCookieBehavior, this);
   prefs_->AddPrefObserver(prefs::kDefaultCharset, this);
 }
 
diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc
index f9d9c9c..9254f73 100644
--- a/chrome/common/pref_names.cc
+++ b/chrome/common/pref_names.cc
@@ -114,7 +114,9 @@ const wchar_t kSafeBrowsingEnabled[] = L"safebrowsing.enabled";
 // Boolean that is true when Suggest support is enabled.
 const wchar_t kSearchSuggestEnabled[] = L"search.suggest_enabled";
 
-// Enum that specifies whether to enforce a third-party cookie blocking policy.
+// OBSOLETE.  Enum that specifies whether to enforce a third-party cookie
+// blocking policy.  This has been superseded by kDefaultContentSettings +
+// kBlockThirdPartyCookies.
 // 0 - allow all cookies.
 // 1 - block third-party cookies
 // 2 - block all cookies
diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h
index 4e0b446..da24ab0 100644
--- a/chrome/common/pref_names.h
+++ b/chrome/common/pref_names.h
@@ -48,7 +48,7 @@ extern const wchar_t kPasswordManagerEnabled[];
 extern const wchar_t kFormAutofillEnabled[];
 extern const wchar_t kSafeBrowsingEnabled[];
 extern const wchar_t kSearchSuggestEnabled[];
-extern const wchar_t kCookieBehavior[];
+extern const wchar_t kCookieBehavior[];  // OBSOLETE
 extern const wchar_t kMixedContentFiltering[];
 extern const wchar_t kDefaultSearchProviderSearchURL[];
 extern const wchar_t kDefaultSearchProviderSuggestURL[];
diff --git a/chrome/test/live_sync/profile_sync_service_test_harness.cc b/chrome/test/live_sync/profile_sync_service_test_harness.cc
index 74b8928..4bee1ce 100644
--- a/chrome/test/live_sync/profile_sync_service_test_harness.cc
+++ b/chrome/test/live_sync/profile_sync_service_test_harness.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -114,8 +114,6 @@ ProfileSyncServiceTestHarness::ProfileSyncServiceTestHarness(
   // Ensure the profile has enough prefs registered for use by sync.
   if (!p->GetPrefs()->FindPreference(prefs::kAcceptLanguages))
     TabContents::RegisterUserPrefs(p->GetPrefs());
-  if (!p->GetPrefs()->FindPreference(prefs::kCookieBehavior))
-    Browser::RegisterUserPrefs(p->GetPrefs());
 }
 
 bool ProfileSyncServiceTestHarness::SetupSync() {