Componentizes builtin_provider_unittest.cc.

Componentizes builtin_provider_unittest.cc.

Removes dependencies on chrome/ and content/ and moves the file to
components/omnibox/browser.

Based on https://codereview.chromium.org/1522303005/ by abhishek21@samsung.com.

BUG=569841
TEST=None

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

[rohitrao (ping after 24h), 2016-02-03 19:24:41]

Description was changed from

==========
Componentizes built_in_provider_unittest.cc.

Removes dependencies on chrome/ and content/ and moves the file to
components/omnibox/browser.

BUG=569841
TEST=None
==========

to

==========
Componentizes builtin_provider_unittest.cc.

Removes dependencies on chrome/ and content/ and moves the file to
components/omnibox/browser.

BUG=569841
TEST=None
==========

[rohitrao (ping after 24h), 2016-02-03 19:52:48]

rohitrao@chromium.org changed reviewers:
+ blundell@chromium.org, pkasting@chromium.org

[rohitrao (ping after 24h), 2016-02-03 19:52:49]

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

[rohitrao (ping after 24h), 2016-02-03 19:52:50]

Taking over from the external contributor.  I think I've addressed the major
comments from that version of the CL.

https://codereview.chromium.org/1668543003/diff/20001/components/omnibox/brow...
File components/omnibox/browser/builtin_provider_unittest.cc (right):

https://codereview.chromium.org/1668543003/diff/20001/components/omnibox/brow...
components/omnibox/browser/builtin_provider_unittest.cc:27: const char
kEmbedderAboutScheme[] = "chrome";
BuiltinProvider calls url_formatter::FixupURL(), which hard-codes the "chrome"
scheme.  That's forcing me to use "chrome" here as well, because the tests fail
if I change to anything else.

I'll file a bug for FixupURL(), but I think it's ok to just keep using "chrome"
in the tests for now.  I've made all the other strings generic.

[rohitrao (ping after 24h), 2016-02-03 20:02:09]

The CQ bit was checked by rohitrao@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-02-03 20:03:48]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1668543003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1668543003/20001

[commit-bot: I haz the power, 2016-02-03 20:51:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-03 20:51:01]

Dry run: This issue passed the CQ dry run.

[Peter Kasting, 2016-02-04 01:14:47]

LGTM

https://codereview.chromium.org/1668543003/diff/20001/components/omnibox/brow...
File components/omnibox/browser/builtin_provider_unittest.cc (right):

https://codereview.chromium.org/1668543003/diff/20001/components/omnibox/brow...
components/omnibox/browser/builtin_provider_unittest.cc:136: const GURL kURL1 =
GURL(kDefaultURL1);
Here and elsewhere in this file: just

  const GURL kURL1(kDefaultURL1);

https://codereview.chromium.org/1668543003/diff/20001/components/omnibox/brow...
components/omnibox/browser/builtin_provider_unittest.cc:150:
{ASCIIToUTF16("chromee:"),   0, {}},
Nit: Why the extra spacing here?

https://codereview.chromium.org/1668543003/diff/20001/components/omnibox/brow...
components/omnibox/browser/builtin_provider_unittest.cc:168:
{ASCIIToUTF16("chROMe://"),   3, {kURL1, kURL2, kURL3}},
And here

https://codereview.chromium.org/1668543003/diff/20001/components/omnibox/brow...
components/omnibox/browser/builtin_provider_unittest.cc:201: // letters "M".
Nit: letter "m"

[blundell, 2016-02-04 08:32:30]

lgtm

Wrt url_fixer.cc, the issue is that the "chrome://" scheme is used in
free-standing functions, i.e., there's no way to inject the scheme from the
embedder that doesn't add ugly complexity. When I discussed the issue with jam@
in the course of componentizing that code, the conclusion was that we would
leave the "chrome://" scheme and worry about it when an embedder that wanted a
different alias for about:// wanted to use url_fixer.cc.

https://codereview.chromium.org/1668543003/diff/20001/components/omnibox/brow...
File components/omnibox/browser/builtin_provider_unittest.cc (right):

