Stop sharing BaseSearchProvider::OnURLFetchComplete between 2 providers

chrome/browser/autocomplete/zero_suggest_provider.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/autocomplete/zero_suggest_provider.h"
 
 #include "base/callback.h"
 #include "base/i18n/case_conversion.h"
 #include "base/json/json_string_value_serializer.h"
 #include "base/metrics/histogram.h"
@@ skipping 148 matching lines @@
 void ZeroSuggestProvider::ModifyProviderInfo(
     metrics::OmniboxEventProto_ProviderInfo* provider_info) const {
   if (!results_.suggest_results.empty() || !results_.navigation_results.empty())
     provider_info->set_times_returned_results_in_session(1);
 }
 
 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) {
 }
 
 ZeroSuggestProvider::~ZeroSuggestProvider() {
 }
 
 bool ZeroSuggestProvider::StoreSuggestionResponse(
     const std::string& json_data,
     const base::Value& parsed_data) {
@@ skipping 24 matching lines @@
   return template_url_service_->GetDefaultSearchProvider();
 }
 
 const AutocompleteInput ZeroSuggestProvider::GetInput(bool is_keyword) const {
   return AutocompleteInput(
       base::string16(), base::string16::npos, base::string16(),
       GURL(current_query_), current_page_classification_, true, false, false,
       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.
   return true;
 }
 
 void ZeroSuggestProvider::StopSuggest() {
-  if (suggest_results_pending_ > 0)
+  if (fetcher_)
     LogOmniboxZeroSuggestRequest(ZERO_SUGGEST_REQUEST_INVALIDATED);
-  suggest_results_pending_ = 0;
   fetcher_.reset();
 }
 
 void ZeroSuggestProvider::ClearAllResults() {
   // We do not call Clear() on |results_| to retain |verbatim_relevance|
   // value in the |results_| object. |verbatim_relevance| is used at the
   // beginning of the next StartZeroSuggest() call to determine the current url
   // match relevance.
   results_.suggest_results.clear();
   results_.navigation_results.clear();
   current_query_.clear();
 }
 
-int ZeroSuggestProvider::GetDefaultResultRelevance() const {
-  return kDefaultZeroSuggestRelevance;
-}
-
 void ZeroSuggestProvider::RecordDeletionResult(bool success) {
   if (success) {
     content::RecordAction(
         base::UserMetricsAction("Omnibox.ZeroSuggestDelete.Success"));
   } else {
     content::RecordAction(
         base::UserMetricsAction("Omnibox.ZeroSuggestDelete.Failure"));
   }
 }
 
-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) {
   for (size_t i = 0; i < results.size(); ++i)
     AddMatchToMap(results[i], std::string(), i, false, map);
 }
 
 AutocompleteMatch ZeroSuggestProvider::NavigationToMatch(
     const SearchSuggestionParser::NavigationResult& navigation) {
   AutocompleteMatch match(this, navigation.relevance(), false,
@@ skipping 15 matching lines @@
 
   match.description =
       AutocompleteMatch::SanitizeString(navigation.description());
   AutocompleteMatch::ClassifyLocationInString(base::string16::npos, 0,
       match.description.length(), ACMatchClassification::NONE,
       &match.description_class);
   return match;
 }
 
 void ZeroSuggestProvider::Run(const GURL& suggest_url) {
-  suggest_results_pending_ = 0;
   const int kFetcherID = 1;
   fetcher_.reset(
       net::URLFetcher::Create(kFetcherID,
           suggest_url,
           net::URLFetcher::GET, this));
   fetcher_->SetRequestContext(profile_->GetRequestContext());
   fetcher_->SetLoadFlags(net::LOAD_DO_NOT_SAVE_COOKIES);
   // Add Chrome experiment state to the request headers.
   net::HttpRequestHeaders headers;
   variations::VariationsHttpHeaderProvider::GetInstance()->AppendHeaders(
       fetcher_->GetOriginalURL(), profile_->IsOffTheRecord(), false, &headers);
   fetcher_->SetExtraRequestHeaders(headers.ToString());
   fetcher_->Start();
 
   if (OmniboxFieldTrial::InZeroSuggestMostVisitedFieldTrial()) {
     most_visited_urls_.clear();
     history::TopSites* ts = profile_->GetTopSites();
     if (ts) {
       ts->GetMostVisitedURLs(
           base::Bind(&ZeroSuggestProvider::OnMostVisitedUrlsAvailable,
                      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;
 }
 
 void ZeroSuggestProvider::ConvertResultsToAutocompleteMatches() {
   matches_.clear();
 
   const TemplateURL* default_provider =
       template_url_service_->GetDefaultSearchProvider();
@@ skipping 105 matching lines @@
 
 void ZeroSuggestProvider::MaybeUseCachedSuggestions() {
   if (!OmniboxFieldTrial::InZeroSuggestPersonalizedFieldTrial())
     return;
 
   std::string json_data = profile_->GetPrefs()->GetString(
       prefs::kZeroSuggestCachedResults);
   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();
     }
   }
 }