Stop sharing BaseSearchProvider::OnURLFetchComplete between 2 providers

Stop sharing BaseSearchProvider::OnURLFetchComplete between 2 providers

Sharing BaseSearchProvider::OnURLFetchComplete() between SearchProvider and ZeroSuggestProvider requires both providers to inherit a lot of virtual methods to customize the method's behavior.
This makes it difficult to track the code path to understand what is happening in OnURLFetchComplete with each provider.

Since SearchSuggestionParser was introduced and OnURLFetchComplete became slimmer, 
it's simpler to have OnURLFetchComplete() implementations in each provider and remove virtual protected methods.

BUG=None

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=288675

[Peter Kasting, 2014-08-08 17:33:24]

LGTM

https://codereview.chromium.org/436833002/diff/60001/chrome/browser/autocompl...
File chrome/browser/autocomplete/search_provider.cc (right):

https://codereview.chromium.org/436833002/diff/60001/chrome/browser/autocompl...
chrome/browser/autocomplete/search_provider.cc:686: suggest_results_pending_--;
Nit: Predecrement

https://codereview.chromium.org/436833002/diff/60001/chrome/browser/autocompl...
File chrome/browser/autocomplete/search_provider.h (right):

https://codereview.chromium.org/436833002/diff/60001/chrome/browser/autocompl...
chrome/browser/autocomplete/search_provider.h:170: void SortResults(bool
is_keyword, SearchSuggestionParser::Results* results);
Nit: Move this down below the virtual functions (and, while here, move
UpdateMatchContentsClass(), declared above, as well).  Let's keep things in
order of:

static functions
virtual functions
non-virtual functions

https://codereview.chromium.org/436833002/diff/60001/chrome/browser/autocompl...
chrome/browser/autocomplete/search_provider.h:185: // if suggested relevances
cause undesriable behavior. Updates |done_|.
Nit undesirable

https://codereview.chromium.org/436833002/diff/60001/chrome/browser/autocompl...
chrome/browser/autocomplete/search_provider.h:223: virtual void
OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
Nit: Move this up to be with the other virtual functions

https://codereview.chromium.org/436833002/diff/60001/chrome/browser/autocompl...
File chrome/browser/autocomplete/zero_suggest_provider.cc (right):

https://codereview.chromium.org/436833002/diff/60001/chrome/browser/autocompl...
chrome/browser/autocomplete/zero_suggest_provider.cc:317: std::string json_data
= SearchSuggestionParser::ExtractJsonData(source);
Nit: Would it make sense to have just these conditional contents be pulled out
as a base-class helper function, which would return the value of
|results_updated|?

https://codereview.chromium.org/436833002/diff/60001/chrome/browser/autocompl...
File chrome/browser/autocomplete/zero_suggest_provider.h (right):

https://codereview.chromium.org/436833002/diff/60001/chrome/browser/autocompl...
chrome/browser/autocomplete/zero_suggest_provider.h:87: const base::Value&
parsed_data);
Nit: See previous comments about function ordering.

https://codereview.chromium.org/436833002/diff/60001/chrome/browser/autocompl...
chrome/browser/autocomplete/zero_suggest_provider.h:112: virtual void
OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
Nit: Same nit.

[hashimoto, 2014-08-11 05:15:05]

https://codereview.chromium.org/436833002/diff/60001/chrome/browser/autocompl...
File chrome/browser/autocomplete/search_provider.cc (right):

https://codereview.chromium.org/436833002/diff/60001/chrome/browser/autocompl...
chrome/browser/autocomplete/search_provider.cc:686: suggest_results_pending_--;
On 2014/08/08 17:33:23, Peter Kasting wrote:
> Nit: Predecrement

Done.

https://codereview.chromium.org/436833002/diff/60001/chrome/browser/autocompl...
File chrome/browser/autocomplete/search_provider.h (right):

https://codereview.chromium.org/436833002/diff/60001/chrome/browser/autocompl...
chrome/browser/autocomplete/search_provider.h:170: void SortResults(bool
is_keyword, SearchSuggestionParser::Results* results);
On 2014/08/08 17:33:23, Peter Kasting wrote:
> Nit: Move this down below the virtual functions (and, while here, move
> UpdateMatchContentsClass(), declared above, as well).  Let's keep things in
> order of:
> 
> static functions
> virtual functions
> non-virtual functions

Done.

https://codereview.chromium.org/436833002/diff/60001/chrome/browser/autocompl...
chrome/browser/autocomplete/search_provider.h:185: // if suggested relevances
cause undesriable behavior. Updates |done_|.
On 2014/08/08 17:33:24, Peter Kasting wrote:
> Nit undesirable

Done.

https://codereview.chromium.org/436833002/diff/60001/chrome/browser/autocompl...
chrome/browser/autocomplete/search_provider.h:223: virtual void
OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
On 2014/08/08 17:33:23, Peter Kasting wrote:
> Nit: Move this up to be with the other virtual functions

Done.

https://codereview.chromium.org/436833002/diff/60001/chrome/browser/autocompl...
File chrome/browser/autocomplete/zero_suggest_provider.cc (right):

https://codereview.chromium.org/436833002/diff/60001/chrome/browser/autocompl...
chrome/browser/autocomplete/zero_suggest_provider.cc:317: std::string json_data
= SearchSuggestionParser::ExtractJsonData(source);
On 2014/08/08 17:33:24, Peter Kasting wrote:
> Nit: Would it make sense to have just these conditional contents be pulled out
> as a base-class helper function, which would return the value of
> |results_updated|?

I guess it doesn't benefit us much because ZeroSuggestProvider needs to call
StoreSuggestionResponse() with |json_data| and abort if it returns true.

https://codereview.chromium.org/436833002/diff/60001/chrome/browser/autocompl...
File chrome/browser/autocomplete/zero_suggest_provider.h (right):

https://codereview.chromium.org/436833002/diff/60001/chrome/browser/autocompl...
chrome/browser/autocomplete/zero_suggest_provider.h:87: const base::Value&
parsed_data);
On 2014/08/08 17:33:24, Peter Kasting wrote:
> Nit: See previous comments about function ordering.

Done.

https://codereview.chromium.org/436833002/diff/60001/chrome/browser/autocompl...
chrome/browser/autocomplete/zero_suggest_provider.h:112: virtual void
OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
On 2014/08/08 17:33:24, Peter Kasting wrote:
> Nit: Same nit.

Done.

[hashimoto, 2014-08-11 05:15:11]

The CQ bit was checked by hashimoto@chromium.org

[commit-bot: I haz the power, 2014-08-11 05:16:13]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hashimoto@chromium.org/436833002/120001

[commit-bot: I haz the power, 2014-08-11 09:15:23]

Change committed as 288675