Fix a bug in ShouldDisplayURL.

Fix a bug in ShouldDisplayURL.
Also update BrowserWithTestWindowTest to correctly simulate cross-site
navigations.

BUG=99519
TEST=Type a URL and hit enter on the NTP. URL should be visible.

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

[Charlie Reis, 2011-10-11 21:47:52]

Splitting this part out into its own CL so that we can close 99519
independently.

[Aaron Boodman, 2011-10-12 07:41:08]

thanks for fixing this

http://codereview.chromium.org/8229031/diff/3001/chrome/browser/ui/toolbar/to...
File chrome/browser/ui/toolbar/toolbar_model_unittest.cc (right):

http://codereview.chromium.org/8229031/diff/3001/chrome/browser/ui/toolbar/to...
chrome/browser/ui/toolbar/toolbar_model_unittest.cc:37:
CommitPendingLoad(&contents->controller());
It would be nice to pull this pattern into a helper function so we can just do:

NavigateAndTest("view-source:www.google.com", true);

That would make it easier to read this test.

[brettw, 2011-10-12 17:43:59]

http://codereview.chromium.org/8229031/diff/3001/chrome/browser/ui/toolbar/to...
File chrome/browser/ui/toolbar/toolbar_model.cc (left):

http://codereview.chromium.org/8229031/diff/3001/chrome/browser/ui/toolbar/to...
chrome/browser/ui/toolbar/toolbar_model.cc:80: if (tab_contents &&
tab_contents->web_ui())
Can we add a comment above the web_ui() function in TabContents about which
WebUI it returns, and pointing callers to GetWebUIForCurrentState for most
things? Or should we even have web_ui() on TabContents?

[Charlie Reis, 2011-10-12 19:02:57]

New patch uploaded.

http://codereview.chromium.org/8229031/diff/3001/chrome/browser/ui/toolbar/to...
File chrome/browser/ui/toolbar/toolbar_model.cc (left):

http://codereview.chromium.org/8229031/diff/3001/chrome/browser/ui/toolbar/to...
chrome/browser/ui/toolbar/toolbar_model.cc:80: if (tab_contents &&
tab_contents->web_ui())
On 2011/10/12 17:43:59, brettw wrote:
> Can we add a comment above the web_ui() function in TabContents about which
> WebUI it returns, and pointing callers to GetWebUIForCurrentState for most
> things? Or should we even have web_ui() on TabContents?

I added a comment.  I don't know if we should be removing web_ui()-- that should
probably happen in a separate CL if so.

http://codereview.chromium.org/8229031/diff/3001/chrome/browser/ui/toolbar/to...
File chrome/browser/ui/toolbar/toolbar_model_unittest.cc (right):

http://codereview.chromium.org/8229031/diff/3001/chrome/browser/ui/toolbar/to...
chrome/browser/ui/toolbar/toolbar_model_unittest.cc:37:
CommitPendingLoad(&contents->controller());
On 2011/10/12 07:41:08, Aaron Boodman wrote:
> It would be nice to pull this pattern into a helper function so we can just
do:
> 
> NavigateAndTest("view-source:www.google.com", true);
> 
> That would make it easier to read this test.

Done.

[brettw, 2011-10-12 19:19:04]

lgtm

[Aaron Boodman, 2011-10-12 20:18:50]

http://codereview.chromium.org/8229031/diff/3001/chrome/browser/ui/toolbar/to...
File chrome/browser/ui/toolbar/toolbar_model_unittest.cc (right):

http://codereview.chromium.org/8229031/diff/3001/chrome/browser/ui/toolbar/to...
chrome/browser/ui/toolbar/toolbar_model_unittest.cc:37:
CommitPendingLoad(&contents->controller());
On 2011/10/12 19:02:57, creis wrote:
> On 2011/10/12 07:41:08, Aaron Boodman wrote:
> > It would be nice to pull this pattern into a helper function so we can just
> do:
> > 
> > NavigateAndTest("view-source:www.google.com", true);
> > 
> > That would make it easier to read this test.
> 
> Done.

Great, thanks!