Update Content Settings Bubbles

chrome/browser/ui/views/content_setting_bubble_contents.cc

Index: chrome/browser/ui/views/content_setting_bubble_contents.cc
diff --git a/chrome/browser/ui/views/content_setting_bubble_contents.cc b/chrome/browser/ui/views/content_setting_bubble_contents.cc
index ff4561cbe822d9d0d8efc54216c52b931936a363..ad73d1b4499564e9a7dd5892808c5e5a54b97b1a 100644
--- a/chrome/browser/ui/views/content_setting_bubble_contents.cc
+++ b/chrome/browser/ui/views/content_setting_bubble_contents.cc
@@ -117,9 +117,7 @@ ContentSettingBubbleContents::ContentSettingBubbleContents(
       info_bubble_(NULL),
       close_button_(NULL),
       manage_link_(NULL),
-      clear_link_(NULL),
-      info_link_(NULL),
-      load_plugins_link_(NULL) {
+      info_link_(NULL) {
   registrar_.Add(this, NotificationType::TAB_CONTENTS_DESTROYED,
                  Source<TabContents>(tab_contents));
 }
@@ -171,24 +169,12 @@ void ContentSettingBubbleContents::LinkActivated(views::Link* source,
     // info bubble, which causes it to close, which deletes us.
     return;
   }
-  if (source == clear_link_) {
-    content_setting_bubble_model_->OnClearLinkClicked();
-    info_bubble_->set_fade_away_on_close(true);
-    info_bubble_->Close();  // CAREFUL: This deletes us.
-    return;
-  }
   if (source == info_link_) {
     content_setting_bubble_model_->OnInfoLinkClicked();
     info_bubble_->set_fade_away_on_close(true);
     info_bubble_->Close();  // CAREFUL: This deletes us.
     return;
   }
-  if (source == load_plugins_link_) {
-    content_setting_bubble_model_->OnLoadPluginsLinkClicked();
-    info_bubble_->set_fade_away_on_close(true);
-    info_bubble_->Close();  // CAREFUL: This deletes us.
-    return;
-  }
 
   PopupLinks::const_iterator i(popup_links_.find(source));
   DCHECK(i != popup_links_.end());
@@ -222,11 +208,11 @@ void ContentSettingBubbleContents::InitControlLayout() {
         bubble_content.title));
     layout->StartRow(0, single_column_set_id);
     layout->AddView(title_label);
-    layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
   }
 
   const std::set<std::string>& plugins = bubble_content.resource_identifiers;
   if (!plugins.empty()) {
+    layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);

[Peter Kasting, 2010/12/07 17:53:00]

Seems like we need to be a little more careful in how we add padding rows. 
Something like this, perhaps:

bool bubble_empty = true;
if (!bubble_content.title.empty()) {
  ...
  layout->AddView(title_label);
  bubble_empty = false;
}

...
if (!plugins.empty()) {
  if (!bubble_empty)
    layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
  for (...) {
    ...
    layout->AddView(new views::Label(name));
    bubble_empty = false;
  }
}

etc., carried throughout the function.

This way, we can add padding rows and separator lines iff they're actually
separating from something.

     for (std::set<std::string>::const_iterator it = plugins.begin();
         it != plugins.end(); ++it) {
       std::wstring name;
@@ -239,7 +225,6 @@ void ContentSettingBubbleContents::InitControlLayout() {
       layout->StartRow(0, single_column_set_id);
       layout->AddView(new views::Label(name));
     }
-    layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
   }
 
   if (content_setting_bubble_model_->content_type() ==
@@ -256,8 +241,7 @@ void ContentSettingBubbleContents::InitControlLayout() {
     for (std::vector<ContentSettingBubbleModel::PopupItem>::const_iterator
          i(bubble_content.popup_items.begin());
          i != bubble_content.popup_items.end(); ++i) {
-      if (i != bubble_content.popup_items.begin())
-        layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
+      layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
       layout->StartRow(0, popup_column_set_id);
 
       views::Link* link = new views::Link(UTF8ToWide(i->title));
@@ -267,31 +251,22 @@ void ContentSettingBubbleContents::InitControlLayout() {
       layout->AddView(new Favicon((*i).bitmap, this, link));
       layout->AddView(link);
     }
-    layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
-
-    views::Separator* separator = new views::Separator;
-    layout->StartRow(0, single_column_set_id);
-    layout->AddView(separator);
-    layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
   }
 
   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), 0);
-    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);
+  if (!radio_group.radio_items.empty()) {
+    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), 0);
+      radio->set_listener(this);
+      radio_group_.push_back(radio);
+      layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
+      layout->StartRow(0, single_column_set_id);
+      layout->AddView(radio);
+    }
+    DCHECK(!radio_group_.empty());
     // Now that the buttons have been added to the view hierarchy, it's safe
     // to call SetChecked() on them.
     radio_group_[radio_group.default_item]->SetChecked(true);
@@ -319,49 +294,22 @@ void ContentSettingBubbleContents::InitControlLayout() {
       layout->StartRow(0, indented_single_column_set_id);
       layout->AddView(new views::Label(UTF8ToWide(*j), domain_font));
     }
-    layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
-  }
-
-  if (!bubble_content.clear_link.empty()) {
-    clear_link_ = new views::Link(UTF8ToWide(bubble_content.clear_link));
-    clear_link_->SetController(this);
-    layout->StartRow(0, single_column_set_id);
-    layout->AddView(clear_link_);
-
-    layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
-    layout->StartRow(0, single_column_set_id);
-    layout->AddView(new views::Separator, 1, 1,
-                    GridLayout::FILL, GridLayout::FILL);
-    layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
   }
 
   if (!bubble_content.info_link.empty()) {
     info_link_ = new views::Link(UTF8ToWide(bubble_content.info_link));
+    info_link_->SetEnabled(bubble_content.info_link_enabled);
     info_link_->SetController(this);
-    layout->StartRow(0, single_column_set_id);
-    layout->AddView(info_link_);
-
     layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
     layout->StartRow(0, single_column_set_id);
-    layout->AddView(new views::Separator, 1, 1,
-                    GridLayout::FILL, GridLayout::FILL);
-    layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
+    layout->AddView(info_link_);
   }
 
-  if (!bubble_content.load_plugins_link_title.empty()) {
-    load_plugins_link_ = new views::Link(
-        UTF8ToWide(bubble_content.load_plugins_link_title));
-    load_plugins_link_->SetEnabled(bubble_content.load_plugins_link_enabled);
-    load_plugins_link_->SetController(this);
-    layout->StartRow(0, single_column_set_id);
-    layout->AddView(load_plugins_link_);
-
-    layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
-    layout->StartRow(0, single_column_set_id);
-    layout->AddView(new views::Separator, 1, 1,
-                    GridLayout::FILL, GridLayout::FILL);
-    layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
-  }
+  layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
+  layout->StartRow(0, single_column_set_id);
+  layout->AddView(new views::Separator, 1, 1,
+                  GridLayout::FILL, GridLayout::FILL);
+  layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
 
   const int double_column_set_id = 1;
   views::ColumnSet* double_column_set =