Add content settings page action for Pepper broker authorization.

Add content settings page action for Pepper broker authorization.

BUG=159199
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=181651

[Bernhard Bauer, 2012-12-14 10:24:15]

Please review.

Markus: content settings
Nico: Cocoa

Thanks!

[Nico, 2012-12-14 23:02:25]

cocoa lgtm

https://codereview.chromium.org/11488009/diff/12015/chrome/browser/ui/cocoa/c...
File chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm
(right):

https://codereview.chromium.org/11488009/diff/12015/chrome/browser/ui/cocoa/c...
chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm:127:
nibPath = @"ContentBlockedSimple"; break;
Do you feel the nib files for the content settings bubbles are worth it? I was
considering moving all the content settings bubble stuff from nibs to code -- do
you have an opinion on that?

[Bernhard Bauer, 2012-12-15 05:44:04]

https://codereview.chromium.org/11488009/diff/12015/chrome/browser/ui/cocoa/c...
File chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm
(right):

https://codereview.chromium.org/11488009/diff/12015/chrome/browser/ui/cocoa/c...
chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm:127:
nibPath = @"ContentBlockedSimple"; break;
On 2012/12/14 23:02:26, Nico wrote:
> Do you feel the nib files for the content settings bubbles are worth it? I was
> considering moving all the content settings bubble stuff from nibs to code --
do
> you have an opinion on that?

Yes, that probably makes sense. For starters we could unify all the different
nibs here to a single one, and then later create that in code instead of loading
and modifying it.

[markusheintz_, 2012-12-17 16:03:58]

LGTM with one comment.

https://codereview.chromium.org/11488009/diff/12015/chrome/browser/ui/content...
File chrome/browser/ui/content_settings/content_setting_image_model.cc (right):

https://codereview.chromium.org/11488009/diff/12015/chrome/browser/ui/content...
chrome/browser/ui/content_settings/content_setting_image_model.cc:120: // For
cookies, only show the accessed bubble if cookies are blocked by
This Change seems not to be related to the issue. Why is this in this CL?

What about if cookies are block due to an exception? Don't we display an page
action icon then?

[Bernhard Bauer, 2012-12-17 16:41:31]

https://codereview.chromium.org/11488009/diff/12015/chrome/browser/ui/content...
File chrome/browser/ui/content_settings/content_setting_image_model.cc (right):

https://codereview.chromium.org/11488009/diff/12015/chrome/browser/ui/content...
chrome/browser/ui/content_settings/content_setting_image_model.cc:120: // For
cookies, only show the accessed bubble if cookies are blocked by
On 2012/12/17 16:03:59, markusheintz_ wrote:
> This Change seems not to be related to the issue. Why is this in this CL?

This behavior (for cookies at least) has been in Chrome for some time, I just
added a comment after I spent a while figuring out why we do it (there used to
be a comment, but it got lost in a refactoring). The only functional change is
that I added the check for CONTENT_SETTINGS_TYPE_COOKIES because now we have
other content types for which we show an "accessed" bubble.

> What about if cookies are block due to an exception? Don't we display an page
> action icon then?

We do (as we do for every time cookies are blocked on a page, by default or via
an exception), but this branch is for when cookies are allowed on a page, to
keep the icon from showing up all the time when the content settings are at
default values.

