Add ContextualSuggestionsService to Omnibox

components/contextual_suggestions/contextual_suggestions_service.h

+// Copyright 2017 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.
+
+#ifndef COMPONENTS_CONTEXTUAL_SUGGESTIONS_SERVICE_H_
+#define COMPONENTS_CONTEXTUAL_SUGGESTIONS_SERVICE_H_

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

There should be an extra CONTEXTUAL_SUGGESTIONS_ here (one for the folder, one
for the file name)

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

Done.

+

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

nit: include <string>, <memory>

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

Done.

+#include "base/feature_list.h"
+#include "base/strings/stringprintf.h"

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

Not needed I think?

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

Removed.

+#include "components/keyed_service/core/keyed_service.h"
+#include "components/search_engines/template_url_service.h"
+#include "components/signin/core/browser/access_token_fetcher.h"
+#include "components/signin/core/browser/signin_manager_base.h"
+#include "google_apis/gaia/oauth2_token_service.h"
+#include "net/url_request/url_fetcher_delegate.h"
+#include "net/url_request/url_request_context_getter.h"
+#include "url/gurl.h"
+
+namespace contextual_suggestions {
+
+// Feature used for using an experimental zero suggest suggestions service
+// for omnibox. This feature redirects requests to a service maintained by
+// the Chrome team.
+extern const base::Feature kZeroSuggestRedirectToChrome;

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

It's common practice to put features into a separate features.h file.

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

Done.

+
+class ContextualSuggestionsService : public KeyedService {
+ public:
+  ContextualSuggestionsService(SigninManagerBase* signin_manager,
+                               OAuth2TokenService* token_service,
+                               TemplateURLService* template_url_service,
+                               net::URLRequestContextGetter* request_context);
+
+  ~ContextualSuggestionsService() override;
+
+  // 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

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

nit: weird line break

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

Fixed.

+  //   or personal data in unencrypted network traffic.
+  // Note: these checks are in addition to CanSendUrl() on the default
+  // contextual suggestion URL.
+  bool UseExperimentalZeroSuggestSuggestions() const;
+
+  using ContextualSuggestionsCallback =
+      base::OnceCallback<void(std::unique_ptr<net::URLFetcher> fetcher)>;
+  // Creates a fetch request for experimental contextual suggestions for
+  // |visited_url|, with |fetch_delegate| as the URLFetherDelegate that will

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

s/fetch_delegate/fetcher_delegate/
s/URLFetherDelegate/URLFetcherDelegate/

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

Done.

+  // process the response.
+  //
+  // Upon successful creation, the function returns |true| and |callback| is
+  // executed with the result as a parameter.

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

Sync or async?

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

sync. I changed the comments to make this clear.

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

No, I don't think that's correct. There are some error cases where it's sync,
but fetching the access token is async. So if you get to the point of trying to
get an access token, then it'll be async.

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

You are right. I made the comment a bit more explicit in that regard. PTAL.

+  //
+  // The function returns |false| if the service is already waiting for a
+  // authenticaiton token.
+  //
+  // The function returns |false| if the service is suggest URL used to redirect

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

Remove "service is"? (I don't understand this sentence)

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

I changed the statement to make it more clear.

+  // the request is invalid.
+  bool CreateContextualSuggestionsRequest(
+      const std::string& visited_url,
+      net::URLFetcherDelegate* fetcher_delegate,
+      ContextualSuggestionsCallback callback);
+
+ private:
+  // Returns a string representing the address of the server where the zero
+  // suggest requests are being redirected.
+  static GURL ExperimentalZeroSuggestURL(const std::string visited_url);

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

&

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

Done.

+
+  // Creates an HTTTP Get request for experimental contextual suggestions. The
+  // return value does not include a header corresponding to an authorization
+  // token.
+  std::unique_ptr<net::URLFetcher> CreateRequest(
+      const std::string& visited_url,
+      net::URLFetcherDelegate* fetcher_delegate);

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

const?

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

Added const.

+
+  // Called when an access token request completes (successfully or not).
+  void AccessTokenAvailable(std::unique_ptr<net::URLFetcher> fetcher,
+                            ContextualSuggestionsCallback callback,
+                            const GoogleServiceAuthError& error,
+                            const std::string& access_token);
+
+  net::URLRequestContextGetter* request_context_;
+  SigninManagerBase* signin_manager_;
+  TemplateURLService* template_url_service_;
+  OAuth2TokenService* token_service_;
+
+  // Helper for fetching OAuth2 access tokens. This is non-null iff an access
+  // token request is currently in progress.
+  std::unique_ptr<AccessTokenFetcher> token_fetcher_;
+};
+
+}  // namespace contextual_suggestions
+
+#endif  // COMPONENTS_CONTEXTUAL_SUGGESTIONS_SERVICE_H_