Signin/OAuth: Create an AccessTokenFetcher helper class

components/signin/core/browser/access_token_fetcher.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_SIGNIN_CORE_BROWSER_ACCESS_TOKEN_FETCHER_H_
+#define COMPONENTS_SIGNIN_CORE_BROWSER_ACCESS_TOKEN_FETCHER_H_
+
+#include <memory>
+#include <string>
+
+#include "base/callback.h"
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "google_apis/gaia/oauth2_token_service.h"
+
+class SigninManagerBase;
+
+// Helper class to ease the task of obtaining an OAuth2 access token. This
+// handles various special cases, e.g. when the refresh token isn't loaded yet
+// (during startup).
+// May only be used on the UI thread.
+class AccessTokenFetcher : public OAuth2TokenService::Observer,
+                           public OAuth2TokenService::Consumer {
+ public:
+  using TokenCallback = base::Callback<void(const std::string&)>;
+
+  // Instantiates a fetcher and immediately starts the process of obtaining an
+  // OAuth2 access token for the given |scopes|. The |callback| will be called
+  // with the access token on success, or with an empty string on failure. If
+  // the AccessTokenFetcher is destroyed before the process completes, the
+  // callback will not be called.
+  AccessTokenFetcher(const std::string& oauth_consumer_name,
+                     const SigninManagerBase* signin_manager,
+                     OAuth2TokenService* token_service,
+                     const OAuth2TokenService::ScopeSet& scopes,
+                     const TokenCallback& callback);

[msarda, 2017/01/18 21:55:19]

I would expect 2 things:
* either the token callback takes 2 arguments (std:sting token, GaiaAuthError
error).
or
* we have 2 callbacks (one for the success and one for the error cases).

I think for simplicity, I would prefer the case with a callback with 2
arguments.

[Marc Treib, 2017/01/19 15:03:56]

I assume you mean GoogleServiceAuthError? The issue is that not all the error
cases here can be properly expressed by that, e.g. the "you're not signed in"
case (if we're keeping that), or the "we didn't get a refresh token" case.

[msarda, 2017/01/23 17:08:09]

Yes, this is what I had in mind. I still think we need some API to pass the
error to the caller here.

+
+  ~AccessTokenFetcher() override;
+
+ private:
+  void Start();
+
+  void NotSignedInCallback();

[msarda, 2017/01/18 21:55:19]

This method names is very unclear.

[Marc Treib, 2017/01/19 15:03:56]

Renamed to "RunCallbackNotSignedIn" - better?

+
+  // OAuth2TokenService::Observer implementation.
+  void OnRefreshTokenAvailable(const std::string& account_id) override;
+  void OnRefreshTokensLoaded() override;
+
+  void StartAccessTokenRequest();

[msarda, 2017/01/18 21:55:19]

Move this method next to the NotSignedInCallback as it is a private method that
is not overridden from a superclass.

[Marc Treib, 2017/01/19 15:03:56]

Done.

+
+  // OAuth2TokenService::Consumer implementation.
+  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;
+
+  const SigninManagerBase* signin_manager_;
+  OAuth2TokenService* token_service_;
+  OAuth2TokenService::ScopeSet scopes_;
+
+  TokenCallback callback_;
+
+  bool waiting_for_refresh_token_ = false;

[msarda, 2017/01/18 21:55:19]

I think we prefer values to be initialized on the constructor.

[Marc Treib, 2017/01/19 15:03:56]

AFAIK there's no general preference for one over the other. But I don't mind
either way, so done.

+
+  std::unique_ptr<OAuth2TokenService::Request> access_token_request_;
+
+  // When a token request gets canceled, we want to retry once.
+  bool access_token_retried_ = false;
+
+  base::WeakPtrFactory<AccessTokenFetcher> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(AccessTokenFetcher);
+};
+
+#endif  // COMPONENTS_SIGNIN_CORE_BROWSER_ACCESS_TOKEN_FETCHER_H_