Fix some bugs related to default-clearing in the HostContentSettingsMap, and clean up various other bits.

This also adds some functionality useful to the Exceptions dialogs.

BUG=none
TEST=none
Review URL: http://codereview.chromium.org/557074

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

6 files changed, 107 insertions, 37 deletions

diff --git a/chrome/browser/host_content_settings_map.cc b/chrome/browser/host_content_settings_map.cc
index 849e204..0cf48a1 100644
--- a/chrome/browser/host_content_settings_map.cc
+++ b/chrome/browser/host_content_settings_map.cc
@@ -11,7 +11,8 @@
 #include "chrome/common/pref_service.h"
 
 // static
-const wchar_t* HostContentSettingsMap::kTypeNames[] = {
+const wchar_t*
+    HostContentSettingsMap::kTypeNames[CONTENT_SETTINGS_NUM_TYPES] = {
   L"cookies",
   L"images",
   L"javascript",
@@ -24,7 +25,6 @@ HostContentSettingsMap::HostContentSettingsMap(Profile* profile)
       block_third_party_cookies_(false) {
   DCHECK_EQ(arraysize(kTypeNames),
             static_cast<size_t>(CONTENT_SETTINGS_NUM_TYPES));
-
   const DictionaryValue* default_settings_dictionary =
       profile_->GetPrefs()->GetDictionary(prefs::kDefaultContentSettings);
   // Careful: The returned value could be NULL if the pref has never been set.
@@ -93,9 +93,9 @@ ContentSettings HostContentSettingsMap::GetContentSettings(
   return output;
 }
 
-void HostContentSettingsMap::GetHostContentSettingsForOneType(
+void HostContentSettingsMap::GetSettingsForOneType(
     ContentSettingsType content_type,
-    HostContentSettingsForOneType* settings) const {
+    SettingsForOneType* settings) const {
   DCHECK(settings);
   settings->clear();
 
@@ -103,8 +103,11 @@ void HostContentSettingsMap::GetHostContentSettingsForOneType(
   for (HostContentSettings::const_iterator i(host_content_settings_.begin());
        i != host_content_settings_.end(); ++i) {
     ContentSetting setting = i->second.settings[content_type];
-    if (setting != CONTENT_SETTING_DEFAULT)
-      (*settings)[i->first] = setting;
+    if (setting != CONTENT_SETTING_DEFAULT) {
+      // Use of push_back() relies on the map iterator traversing in order of
+      // ascending keys.
+      settings->push_back(std::make_pair(i->first, setting));
+    }
   }
 }
 
@@ -128,7 +131,7 @@ void HostContentSettingsMap::SetContentSetting(const std::string& host,
                                                ContentSetting setting) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
 
-  bool all_default = true;
+  bool all_default;
   {
     AutoLock auto_lock(lock_);
     if (!host_content_settings_.count(host))
@@ -136,12 +139,7 @@ void HostContentSettingsMap::SetContentSetting(const std::string& host,
     HostContentSettings::iterator i(host_content_settings_.find(host));
     ContentSettings& settings = i->second;
     settings.settings[content_type] = setting;
-    for (size_t i = 0; i < arraysize(settings.settings); ++i) {
-      if (settings.settings[i] != CONTENT_SETTING_DEFAULT) {
-        all_default = false;
-        break;
-      }
-    }
+    all_default = AllDefault(settings);
     if (all_default)
       host_content_settings_.erase(i);
   }
@@ -162,9 +160,42 @@ void HostContentSettingsMap::SetContentSetting(const std::string& host,
     all_settings_dictionary->SetWithoutPathExpansion(
         wide_host, host_settings_dictionary);
   }
-  host_settings_dictionary->SetWithoutPathExpansion(
-      std::wstring(kTypeNames[content_type]),
-      Value::CreateIntegerValue(setting));
+  std::wstring dictionary_path(kTypeNames[content_type]);
+  if (setting == CONTENT_SETTING_DEFAULT) {
+    host_settings_dictionary->RemoveWithoutPathExpansion(dictionary_path, NULL);
+  } else {
+    host_settings_dictionary->SetWithoutPathExpansion(
+        dictionary_path, Value::CreateIntegerValue(setting));
+  }
+}
+
+void HostContentSettingsMap::ClearSettingsForOneType(
+    ContentSettingsType content_type) {
+  AutoLock auto_lock(lock_);
+  for (HostContentSettings::iterator i(host_content_settings_.begin());
+       i != host_content_settings_.end(); ) {
+    if (i->second.settings[content_type] != CONTENT_SETTING_DEFAULT) {
+      i->second.settings[content_type] = CONTENT_SETTING_DEFAULT;
+      std::wstring wide_host(UTF8ToWide(i->first));
+      DictionaryValue* all_settings_dictionary =
+          profile_->GetPrefs()->GetMutableDictionary(
+              prefs::kPerHostContentSettings);
+      if (AllDefault(i->second)) {
+        all_settings_dictionary->RemoveWithoutPathExpansion(wide_host, NULL);
+        host_content_settings_.erase(i++);
+      } else {
+        DictionaryValue* host_settings_dictionary;
+        bool found = all_settings_dictionary->GetDictionaryWithoutPathExpansion(
+            wide_host, &host_settings_dictionary);
+        DCHECK(found);
+        host_settings_dictionary->RemoveWithoutPathExpansion(
+            kTypeNames[content_type], NULL);
+        ++i;
+      }
+    } else {
+      ++i;
+    }
+  }
 }
 
 void HostContentSettingsMap::SetBlockThirdPartyCookies(bool block) {
@@ -230,3 +261,12 @@ void HostContentSettingsMap::ForceDefaultsToBeExplicit() {
       default_content_settings_.settings[i] = kDefaultSettings[i];
   }
 }
+
+bool HostContentSettingsMap::AllDefault(const ContentSettings& settings) const {
+  for (size_t i = 0; i < arraysize(settings.settings); ++i) {
+    if (settings.settings[i] != CONTENT_SETTING_DEFAULT)
+      return false;
+  }
+
+  return true;
+}
diff --git a/chrome/browser/host_content_settings_map.h b/chrome/browser/host_content_settings_map.h
index e068b06..c24a412 100644
--- a/chrome/browser/host_content_settings_map.h
+++ b/chrome/browser/host_content_settings_map.h
@@ -10,6 +10,8 @@
 
 #include <map>
 #include <string>
+#include <utility>
+#include <vector>
 
 #include "base/basictypes.h"
 #include "base/lock.h"
@@ -23,7 +25,8 @@ class Profile;
 class HostContentSettingsMap
     : public base::RefCountedThreadSafe<HostContentSettingsMap> {
  public:
-  typedef std::map<std::string, ContentSetting> HostContentSettingsForOneType;
+  typedef std::pair<std::string, ContentSetting> HostSettingPair;
+  typedef std::vector<HostSettingPair> SettingsForOneType;
 
   explicit HostContentSettingsMap(Profile* profile);
 
@@ -47,12 +50,12 @@ class HostContentSettingsMap
   ContentSettings GetContentSettings(const std::string& host) const;
 
   // For a given content type, returns all hosts with a non-default setting,
-  // mapped to their actual settings.  |settings| must be a non-NULL outparam.
+  // mapped to their actual settings, in lexicographical order.  |settings| must
+  // be a non-NULL outparam.
   //
   // This may be called on any thread.
-  void GetHostContentSettingsForOneType(
-      ContentSettingsType content_type,
-      HostContentSettingsForOneType* settings) const;
+  void GetSettingsForOneType(ContentSettingsType content_type,
+                             SettingsForOneType* settings) const;
 
   // Sets the default setting for a particular content type.
   //
@@ -69,6 +72,11 @@ class HostContentSettingsMap
                          ContentSettingsType content_type,
                          ContentSetting setting);
 
+  // Clears all host-specific settings for one content type.
+  //
+  // This should only be called on the UI thread.
+  void ClearSettingsForOneType(ContentSettingsType content_type);
+
   // This setting trumps any host-specific settings.
   bool BlockThirdPartyCookies() const { return block_third_party_cookies_; }
 
@@ -100,6 +108,9 @@ class HostContentSettingsMap
   // being CONTENT_SETTING_DEFAULT.
   void ForceDefaultsToBeExplicit();
 
+  // Returns true if |settings| consists entirely of CONTENT_SETTING_DEFAULT.
+  bool AllDefault(const ContentSettings& settings) const;
+
   // The profile we're associated with.
   Profile* profile_;
 
diff --git a/chrome/browser/host_content_settings_map_unittest.cc b/chrome/browser/host_content_settings_map_unittest.cc
index eaaa9ce..c4b2f86 100644
--- a/chrome/browser/host_content_settings_map_unittest.cc
+++ b/chrome/browser/host_content_settings_map_unittest.cc
@@ -102,20 +102,39 @@ TEST_F(HostContentSettingsMapTest, DefaultValues) {
       CONTENT_SETTINGS_TYPE_IMAGES, CONTENT_SETTING_BLOCK);
   host_content_settings_map->SetContentSetting(host2,
       CONTENT_SETTINGS_TYPE_PLUGINS, CONTENT_SETTING_BLOCK);
-  HostContentSettingsMap::HostContentSettingsForOneType host_settings;
-  host_content_settings_map->GetHostContentSettingsForOneType(
-      CONTENT_SETTINGS_TYPE_IMAGES, &host_settings);
+  HostContentSettingsMap::SettingsForOneType host_settings;
+  host_content_settings_map->GetSettingsForOneType(CONTENT_SETTINGS_TYPE_IMAGES,
+                                                   &host_settings);
   EXPECT_EQ(1U, host_settings.size());
-  host_content_settings_map->GetHostContentSettingsForOneType(
+  host_content_settings_map->GetSettingsForOneType(
       CONTENT_SETTINGS_TYPE_PLUGINS, &host_settings);
   EXPECT_EQ(2U, host_settings.size());
-  host_content_settings_map->GetHostContentSettingsForOneType(
-      CONTENT_SETTINGS_TYPE_POPUPS, &host_settings);
+  host_content_settings_map->GetSettingsForOneType(CONTENT_SETTINGS_TYPE_POPUPS,
+                                                   &host_settings);
   EXPECT_EQ(0U, host_settings.size());
   host_content_settings_map->ResetToDefaults();
-  host_content_settings_map->GetHostContentSettingsForOneType(
+  host_content_settings_map->GetSettingsForOneType(
       CONTENT_SETTINGS_TYPE_PLUGINS, &host_settings);
   EXPECT_EQ(0U, host_settings.size());
+
+  // Check clearing one type.
+  std::string host3("example.net");
+  host_content_settings_map->SetContentSetting(host,
+      CONTENT_SETTINGS_TYPE_IMAGES, CONTENT_SETTING_BLOCK);
+  host_content_settings_map->SetContentSetting(host2,
+      CONTENT_SETTINGS_TYPE_IMAGES, CONTENT_SETTING_BLOCK);
+  host_content_settings_map->SetContentSetting(host2,
+      CONTENT_SETTINGS_TYPE_PLUGINS, CONTENT_SETTING_BLOCK);
+  host_content_settings_map->SetContentSetting(host3,
+      CONTENT_SETTINGS_TYPE_IMAGES, CONTENT_SETTING_BLOCK);
+  host_content_settings_map->ClearSettingsForOneType(
+      CONTENT_SETTINGS_TYPE_IMAGES);
+  host_content_settings_map->GetSettingsForOneType(CONTENT_SETTINGS_TYPE_IMAGES,
+                                                   &host_settings);
+  EXPECT_EQ(0U, host_settings.size());
+  host_content_settings_map->GetSettingsForOneType(
+      CONTENT_SETTINGS_TYPE_PLUGINS, &host_settings);
+  EXPECT_EQ(1U, host_settings.size());
 }
 
 }  // namespace
diff --git a/chrome/browser/views/content_blocked_bubble_contents.cc b/chrome/browser/views/content_blocked_bubble_contents.cc
index 4ba0b34..82c9eca 100644
--- a/chrome/browser/views/content_blocked_bubble_contents.cc
+++ b/chrome/browser/views/content_blocked_bubble_contents.cc
@@ -85,7 +85,7 @@ void ContentBlockedBubbleContents::InitControlLayout() {
   column_set->AddColumn(GridLayout::LEADING, GridLayout::FILL, 1,
                         GridLayout::USE_PREF, 0, 0);
 
-  static const int kTitleIDs[] = {
+  static const int kTitleIDs[CONTENT_SETTINGS_NUM_TYPES] = {
     IDS_BLOCKED_COOKIES_TITLE,
     IDS_BLOCKED_IMAGES_TITLE,
     IDS_BLOCKED_JAVASCRIPT_TITLE,
@@ -119,7 +119,7 @@ void ContentBlockedBubbleContents::InitControlLayout() {
   }
 
   if (content_type_ != CONTENT_SETTINGS_TYPE_COOKIES) {
-    static const int kAllowIDs[] = {
+    static const int kAllowIDs[CONTENT_SETTINGS_NUM_TYPES] = {
       0,  // Not displayed for cookies
       IDS_BLOCKED_IMAGES_UNBLOCK,
       IDS_BLOCKED_JAVASCRIPT_UNBLOCK,
@@ -134,7 +134,7 @@ void ContentBlockedBubbleContents::InitControlLayout() {
         radio_button_group);
     allow_radio_->set_listener(this);
 
-    static const int kBlockIDs[] = {
+    static const int kBlockIDs[CONTENT_SETTINGS_NUM_TYPES] = {
       0,  // Not displayed for cookies
       IDS_BLOCKED_IMAGES_NO_ACTION,
       IDS_BLOCKED_JAVASCRIPT_NO_ACTION,
@@ -172,7 +172,7 @@ void ContentBlockedBubbleContents::InitControlLayout() {
   double_column_set->AddColumn(GridLayout::TRAILING, GridLayout::CENTER, 0,
                         GridLayout::USE_PREF, 0, 0);
 
-  static const int kLinkIDs[] = {
+  static const int kLinkIDs[CONTENT_SETTINGS_NUM_TYPES] = {
     IDS_BLOCKED_COOKIES_LINK,
     IDS_BLOCKED_IMAGES_LINK,
     IDS_BLOCKED_JAVASCRIPT_LINK,
diff --git a/chrome/browser/views/location_bar_view.cc b/chrome/browser/views/location_bar_view.cc
index 846ef73..39773b7 100644
--- a/chrome/browser/views/location_bar_view.cc
+++ b/chrome/browser/views/location_bar_view.cc
@@ -1333,7 +1333,7 @@ LocationBarView::ContentBlockedImageView::ContentBlockedImageView(
       info_bubble_(NULL),
       bubble_positioner_(bubble_positioner) {
   if (!icons_[CONTENT_SETTINGS_TYPE_COOKIES]) {
-    static const int kIconIDs[] = {
+    static const int kIconIDs[CONTENT_SETTINGS_NUM_TYPES] = {
       IDR_BLOCKED_COOKIES,
       IDR_BLOCKED_IMAGES,
       IDR_BLOCKED_JAVASCRIPT,
@@ -1348,7 +1348,7 @@ LocationBarView::ContentBlockedImageView::ContentBlockedImageView(
   }
   SetImage(icons_[content_type_]);
 
-  static const int kTooltipIDs[] = {
+  static const int kTooltipIDs[CONTENT_SETTINGS_NUM_TYPES] = {
     IDS_BLOCKED_COOKIES_TITLE,
     IDS_BLOCKED_IMAGES_TITLE,
     IDS_BLOCKED_JAVASCRIPT_TITLE,
diff --git a/chrome/browser/views/options/content_filter_page_view.cc b/chrome/browser/views/options/content_filter_page_view.cc
index bb61d7c..4be6291 100644
--- a/chrome/browser/views/options/content_filter_page_view.cc
+++ b/chrome/browser/views/options/content_filter_page_view.cc
@@ -49,7 +49,7 @@ void ContentFilterPageView::InitControlLayout() {
                         GridLayout::USE_PREF, 0, 0);
   layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
 
-  static const int kTitleIDs[] = {
+  static const int kTitleIDs[CONTENT_SETTINGS_NUM_TYPES] = {
     0,  // This dialog isn't used for cookies.
     IDS_IMAGES_SETTING_LABEL,
     IDS_JS_SETTING_LABEL,
@@ -67,7 +67,7 @@ void ContentFilterPageView::InitControlLayout() {
   layout->AddView(title_label);
   layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
 
-  static const int kAllowIDs[] = {
+  static const int kAllowIDs[CONTENT_SETTINGS_NUM_TYPES] = {
     0,  // This dialog isn't used for cookies.
     IDS_IMAGES_LOAD_RADIO,
     IDS_JS_ALLOW_RADIO,
@@ -82,7 +82,7 @@ void ContentFilterPageView::InitControlLayout() {
   allow_radio_->set_listener(this);
   allow_radio_->SetMultiLine(true);
 
-  static const int kBlockIDs[] = {
+  static const int kBlockIDs[CONTENT_SETTINGS_NUM_TYPES] = {
     0,  // This dialog isn't used for cookies.
     IDS_IMAGES_NOLOAD_RADIO,
     IDS_JS_DONOTALLOW_RADIO,