Add ContextualSuggestionsService to Omnibox

components/omnibox/browser/zero_suggest_provider.cc

Index: components/omnibox/browser/zero_suggest_provider.cc
diff --git a/components/omnibox/browser/zero_suggest_provider.cc b/components/omnibox/browser/zero_suggest_provider.cc
index 45e7d86b1172c646767a6874edd5866065ef9424..440ef96e850dac272a8cd5cf3300805bca77ff1e 100644
--- a/components/omnibox/browser/zero_suggest_provider.cc
+++ b/components/omnibox/browser/zero_suggest_provider.cc
@@ -85,32 +85,6 @@ const int kDefaultZeroSuggestRelevance = 100;
 // Used for testing whether zero suggest is ever available.
 constexpr char kArbitraryInsecureUrlString[] = "http://www.google.com/";
 
-// Returns true if the folowing conditions are valid:
-// * The |default_provider| is Google.
-// * The user is in the ZeroSuggestRedirectToChrome field trial.
-// * The field trial provides a valid server address where the suggest request
-//   is redirected.
-// * The suggest request is over HTTPS. This avoids leaking the current page URL
-//   or personal data in unencrypted network traffic.
-// Note: these checks are in addition to CanSendUrl() on the default contextual
-// suggestion URL.
-bool UseExperimentalSuggestService(const TemplateURLService& default_provider) {
-  const TemplateURL& default_provider_url =
-      *default_provider.GetDefaultSearchProvider();
-  const SearchTermsData& search_terms_data =
-      default_provider.search_terms_data();
-  if ((default_provider_url.GetEngineType(search_terms_data) !=
-       SEARCH_ENGINE_GOOGLE) ||
-      !OmniboxFieldTrial::InZeroSuggestRedirectToChromeFieldTrial())
-    return false;
-  // Check that the suggest URL for redirect to chrome field trial is valid.
-  const GURL suggest_url(
-      OmniboxFieldTrial::ZeroSuggestRedirectToChromeServerAddress());
-  if (!suggest_url.is_valid())
-    return false;
-  return suggest_url.SchemeIsCryptographic();
-}
-
 }  // namespace
 
 // static
@@ -177,20 +151,31 @@ void ZeroSuggestProvider::Start(const AutocompleteInput& input,
   if (can_send_current_url &&
       !OmniboxFieldTrial::InZeroSuggestPersonalizedFieldTrial() &&
       !OmniboxFieldTrial::InZeroSuggestMostVisitedFieldTrial()) {
-    // Update suggest_url to include the current_page_url.
-    if (UseExperimentalSuggestService(*template_url_service)) {
-      suggest_url = GURL(
-          OmniboxFieldTrial::ZeroSuggestRedirectToChromeServerAddress() +
-          "/url=" + net::EscapePath(current_query_) +
-          OmniboxFieldTrial::ZeroSuggestRedirectToChromeAdditionalFields());
-    } else {
-      base::string16 prefix;
-      TemplateURLRef::SearchTermsArgs search_term_args(prefix);
-      search_term_args.current_page_url = current_query_;
-      suggest_url =
-          GURL(default_provider->suggestions_url_ref().ReplaceSearchTerms(
-              search_term_args, template_url_service->search_terms_data()));
+    // Maybe redirect the request to the service maintained by Chrome, which
+    // provides experimental contextual suggestions.
+    contextual_suggestions::ContextualSuggestionsService*
+        contextual_suggestions_service =
+            client()->GetContextualSuggestionsService();
+    if (contextual_suggestions_service != nullptr &&
+        contextual_suggestions_service
+            ->UseExperimentalZeroSuggestSuggestions()) {
+      // Create a request for experimental suggestions with |this| as the
+      // fether delagate.

[Marc Treib, 2017/07/06 08:32:21]

s/fether delagate/fetcher delegate/

[gcomanici, 2017/07/06 15:34:18]

Done.

+      contextual_suggestions_service->CreateContextualSuggestionsRequest(
+          current_query_, /*fetcher_delegate=*/this,
+          base::BindOnce(
+              &ZeroSuggestProvider::OnContextualSuggestionsFetcherAvailable,
+              base::Unretained(this)));

[Marc Treib, 2017/07/06 08:32:21]

Is Unretained() safe to use here? If so, please add a comment explaining why. If
not, use a WeakPtr instead.

[gcomanici, 2017/07/06 15:34:19]

I am not sure how to answer your question, so I will go with WeakPtr instead to
be safe.

[Marc Treib, 2017/07/06 16:02:45]

Unretained is only safe if you're absolutely sure that the callback won't be
called after "this" has been destroyed. Maybe that's the case here, I can't
tell, but anyway weakptr doesn't hurt.

[gcomanici, 2017/07/06 16:54:16]

Thanks. Indeed, we should go with weakptr.

+      return;
     }
+
+    // Update suggest_url to include the current_page_url.
+    base::string16 prefix;
+    TemplateURLRef::SearchTermsArgs search_term_args(prefix);
+    search_term_args.current_page_url = current_query_;
+    suggest_url =
+        GURL(default_provider->suggestions_url_ref().ReplaceSearchTerms(
+            search_term_args, template_url_service->search_terms_data()));
   } else if (!ShouldShowNonContextualZeroSuggest(input.current_url())) {
     return;
   }
@@ -203,6 +188,16 @@ void ZeroSuggestProvider::Start(const AutocompleteInput& input,
   Run(suggest_url);
 }
 
+void ZeroSuggestProvider::OnContextualSuggestionsFetcherAvailable(
+    std::unique_ptr<net::URLFetcher> fetcher) {
+  // fetcher->Start();

[Marc Treib, 2017/07/06 08:32:21]

?

[gcomanici, 2017/07/06 15:34:19]

Removed.

+  if (fetcher == nullptr)
+    return;
+  fetcher_ = std::move(fetcher);
+  done_ = false;
+  fetcher_->Start();
+}
+
 void ZeroSuggestProvider::Stop(bool clear_cached_results,
                                bool due_to_user_inactivity) {
   if (fetcher_)