[commit-bot: I haz the power, 2012-12-17 16:57:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bauerb@chromium.org/11488009/23002

[commit-bot: I haz the power, 2012-12-17 16:57:40]

Failed to apply patch for chrome/browser/pepper_broker_infobar_delegate.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/pepper_broker_infobar_delegate.cc
  Hunk #2 FAILED at 46.
  1 out of 6 hunks FAILED -- saving rejects to file
chrome/browser/pepper_broker_infobar_delegate.cc.rej

Patch:       chrome/browser/pepper_broker_infobar_delegate.cc
Index: chrome/browser/pepper_broker_infobar_delegate.cc
diff --git a/chrome/browser/pepper_broker_infobar_delegate.cc
b/chrome/browser/pepper_broker_infobar_delegate.cc
index
3ef8edf877ee9d5f03b9956a652004ff664c78d2..5c1f0e5b43ce85b28a9160bc69f8925268b8b446
100644
--- a/chrome/browser/pepper_broker_infobar_delegate.cc
+++ b/chrome/browser/pepper_broker_infobar_delegate.cc
@@ -5,6 +5,7 @@
 #include "chrome/browser/pepper_broker_infobar_delegate.h"
 
 #include "chrome/browser/content_settings/host_content_settings_map.h"
+#include "chrome/browser/content_settings/tab_specific_content_settings.h"
 #include "chrome/browser/infobars/infobar_tab_helper.h"
 #include "chrome/browser/plugins/plugin_finder.h"
 #include "chrome/browser/plugins/plugin_metadata.h"
@@ -45,6 +46,8 @@ void PepperBrokerInfoBarDelegate::Show(
 
   Profile* profile =
       Profile::FromBrowserContext(web_contents->GetBrowserContext());
+  TabSpecificContentSettings* tab_content_settings =
+      TabSpecificContentSettings::FromWebContents(web_contents);
   HostContentSettingsMap* content_settings =
       profile->GetHostContentSettingsMap();
   ContentSetting setting =
@@ -55,12 +58,14 @@ void PepperBrokerInfoBarDelegate::Show(
     case CONTENT_SETTING_ALLOW: {
       content::RecordAction(
           content::UserMetricsAction("PPAPI.BrokerSettingAllow"));
+      tab_content_settings->SetPepperBrokerAllowed(true);
       callback.Run(true);
       break;
     }
     case CONTENT_SETTING_BLOCK: {
       content::RecordAction(
           content::UserMetricsAction("PPAPI.BrokerSettingDeny"));
+      tab_content_settings->SetPepperBrokerAllowed(false);
       callback.Run(false);
       break;
     }
@@ -75,7 +80,7 @@ void PepperBrokerInfoBarDelegate::Show(
       infobar_helper->AddInfoBar(
           new PepperBrokerInfoBarDelegate(
               infobar_helper, url, plugin_path, languages, content_settings,
-              callback));
+              tab_content_settings, callback));
       break;
     }
     default:
@@ -89,12 +94,14 @@ PepperBrokerInfoBarDelegate::PepperBrokerInfoBarDelegate(
     const FilePath& plugin_path,
     const std::string& languages,
     HostContentSettingsMap* content_settings,
+    TabSpecificContentSettings* tab_content_settings,
     const base::Callback<void(bool)>& callback)
     : ConfirmInfoBarDelegate(helper),
       url_(url),
       plugin_path_(plugin_path),
       languages_(languages),
       content_settings_(content_settings),
+      tab_content_settings_(tab_content_settings),
       callback_(callback) {
 }
 
@@ -175,4 +182,5 @@ void PepperBrokerInfoBarDelegate::DispatchCallback(bool
result) {
       ContentSettingsPattern::Wildcard(),
       CONTENT_SETTINGS_TYPE_PPAPI_BROKER,
       std::string(), result ? CONTENT_SETTING_ALLOW : CONTENT_SETTING_BLOCK);
+  tab_content_settings_->SetPepperBrokerAllowed(result);
 }

