Update Content Settings Bubbles

chrome/browser/content_setting_bubble_model.cc

Index: chrome/browser/content_setting_bubble_model.cc
diff --git a/chrome/browser/content_setting_bubble_model.cc b/chrome/browser/content_setting_bubble_model.cc
index 3f9cbb7a8369779d52aec82de4c219aa6bab10a1..2907311c6560e83ba3c22d17ec548df6bcbc7f2b 100644
--- a/chrome/browser/content_setting_bubble_model.cc
+++ b/chrome/browser/content_setting_bubble_model.cc
@@ -118,52 +118,44 @@ class ContentSettingTitleAndLinkModel : public ContentSettingBubbleModel {
   }
 };
 
-class ContentSettingTitleLinkAndInfoModel
+class ContentSettingTitleLinkAndCustomModel
     : public ContentSettingTitleAndLinkModel {
  public:
-  ContentSettingTitleLinkAndInfoModel(TabContents* tab_contents,
-                                      Profile* profile,
-                                      ContentSettingsType content_type)
+  ContentSettingTitleLinkAndCustomModel(TabContents* tab_contents,
+                                        Profile* profile,
+                                        ContentSettingsType content_type)
       : ContentSettingTitleAndLinkModel(tab_contents, profile, content_type) {
-    SetInfoLink();
+    SetCustomLink();
   }
 
  private:
-  void SetInfoLink() {
-    static const int kInfoIDs[] = {
+  void SetCustomLink() {
+    static const int kCustomIDs[] = {
       IDS_BLOCKED_COOKIES_INFO,
-      0,  // Images do not have an info link.
-      0,  // Javascript doesn't have an info link.
-      0,  // Plugins do not have an info link.
-      0,  // Popups do not have an info link.
-      0,  // Geolocation does not have an info link.
+      0,  // Images do not have a custom link.
+      0,  // Javascript doesn't have a custom link.
+      IDS_BLOCKED_PLUGINS_LOAD_ALL,
+      0,  // Popups do not have a custom link.
+      0,  // Geolocation custom links are set within that class.
       0,  // Notifications do not have a bubble.
     };
-    COMPILE_ASSERT(arraysize(kInfoIDs) == CONTENT_SETTINGS_NUM_TYPES,
+    COMPILE_ASSERT(arraysize(kCustomIDs) == CONTENT_SETTINGS_NUM_TYPES,
                    Need_a_setting_for_every_content_settings_type);
-    if (kInfoIDs[content_type()])
-      set_info_link(l10n_util::GetStringUTF8(kInfoIDs[content_type()]));
+    if (kCustomIDs[content_type()])
+      set_custom_link(l10n_util::GetStringUTF8(kCustomIDs[content_type()]));
   }
 
-  virtual void OnInfoLinkClicked() {
-    DCHECK(content_type() == CONTENT_SETTINGS_TYPE_COOKIES);
-    if (tab_contents()) {
-      NotificationService::current()->Notify(
-          NotificationType::COLLECTED_COOKIES_SHOWN,
-          Source<TabSpecificContentSettings>(
-              tab_contents()->GetTabSpecificContentSettings()),
-          NotificationService::NoDetails());
-      tab_contents()->delegate()->ShowCollectedCookiesDialog(tab_contents());
-    }
-  }
+  virtual void OnCustomLinkClicked() {}
 };
 
 
-class ContentSettingSingleRadioGroup : public ContentSettingTitleAndLinkModel {
+class ContentSettingSingleRadioGroup
+    : public ContentSettingTitleLinkAndCustomModel {
  public:
   ContentSettingSingleRadioGroup(TabContents* tab_contents, Profile* profile,

[Peter Kasting, 2010/12/09 17:56:34]

Nit: While you're here, can you fix the parameter wrapping to be one per line a
la the ContentSettingTitleLinkAndCustomModel above?

       ContentSettingsType content_type)
-      : ContentSettingTitleAndLinkModel(tab_contents, profile, content_type),
+      : ContentSettingTitleLinkAndCustomModel(tab_contents, profile,
+                                              content_type),
         block_setting_(CONTENT_SETTING_BLOCK) {
     SetRadioGroup();
   }
@@ -286,27 +278,44 @@ class ContentSettingSingleRadioGroup : public ContentSettingTitleAndLinkModel {
   }
 };
 
+class ContentSettingCookiesBubbleModel
+    : public ContentSettingTitleLinkAndCustomModel {
+ public:
+  ContentSettingCookiesBubbleModel(TabContents* tab_contents, Profile* profile,

[Peter Kasting, 2010/12/09 17:56:34]

Nit: |profile| goes on its own line.

+                                   ContentSettingsType content_type)
+      : ContentSettingTitleLinkAndCustomModel(tab_contents, profile,
+                                              content_type) {
+    DCHECK_EQ(content_type, CONTENT_SETTINGS_TYPE_COOKIES);

[Peter Kasting, 2010/12/09 17:56:34]

Nit: DCHECK_EQ(EXPECTED, ACTUAL);

+  }
+
+ private:
+  virtual void OnCustomLinkClicked() OVERRIDE {
+    if (tab_contents()) {
+      NotificationService::current()->Notify(
+          NotificationType::COLLECTED_COOKIES_SHOWN,
+          Source<TabSpecificContentSettings>(
+              tab_contents()->GetTabSpecificContentSettings()),
+          NotificationService::NoDetails());
+      tab_contents()->delegate()->ShowCollectedCookiesDialog(tab_contents());
+    }
+  }
+};
+
 class ContentSettingPluginBubbleModel : public ContentSettingSingleRadioGroup {
  public:
   ContentSettingPluginBubbleModel(TabContents* tab_contents, Profile* profile,
                                   ContentSettingsType content_type)
       : ContentSettingSingleRadioGroup(tab_contents, profile, content_type) {
     DCHECK_EQ(content_type, CONTENT_SETTINGS_TYPE_PLUGINS);
-    SetLoadPluginsLinkTitle();
   }
 
  private:
-  void SetLoadPluginsLinkTitle() {
-    set_load_plugins_link_title(
-        l10n_util::GetStringUTF8(IDS_BLOCKED_PLUGINS_LOAD_ALL));
-   }
-
-  virtual void OnLoadPluginsLinkClicked() {
+  virtual void OnCustomLinkClicked() OVERRIDE {
     UserMetrics::RecordAction(UserMetricsAction("ClickToPlay_LoadAll_Bubble"));
     if (tab_contents()) {

[Peter Kasting, 2010/12/09 17:56:34]

Nit: While here, remove extra {}

       tab_contents()->render_view_host()->LoadBlockedPlugins();
     }
-    set_load_plugins_link_enabled(false);
+    set_custom_link_enabled(false);
     TabSpecificContentSettings* settings =
         tab_contents()->GetTabSpecificContentSettings();
     settings->set_load_plugins_link_enabled(false);
@@ -350,6 +359,7 @@ class ContentSettingPopupBubbleModel : public ContentSettingSingleRadioGroup {
           bubble_content().popup_items[index].tab_contents);
     }
   }
+  virtual void OnCustomLinkClicked() OVERRIDE {}

[Peter Kasting, 2010/12/09 17:56:34]

Nit: No need for this since the base class provides an empty body.

 };
 
 class ContentSettingDomainListBubbleModel
@@ -361,7 +371,7 @@ class ContentSettingDomainListBubbleModel
       : ContentSettingTitleAndLinkModel(tab_contents, profile, content_type) {
     DCHECK_EQ(CONTENT_SETTINGS_TYPE_GEOLOCATION, content_type) <<
         "SetDomains currently only supports geolocation content type";
-    SetDomainsAndClearLink();
+    SetDomainsAndCustomLink();
   }
 
  private:
@@ -373,7 +383,7 @@ class ContentSettingDomainListBubbleModel
       add_domain_list(domain_list);
     }
   }
-  void SetDomainsAndClearLink() {
+  void SetDomainsAndCustomLink() {
     TabSpecificContentSettings* content_settings =
         tab_contents()->GetTabSpecificContentSettings();
     const GeolocationSettingsState& settings =
@@ -390,21 +400,20 @@ class ContentSettingDomainListBubbleModel
                        IDS_GEOLOCATION_BUBBLE_SECTION_DENIED);
 
     if (tab_state_flags & GeolocationSettingsState::TABSTATE_HAS_EXCEPTION) {
-      set_clear_link(
-          l10n_util::GetStringUTF8(IDS_GEOLOCATION_BUBBLE_CLEAR_LINK));
+      set_custom_link(l10n_util::GetStringUTF8(
+                      IDS_GEOLOCATION_BUBBLE_CLEAR_LINK));
+      set_custom_link_enabled(true);
     } else if (tab_state_flags &
                GeolocationSettingsState::TABSTATE_HAS_CHANGED) {
-      // It is a slight abuse of the domain list field to use it for the reload
-      // hint, but works fine for now. TODO(joth): If we need to style it
-      // differently, consider adding an explicit field, or generalize the
-      // domain list to be a flat list of style formatted lines.
-      DomainList reload_section;
-      reload_section.title = l10n_util::GetStringUTF8(
-          IDS_GEOLOCATION_BUBBLE_REQUIRE_RELOAD_TO_CLEAR);
-      add_domain_list(reload_section);
+      set_custom_link(l10n_util::GetStringUTF8(
+                      IDS_GEOLOCATION_BUBBLE_REQUIRE_RELOAD_TO_CLEAR));
+      set_custom_link_enabled(false);

[Peter Kasting, 2010/12/09 17:56:34]

Nit: You can remove this line and the conditional arm below it, since a disabled
empty link is the default constructed state.

+    } else {
+      set_custom_link(std::string());
+      set_custom_link_enabled(false);
     }
   }
-  virtual void OnClearLinkClicked() {
+  virtual void OnCustomLinkClicked() OVERRIDE {
     if (!tab_contents())
       return;
     // Reset this embedder's entry to default for each of the requesting
@@ -431,8 +440,8 @@ ContentSettingBubbleModel*
         Profile* profile,
         ContentSettingsType content_type) {
   if (content_type == CONTENT_SETTINGS_TYPE_COOKIES) {
-    return new ContentSettingTitleLinkAndInfoModel(tab_contents, profile,
-                                                   content_type);
+    return new ContentSettingCookiesBubbleModel(tab_contents, profile,
+                                                content_type);
   }
   if (content_type == CONTENT_SETTINGS_TYPE_POPUPS) {
     return new ContentSettingPopupBubbleModel(tab_contents, profile,
@@ -458,9 +467,9 @@ ContentSettingBubbleModel::ContentSettingBubbleModel(
   if (tab_contents) {

[Peter Kasting, 2010/12/09 17:56:34]

Nit: This whole conditional could be reduced to just:

  set_custom_link_enabled(!tab_contents || tab_contents->
      GetTabSpecificContentSettings()->load_plugins_link_enabled());

     TabSpecificContentSettings* settings =
         tab_contents->GetTabSpecificContentSettings();
-    set_load_plugins_link_enabled(settings->load_plugins_link_enabled());
+    set_custom_link_enabled(settings->load_plugins_link_enabled());
   } else {
-    set_load_plugins_link_enabled(true);
+    set_custom_link_enabled(true);
   }
   registrar_.Add(this, NotificationType::TAB_CONTENTS_DESTROYED,
                  Source<TabContents>(tab_contents));
@@ -478,7 +487,7 @@ ContentSettingBubbleModel::DomainList::DomainList() {}
 ContentSettingBubbleModel::DomainList::~DomainList() {}
 
 ContentSettingBubbleModel::BubbleContent::BubbleContent()
-    : load_plugins_link_enabled(false) {
+    : custom_link_enabled(false) {
 }
 
 ContentSettingBubbleModel::BubbleContent::~BubbleContent() {}