Remove some chrome-specific methods from content::WebUI.

Remove some chrome-specific methods from content::WebUI.

HideFavicon wasn't used anymore. This is probably because now we show a search icon there.
FocusLocationBarByDefault can just be implemented in Browser (like Instant).
HideURL can just be implemented in ToolbarModelImpl (again, like Instant).
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=193998

[Avi (use Gerrit), 2013-04-10 23:12:15]

https://codereview.chromium.org/14080004/diff/18001/chrome/browser/favicon/fa...
File chrome/browser/favicon/favicon_tab_helper.cc (right):

https://codereview.chromium.org/14080004/diff/18001/chrome/browser/favicon/fa...
chrome/browser/favicon/favicon_tab_helper.cc:25: #include
"content/public/browser/web_ui.h"
Is this include still needed, then?

https://codereview.chromium.org/14080004/diff/18001/chrome/browser/ui/toolbar...
File chrome/browser/ui/toolbar/toolbar_model_impl.cc (right):

https://codereview.chromium.org/14080004/diff/18001/chrome/browser/ui/toolbar...
chrome/browser/ui/toolbar/toolbar_model_impl.cc:150: //   can be NTPs that have
extension schemes (e.g. the bookmark manager). In
"NTPs that have extension schemes"? I'm not sure all these instances of "WebUI"
should be altered this way.

https://codereview.chromium.org/14080004/diff/18001/content/browser/web_conte...
File content/browser/web_contents/web_contents_impl.cc (left):

https://codereview.chromium.org/14080004/diff/18001/content/browser/web_conte...
content/browser/web_contents/web_contents_impl.cc:1940: 
This deletion doesn't seem relevant to this CL.

https://codereview.chromium.org/14080004/diff/18001/content/public/browser/we...
File content/public/browser/web_contents.h (left):

https://codereview.chromium.org/14080004/diff/18001/content/public/browser/we...
content/public/browser/web_contents.h:177: // should use GetWebUIForCurrentState
instead.
Same here; is this deletion part of this CL?

https://codereview.chromium.org/14080004/diff/18001/content/public/browser/we...
content/public/browser/web_contents.h:390: 
Same here; is this deletion part of this CL?

[jam, 2013-04-10 23:45:35]

https://codereview.chromium.org/14080004/diff/18001/chrome/browser/favicon/fa...
File chrome/browser/favicon/favicon_tab_helper.cc (right):

https://codereview.chromium.org/14080004/diff/18001/chrome/browser/favicon/fa...
chrome/browser/favicon/favicon_tab_helper.cc:25: #include
"content/public/browser/web_ui.h"
On 2013/04/10 23:12:15, Avi wrote:
> Is this include still needed, then?

Done.

https://codereview.chromium.org/14080004/diff/18001/chrome/browser/ui/toolbar...
File chrome/browser/ui/toolbar/toolbar_model_impl.cc (right):

https://codereview.chromium.org/14080004/diff/18001/chrome/browser/ui/toolbar...
chrome/browser/ui/toolbar/toolbar_model_impl.cc:150: //   can be NTPs that have
extension schemes (e.g. the bookmark manager). In
On 2013/04/10 23:12:15, Avi wrote:
> "NTPs that have extension schemes"? I'm not sure all these instances of
"WebUI"
> should be altered this way.

updated comment

https://codereview.chromium.org/14080004/diff/18001/content/browser/web_conte...
File content/browser/web_contents/web_contents_impl.cc (left):

https://codereview.chromium.org/14080004/diff/18001/content/browser/web_conte...
content/browser/web_contents/web_contents_impl.cc:1940: 
On 2013/04/10 23:12:15, Avi wrote:
> This deletion doesn't seem relevant to this CL.

this method isn't used anymore (see the diff below)

[Avi (use Gerrit), 2013-04-11 00:51:19]

comment nitpickiness lgtm

https://codereview.chromium.org/14080004/diff/26001/chrome/browser/ui/toolbar...
File chrome/browser/ui/toolbar/toolbar_model_impl.cc (right):

https://codereview.chromium.org/14080004/diff/26001/chrome/browser/ui/toolbar...
chrome/browser/ui/toolbar/toolbar_model_impl.cc:150: //   extensions can
implement the NTP.
"because extensions can implement the NTP"? Drop the "there"

[jam, 2013-04-11 16:56:55]

Avi: ptal, I updated the cl to fix a few failing tests.

[Avi (use Gerrit), 2013-04-11 17:01:37]

slgtm

https://codereview.chromium.org/14080004/diff/22005/chrome/browser/ui/toolbar...
File chrome/browser/ui/toolbar/toolbar_model_impl.cc (right):

https://codereview.chromium.org/14080004/diff/22005/chrome/browser/ui/toolbar...
chrome/browser/ui/toolbar/toolbar_model_impl.cc:151: //   that case, we should
prefer what the WebUI instance says.
Oh, that's much clearer. Thanks.

[jam, 2013-04-12 17:27:22]

Avi, sorry for all the pings. I've had to make minor changes to get the
interactive test to pass on mac, and also to fix a test failure on cros that I
originally thought was a flake. everything passes now.

It took me a while to figure out why the interactive test was failing on mac. I
didn't know that ui_test_utils::BringBrowserWindowToFront has to be called for
Mac or else the focus methods will all fail since the browser window is behind
the shell. I'm not sure where to put a warning about this? perhaps in
interactive_test_utils.h? should the harness try to do this itself (but then it
won't work for new windows?)?

[Avi (use Gerrit), 2013-04-12 17:49:03]

I don't know. I've never been into the whole testing mechanism stuff, and am
still being schooled by browser tests.

SLGTM