https://codereview.chromium.org/1668543003/diff/20001/components/omnibox/brow...
components/omnibox/browser/builtin_provider_unittest.cc:44: class
FakeAutocompleteProviderClient : public MockAutocompleteProviderClient {
you get a FakeAutocompleteProviderClient! you get a
FakeAutocompleteProviderClient!

[blundell, 2016-02-04 08:35:21]

Also, please reference in the description that this builds on
https://codereview.chromium.org/1522303005/ by abhishek21@samsung.com.

[rohitrao (ping after 24h), 2016-02-04 12:20:05]

Description was changed from

==========
Componentizes builtin_provider_unittest.cc.

Removes dependencies on chrome/ and content/ and moves the file to
components/omnibox/browser.

BUG=569841
TEST=None
==========

to

==========
Componentizes builtin_provider_unittest.cc.

Removes dependencies on chrome/ and content/ and moves the file to
components/omnibox/browser.

Based on https://codereview.chromium.org/1522303005/ by abhishek21@samsung.com.

BUG=569841
TEST=None
==========

[rohitrao (ping after 24h), 2016-02-04 12:34:27]

https://codereview.chromium.org/1668543003/diff/20001/components/omnibox/brow...
File components/omnibox/browser/builtin_provider_unittest.cc (right):

https://codereview.chromium.org/1668543003/diff/20001/components/omnibox/brow...
components/omnibox/browser/builtin_provider_unittest.cc:136: const GURL kURL1 =
GURL(kDefaultURL1);
On 2016/02/04 01:14:47, Peter Kasting wrote:
> Here and elsewhere in this file: just
> 
>   const GURL kURL1(kDefaultURL1);

Done.

https://codereview.chromium.org/1668543003/diff/20001/components/omnibox/brow...
components/omnibox/browser/builtin_provider_unittest.cc:150:
{ASCIIToUTF16("chromee:"),   0, {}},
On 2016/02/04 01:14:47, Peter Kasting wrote:
> Nit: Why the extra spacing here?

I originally changed the "chrome" scheme to "embedder", which is two characters
longer.  Now that I've switched back, the extra spacing is unnecessary.

https://codereview.chromium.org/1668543003/diff/20001/components/omnibox/brow...
components/omnibox/browser/builtin_provider_unittest.cc:201: // letters "M".
On 2016/02/04 01:14:47, Peter Kasting wrote:
> Nit: letter "m"

Done.

[rohitrao (ping after 24h), 2016-02-04 12:34:33]

The CQ bit was checked by rohitrao@chromium.org

[rohitrao (ping after 24h), 2016-02-04 12:34:33]

The patchset sent to the CQ was uploaded after l-g-t-m from
pkasting@chromium.org, blundell@chromium.org
Link to the patchset: https://codereview.chromium.org/1668543003/#ps40001
(title: "Review.")

[commit-bot: I haz the power, 2016-02-04 12:35:03]

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

[commit-bot: I haz the power, 2016-02-04 13:25:08]

Description was changed from

==========
Componentizes builtin_provider_unittest.cc.

Removes dependencies on chrome/ and content/ and moves the file to
components/omnibox/browser.

Based on https://codereview.chromium.org/1522303005/ by abhishek21@samsung.com.

BUG=569841
TEST=None
==========

to

==========
Componentizes builtin_provider_unittest.cc.

Removes dependencies on chrome/ and content/ and moves the file to
components/omnibox/browser.

Based on https://codereview.chromium.org/1522303005/ by abhishek21@samsung.com.

BUG=569841
TEST=None
==========

[commit-bot: I haz the power, 2016-02-04 13:25:09]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-02-04 13:27:00]

Description was changed from

==========
Componentizes builtin_provider_unittest.cc.

Removes dependencies on chrome/ and content/ and moves the file to
components/omnibox/browser.

Based on https://codereview.chromium.org/1522303005/ by abhishek21@samsung.com.

BUG=569841
TEST=None
==========

to

==========
Componentizes builtin_provider_unittest.cc.

Removes dependencies on chrome/ and content/ and moves the file to
components/omnibox/browser.

Based on https://codereview.chromium.org/1522303005/ by abhishek21@samsung.com.

BUG=569841
TEST=None

Committed: https://crrev.com/7decccec1fc39bf69e2fe287757f4c9fd8ff13a7
Cr-Commit-Position: refs/heads/master@{#373522}
==========

[commit-bot: I haz the power, 2016-02-04 13:27:01]

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