Update Content Settings Bubbles

chrome/browser/content_setting_bubble_model.cc

 // Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/content_setting_bubble_model.h"
 
 #include "app/l10n_util.h"
 #include "base/command_line.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/blocked_content_container.h"
@@ skipping 116 matching lines @@
       : ContentSettingTitleAndLinkModel(tab_contents, profile, content_type) {
     SetInfoLink();
   }
 
  private:
   void SetInfoLink() {
     static const int kInfoIDs[] = {
       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.
+      IDS_BLOCKED_PLUGINS_LOAD_ALL,
       0,  // Popups do not have an info link.
-      0,  // Geolocation does not have an info link.
+      0,  // Geolocation links are set within that class.
       0,  // Notifications do not have a bubble.
     };
     COMPILE_ASSERT(arraysize(kInfoIDs) == 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()]));
   }
 
   virtual void OnInfoLinkClicked() {
     DCHECK(content_type() == CONTENT_SETTINGS_TYPE_COOKIES);

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

Nit: Now that we've combined all these, it seems wrong to me for the base class
to handle the cookie case and overrides the others.  I would prefer to also make
the cookie case be a lightweight subclass that overrides this function.

     if (tab_contents()) {
       NotificationService::current()->Notify(
           NotificationType::COLLECTED_COOKIES_SHOWN,
           Source<TabSpecificContentSettings>(
               tab_contents()->GetTabSpecificContentSettings()),
           NotificationService::NoDetails());
       tab_contents()->delegate()->ShowCollectedCookiesDialog(tab_contents());
     }
   }
 };
 
 
-class ContentSettingSingleRadioGroup : public ContentSettingTitleAndLinkModel {
+class ContentSettingSingleRadioGroup
+    : public ContentSettingTitleLinkAndInfoModel {
  public:
   ContentSettingSingleRadioGroup(TabContents* tab_contents, Profile* profile,
       ContentSettingsType content_type)
-      : ContentSettingTitleAndLinkModel(tab_contents, profile, content_type),
+      : ContentSettingTitleLinkAndInfoModel(tab_contents,
+                                            profile,
+                                            content_type),
         block_setting_(CONTENT_SETTING_BLOCK) {
     SetRadioGroup();
   }
 
  private:
   ContentSetting block_setting_;
 
   // Initialize the radio group by setting the appropriate labels for the
   // content type and setting the default value based on the content setting.
   void SetRadioGroup() {
@@ skipping 108 matching lines @@
     }
   }
 };
 
 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 OnInfoLinkClicked() OVERRIDE {
     UserMetrics::RecordAction(UserMetricsAction("ClickToPlay_LoadAll_Bubble"));
     if (tab_contents()) {
       tab_contents()->render_view_host()->LoadBlockedPlugins();
     }
-    set_load_plugins_link_enabled(false);
+    set_info_link_enabled(false);
     TabSpecificContentSettings* settings =
         tab_contents()->GetTabSpecificContentSettings();
     settings->set_load_plugins_link_enabled(false);

[msw, 2010/12/07 03:28:58]

If we want, I can rename
TabSpecificContentSettings::set_load_plugins_link_enabled()

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

No, I think that'd be wrong.  From that class' perspective the link to load
plugins is distinct from the links that do other things, so we'd want the
concepts to remain separate.

The only change that would make sense to me is to convert from a bool to a
bitfield, e.g. "info_link_enabled = CONTENT_SETTINGS_TYPE_PLUGINS |
CONTENT_SETTINGS_TYPE_GEOLOCATION".  But I'm not sure that buys much in today's
world.

   }
 };
 
 class ContentSettingPopupBubbleModel : public ContentSettingSingleRadioGroup {
  public:
   ContentSettingPopupBubbleModel(TabContents* tab_contents, Profile* profile,
       ContentSettingsType content_type)
       : ContentSettingSingleRadioGroup(tab_contents, profile, content_type) {
     SetPopups();
   }
@@ skipping 20 matching lines @@
       add_popup(popup_item);
     }
   }
 
   virtual void OnPopupClicked(int index) {
     if (tab_contents() && tab_contents()->blocked_content_container()) {
       tab_contents()->blocked_content_container()->LaunchForContents(
           bubble_content().popup_items[index].tab_contents);
     }
   }
