Allow fetching personalized snippets from ChromeReader.

components/ntp_snippets/ntp_snippets_fetcher.h

 // Copyright 2016 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_NTP_SNIPPETS_NTP_SNIPPETS_FETCHER_H_
 #define COMPONENTS_NTP_SNIPPETS_NTP_SNIPPETS_FETCHER_H_
 
 #include <memory>
 #include <set>
 #include <string>
 
 #include "base/callback.h"
 #include "base/callback_list.h"
 #include "base/memory/weak_ptr.h"
+#include "components/signin/core/browser/signin_manager_base.h"
+#include "google_apis/gaia/oauth2_token_service.h"
+#include "net/http/http_request_headers.h"

[Marc Treib, 2016/05/04 11:50:10]

I think you can forward-declare this, and keep the include in the .cc

[jkrcal, 2016/05/04 14:04:09]

Done. SigninManagerBase as well.

 #include "net/url_request/url_fetcher_delegate.h"
 #include "net/url_request/url_request_context_getter.h"
 
 namespace ntp_snippets {
 
 // Fetches snippet data for the NTP from the server
-class NTPSnippetsFetcher : public net::URLFetcherDelegate {
+class NTPSnippetsFetcher : public OAuth2TokenService::Consumer,
+                           public OAuth2TokenService::Observer,
+                           public net::URLFetcherDelegate {
  public:
   // If problems occur (explained in |status_message|), |snippets_json| is
   // empty; otherwise, |status_message| is empty.
   using SnippetsAvailableCallback =
       base::Callback<void(const std::string& snippets_json,
                           const std::string& status_message)>;
   using SnippetsAvailableCallbackList =
       base::CallbackList<void(const std::string&, const std::string&)>;
 
   NTPSnippetsFetcher(
+      SigninManagerBase* signin_manager,

[Bernhard Bauer, 2016/05/04 11:46:58]

If you take ownership of these pointers, you should take a std::unique_ptr (or
at the very least document that you take ownership, but forget that immediately
again, because taking a unique_ptr is better anyway 😃). Then when you do that,
you should realize that you don't actually have ownership of the SigninManager
and the OAuthTokenService, because they are keyed services whose lifetime is
tied to the profile.

[jkrcal, 2016/05/04 14:04:09]

Done.

+      OAuth2TokenService* oauth2_token_service,
       scoped_refptr<net::URLRequestContextGetter> url_request_context_getter,
       bool is_stable_channel);
   ~NTPSnippetsFetcher() override;
 
   // Adds a callback that is called when a new set of snippets are downloaded.
   std::unique_ptr<SnippetsAvailableCallbackList::Subscription> AddCallback(
       const SnippetsAvailableCallback& callback) WARN_UNUSED_RESULT;
 
   // Fetches snippets from the server. |hosts| can be used to restrict the
   // results to a set of hosts, e.g. "www.google.com". If it is empty, no
   // restrictions are applied.
   //
   // If an ongoing fetch exists, it will be cancelled and a new one started,
   // without triggering additional callbacks (i.e. not noticeable by
   // subscribers).
-  void FetchSnippets(const std::set<std::string>& hosts, int count);
+  void FetchSnippets(const std::set<std::string>& hosts,
+                     const std::string& language_code,
+                     int count);
 
  private:
+  void FetchSnippetsImpl(const std::string& url,
+                         net::HttpRequestHeaders& headers,

[Marc Treib, 2016/05/04 11:50:10]

const please. If you need it to be non-const, use a pointer instead (or, better,
refactor the code so you don't need mutable parameters :) ).

[jkrcal, 2016/05/04 14:04:09]

Done.

+                         const std::string& request);
+  std::string GetHostsRestrict() const;
+  bool UseAuthenticated();

[Marc Treib, 2016/05/04 11:50:10]

Authenticated what? Either "UseAuthentication" or "UseAuthenticated<Something>"
please.

[jkrcal, 2016/05/04 14:04:09]

Done.

+  void FetchSnippetsNonAuthenticated();
+  void FetchSnippetsAuthenticated(const std::string& account_id,
+                                  const std::string& oauth_access_token);
+  void StartTokenRequest();
+
+  // OAuth2TokenService::Consumer overrides:
+  void OnGetTokenSuccess(const OAuth2TokenService::Request* request,
+                         const std::string& access_token,
+                         const base::Time& expiration_time) override;
+  void OnGetTokenFailure(const OAuth2TokenService::Request* request,
+                         const GoogleServiceAuthError& error) override;
+
+  // OAuth2TokenService::Observer overrides:
+  void OnRefreshTokenAvailable(const std::string& account_id) override;
+
   // URLFetcherDelegate implementation.
   void OnURLFetchComplete(const net::URLFetcher* source) override;
 
+  // The SequencedTaskRunner on which file system operations will be run.
+  scoped_refptr<base::SequencedTaskRunner> file_task_runner_;

[Bernhard Bauer, 2016/05/04 11:46:58]

Why do we add this again? Bad merge conflict?

[Marc Treib, 2016/05/04 11:50:10]

Doesn't seem to be needed?

[jkrcal, 2016/05/04 14:04:09]

Done.

[jkrcal, 2016/05/04 14:04:09]

Sorry about that, a merging error of mine.

+
   // Holds the URL request context.
   scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_;
 
+  // Hosts to restrict the snippets to

[Marc Treib, 2016/05/04 11:50:10]

nit: Periods at the end of comments please.

[jkrcal, 2016/05/04 14:04:09]

Done.

+  std::set<std::string> hosts_;
+
+  // Count of snippets to fetch
+  int count_;
+
+  // Language code to restrict to for personalized results
+  std::string locale_;
+
   // The fetcher for downloading the snippets.
   std::unique_ptr<net::URLFetcher> url_fetcher_;
 
   // The callbacks to notify when new snippets get fetched.
   SnippetsAvailableCallbackList callback_list_;
 
+  // Authorization for signed-in users
+  std::unique_ptr<SigninManagerBase> signin_manager_;
+  std::unique_ptr<OAuth2TokenService> token_service_;
+  std::unique_ptr<OAuth2TokenService::Request> oauth_request_;
+  bool waiting_for_refresh_token_;
+
   // Flag for picking the right (stable/non-stable) API key for Chrome Reader
   bool is_stable_channel_;
 
+  // The variant of the fetching to use.
+  std::string variant_;

[Bernhard Bauer, 2016/05/04 11:46:58]

I would store this as an enum and only use the string values for getting the
parameter from variations.

[Marc Treib, 2016/05/04 11:50:10]

Make this an enum?

[jkrcal, 2016/05/04 14:04:09]

Done.

[jkrcal, 2016/05/04 14:04:09]

Done.

+
   DISALLOW_COPY_AND_ASSIGN(NTPSnippetsFetcher);
 };
 }  // namespace ntp_snippets
 
 #endif  // COMPONENTS_NTP_SNIPPETS_NTP_SNIPPETS_FETCHER_H_