Only commit changes from content settings bubbles on their dismissal

BUG=48484
TEST=as described in the bug

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

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

2 files changed, 39 insertions, 18 deletions

diff --git a/chrome/browser/content_setting_bubble_model.cc b/chrome/browser/content_setting_bubble_model.cc
index c6f7a8e..c2f058c 100644
--- a/chrome/browser/content_setting_bubble_model.cc
+++ b/chrome/browser/content_setting_bubble_model.cc
@@ -35,6 +35,8 @@ class ContentSettingTitleAndLinkModel : public ContentSettingBubbleModel {
      SetManageLink();
   }
 
+  virtual ~ContentSettingTitleAndLinkModel() {}
+
  private:
   void SetBlockedResources() {
     TabSpecificContentSettings* settings =
@@ -128,6 +130,8 @@ class ContentSettingTitleLinkAndCustomModel
     SetCustomLink();
   }
 
+  virtual ~ContentSettingTitleLinkAndCustomModel() {}
+
  private:
   void SetCustomLink() {
     static const int kCustomIDs[] = {
@@ -157,12 +161,31 @@ class ContentSettingSingleRadioGroup
                                  ContentSettingsType content_type)
       : ContentSettingTitleLinkAndCustomModel(tab_contents, profile,
                                               content_type),
-        block_setting_(CONTENT_SETTING_BLOCK) {
+        block_setting_(CONTENT_SETTING_BLOCK),
+        selected_item_(0) {
     SetRadioGroup();
   }
 
+  virtual ~ContentSettingSingleRadioGroup() {
+    if (selected_item_ != bubble_content().radio_group.default_item) {
+      ContentSetting setting =
+          selected_item_ == 0 ? CONTENT_SETTING_ALLOW : block_setting_;
+      const std::set<std::string>& resources =
+          bubble_content().resource_identifiers;
+      if (resources.empty()) {
+        AddException(setting, std::string());
+      } else {
+        for (std::set<std::string>::const_iterator it = resources.begin();
+             it != resources.end(); ++it) {
+          AddException(setting, *it);
+        }
+      }
+    }
+  }
+
  private:
   ContentSetting block_setting_;
+  int selected_item_;
 
   // Initialize the radio group by setting the appropriate labels for the
   // content type and setting the default value based on the content setting.
@@ -253,6 +276,7 @@ class ContentSettingSingleRadioGroup
       radio_group.default_item = 1;
       block_setting_ = mostRestrictiveSetting;
     }
+    selected_item_ = radio_group.default_item;
     set_radio_group(radio_group);
   }
 
@@ -264,18 +288,7 @@ class ContentSettingSingleRadioGroup
   }
 
   virtual void OnRadioClicked(int radio_index) {
-    ContentSetting setting =
-        radio_index == 0 ? CONTENT_SETTING_ALLOW : block_setting_;
-    const std::set<std::string>& resources =
-        bubble_content().resource_identifiers;
-    if (resources.empty()) {
-      AddException(setting, std::string());
-    } else {
-      for (std::set<std::string>::const_iterator it = resources.begin();
-           it != resources.end(); ++it) {
-        AddException(setting, *it);
-      }
-    }
+    selected_item_ = radio_index;
   }
 };
 
@@ -291,6 +304,8 @@ class ContentSettingCookiesBubbleModel
     set_custom_link_enabled(true);
   }
 
+  virtual ~ContentSettingCookiesBubbleModel() {}
+
  private:
   virtual void OnCustomLinkClicked() OVERRIDE {
     if (tab_contents()) {
@@ -315,6 +330,8 @@ class ContentSettingPluginBubbleModel : public ContentSettingSingleRadioGroup {
         GetTabSpecificContentSettings()->load_plugins_link_enabled());
   }
 
+  virtual ~ContentSettingPluginBubbleModel() {}
+
  private:
   virtual void OnCustomLinkClicked() OVERRIDE {
     UserMetrics::RecordAction(UserMetricsAction("ClickToPlay_LoadAll_Bubble"));
@@ -335,6 +352,8 @@ class ContentSettingPopupBubbleModel : public ContentSettingSingleRadioGroup {
     SetPopups();
   }
 
+  virtual ~ContentSettingPopupBubbleModel() {}
+
  private:
   void SetPopups() {
     // check for crbug.com/53176
@@ -378,6 +397,8 @@ class ContentSettingDomainListBubbleModel
     SetDomainsAndCustomLink();
   }
 
+  virtual ~ContentSettingDomainListBubbleModel() {}
+
  private:
   void MaybeAddDomainList(const std::set<std::string>& hosts, int title_id) {
     if (!hosts.empty()) {
diff --git a/chrome/browser/content_setting_bubble_model_unittest.cc b/chrome/browser/content_setting_bubble_model_unittest.cc
index d2eebde..00f38c8 100644
--- a/chrome/browser/content_setting_bubble_model_unittest.cc
+++ b/chrome/browser/content_setting_bubble_model_unittest.cc
@@ -132,23 +132,23 @@ TEST_F(ContentSettingBubbleModelTest, MultiplePlugins) {
   EXPECT_EQ(1, bubble_content.radio_group.default_item);
 
   content_setting_bubble_model->OnRadioClicked(0);
-  // Both plug-ins should be allowed now.
+  // Nothing should have changed.
   EXPECT_EQ(CONTENT_SETTING_ALLOW,
             map->GetContentSetting(url,
                                    CONTENT_SETTINGS_TYPE_PLUGINS,
                                    fooPlugin));
-  EXPECT_EQ(CONTENT_SETTING_ALLOW,
+  EXPECT_EQ(CONTENT_SETTING_ASK,
             map->GetContentSetting(url,
                                    CONTENT_SETTINGS_TYPE_PLUGINS,
                                    barPlugin));
 
-  content_setting_bubble_model->OnRadioClicked(1);
+  content_setting_bubble_model.reset();
   // Both plug-ins should be click-to-play now.
-  EXPECT_EQ(CONTENT_SETTING_ASK,
+  EXPECT_EQ(CONTENT_SETTING_ALLOW,
             map->GetContentSetting(url,
                                    CONTENT_SETTINGS_TYPE_PLUGINS,
                                    fooPlugin));
-  EXPECT_EQ(CONTENT_SETTING_ASK,
+  EXPECT_EQ(CONTENT_SETTING_ALLOW,
             map->GetContentSetting(url,
                                    CONTENT_SETTINGS_TYPE_PLUGINS,
                                    barPlugin));