Stop sharing BaseSearchProvider::OnURLFetchComplete between 2 providers

chrome/browser/autocomplete/zero_suggest_provider.cc

Index: chrome/browser/autocomplete/zero_suggest_provider.cc
diff --git a/chrome/browser/autocomplete/zero_suggest_provider.cc b/chrome/browser/autocomplete/zero_suggest_provider.cc
index 709fa4c745ac9ba9c3af21c072cd34a680876de8..b48cc0aad14e1ff718e126e045f8cd0b7dbe491a 100644
--- a/chrome/browser/autocomplete/zero_suggest_provider.cc
+++ b/chrome/browser/autocomplete/zero_suggest_provider.cc
@@ -166,8 +166,9 @@ ZeroSuggestProvider::ZeroSuggestProvider(
   AutocompleteProviderListener* listener,
   TemplateURLService* template_url_service,
   Profile* profile)
-    : BaseSearchProvider(listener, template_url_service, profile,
+    : BaseSearchProvider(template_url_service, profile,
                          AutocompleteProvider::TYPE_ZERO_SUGGEST),
+      listener_(listener),
       results_from_cache_(false),
       weak_ptr_factory_(this) {
 }
@@ -212,12 +213,6 @@ const AutocompleteInput ZeroSuggestProvider::GetInput(bool is_keyword) const {
       true, ChromeAutocompleteSchemeClassifier(profile_));
 }
 
-SearchSuggestionParser::Results* ZeroSuggestProvider::GetResultsToFill(
-    bool is_keyword) {
-  DCHECK(!is_keyword);
-  return &results_;
-}
-
 bool ZeroSuggestProvider::ShouldAppendExtraParams(
       const SearchSuggestionParser::SuggestResult& result) const {
   // We always use the default provider for search, so append the params.
@@ -225,9 +220,8 @@ bool ZeroSuggestProvider::ShouldAppendExtraParams(
 }
 
 void ZeroSuggestProvider::StopSuggest() {
-  if (suggest_results_pending_ > 0)
+  if (fetcher_)
     LogOmniboxZeroSuggestRequest(ZERO_SUGGEST_REQUEST_INVALIDATED);
-  suggest_results_pending_ = 0;
   fetcher_.reset();
 }
 
@@ -241,10 +235,6 @@ void ZeroSuggestProvider::ClearAllResults() {
   current_query_.clear();
 }
 
-int ZeroSuggestProvider::GetDefaultResultRelevance() const {
-  return kDefaultZeroSuggestRelevance;
-}
-
 void ZeroSuggestProvider::RecordDeletionResult(bool success) {
   if (success) {
     content::RecordAction(
@@ -255,22 +245,6 @@ void ZeroSuggestProvider::RecordDeletionResult(bool success) {
   }
 }
 
-void ZeroSuggestProvider::LogFetchComplete(bool success, bool is_keyword) {
-  LogOmniboxZeroSuggestRequest(ZERO_SUGGEST_REPLY_RECEIVED);
-}
-
-bool ZeroSuggestProvider::IsKeywordFetcher(
-    const net::URLFetcher* fetcher) const {
-  // ZeroSuggestProvider does not have a keyword provider.
-  DCHECK_EQ(fetcher, fetcher_.get());
-  return false;
-}
-
-void ZeroSuggestProvider::UpdateMatches() {
-  done_ = true;
-  ConvertResultsToAutocompleteMatches();
-}
-
 void ZeroSuggestProvider::AddSuggestResultsToMap(
     const SearchSuggestionParser::SuggestResults& results,
     MatchMap* map) {
@@ -306,7 +280,6 @@ AutocompleteMatch ZeroSuggestProvider::NavigationToMatch(
 }
 
 void ZeroSuggestProvider::Run(const GURL& suggest_url) {
-  suggest_results_pending_ = 0;
   const int kFetcherID = 1;
   fetcher_.reset(
       net::URLFetcher::Create(kFetcherID,
@@ -330,10 +303,33 @@ void ZeroSuggestProvider::Run(const GURL& suggest_url) {
                      weak_ptr_factory_.GetWeakPtr()), false);
     }
   }
-  suggest_results_pending_ = 1;
   LogOmniboxZeroSuggestRequest(ZERO_SUGGEST_REQUEST_SENT);
 }
 
+void ZeroSuggestProvider::OnURLFetchComplete(const net::URLFetcher* source) {
+  DCHECK(!done_);
+  DCHECK_EQ(fetcher_.get(), source);
+
+  LogOmniboxZeroSuggestRequest(ZERO_SUGGEST_REPLY_RECEIVED);
+
+  bool results_updated = false;
+  if (source->GetStatus().is_success() && source->GetResponseCode() == 200) {
+    std::string json_data = SearchSuggestionParser::ExtractJsonData(source);

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

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|?

[hashimoto, 2014/08/11 05:15:04]

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

+    scoped_ptr<base::Value> data(
+        SearchSuggestionParser::DeserializeJsonData(json_data));
+    if (data) {
+      if (StoreSuggestionResponse(json_data, *data))
+        return;
+      results_updated = ParseSuggestResults(
+          *data, kDefaultZeroSuggestRelevance, false, &results_);
+    }
+  }
+  fetcher_.reset();
+  done_ = true;
+  ConvertResultsToAutocompleteMatches();
+  listener_->OnProviderUpdate(results_updated);
+}
+
 void ZeroSuggestProvider::OnMostVisitedUrlsAvailable(
     const history::MostVisitedURLList& urls) {
   most_visited_urls_ = urls;
@@ -459,7 +455,8 @@ void ZeroSuggestProvider::MaybeUseCachedSuggestions() {
   if (!json_data.empty()) {
     scoped_ptr<base::Value> data(
         SearchSuggestionParser::DeserializeJsonData(json_data));
-    if (data && ParseSuggestResults(*data.get(), false, &results_)) {
+    if (data && ParseSuggestResults(
+            *data, kDefaultZeroSuggestRelevance, false, &results_)) {
       ConvertResultsToAutocompleteMatches();
       results_from_cache_ = !matches_.empty();
     }