[commit-bot: I haz the power, 2012-12-17 17:15:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bauerb@chromium.org/11488009/29002

[commit-bot: I haz the power, 2012-12-17 23:30:34]

Retried try job too often on win_rel for step(s) browser_tests

[commit-bot: I haz the power, 2012-12-18 17:04:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bauerb@chromium.org/11488009/29002

[commit-bot: I haz the power, 2012-12-18 22:43:30]

Retried try job too often on win_rel for step(s) browser_tests

[commit-bot: I haz the power, 2013-02-07 18:07:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bauerb@chromium.org/11488009/29002

[commit-bot: I haz the power, 2013-02-07 18:07:47]

Failed to apply patch for
chrome/browser/content_settings/tab_specific_content_settings.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/content_settings/tab_specific_content_settings.cc
  Hunk #1 FAILED at 205.
  Hunk #2 FAILED at 223.
  Hunk #3 succeeded at 475 (offset 10 lines).
  2 out of 3 hunks FAILED -- saving rejects to file
chrome/browser/content_settings/tab_specific_content_settings.cc.rej

Patch:       chrome/browser/content_settings/tab_specific_content_settings.cc
Index: chrome/browser/content_settings/tab_specific_content_settings.cc
diff --git a/chrome/browser/content_settings/tab_specific_content_settings.cc
b/chrome/browser/content_settings/tab_specific_content_settings.cc
index
8374180ed8379cf9d8430212a14da950a3d04326..d3377b105966e63e57d1a2e263202e1a6f52736f
100644
--- a/chrome/browser/content_settings/tab_specific_content_settings.cc
+++ b/chrome/browser/content_settings/tab_specific_content_settings.cc
@@ -205,7 +205,8 @@ bool TabSpecificContentSettings::IsContentBlocked(
       content_type == CONTENT_SETTINGS_TYPE_PLUGINS ||
       content_type == CONTENT_SETTINGS_TYPE_COOKIES ||
       content_type == CONTENT_SETTINGS_TYPE_POPUPS ||
-      content_type == CONTENT_SETTINGS_TYPE_MIXEDSCRIPT)
+      content_type == CONTENT_SETTINGS_TYPE_MIXEDSCRIPT ||
+      content_type == CONTENT_SETTINGS_TYPE_PPAPI_BROKER)
     return content_blocked_[content_type];
 
   return false;
@@ -223,9 +224,12 @@ void
TabSpecificContentSettings::SetBlockageHasBeenIndicated(
 
 bool TabSpecificContentSettings::IsContentAccessed(
     ContentSettingsType content_type) const {
-  // This method currently only returns meaningful values for cookies.
-  if (content_type != CONTENT_SETTINGS_TYPE_COOKIES)
+  // This method currently only returns meaningful values for cookies
+  /// and PPAPI broker.
+  if (content_type != CONTENT_SETTINGS_TYPE_COOKIES &&
+      content_type != CONTENT_SETTINGS_TYPE_PPAPI_BROKER) {
     return false;
+  }
 
   return content_accessed_[content_type];
 }
@@ -465,6 +469,14 @@ void
TabSpecificContentSettings::ClearGeolocationContentSettings() {
   geolocation_settings_state_.ClearStateMap();
 }
 
+void TabSpecificContentSettings::SetPepperBrokerAllowed(bool allowed) {
+  if (allowed) {
+    OnContentAccessed(CONTENT_SETTINGS_TYPE_PPAPI_BROKER);
+  } else {
+    OnContentBlocked(CONTENT_SETTINGS_TYPE_PPAPI_BROKER, std::string());
+  }
+}
+
 void TabSpecificContentSettings::RenderViewForInterstitialPageCreated(
     RenderViewHost* render_view_host) {
   // We want to tell the renderer-side code to ignore content settings for this

[commit-bot: I haz the power, 2013-02-07 18:30:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bauerb@chromium.org/11488009/47002

[commit-bot: I haz the power, 2013-02-07 18:55:04]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_clang_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-02-08 09:22:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bauerb@chromium.org/11488009/48004

[commit-bot: I haz the power, 2013-02-08 09:47:55]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-02-09 10:10:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bauerb@chromium.org/11488009/59004

[commit-bot: I haz the power, 2013-02-10 16:22:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bauerb@chromium.org/11488009/59004

[commit-bot: I haz the power, 2013-02-10 16:23:03]

Change committed as 181651