+  virtual void OnInfoLinkClicked() OVERRIDE {}
 };
 
 class ContentSettingDomainListBubbleModel
     : public ContentSettingTitleAndLinkModel {
  public:
   ContentSettingDomainListBubbleModel(TabContents* tab_contents,
                                       Profile* profile,
                                       ContentSettingsType content_type)
       : ContentSettingTitleAndLinkModel(tab_contents, profile, content_type) {
     DCHECK_EQ(CONTENT_SETTINGS_TYPE_GEOLOCATION, content_type) <<
         "SetDomains currently only supports geolocation content type";
-    SetDomainsAndClearLink();
+    SetDomainsAndInfoLink();
   }
 
  private:
   void MaybeAddDomainList(const std::set<std::string>& hosts, int title_id) {
     if (!hosts.empty()) {
       DomainList domain_list;
       domain_list.title = l10n_util::GetStringUTF8(title_id);
       domain_list.hosts = hosts;
       add_domain_list(domain_list);
     }
   }
-  void SetDomainsAndClearLink() {
+  void SetDomainsAndInfoLink() {
     TabSpecificContentSettings* content_settings =
         tab_contents()->GetTabSpecificContentSettings();
     const GeolocationSettingsState& settings =
         content_settings->geolocation_settings_state();
     GeolocationSettingsState::FormattedHostsPerState formatted_hosts_per_state;
     unsigned int tab_state_flags = 0;
     settings.GetDetailedInfo(&formatted_hosts_per_state, &tab_state_flags);
     // Divide the tab's current geolocation users into sets according to their
     // permission state.
     MaybeAddDomainList(formatted_hosts_per_state[CONTENT_SETTING_ALLOW],
                        IDS_GEOLOCATION_BUBBLE_SECTION_ALLOWED);
 
     MaybeAddDomainList(formatted_hosts_per_state[CONTENT_SETTING_BLOCK],
                        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_info_link(l10n_util::GetStringUTF8(
+                    IDS_GEOLOCATION_BUBBLE_CLEAR_LINK));
+      set_info_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_info_link(l10n_util::GetStringUTF8(
+                    IDS_GEOLOCATION_BUBBLE_REQUIRE_RELOAD_TO_CLEAR));
+      set_info_link_enabled(false);
+    } else {
+      set_info_link(std::string());
+      set_info_link_enabled(false);
     }
   }
-  virtual void OnClearLinkClicked() {
+  virtual void OnInfoLinkClicked() OVERRIDE {
     if (!tab_contents())
       return;
     // Reset this embedder's entry to default for each of the requesting
     // origins currently on the page.
     const GURL& embedder_url = tab_contents()->GetURL();
     TabSpecificContentSettings* content_settings =
         tab_contents()->GetTabSpecificContentSettings();
     const GeolocationSettingsState::StateMap& state_map =
         content_settings->geolocation_settings_state().state_map();
     GeolocationContentSettingsMap* settings_map =
@@ skipping 33 matching lines @@
 }
 
 ContentSettingBubbleModel::ContentSettingBubbleModel(
     TabContents* tab_contents, Profile* profile,
     ContentSettingsType content_type)
     : tab_contents_(tab_contents), profile_(profile),
       content_type_(content_type) {
   if (tab_contents) {
     TabSpecificContentSettings* settings =
         tab_contents->GetTabSpecificContentSettings();
-    set_load_plugins_link_enabled(settings->load_plugins_link_enabled());
+    set_info_link_enabled(settings->load_plugins_link_enabled());
   } else {
-    set_load_plugins_link_enabled(true);
+    set_info_link_enabled(true);
   }
   registrar_.Add(this, NotificationType::TAB_CONTENTS_DESTROYED,
                  Source<TabContents>(tab_contents));
 }
 
 ContentSettingBubbleModel::~ContentSettingBubbleModel() {
 }
 
 ContentSettingBubbleModel::RadioGroup::RadioGroup() : default_item(0) {}
 
 ContentSettingBubbleModel::RadioGroup::~RadioGroup() {}
 
 ContentSettingBubbleModel::DomainList::DomainList() {}
 
 ContentSettingBubbleModel::DomainList::~DomainList() {}
 
 ContentSettingBubbleModel::BubbleContent::BubbleContent()
-    : load_plugins_link_enabled(false) {
+    : info_link_enabled(false) {
 }
 
 ContentSettingBubbleModel::BubbleContent::~BubbleContent() {}
 
 
 void ContentSettingBubbleModel::AddBlockedResource(
     const std::string& resource_identifier) {
   bubble_content_.resource_identifiers.insert(resource_identifier);
 }
 
 void ContentSettingBubbleModel::Observe(NotificationType type,
                                         const NotificationSource& source,
                                         const NotificationDetails& details) {
   DCHECK(type == NotificationType::TAB_CONTENTS_DESTROYED);
   DCHECK(source == Source<TabContents>(tab_contents_));
   tab_contents_ = NULL;
 }