Include a URL what-you-typed option when the default search provider is not available and the input…

Include a URL what-you-typed option when the default search provider is not available and the input type is unknown.

Previously, when the default search provider was disabled via an enterprise policy, backspacing through the URL in the omnibar would get stuck as soon as the input got short enough to be classified as unknown. Normally, this is when the top autocomplete option would become a search for what you typed. But with search disabled, the top autocomplete option became the full previously visited URL. This change makes a URL what-you-typed autocomplete option to be provided instead under these circumstances.

BUG=363656
R=mpearson@chromium.org

Committed: https://crrev.com/7d6b4f8b303603f2cf27220f0871df3812880a25
Cr-Commit-Position: refs/heads/master@{#301295}

[Mark P, 2014-10-23 21:41:12]

mpearson@chromium.org changed reviewers:
+ pkasting@chromium.org

[Mark P, 2014-10-23 21:41:12]

mpearson@chromium.org changed required reviewers:
+ pkasting@chromium.org

[Mark P, 2014-10-23 21:42:49]

This looks fine to me.  It doesn't entirely fix the problem but it gets a
substantial part of the way there.  I think pkasting@ would be a better
reviewerer than me; he's always more touchy about things like this.  I added him
to the reviewer list.  Assuming it's okay with him, I imagine he'll ask you to
write a trivial unit test for this code, and then you'll get to submit it.

--mark

[Peter Kasting, 2014-10-24 01:10:07]

Yeah, I think this is fine.  A unittest would be good.  I'd just verify that we
create a WYT match here when you only have a history provider, and then once you
have a search provider also, we no longer create the match.

Once that's done, LGTM.

Mark, I'm OOO for a few days, and can't get at the CLA spreadsheet while at
home, so could you verify the CLA entry here for me?

https://codereview.chromium.org/672773003/diff/1/chrome/browser/autocomplete/...
File chrome/browser/autocomplete/history_url_provider.cc (right):

https://codereview.chromium.org/672773003/diff/1/chrome/browser/autocomplete/...
chrome/browser/autocomplete/history_url_provider.cc:763: // "https:".
Nit: These comments should be updated.  How about:

...We consider the what you typed match eligible for display when it's navigable
and there's a reasonable chance the user intended to do something other than
search.  We use a variety of heuristics to determine this, e.g. whether the user
explicitly typed a scheme, or if omnibox searching has been disabled by policy. 
In the cases where we've parsed as UNKNOWN, ...

https://codereview.chromium.org/672773003/diff/1/chrome/browser/autocomplete/...
chrome/browser/autocomplete/history_url_provider.cc:773:
(params->default_search_provider == NULL));
Nit: Just do

      !params->default_search_provider);

[Chris Vasselli, 2014-10-24 21:12:28]

Thanks! I think I've addressed all the things mentioned here.

There seemed to already be a WhatYouTyped test case checking that no
what-you-typed matches are created for certain short queries. So I created a
WhatYouTypedNoSearchProvider test case that duplicates the tests from
WhatYouTyped that look like they would be meaningful to test with the default
search provider disabled.

[Peter Kasting, 2014-10-24 22:02:13]

LGTM

https://codereview.chromium.org/672773003/diff/20001/chrome/browser/autocompl...
File chrome/browser/autocomplete/history_url_provider.cc (right):

https://codereview.chromium.org/672773003/diff/20001/chrome/browser/autocompl...
chrome/browser/autocomplete/history_url_provider.cc:772:
(!params->default_search_provider));
Nit: No parens around unary operation

https://codereview.chromium.org/672773003/diff/20001/chrome/browser/autocompl...
File chrome/browser/autocomplete/history_url_provider_unittest.cc (right):

https://codereview.chromium.org/672773003/diff/20001/chrome/browser/autocompl...
chrome/browser/autocomplete/history_url_provider_unittest.cc:237: virtual void
SetUp() {
Nit: virtual -> override (2 places)

I realize the rest of this file hasn't been fixed to do that yet; up to you
whether you wish to convert the other places or not.

https://codereview.chromium.org/672773003/diff/20001/chrome/browser/autocompl...
chrome/browser/autocomplete/history_url_provider_unittest.cc:499: // for text
that could be a URL
Nit: Trailing period.

[Chris Vasselli, 2014-10-25 15:08:58]

Fixed up those nits!

[Mark P, 2014-10-25 17:44:46]

Then check the "Commit" box on the codereview page. :-)

--mark

[Chris Vasselli, 2014-10-25 18:23:35]

The CQ bit was checked by clindsay@gmail.com

[commit-bot: I haz the power, 2014-10-25 18:24:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/672773003/40001

[commit-bot: I haz the power, 2014-10-25 21:53:27]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-10-25 21:54:08]

Patchset 3 (id:??) landed as
https://crrev.com/7d6b4f8b303603f2cf27220f0871df3812880a25
Cr-Commit-Position: refs/heads/master@{#301295}