BrowserRootView: rewritten the GetPasteAndGoUrl function....

BrowserRootView: rewritten the GetPasteAndGoUrl function.

Now the classification of text in a query or url can be made by SearchVersusNavigateClassifier, so now
just use it to avoid the requirement of AutocompleteEdit.

BUG=21317
TEST=None

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

[Peter Kasting, 2009-10-29 20:37:54]

http://codereview.chromium.org/347020/diff/2001/4
File chrome/browser/views/frame/browser_root_view.cc (right):

http://codereview.chromium.org/347020/diff/2001/4#newcode152
Line 152: return false;
You do not need to NULL check any of these three return values.

http://codereview.chromium.org/347020/diff/2001/4#newcode159
Line 159: GURL destination_url;
Nit: Instead of using a temp, just pass |url| directly.

http://codereview.chromium.org/347020/diff/2001/4#newcode164
Line 164: return false;
This is wrong.  As with the autocomplete code, "CanPasteAndGo()" returned true
for navigation _or_ search.  Instead you should be returning whether the result
URL is valid.

[tfarina (gmail-do not use), 2009-10-29 20:58:39]

http://codereview.chromium.org/347020/diff/2001/4
File chrome/browser/views/frame/browser_root_view.cc (right):

http://codereview.chromium.org/347020/diff/2001/4#newcode152
Line 152: return false;
On 2009/10/29 20:37:54, Peter Kasting wrote:
> You do not need to NULL check any of these three return values.

Removed the checks.

http://codereview.chromium.org/347020/diff/2001/4#newcode159
Line 159: GURL destination_url;
On 2009/10/29 20:37:54, Peter Kasting wrote:
> Nit: Instead of using a temp, just pass |url| directly.

Removed |is_search|, and passing |url| directly.

http://codereview.chromium.org/347020/diff/2001/4#newcode164
Line 164: return false;
On 2009/10/29 20:37:54, Peter Kasting wrote:
> This is wrong.  As with the autocomplete code, "CanPasteAndGo()" returned true
> for navigation _or_ search.  Instead you should be returning whether the
result
> URL is valid.

Done.

[Peter Kasting, 2009-10-29 21:08:54]

http://codereview.chromium.org/347020/diff/1002/1003
File chrome/browser/views/frame/browser_root_view.cc (right):

http://codereview.chromium.org/347020/diff/1002/1003#newcode142
Line 142: profile->GetSearchVersusNavigateClassifier();
Nit: Just roll all these into the Classify() call below.

http://codereview.chromium.org/347020/diff/1002/1003#newcode150
Line 150: if (!url || !url->is_valid())
Hmm.  This isn't quite right because we may be able to paste-and-go without
|url| being set (see one of the callers).

I guess my first advice was wrong, and you'll have to put this back to:

GURL destination_url;
Classify(&destination_url);
if (!destination_url.is_valid())
  return false;
if (url)
  *url = destination_url;
return true;

Sorry about that.

[tfarina (gmail-do not use), 2009-10-29 21:29:59]

http://codereview.chromium.org/347020/diff/1002/1003
File chrome/browser/views/frame/browser_root_view.cc (right):

http://codereview.chromium.org/347020/diff/1002/1003#newcode142
Line 142: profile->GetSearchVersusNavigateClassifier();
On 2009/10/29 21:08:54, Peter Kasting wrote:
> Nit: Just roll all these into the Classify() call below.

Just moved below.

http://codereview.chromium.org/347020/diff/1002/1003#newcode150
Line 150: if (!url || !url->is_valid())
On 2009/10/29 21:08:54, Peter Kasting wrote:
> Hmm.  This isn't quite right because we may be able to paste-and-go without
> |url| being set (see one of the callers).
> 
Oh that is true, we can paste-and-go without url.
I verified the callers, but not noticied that.

> I guess my first advice was wrong, and you'll have to put this back to:
> 
> GURL destination_url;
> Classify(&destination_url);
> if (!destination_url.is_valid())
>   return false;
> if (url)
>   *url = destination_url;
> return true;
> 
> Sorry about that.

Rolled back.

[Peter Kasting, 2009-10-29 21:32:21]

LGTM, I'll just modify my nits on landing.

[Peter Kasting, 2009-10-29 21:38:36]

Landed in r30519.