Updated Plugin bubble model to add "learn more" link and to appear with a sliding yellow thing

chrome/browser/ui/content_settings/content_setting_bubble_model.cc

Index: chrome/browser/ui/content_settings/content_setting_bubble_model.cc
diff --git a/chrome/browser/ui/content_settings/content_setting_bubble_model.cc b/chrome/browser/ui/content_settings/content_setting_bubble_model.cc
index 2acc49698b1cf1f11cbe5ec009093b2ccdbdab24..b4b66ba1b7208ed051178f2e4529bc38e2f58f70 100644
--- a/chrome/browser/ui/content_settings/content_setting_bubble_model.cc
+++ b/chrome/browser/ui/content_settings/content_setting_bubble_model.cc
@@ -95,6 +95,7 @@ ContentSettingTitleAndLinkModel::ContentSettingTitleAndLinkModel(
   DCHECK_NE(content_type, CONTENT_SETTINGS_TYPE_NOTIFICATIONS);
   SetTitle();
   SetManageLink();
+  SetLearnMoreLink();
 }
 
 void ContentSettingTitleAndLinkModel::SetTitle() {
@@ -156,6 +157,22 @@ void ContentSettingTitleAndLinkModel::OnManageLinkClicked() {
     delegate_->ShowContentSettingsPage(content_type());
 }
 
+void ContentSettingTitleAndLinkModel::SetLearnMoreLink() {
+  static const ContentSettingsTypeIdEntry kLearnMoreIDs[] = {
+    {CONTENT_SETTINGS_TYPE_PLUGINS, IDS_LEARN_MORE},
+  };
+  int learn_more_id =
+      GetIdForContentType(kLearnMoreIDs, arraysize(kLearnMoreIDs),
+                          content_type());
+  if (learn_more_id)
+    set_learn_more_link(l10n_util::GetStringUTF8(learn_more_id));
+}
+
+void ContentSettingTitleAndLinkModel::OnLearnMoreLinkClicked() {
+  if (delegate_)
+    delegate_->ShowLearnMorePage(content_type());
+}
+
 class ContentSettingTitleLinkAndCustomModel
     : public ContentSettingTitleAndLinkModel {
  public:
@@ -183,7 +200,6 @@ ContentSettingTitleLinkAndCustomModel::ContentSettingTitleLinkAndCustomModel(
 void ContentSettingTitleLinkAndCustomModel::SetCustomLink() {
   static const ContentSettingsTypeIdEntry kCustomIDs[] = {
     {CONTENT_SETTINGS_TYPE_COOKIES, IDS_BLOCKED_COOKIES_INFO},
-    {CONTENT_SETTINGS_TYPE_PLUGINS, IDS_BLOCKED_PLUGINS_LOAD_ALL},
     {CONTENT_SETTINGS_TYPE_MIXEDSCRIPT, IDS_ALLOW_INSECURE_CONTENT_BUTTON},
   };
   int custom_link_id =
@@ -267,7 +283,6 @@ void ContentSettingSingleRadioGroup::SetRadioGroup() {
     {CONTENT_SETTINGS_TYPE_COOKIES, IDS_BLOCKED_COOKIES_UNBLOCK},
     {CONTENT_SETTINGS_TYPE_IMAGES, IDS_BLOCKED_IMAGES_UNBLOCK},
     {CONTENT_SETTINGS_TYPE_JAVASCRIPT, IDS_BLOCKED_JAVASCRIPT_UNBLOCK},
-    {CONTENT_SETTINGS_TYPE_PLUGINS, IDS_BLOCKED_PLUGINS_UNBLOCK_ALL},
     {CONTENT_SETTINGS_TYPE_POPUPS, IDS_BLOCKED_POPUPS_UNBLOCK},
     {CONTENT_SETTINGS_TYPE_PPAPI_BROKER, IDS_BLOCKED_PPAPI_BROKER_UNBLOCK},
     {CONTENT_SETTINGS_TYPE_AUTOMATIC_DOWNLOADS, IDS_BLOCKED_DOWNLOAD_UNBLOCK},
@@ -299,7 +314,6 @@ void ContentSettingSingleRadioGroup::SetRadioGroup() {
     {CONTENT_SETTINGS_TYPE_COOKIES, IDS_BLOCKED_COOKIES_NO_ACTION},
     {CONTENT_SETTINGS_TYPE_IMAGES, IDS_BLOCKED_IMAGES_NO_ACTION},
     {CONTENT_SETTINGS_TYPE_JAVASCRIPT, IDS_BLOCKED_JAVASCRIPT_NO_ACTION},
-    {CONTENT_SETTINGS_TYPE_PLUGINS, IDS_BLOCKED_PLUGINS_NO_ACTION},
     {CONTENT_SETTINGS_TYPE_POPUPS, IDS_BLOCKED_POPUPS_NO_ACTION},
     {CONTENT_SETTINGS_TYPE_PPAPI_BROKER, IDS_BLOCKED_PPAPI_BROKER_NO_ACTION},
     {CONTENT_SETTINGS_TYPE_AUTOMATIC_DOWNLOADS, IDS_BLOCKED_DOWNLOAD_NO_ACTION},
@@ -329,7 +343,6 @@ void ContentSettingSingleRadioGroup::SetRadioGroup() {
   radio_group.radio_items.push_back(radio_block_label);
   ContentSetting setting;
   SettingSource setting_source = SETTING_SOURCE_NONE;
-  bool setting_is_wildcard = false;
 
   if (content_type() == CONTENT_SETTINGS_TYPE_COOKIES) {
     CookieSettings* cookie_settings =
@@ -343,19 +356,9 @@ void ContentSettingSingleRadioGroup::SetRadioGroup() {
         url, url, content_type(), std::string(), &info));
     setting = content_settings::ValueToContentSetting(value.get());
     setting_source = info.source;
-    setting_is_wildcard =
-        info.primary_pattern == ContentSettingsPattern::Wildcard() &&
-        info.secondary_pattern == ContentSettingsPattern::Wildcard();
   }
 
-  if (content_type() == CONTENT_SETTINGS_TYPE_PLUGINS &&
-      setting == CONTENT_SETTING_ALLOW &&
-      setting_is_wildcard) {
-    // In the corner case of unrecognized plugins (which are now blocked by
-    // default) we indicate the blocked state in the UI and allow the user to
-    // whitelist.
-    radio_group.default_item = 1;
-  } else if (setting == CONTENT_SETTING_ALLOW) {
+  if (setting == CONTENT_SETTING_ALLOW) {
     radio_group.default_item = kAllowButtonIndex;
     // |block_setting_| is already set to |CONTENT_SETTING_BLOCK|.
   } else {
@@ -431,7 +434,7 @@ void ContentSettingCookiesBubbleModel::OnCustomLinkClicked() {
   delegate()->ShowCollectedCookiesDialog(web_contents());
 }
 
-class ContentSettingPluginBubbleModel : public ContentSettingSingleRadioGroup {
+class ContentSettingPluginBubbleModel : public ContentSettingTitleAndLinkModel {
  public:
   ContentSettingPluginBubbleModel(Delegate* delegate,
                                   WebContents* web_contents,
@@ -441,7 +444,20 @@ class ContentSettingPluginBubbleModel : public ContentSettingSingleRadioGroup {
   virtual ~ContentSettingPluginBubbleModel();
 
  private:
-  virtual void OnCustomLinkClicked() OVERRIDE;
+   void SetRadioGroup();
+
+   // Cookie bubble implementation.
+   virtual void OnRadioClicked(int radio_index) OVERRIDE;
+
+   // Function to load plugins.
+   void OnRunPlugins();
+
+   bool settings_changed() const;
+   int selected_item() const { return selected_item_; }
+
+   ContentSetting block_setting_;
+   int selected_item_;
+   int kAllowOnceButtonIndex;

[Bernhard Bauer, 2014/06/16 09:43:15]

Why is this a member variable?

[radhikabhar, 2014/06/16 18:21:37]

Removed it and made it as a global.

 };
 
 ContentSettingPluginBubbleModel::ContentSettingPluginBubbleModel(
@@ -449,27 +465,109 @@ ContentSettingPluginBubbleModel::ContentSettingPluginBubbleModel(
     WebContents* web_contents,
     Profile* profile,
     ContentSettingsType content_type)
-    : ContentSettingSingleRadioGroup(
-          delegate, web_contents, profile, content_type) {
+    : ContentSettingTitleAndLinkModel(
+          delegate, web_contents, profile, content_type),
+      block_setting_(CONTENT_SETTING_BLOCK),
+      selected_item_(0),
+      kAllowOnceButtonIndex(1) {
   DCHECK_EQ(content_type, CONTENT_SETTINGS_TYPE_PLUGINS);
-  // Disable the "Run all plugins this time" link if the setting is managed and
-  // can't be controlled by the user or if the user already clicked on the link
-  // and ran all plugins.
-  set_custom_link_enabled(!setting_is_managed() &&
-                          web_contents &&
-                          TabSpecificContentSettings::FromWebContents(
-                              web_contents)->load_plugins_link_enabled());
+  SetRadioGroup();
 }
 
 ContentSettingPluginBubbleModel::~ContentSettingPluginBubbleModel() {
-  if (settings_changed()) {
+  if (!settings_changed())
+    return;
+  ContentSetting setting;
+  if (selected_item_ == kAllowButtonIndex) {
+    setting = CONTENT_SETTING_ALLOW;
     // If the user elected to allow all plugins then run plugins at this time.
-    if (selected_item() == kAllowButtonIndex)
-      OnCustomLinkClicked();
+    OnRunPlugins();
+  } else if (selected_item_ == kAllowOnceButtonIndex) {
+    setting = CONTENT_SETTING_DEFAULT;

[Bernhard Bauer, 2014/06/16 09:43:16]

Wait, this means that we will run all plugins currently on the page, but remove
any setting for this site. Is this really the desired behavior?

[radhikabhar, 2014/06/16 18:21:37]

I added this for the corner case when the user selects Always run on this site
and then Run this time. When the user selects "Always" it will add an exception
with Allow but shouldn't this content setting be removed when he selects "Run
this time". Is this the behavior that we want or should we remember that the
user selected "Always run on this site" and let the content setting remain to
allow?   
On 2014/06/16 09:43:16, Bernhard Bauer wrote:

+    OnRunPlugins();
+  } else {

[Bernhard Bauer, 2014/06/16 09:43:16]

Can you DCHECK that the selected item is the block radio button?

[radhikabhar, 2014/06/16 18:21:37]

Done.

+    setting = block_setting_;
+  }
+  TabSpecificContentSettings::FromWebContents(web_contents())->
+      set_plugin_bubble_setting(setting);
+  if (profile()) {

[Bernhard Bauer, 2014/06/16 09:43:15]

When is profile() NULL?

[radhikabhar, 2014/06/16 18:21:37]

Removed it.

+    profile()->GetHostContentSettingsMap()->AddExceptionForURL(
+        bubble_content().radio_group.url,

[Bernhard Bauer, 2014/06/16 09:43:15]

Also, this will add an exception for a plugin with the URL of the current page,
embedded on the current page (which doesn't make sense). Did you actually verify
that this CL will have the correct effect on content settings?

[radhikabhar, 2014/06/16 18:21:37]

So previously the destructor was adding an exception for the URL and I just took
it form there to do the same thing. I verified the effect on the content
settings and it had the same effect with and without the patch.

Is there something I am missing or anything else that I should be doing?

+        bubble_content().radio_group.url,
+        content_type(),
+        setting);
+  }
+}
+
+bool ContentSettingPluginBubbleModel::settings_changed() const {

[Bernhard Bauer, 2014/06/16 09:43:15]

This isn't really a trivial accessor method, so it shouldn't be named
unix_hacker_style. 

[radhikabhar, 2014/06/16 18:21:38]

Done.

+  return selected_item_ != bubble_content().radio_group.default_item;
+}
+
+void ContentSettingPluginBubbleModel::OnRadioClicked(int radio_index) {
+  selected_item_ = radio_index;
+}
+
+void ContentSettingPluginBubbleModel::SetRadioGroup() {
+  GURL url = web_contents()->GetURL();
+  TabSpecificContentSettings* content_settings =
+      TabSpecificContentSettings::FromWebContents(web_contents());
+  bool allowed =

[Bernhard Bauer, 2014/06/16 09:43:15]

I think this is only necessary for cookies.

[radhikabhar, 2014/06/16 18:21:37]

Done.

+      !content_settings->IsContentBlocked(content_type());
+  DCHECK(!allowed ||
+         content_settings->IsContentAllowed(content_type()));
+  RadioGroup radio_group;
+  radio_group.url = url;
+
+  std::string radio_allow_label =
+      l10n_util::GetStringUTF8(IDS_BLOCKED_PLUGINS_UNBLOCK_ALL);
+  std::string radio_allow_once_label =
+      l10n_util::GetStringUTF8(IDS_BLOCKED_PLUGINS_LOAD_ALL);
+  std::string radio_block_label =
+      l10n_util::GetStringUTF8(IDS_BLOCKED_PLUGINS_NO_ACTION);
+
+  radio_group.radio_items.push_back(radio_allow_label);
+  radio_group.radio_items.push_back(radio_allow_once_label);
+  radio_group.radio_items.push_back(radio_block_label);
+
+  ContentSetting setting;
+  SettingSource setting_source = SETTING_SOURCE_NONE;
+  bool setting_is_wildcard = false;
+  SettingInfo info;
+  HostContentSettingsMap* map = profile()->GetHostContentSettingsMap();
+  scoped_ptr<base::Value> value(map->GetWebsiteSetting(
+      url, url, content_type(), std::string(), &info));
+  setting = content_settings->plugin_bubble_setting();
+  setting_source = info.source;
+  setting_is_wildcard =
+      info.primary_pattern == ContentSettingsPattern::Wildcard() &&
+      info.secondary_pattern == ContentSettingsPattern::Wildcard();
+
+  if (setting == CONTENT_SETTING_ALLOW && setting_is_wildcard) {

[Bernhard Bauer, 2014/06/16 09:43:16]

Really, you could fold this case into the next one, and make the whole thing a
switch statement.

[radhikabhar, 2014/06/16 18:21:37]

Since I changed the way I remember the user selection for run this time I still
do it as an if statement.

+    // In the corner case of unrecognized plugins (which are now blocked by
+    // default) we indicate the blocked state in the UI and allow the user to
+    // whitelist.

[Bernhard Bauer, 2014/06/16 09:43:16]

Are we actually still handling this case? We get |setting| from
TabSpecificContentSettings, but there it is only set if the bubble closes, and
the default value is BLOCK. So, for unrecognized plugins we will still get a
BLOCK value out of TabSpecificContentSetting.

[radhikabhar, 2014/06/16 18:21:37]

Removed it

+    radio_group.default_item = 2;

[Bernhard Bauer, 2014/06/16 09:43:16]

Can this 2 also get a constant?

[radhikabhar, 2014/06/16 18:21:37]

Done.

+  } else if (setting == CONTENT_SETTING_ALLOW) {
+    radio_group.default_item = kAllowButtonIndex;
+  } else if (setting == CONTENT_SETTING_DEFAULT) {
+    radio_group.default_item = kAllowOnceButtonIndex;
+    // |block_setting_| is already set to |CONTENT_SETTING_BLOCK|.
+  } else {
+    radio_group.default_item = 2;

[Bernhard Bauer, 2014/06/16 09:43:16]

DCHECK that the setting is BLOCK?

[radhikabhar, 2014/06/16 18:21:37]

Done.

+    block_setting_ = setting;
   }
+
+  set_setting_is_managed(setting_source != SETTING_SOURCE_USER);
+  if (setting_source != SETTING_SOURCE_USER) {
+    set_radio_group_enabled(false);

[Bernhard Bauer, 2014/06/16 09:43:15]

Extract `setting_source != SETTING_SOURCE_USER` into a local variable and use
that for set_setting_is_managed() and set_radio_group_enabled()?

[radhikabhar, 2014/06/16 18:21:38]

Done.

+  } else {
+    set_radio_group_enabled(true);
+  }
+  selected_item_ = radio_group.default_item;
+  set_radio_group(radio_group);
 }
 
-void ContentSettingPluginBubbleModel::OnCustomLinkClicked() {
+void ContentSettingPluginBubbleModel::OnRunPlugins() {
   content::RecordAction(UserMetricsAction("ClickToPlay_LoadAll_Bubble"));
   DCHECK(web_contents());
 #if defined(ENABLE_PLUGINS)
@@ -477,9 +575,6 @@ void ContentSettingPluginBubbleModel::OnCustomLinkClicked() {
   ChromePluginServiceFilter::GetInstance()->AuthorizeAllPlugins(
       web_contents(), true, std::string());
 #endif
-  set_custom_link_enabled(false);
-  TabSpecificContentSettings::FromWebContents(web_contents())->
-      set_load_plugins_link_enabled(false);
 }
 
 class ContentSettingPopupBubbleModel : public ContentSettingSingleRadioGroup {