Switch ScriptBadgeController and PageActionController to use WebContents.

Switch ScriptBadgeController and PageActionController to use WebContents.

BUG=107201
TEST=no visible change


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

[Aaron Boodman, 2012-09-05 22:02:00]

LGTM

http://codereview.chromium.org/10911105/diff/1/chrome/browser/extensions/page...
File chrome/browser/extensions/page_action_controller.h (right):

http://codereview.chromium.org/10911105/diff/1/chrome/browser/extensions/page...
chrome/browser/extensions/page_action_controller.h:24: explicit
PageActionController(content::WebContents* web_contents);
General comment. It is really important that all of these things which were
previously relying on TabContents still only happen for the case that
WebContents is actually a tab.

I think you're most likely aware of this, just wanted to be sure.

[Avi (use Gerrit), 2012-09-05 22:06:33]

It's a little more subtle than that. When the existing code requires that
something be a tab, what they really mean is that they rely on certain tab
helpers existing for the webcontents. I'm keeping an eye out for the
dependencies, in either case.

[commit-bot: I haz the power, 2012-09-05 22:08:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/avi@chromium.org/10911105/1

[commit-bot: I haz the power, 2012-09-05 22:08:24]

Presubmit check for 10911105-1 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome/browser/ui/gtk
    chrome
    chrome/browser/ui/views

Presubmit checks took 1.2s to calculate.

[Avi (use Gerrit), 2012-09-05 22:09:00]

Ben, OWNER?

[Ben Goodger (Google), 2012-09-06 21:19:55]

ui lgtm

[Aaron Boodman, 2012-09-06 21:53:28]

On 2012/09/05 22:06:33, Avi wrote:
> It's a little more subtle than that. When the existing code requires that
> something be a tab, what they really mean is that they rely on certain tab
> helpers existing for the webcontents. I'm keeping an eye out for the
> dependencies, in either case.

No in many cases we actually only want this behavior for tabs. Not notification
balloons or cros login dialogs or other weird cases we show HTML.

[Avi (use Gerrit), 2012-09-06 23:23:04]

They exist because they're created by the Extensions::TabHelper, and I'm going
to make sure I create that tab helper only for WebContents that are hosted by a
Browser as tabs.

[Avi (use Gerrit), 2012-09-06 23:27:45]

In fact this is going to be better than it is today. Today, TabContents are
created for everything from CrOS login pages to background printing. Tomorrow,
classes like this will _only_ be for real tabs.

[Aaron Boodman, 2012-09-06 23:53:50]

On Thu, Sep 6, 2012 at 4:27 PM,  <avi@chromium.org> wrote:
> In fact this is going to be better than it is today. Today, TabContents are
> created for everything from CrOS login pages to background printing.
> Tomorrow,
> classes like this will _only_ be for real tabs.

ok, sg

[commit-bot: I haz the power, 2012-09-07 00:51:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/avi@chromium.org/10911105/1

[commit-bot: I haz the power, 2012-09-07 00:53:40]

Try job failure for 10911105-1 on win for step "update".
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

Step "update" is always a major failure.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-09-07 01:36:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/avi@chromium.org/10911105/1

[commit-bot: I haz the power, 2012-09-07 13:06:10]

Change committed as 155378