Prepare AutocompleteController for componentization

Prepare AutocompleteController for componentization

Minor abstractions required as well as changing AutocompleteController to take
in a scoped_ptr<AutocompleteProviderClient> rather than constructing one
internally.

BUG=371538
TBR=miguelg

Committed: https://crrev.com/b11c2787e59eedf657e63d03cd3d24efa067a0f9
Cr-Commit-Position: refs/heads/master@{#335482}

[blundell, 2015-06-19 12:49:44]

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

[blundell, 2015-06-19 12:49:45]

You were correct that in the end a factory was not worth the trouble.

[Peter Kasting, 2015-06-19 19:19:22]

LGTM

https://codereview.chromium.org/1192373002/diff/20001/chrome/browser/autocomp...
File chrome/browser/autocomplete/autocomplete_controller.cc (right):

https://codereview.chromium.org/1192373002/diff/20001/chrome/browser/autocomp...
chrome/browser/autocomplete/autocomplete_controller.cc:183: if (provider_types &
AutocompleteProvider::TYPE_HISTORY_QUICK)
Nit: Why blank lines above and below this particular block?

https://codereview.chromium.org/1192373002/diff/20001/chrome/browser/autocomp...
File chrome/browser/autocomplete/chrome_autocomplete_provider_client.cc (right):

https://codereview.chromium.org/1192373002/diff/20001/chrome/browser/autocomp...
chrome/browser/autocomplete/chrome_autocomplete_provider_client.cc:164: void
ChromeAutocompleteProviderClient::ConfigureKeywordProvider(
Nit: Why not instead have something like

scoped_ptr<KeywordExtensionsDelegate>
GetKeywordExtensionsDelegate(KeywordProvider* provider);

... and when ENABLE_EXTENSIONS is off, this can return null; then have the
keyword provider call this itself?  That will eliminate the need of the
set_extensions_delegate() public method on that provider and make the API of
this class a bit easier to use and more parallel to other getters.

https://codereview.chromium.org/1192373002/diff/20001/chrome/browser/autocomp...
chrome/browser/autocomplete/chrome_autocomplete_provider_client.cc:168:
scoped_ptr<KeywordExtensionsDelegate>(
Nit: make_scoped_ptr?

https://codereview.chromium.org/1192373002/diff/20001/chrome/browser/autocomp...
File chrome/browser/autocomplete/search_provider_unittest.cc (right):

https://codereview.chromium.org/1192373002/diff/20001/chrome/browser/autocomp...
chrome/browser/autocomplete/search_provider_unittest.cc:1132:
make_scoped_ptr(new ChromeAutocompleteProviderClient(&profile_)), NULL,
Nit: While here: nullptr

[blundell, 2015-06-22 08:50:28]

Patchset #4 (id:60001) has been deleted

[blundell, 2015-06-22 08:58:59]

Patchset #4 (id:80001) has been deleted

[blundell, 2015-06-22 09:01:36]

blundell@chromium.org changed reviewers:
+ miguelg@chromium.org

[blundell, 2015-06-22 09:01:37]

TBR=miguelg for //chrome/browser/android/omnibox

https://codereview.chromium.org/1192373002/diff/20001/chrome/browser/autocomp...
File chrome/browser/autocomplete/autocomplete_controller.cc (right):

https://codereview.chromium.org/1192373002/diff/20001/chrome/browser/autocomp...
chrome/browser/autocomplete/autocomplete_controller.cc:183: if (provider_types &
AutocompleteProvider::TYPE_HISTORY_QUICK)
On 2015/06/19 19:19:21, Peter Kasting wrote:
> Nit: Why blank lines above and below this particular block?

Done.

https://codereview.chromium.org/1192373002/diff/20001/chrome/browser/autocomp...
File chrome/browser/autocomplete/chrome_autocomplete_provider_client.cc (right):

https://codereview.chromium.org/1192373002/diff/20001/chrome/browser/autocomp...
chrome/browser/autocomplete/chrome_autocomplete_provider_client.cc:164: void
ChromeAutocompleteProviderClient::ConfigureKeywordProvider(
On 2015/06/19 19:19:21, Peter Kasting wrote:
> Nit: Why not instead have something like
> 
> scoped_ptr<KeywordExtensionsDelegate>
> GetKeywordExtensionsDelegate(KeywordProvider* provider);
> 
> ... and when ENABLE_EXTENSIONS is off, this can return null; then have the
> keyword provider call this itself?  That will eliminate the need of the
> set_extensions_delegate() public method on that provider and make the API of
> this class a bit easier to use and more parallel to other getters.

Done.

https://codereview.chromium.org/1192373002/diff/20001/chrome/browser/autocomp...
chrome/browser/autocomplete/chrome_autocomplete_provider_client.cc:168:
scoped_ptr<KeywordExtensionsDelegate>(
On 2015/06/19 19:19:21, Peter Kasting wrote:
> Nit: make_scoped_ptr?

Done.

https://codereview.chromium.org/1192373002/diff/20001/chrome/browser/autocomp...
File chrome/browser/autocomplete/search_provider_unittest.cc (right):

https://codereview.chromium.org/1192373002/diff/20001/chrome/browser/autocomp...
chrome/browser/autocomplete/search_provider_unittest.cc:1132:
make_scoped_ptr(new ChromeAutocompleteProviderClient(&profile_)), NULL,
On 2015/06/19 19:19:21, Peter Kasting wrote:
> Nit: While here: nullptr

Done.

[blundell, 2015-06-22 09:02:04]

The CQ bit was checked by blundell@chromium.org

[blundell, 2015-06-22 09:02:05]

The patchset sent to the CQ was uploaded after l-g-t-m from
pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/1192373002/#ps100001
(title: "Response to review")

[commit-bot: I haz the power, 2015-06-22 09:02:09]

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

[commit-bot: I haz the power, 2015-06-22 10:05:22]

Committed patchset #4 (id:100001)

[commit-bot: I haz the power, 2015-06-22 10:06:05]

Patchset 4 (id:??) landed as
https://crrev.com/b11c2787e59eedf657e63d03cd3d24efa067a0f9
Cr-Commit-Position: refs/heads/master@{#335482}