Move RequestPpapiBrokerPermission from WebContentsObserver to WebContentsDelegate.

Move RequestPpapiBrokerPermission from WebContentsObserver to WebContentsDelegate.

TBR=joth@chromium.org

BUG=142639
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=169360

[Bernhard Bauer, 2012-11-16 09:31:57]

Please review.
John: content/
Scott: chrome/

Thanks!

[Avi (use Gerrit), 2012-11-16 15:15:43]

drive-by

https://codereview.chromium.org/11416013/diff/1016/chrome/browser/pepper_brok...
File chrome/browser/pepper_broker_infobar_delegate.h (right):

https://codereview.chromium.org/11416013/diff/1016/chrome/browser/pepper_brok...
chrome/browser/pepper_broker_infobar_delegate.h:14: class
HostContentSettingsMap;
h < i

https://codereview.chromium.org/11416013/diff/1016/chrome/browser/pepper_brok...
chrome/browser/pepper_broker_infobar_delegate.h:41: PepperBrokerInfoBarDelegate(
indentation wrong

[Bernhard Bauer, 2012-11-16 17:02:40]

https://codereview.chromium.org/11416013/diff/1016/chrome/browser/pepper_brok...
File chrome/browser/pepper_broker_infobar_delegate.h (right):

https://codereview.chromium.org/11416013/diff/1016/chrome/browser/pepper_brok...
chrome/browser/pepper_broker_infobar_delegate.h:14: class
HostContentSettingsMap;
On 2012/11/16 15:15:43, Avi wrote:
> h < i

Done.

https://codereview.chromium.org/11416013/diff/1016/chrome/browser/pepper_brok...
chrome/browser/pepper_broker_infobar_delegate.h:41: PepperBrokerInfoBarDelegate(
On 2012/11/16 15:15:43, Avi wrote:
> indentation wrong

Done.

[jam, 2012-11-16 17:46:40]

https://codereview.chromium.org/11416013/diff/7017/content/public/browser/web...
File content/public/browser/web_contents_delegate.cc (right):

https://codereview.chromium.org/11416013/diff/7017/content/public/browser/web...
content/public/browser/web_contents_delegate.cc:151: callback.Run(false);
nit: don't add this function body, just leave it empty. in general, void
functions are empty in the delegate interfaces. these are not meant to be
"sample" or "basic" interfaces. if you really want, you can have a bool return
value here since we do implement bodies with return values.

[sky, 2012-11-16 21:48:30]

LGTM

https://codereview.chromium.org/11416013/diff/7017/chrome/browser/pepper_brok...
File chrome/browser/pepper_broker_infobar_delegate.cc (right):

https://codereview.chromium.org/11416013/diff/7017/chrome/browser/pepper_brok...
chrome/browser/pepper_broker_infobar_delegate.cc:43: if
(Profile::IsGuestSession()) {
Move this before 40.

https://codereview.chromium.org/11416013/diff/7017/chrome/browser/pepper_brok...
File chrome/browser/pepper_broker_infobar_delegate.h (right):

https://codereview.chromium.org/11416013/diff/7017/chrome/browser/pepper_brok...
chrome/browser/pepper_broker_infobar_delegate.h:20: class
PepperBrokerInfoBarDelegate : public ConfirmInfoBarDelegate {
Add description.

https://codereview.chromium.org/11416013/diff/7017/chrome/browser/pepper_brok...
chrome/browser/pepper_broker_infobar_delegate.h:28: virtual
~PepperBrokerInfoBarDelegate();
constructor/destructor before other methods.

[Bernhard Bauer, 2012-11-19 08:55:17]

http://codereview.chromium.org/11416013/diff/7017/chrome/browser/pepper_broke...
File chrome/browser/pepper_broker_infobar_delegate.cc (right):

http://codereview.chromium.org/11416013/diff/7017/chrome/browser/pepper_broke...
chrome/browser/pepper_broker_infobar_delegate.cc:43: if
(Profile::IsGuestSession()) {
On 2012/11/16 21:48:30, sky wrote:
> Move this before 40.

Done.

http://codereview.chromium.org/11416013/diff/7017/chrome/browser/pepper_broke...
File chrome/browser/pepper_broker_infobar_delegate.h (right):

http://codereview.chromium.org/11416013/diff/7017/chrome/browser/pepper_broke...
chrome/browser/pepper_broker_infobar_delegate.h:20: class
PepperBrokerInfoBarDelegate : public ConfirmInfoBarDelegate {
On 2012/11/16 21:48:30, sky wrote:
> Add description.

Done.

http://codereview.chromium.org/11416013/diff/7017/chrome/browser/pepper_broke...
chrome/browser/pepper_broker_infobar_delegate.h:28: virtual
~PepperBrokerInfoBarDelegate();
On 2012/11/16 21:48:30, sky wrote:
> constructor/destructor before other methods.

Done.

http://codereview.chromium.org/11416013/diff/7017/content/public/browser/web_...
File content/public/browser/web_contents_delegate.cc (right):

http://codereview.chromium.org/11416013/diff/7017/content/public/browser/web_...
content/public/browser/web_contents_delegate.cc:151: callback.Run(false);
On 2012/11/16 17:46:40, John Abd-El-Malek wrote:
> nit: don't add this function body, just leave it empty. in general, void
> functions are empty in the delegate interfaces. these are not meant to be
> "sample" or "basic" interfaces. if you really want, you can have a bool return
> value here since we do implement bodies with return values.

Done. I use a return value of false here and true in "real" implementations now
to make sure that we don't ever drop the callback (which would result in very
subtle issues).

[jam, 2012-11-19 19:00:00]

lgtm

[commit-bot: I haz the power, 2012-11-19 19:09:06]

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

[commit-bot: I haz the power, 2012-11-19 19:22:18]

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, 2012-11-20 09:45:28]

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

[commit-bot: I haz the power, 2012-11-20 09:56:52]

Sorry for I got bad news for ya.
Compile failed with a clobber build on ios_dbg_simulator.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...
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, 2012-11-22 10:05:12]

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

[commit-bot: I haz the power, 2012-11-22 10:05:23]

Failed to apply patch for content/public/browser/web_contents_observer.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file content/public/browser/web_contents_observer.h
  Hunk #2 FAILED at 136.
  1 out of 2 hunks FAILED -- saving rejects to file
content/public/browser/web_contents_observer.h.rej

Patch:       content/public/browser/web_contents_observer.h
Index: content/public/browser/web_contents_observer.h
diff --git a/content/public/browser/web_contents_observer.h
b/content/public/browser/web_contents_observer.h
index
59e0a1b84272768c89c75e1d274abb6a9aef0f4e..1ab01e188d48bd76ef7f7c1de63a24d6d6ee129a
100644
--- a/content/public/browser/web_contents_observer.h
+++ b/content/public/browser/web_contents_observer.h
@@ -5,7 +5,6 @@
 #ifndef CONTENT_PUBLIC_BROWSER_WEB_CONTENTS_OBSERVER_H_
 #define CONTENT_PUBLIC_BROWSER_WEB_CONTENTS_OBSERVER_H_
 
-#include "base/callback_forward.h"
 #include "base/process_util.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/navigation_controller.h"
@@ -137,18 +136,6 @@ class CONTENT_EXPORT WebContentsObserver : public
IPC::Listener,
   // Called when the user agent override for a WebContents has been changed.
   virtual void UserAgentOverrideSet(const std::string& user_agent) {}
 
-  // Requests permission to access the PPAPI broker. If the object handles the
-  // request, it should return true and eventually call the passed in
|callback|
-  // with the result. Otherwise it should return false, in which case the next
-  // observer will be called.
-  // Implementations should make sure not to call the callback after the
-  // WebContents has been destroyed.
-  virtual bool RequestPpapiBrokerPermission(
-      WebContents* web_contents,
-      const GURL& url,
-      const FilePath& plugin_path,
-      const base::Callback<void(bool)>& callback);
-
   // IPC::Listener implementation.
   virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;

[commit-bot: I haz the power, 2012-11-22 12:59:44]

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

[commit-bot: I haz the power, 2012-11-22 13:20:53]

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, 2012-11-22 19:17:24]

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

[commit-bot: I haz the power, 2012-11-22 19:35: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, 2012-11-23 17:00:57]

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

[commit-bot: I haz the power, 2012-11-23 18:57:21]

Change committed as 169360