Making OAuth2TokenService multi-login aware, updating callers, minor fixes

chrome/browser/managed_mode/managed_user_refresh_token_fetcher.h

 // Copyright 2013 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 CHROME_BROWSER_MANAGED_MODE_MANAGED_USER_REFRESH_TOKEN_FETCHER_H_
 #define CHROME_BROWSER_MANAGED_MODE_MANAGED_USER_REFRESH_TOKEN_FETCHER_H_
 
 #include <string>
 
 #include "base/callback_forward.h"
@@ skipping 19 matching lines @@
 //      user and return it to the caller. The refresh token can only be used to
 //      mint tokens with the special managed user Sync scope.
 class ManagedUserRefreshTokenFetcher {
  public:
   typedef base::Callback<void(const GoogleServiceAuthError& /* error */,
                               const std::string& /* refresh_token */)>
       TokenCallback;
 
   static scoped_ptr<ManagedUserRefreshTokenFetcher> Create(
       OAuth2TokenService* oauth2_token_service,
+      const std::string& account_id,

[Andrew T Wilson (Slow), 2013/09/03 14:04:24]

This is OK and is consistent with the API changes we're making, but note that
ManagedUsers don't really have account_ids, so we'll have to invent an account
ID for this class (similar to how we had to invent an account ID for
DeviceOAuth2TokenService) - this is yet another indication that OA2TS really
doesn't want to be multi-account aware because there are at least two instances
where account_id doesn't make sense.

Not a huge deal - just a minor API wart that probably doesn't warrant doing a
big refactoring, but wanted to point it out.

The alternative would be to have multiple OAuth2TokenService instances
associated with a single profile, and instead of adding plumbing where we pass
an account_id through all of these APIs, we instead have something like this:

ProfileOAuth2TokenServiceManager* manager =
ProfileOAuth2TokenServiceManager::GetForProfile(profile);
ProfileOAuth2TokenService* service =
manager->GetTokenServiceForAccount(manager->GetPrimaryAccountId());
if (service) {
  // This account is signed in...
  service->StartRequest(scopes, consumer);
}

That way we don't have account ID polluting the StartRequest() API when many of
the subclasses don't support the concept of accounts.

Again, just a suggestion - we can definitely live with it the way it is.

[Roger Tawa OOO till Jul 10th, 2013/09/03 21:06:42]

Agreed that the |account_id| is not needed here, since its always the account
connected to the profile that counts, i.e., the primary account.  I don't think
this has any bearing on the O2TS api though.

The OAuth2TokenService today is acting like both the manager you describe and
token service instances.  But as you say, I don't think this warrants a big
refactoring.

[fgorski, 2013/09/03 23:29:25]

If we decide not to go with account_id here we'll have to use
ProfileOAuth2TokenService instead of OAuth2TokenService. I think it is best to
make a change once bauerb comes up with ManagedUserOAuth2TokenService and
exactly what is needed then.

       net::URLRequestContextGetter* context);
 
   virtual ~ManagedUserRefreshTokenFetcher();
 
   virtual void Start(const std::string& managed_user_id,
                      const std::string& device_name,
                      const TokenCallback& callback) = 0;
 };
 
 #endif  // CHROME_BROWSER_MANAGED_MODE_MANAGED_USER_REFRESH_TOKEN_FETCHER_H_