Simplify content settings bubble model "radio groups" to a single radio group.

There's no longer a use case for multiple radio groups (was originally planned for Geolocation), simplify the model and its views.

BUG=39073
TEST=content_setting_bubble_model_unittest

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

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

9 files changed, 71 insertions, 89 deletions

diff --git a/chrome/browser/cocoa/content_blocked_bubble_controller.mm b/chrome/browser/cocoa/content_blocked_bubble_controller.mm
index 0455da1..fd4d979 100644
--- a/chrome/browser/cocoa/content_blocked_bubble_controller.mm
+++ b/chrome/browser/cocoa/content_blocked_bubble_controller.mm
@@ -171,14 +171,12 @@ NSTextField* LabelWithFrame(NSString* text, const NSRect& frame) {
 
 - (void)initializeRadioGroup {
   // Configure the radio group. For now, only deal with the
-  // strictly needed case of 1 radio group (containing 2 radio buttons).
+  // strictly needed case of group containing 2 radio buttons.
   // TODO(joth): Implement the generic case, getting localized strings from the
   // bubble model instead of the xib, or remove it if it's never needed.
   // http://crbug.com/38432
-  const ContentSettingBubbleModel::RadioGroups& radioGroups =
-      contentSettingBubbleModel_->bubble_content().radio_groups;
-  DCHECK_EQ(1u, radioGroups.size()) << "Only one radio group supported";
-  const ContentSettingBubbleModel::RadioGroup& radioGroup = radioGroups.at(0);
+  const ContentSettingBubbleModel::RadioGroup& radioGroup =
+      contentSettingBubbleModel_->bubble_content().radio_group;
 
   // Select appropriate radio button..
   [allowBlockRadioGroup_ selectCellWithTag:
@@ -460,7 +458,7 @@ NSTextField* LabelWithFrame(NSString* text, const NSRect& frame) {
 - (IBAction)allowBlockToggled:(id)sender {
   NSButtonCell *selectedCell = [sender selectedCell];
   contentSettingBubbleModel_->OnRadioClicked(
-      0, [selectedCell tag] == kAllowTag ? 0 : 1);
+      [selectedCell tag] == kAllowTag ? 0 : 1);
 }
 
 - (IBAction)closeBubble:(id)sender {
diff --git a/chrome/browser/cocoa/content_blocked_bubble_controller_unittest.mm b/chrome/browser/cocoa/content_blocked_bubble_controller_unittest.mm
index 2bd8533d..a7b940e 100644
--- a/chrome/browser/cocoa/content_blocked_bubble_controller_unittest.mm
+++ b/chrome/browser/cocoa/content_blocked_bubble_controller_unittest.mm
@@ -21,7 +21,7 @@ class DummyContentSettingBubbleModel : public ContentSettingBubbleModel {
     RadioGroup radio_group;
     radio_group.default_item = 0;
     radio_group.radio_items.resize(2);
-    add_radio_group(radio_group);
+    set_radio_group(radio_group);
   }
 };
 
diff --git a/chrome/browser/content_setting_bubble_model.cc b/chrome/browser/content_setting_bubble_model.cc
index 1318ac0..e385029 100644
--- a/chrome/browser/content_setting_bubble_model.cc
+++ b/chrome/browser/content_setting_bubble_model.cc
@@ -67,11 +67,11 @@ class ContentSettingSingleRadioGroup : public ContentSettingTitleAndLinkModel {
   ContentSettingSingleRadioGroup(TabContents* tab_contents, Profile* profile,
       ContentSettingsType content_type)
       : ContentSettingTitleAndLinkModel(tab_contents, profile, content_type) {
-    SetRadioGroups();
+    SetRadioGroup();
   }
 
  private:
-  void SetRadioGroups() {
+  void SetRadioGroup() {
     GURL url = tab_contents()->GetURL();
     std::wstring display_host_wide;
     net::AppendFormattedHost(url,
@@ -115,13 +115,13 @@ class ContentSettingSingleRadioGroup : public ContentSettingTitleAndLinkModel {
     radio_group.default_item =
         profile()->GetHostContentSettingsMap()->GetContentSetting(url,
             content_type()) == CONTENT_SETTING_ALLOW ? 0 : 1;
-    add_radio_group(radio_group);
+    set_radio_group(radio_group);
   }
 
-  virtual void OnRadioClicked(int radio_group, int radio_index) {
+  virtual void OnRadioClicked(int radio_index) {
     profile()->GetHostContentSettingsMap()->SetContentSetting(
         HostContentSettingsMap::Pattern::FromURL(
-            bubble_content().radio_groups[radio_group].url),
+            bubble_content().radio_group.url),
         content_type(),
         radio_index == 0 ? CONTENT_SETTING_ALLOW : CONTENT_SETTING_BLOCK);
   }
diff --git a/chrome/browser/content_setting_bubble_model.h b/chrome/browser/content_setting_bubble_model.h
index 5c21460..18cf6ac 100644
--- a/chrome/browser/content_setting_bubble_model.h
+++ b/chrome/browser/content_setting_bubble_model.h
@@ -46,7 +46,6 @@ class ContentSettingBubbleModel : public NotificationObserver {
     RadioItems radio_items;
     int default_item;
   };
-  typedef std::vector<RadioGroup> RadioGroups;
 
   struct DomainList {
     std::string title;
@@ -56,7 +55,7 @@ class ContentSettingBubbleModel : public NotificationObserver {
   struct BubbleContent {
     std::string title;
     PopupItems popup_items;
-    RadioGroups radio_groups;
+    RadioGroup radio_group;
     std::vector<DomainList> domain_lists;
     std::string manage_link;
     std::string clear_link;
@@ -69,7 +68,7 @@ class ContentSettingBubbleModel : public NotificationObserver {
                        const NotificationSource& source,
                        const NotificationDetails& details);
 
-  virtual void OnRadioClicked(int radio_group, int radio_index) {}
+  virtual void OnRadioClicked(int radio_index) {}
   virtual void OnPopupClicked(int index) {}
   virtual void OnManageLinkClicked() {}
   virtual void OnClearLinkClicked() {}
@@ -85,8 +84,8 @@ class ContentSettingBubbleModel : public NotificationObserver {
   void add_popup(const PopupItem& popup) {
     bubble_content_.popup_items.push_back(popup);
   }
-  void add_radio_group(const RadioGroup& radio_group) {
-    bubble_content_.radio_groups.push_back(radio_group);
+  void set_radio_group(const RadioGroup& radio_group) {
+    bubble_content_.radio_group = radio_group;
   }
   void add_domain_list(const DomainList& domain_list) {
     bubble_content_.domain_lists.push_back(domain_list);
diff --git a/chrome/browser/content_setting_bubble_model_unittest.cc b/chrome/browser/content_setting_bubble_model_unittest.cc
index d6555014..dbcd2b0 100644
--- a/chrome/browser/content_setting_bubble_model_unittest.cc
+++ b/chrome/browser/content_setting_bubble_model_unittest.cc
@@ -24,8 +24,8 @@ class ContentSettingBubbleModelTest : public RenderViewHostTestHarness {
            contents(), profile_.get(), CONTENT_SETTINGS_TYPE_GEOLOCATION));
     const ContentSettingBubbleModel::BubbleContent& bubble_content =
         content_setting_bubble_model->bubble_content();
+    EXPECT_EQ(0U, bubble_content.radio_group.radio_items.size());
     EXPECT_EQ(0U, bubble_content.popup_items.size());
-    EXPECT_EQ(0U, bubble_content.radio_groups.size());
     // The reload hint is currently implemented as a tacked on domain title, so
     // account for this.
     if (expect_reload_hint)
@@ -51,9 +51,8 @@ TEST_F(ContentSettingBubbleModelTest, ImageRadios) {
          contents(), profile_.get(), CONTENT_SETTINGS_TYPE_IMAGES));
   const ContentSettingBubbleModel::BubbleContent& bubble_content =
       content_setting_bubble_model->bubble_content();
-  EXPECT_EQ(1U, bubble_content.radio_groups.size());
-  EXPECT_EQ(2U, bubble_content.radio_groups[0].radio_items.size());
-  EXPECT_EQ(0, bubble_content.radio_groups[0].default_item);
+  EXPECT_EQ(2U, bubble_content.radio_group.radio_items.size());
+  EXPECT_EQ(0, bubble_content.radio_group.default_item);
   EXPECT_NE(std::string(), bubble_content.manage_link);
   EXPECT_NE(std::string(), bubble_content.title);
 }
@@ -67,7 +66,7 @@ TEST_F(ContentSettingBubbleModelTest, Cookies) {
          contents(), profile_.get(), CONTENT_SETTINGS_TYPE_COOKIES));
   const ContentSettingBubbleModel::BubbleContent& bubble_content =
       content_setting_bubble_model->bubble_content();
-  EXPECT_EQ(0U, bubble_content.radio_groups.size());
+  EXPECT_EQ(0U, bubble_content.radio_group.radio_items.size());
   EXPECT_NE(std::string(), bubble_content.manage_link);
   EXPECT_NE(std::string(), bubble_content.title);
 }
diff --git a/chrome/browser/gtk/content_setting_bubble_gtk.cc b/chrome/browser/gtk/content_setting_bubble_gtk.cc
index 05e170e..48430eb 100644
--- a/chrome/browser/gtk/content_setting_bubble_gtk.cc
+++ b/chrome/browser/gtk/content_setting_bubble_gtk.cc
@@ -120,33 +120,31 @@ void ContentSettingBubbleGtk::BuildBubble() {
 
   if (content_setting_bubble_model_->content_type() !=
       CONTENT_SETTINGS_TYPE_COOKIES) {
-    const ContentSettingBubbleModel::RadioGroups& radio_groups =
-        content.radio_groups;
-    for (ContentSettingBubbleModel::RadioGroups::const_iterator i =
-         radio_groups.begin(); i != radio_groups.end(); ++i) {
-      const ContentSettingBubbleModel::RadioItems& radio_items = i->radio_items;
-      RadioGroupGtk radio_group_gtk;
-      for (ContentSettingBubbleModel::RadioItems::const_iterator j =
-           radio_items.begin(); j != radio_items.end(); ++j) {
-        GtkWidget* radio =
-            radio_group_gtk.empty() ?
-                gtk_radio_button_new_with_label(NULL, j->c_str()) :
-                gtk_radio_button_new_with_label_from_widget(
-                    GTK_RADIO_BUTTON(radio_group_gtk[0]),
-                    j->c_str());
-        gtk_box_pack_start(GTK_BOX(bubble_content), radio, FALSE, FALSE, 0);
-        if (j - radio_items.begin() == i->default_item) {
-          // We must set the default value before we attach the signal handlers
-          // or pain occurs.
-          gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(radio), TRUE);
-        }
-        radio_group_gtk.push_back(radio);
+    const ContentSettingBubbleModel::RadioGroup& radio_group =
+        content.radio_group;
+    for (ContentSettingBubbleModel::RadioItems::const_iterator i =
+         radio_group.radio_items.begin();
+         i != radio_group.radio_items.end(); ++i) {
+      GtkWidget* radio =
+          radio_group_gtk_.empty() ?
+              gtk_radio_button_new_with_label(NULL, i->c_str()) :
+              gtk_radio_button_new_with_label_from_widget(
+                  GTK_RADIO_BUTTON(radio_group_gtk_[0]),
+                  i->c_str());
+      gtk_box_pack_start(GTK_BOX(bubble_content), radio, FALSE, FALSE, 0);
+      if (i - radio_group.radio_items.begin() == radio_group.default_item) {
+        // We must set the default value before we attach the signal handlers
+        // or pain occurs.
+        gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(radio), TRUE);
       }
-      for (std::vector<GtkWidget*>::const_iterator j = radio_group_gtk.begin();
-           j != radio_group_gtk.end(); ++j) {
-        g_signal_connect(*j, "toggled", G_CALLBACK(OnRadioToggled), this);
-      }
-      radio_groups_gtk_.push_back(radio_group_gtk);
+      radio_group_gtk_.push_back(radio);
+    }
+    for (std::vector<GtkWidget*>::const_iterator i = radio_group_gtk_.begin();
+         i != radio_group_gtk_.end(); ++i) {
+      // We can attach signal handlers now that all defaults are set.
+      g_signal_connect(*i, "toggled", G_CALLBACK(OnRadioToggled), this);
+    }
+    if (!radio_group_gtk_.empty()) {
       gtk_box_pack_start(GTK_BOX(bubble_content), gtk_hseparator_new(), FALSE,
                          FALSE, 0);
     }
@@ -247,16 +245,13 @@ void ContentSettingBubbleGtk::OnPopupLinkClicked(
 void ContentSettingBubbleGtk::OnRadioToggled(
     GtkWidget* widget,
     ContentSettingBubbleGtk* bubble) {
-  for (std::vector<RadioGroupGtk>::const_iterator i =
-       bubble->radio_groups_gtk_.begin();
-       i != bubble->radio_groups_gtk_.end(); ++i) {
-    for (RadioGroupGtk::const_iterator j = i->begin(); j != i->end(); j++) {
-      if (widget == *j) {
-        bubble->content_setting_bubble_model_->OnRadioClicked(
-            i - bubble->radio_groups_gtk_.begin(),
-            j - i->begin());
-        return;
-      }
+  for (ContentSettingBubbleGtk::RadioGroupGtk::const_iterator i =
+       bubble->radio_group_gtk_.begin();
+       i != bubble->radio_group_gtk_.end(); ++i) {
+    if (widget == *i) {
+      bubble->content_setting_bubble_model_->OnRadioClicked(
+          i - bubble->radio_group_gtk_.begin());
+      return;
     }
   }
   NOTREACHED() << "unknown radio toggled";
diff --git a/chrome/browser/gtk/content_setting_bubble_gtk.h b/chrome/browser/gtk/content_setting_bubble_gtk.h
index 813f170..a09acab 100644
--- a/chrome/browser/gtk/content_setting_bubble_gtk.h
+++ b/chrome/browser/gtk/content_setting_bubble_gtk.h
@@ -90,7 +90,7 @@ class ContentSettingBubbleGtk : public InfoBubbleGtkDelegate,
   PopupMap popup_icons_;
 
   typedef std::vector<GtkWidget*> RadioGroupGtk;
-  std::vector<RadioGroupGtk> radio_groups_gtk_;
+  RadioGroupGtk radio_group_gtk_;
 };
 
 #endif  // CHROME_BROWSER_GTK_CONTENT_SETTING_BUBBLE_GTK_H_
diff --git a/chrome/browser/views/content_blocked_bubble_contents.cc b/chrome/browser/views/content_blocked_bubble_contents.cc
index f423b04..e64d607 100644
--- a/chrome/browser/views/content_blocked_bubble_contents.cc
+++ b/chrome/browser/views/content_blocked_bubble_contents.cc
@@ -124,14 +124,11 @@ void ContentSettingBubbleContents::ButtonPressed(views::Button* sender,
     return;
   }
 
-  for (std::vector<RadioGroup>::const_iterator i = radio_groups_.begin();
-       i != radio_groups_.end(); ++i) {
-    for (RadioGroup::const_iterator j = i->begin(); j != i->end(); ++j) {
-      if (sender == *j) {
-        content_setting_bubble_model_->OnRadioClicked(
-            i - radio_groups_.begin(), j - i->begin());
-        return;
-      }
+  for (RadioGroup::const_iterator i = radio_group_.begin();
+       i != radio_group_.end(); ++i) {
+    if (sender == *i) {
+      content_setting_bubble_model_->OnRadioClicked(i - radio_group_.begin());
+      return;
     }
   }
   NOTREACHED() << "unknown radio";
@@ -218,33 +215,27 @@ void ContentSettingBubbleContents::InitControlLayout() {
     layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
   }
 
-
-
-
-  const ContentSettingBubbleModel::RadioGroups& radio_groups =
-      bubble_content.radio_groups;
-  for (ContentSettingBubbleModel::RadioGroups::const_iterator i =
-       radio_groups.begin(); i != radio_groups.end(); ++i) {
-    const ContentSettingBubbleModel::RadioItems& radio_items = i->radio_items;
-    RadioGroup radio_group;
-    for (ContentSettingBubbleModel::RadioItems::const_iterator j =
-         radio_items.begin(); j != radio_items.end(); ++j) {
-      views::RadioButton* radio = new views::RadioButton(
-            UTF8ToWide(*j), i - radio_groups.begin());
-      radio->set_listener(this);
-      radio_group.push_back(radio);
-      layout->StartRow(0, single_column_set_id);
-      layout->AddView(radio);
-      layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
-    }
-    radio_groups_.push_back(radio_group);
+  const ContentSettingBubbleModel::RadioGroup& radio_group =
+      bubble_content.radio_group;
+  for (ContentSettingBubbleModel::RadioItems::const_iterator i =
+       radio_group.radio_items.begin();
+       i != radio_group.radio_items.end(); ++i) {
+    views::RadioButton* radio = new views::RadioButton(
+          UTF8ToWide(*i), i - radio_group.radio_items.begin());
+    radio->set_listener(this);
+    radio_group_.push_back(radio);
+    layout->StartRow(0, single_column_set_id);
+    layout->AddView(radio);
+    layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
+  }
+  if (!radio_group_.empty()) {
     views::Separator* separator = new views::Separator;
     layout->StartRow(0, single_column_set_id);
     layout->AddView(separator, 1, 1, GridLayout::FILL, GridLayout::FILL);
     layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
     // Now that the buttons have been added to the view hierarchy, it's safe
     // to call SetChecked() on them.
-    radio_group[i->default_item]->SetChecked(true);
+    radio_group_[radio_group.default_item]->SetChecked(true);
   }
 
   gfx::Font domain_font =
diff --git a/chrome/browser/views/content_blocked_bubble_contents.h b/chrome/browser/views/content_blocked_bubble_contents.h
index 8e4c5810..ad4511c 100644
--- a/chrome/browser/views/content_blocked_bubble_contents.h
+++ b/chrome/browser/views/content_blocked_bubble_contents.h
@@ -89,7 +89,7 @@ class ContentSettingBubbleContents : public views::View,
   // message.
   PopupLinks popup_links_;
   typedef std::vector<views::RadioButton*> RadioGroup;
-  std::vector<RadioGroup> radio_groups_;
+  RadioGroup radio_group_;
   views::NativeButton* close_button_;
   views::Link* manage_link_;
   views::Link* clear_link_;