Refactor AO2TS to make it easier to componentize.

chrome/browser/chromeos/settings/device_oauth2_token_service.cc

 // 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.
 
 #include "chrome/browser/chromeos/settings/device_oauth2_token_service.h"
 
 #include <string>
 #include <vector>
 
 #include "base/bind.h"
@@ skipping 25 matching lines @@
         client_id(client_id),
         client_secret(client_secret),
         scopes(scopes) {}
 
   const base::WeakPtr<RequestImpl> request;
   const std::string client_id;
   const std::string client_secret;
   const ScopeSet scopes;
 };
 
+DeviceOAuth2TokenService::DeviceOAuth2TokenServiceDelegate::

[Mattias Nissler (ping if slow), 2015/06/29 08:20:06]

It's pretty odd that the CL introduces a delegate interface that the code here
then implements trivially to delegate back to DeviceOAuth2TokenService. So in a
sense, DeviceOAuth2TokenService is now its own delegate, which seems a bit
silly. Is there a longer-term plan for this that I'm missing?

+    DeviceOAuth2TokenServiceDelegate(
+        DeviceOAuth2TokenService* device_token_service)
+    : device_token_service_(device_token_service) {
+}
+
+DeviceOAuth2TokenService::DeviceOAuth2TokenServiceDelegate::
+    ~DeviceOAuth2TokenServiceDelegate() {
+}
+
+OAuth2AccessTokenFetcher* DeviceOAuth2TokenService::
+    DeviceOAuth2TokenServiceDelegate::CreateAccessTokenFetcher(
+        const std::string& account_id,
+        net::URLRequestContextGetter* getter,
+        OAuth2AccessTokenConsumer* consumer) {
+  return device_token_service_->CreateAccessTokenFetcherImpl(account_id, getter,
+                                                             consumer);
+}
+
+bool DeviceOAuth2TokenService::DeviceOAuth2TokenServiceDelegate::
+    RefreshTokenIsAvailable(const std::string& account_id) const {
+  return device_token_service_->RefreshTokenIsAvailableImpl(account_id);
+}
+
+net::URLRequestContextGetter*
+DeviceOAuth2TokenService::DeviceOAuth2TokenServiceDelegate::GetRequestContext()
+    const {
+  return device_token_service_->GetRequestContextImpl();
+}
+
+void DeviceOAuth2TokenService::DeviceOAuth2TokenServiceDelegate::
+    UpdateAuthError(const std::string& account_id,
+                    const GoogleServiceAuthError& error) {

[Mattias Nissler (ping if slow), 2015/06/29 08:20:06]

Should have a comment explaining why it's OK that this is empty.

+}
+
+std::vector<std::string>
+DeviceOAuth2TokenService::DeviceOAuth2TokenServiceDelegate::GetAccounts() {
+  std::vector<std::string> accounts;
+  std::string account = device_token_service_->GetRobotAccountId();
+  if (!account.empty())
+    accounts.push_back(account);
+  return accounts;
+}
+
+void DeviceOAuth2TokenService::DeviceOAuth2TokenServiceDelegate::
+    RevokeAllCredentials() {

[Mattias Nissler (ping if slow), 2015/06/29 08:20:06]

Again, please explain why it's OK that this is empty.

+}
+
 void DeviceOAuth2TokenService::OnServiceAccountIdentityChanged() {
   if (!GetRobotAccountId().empty() && !refresh_token_.empty())
-    FireRefreshTokenAvailable(GetRobotAccountId());
+    device_oauth2_token_service_delegate_->FireRefreshTokenAvailable(
+        GetRobotAccountId());
 }
 
 DeviceOAuth2TokenService::DeviceOAuth2TokenService(
     net::URLRequestContextGetter* getter,
     PrefService* local_state)
-    : url_request_context_getter_(getter),
+    : OAuth2TokenService(new DeviceOAuth2TokenServiceDelegate(this)),
+      url_request_context_getter_(getter),
       local_state_(local_state),
       state_(STATE_LOADING),
       max_refresh_token_validation_retries_(3),
       service_account_identity_subscription_(
-          CrosSettings::Get()->AddSettingsObserver(
-              kServiceAccountIdentity,
-              base::Bind(
-                  &DeviceOAuth2TokenService::OnServiceAccountIdentityChanged,
-                  base::Unretained(this))).Pass()),
+          CrosSettings::Get()
+              ->AddSettingsObserver(
+                    kServiceAccountIdentity,
+                    base::Bind(&DeviceOAuth2TokenService::
+                                   OnServiceAccountIdentityChanged,
+                               base::Unretained(this)))
+              .Pass()),
       weak_ptr_factory_(this) {
+  device_oauth2_token_service_delegate_ =
+      (DeviceOAuth2TokenServiceDelegate*)GetDelegate();

[xiyuan, 2015/06/26 18:15:52]

nit: static_cast<DeviceOAuth2TokenServiceDelegate*>(GetDelegate());

[Mattias Nissler (ping if slow), 2015/06/29 08:20:06]

Actually, it looks like none of the uses of
|device_oauth2_token_service_delegate_| requires the specific subtype, i.e.
DeviceOAuth2TokenServiceDelegate. If so, does it make sense at all to keep a
pointer of type DeviceOAuth2TokenServiceDelegate?

   // Pull in the system salt.
   SystemSaltGetter::Get()->GetSystemSalt(
       base::Bind(&DeviceOAuth2TokenService::DidGetSystemSalt,
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
 DeviceOAuth2TokenService::~DeviceOAuth2TokenService() {
   FlushPendingRequests(false, GoogleServiceAuthError::REQUEST_CANCELED);
   FlushTokenSaveCallbacks(false);
 }
@@ skipping 10 matching lines @@
   FlushPendingRequests(false, GoogleServiceAuthError::REQUEST_CANCELED);
 
   bool waiting_for_salt = state_ == STATE_LOADING;
   refresh_token_ = refresh_token;
   state_ = STATE_VALIDATION_PENDING;
 
   // If the robot account ID is not available yet, do not announce the token. It
   // will be done from OnServiceAccountIdentityChanged() once the robot account
   // ID becomes available as well.
   if (!GetRobotAccountId().empty())
-    FireRefreshTokenAvailable(GetRobotAccountId());
+    device_oauth2_token_service_delegate_->FireRefreshTokenAvailable(
+        GetRobotAccountId());
 
   token_save_callbacks_.push_back(result_callback);
   if (!waiting_for_salt) {
     if (system_salt_.empty())
       FlushTokenSaveCallbacks(false);
     else
       EncryptAndSaveToken();
   }
 }
 
-bool DeviceOAuth2TokenService::RefreshTokenIsAvailable(
+bool DeviceOAuth2TokenService::RefreshTokenIsAvailableImpl(
     const std::string& account_id) const {
   switch (state_) {
     case STATE_NO_TOKEN:
     case STATE_TOKEN_INVALID:
       return false;
     case STATE_LOADING:
     case STATE_VALIDATION_PENDING:
     case STATE_VALIDATION_STARTED:
     case STATE_TOKEN_VALID:
       return account_id == GetRobotAccountId();
@@ skipping 57 matching lines @@
     case STATE_VALIDATION_PENDING:
     case STATE_VALIDATION_STARTED:
     case STATE_TOKEN_VALID:
       return refresh_token_;
   }
 
   NOTREACHED() << "Unhandled state " << state_;
   return std::string();
 }
 
-net::URLRequestContextGetter* DeviceOAuth2TokenService::GetRequestContext() {
+net::URLRequestContextGetter*
+DeviceOAuth2TokenService::GetRequestContextImpl() {
   return url_request_context_getter_.get();
 }
 
 void DeviceOAuth2TokenService::FetchOAuth2Token(
     RequestImpl* request,
     const std::string& account_id,
     net::URLRequestContextGetter* getter,
     const std::string& client_id,
     const std::string& client_secret,
     const ScopeSet& scopes) {
@@ skipping 17 matching lines @@
     case STATE_TOKEN_VALID:
       // Pass through to OAuth2TokenService to satisfy the request.
       OAuth2TokenService::FetchOAuth2Token(
           request, account_id, getter, client_id, client_secret, scopes);
       return;
   }
 
   NOTREACHED() << "Unexpected state " << state_;
 }
 
-OAuth2AccessTokenFetcher* DeviceOAuth2TokenService::CreateAccessTokenFetcher(
+OAuth2AccessTokenFetcher*
+DeviceOAuth2TokenService::CreateAccessTokenFetcherImpl(
     const std::string& account_id,
     net::URLRequestContextGetter* getter,
     OAuth2AccessTokenConsumer* consumer) {
   std::string refresh_token = GetRefreshToken(account_id);
   DCHECK(!refresh_token.empty());
   return new OAuth2AccessTokenFetcherImpl(consumer, getter, refresh_token);
 }
 
 
 void DeviceOAuth2TokenService::DidGetSystemSalt(
     const std::string& system_salt) {
   system_salt_ = system_salt;
 
   // Bail out if system salt is not available.
   if (system_salt_.empty()) {
     LOG(ERROR) << "Failed to get system salt.";
     FlushTokenSaveCallbacks(false);
     state_ = STATE_NO_TOKEN;
-    FireRefreshTokensLoaded();
+    device_oauth2_token_service_delegate_->FireRefreshTokensLoaded();
     return;
   }
 
   // If the token has been set meanwhile, write it to |local_state_|.
   if (!refresh_token_.empty()) {
     EncryptAndSaveToken();
-    FireRefreshTokensLoaded();
+    device_oauth2_token_service_delegate_->FireRefreshTokensLoaded();
     return;
   }
 
   // Otherwise, load the refresh token from |local_state_|.
   std::string encrypted_refresh_token =
       local_state_->GetString(prefs::kDeviceRobotAnyApiRefreshToken);
   if (!encrypted_refresh_token.empty()) {
     CryptohomeTokenEncryptor encryptor(system_salt_);
     refresh_token_ = encryptor.DecryptWithSystemSalt(encrypted_refresh_token);
     if (refresh_token_.empty()) {
       LOG(ERROR) << "Failed to decrypt refresh token.";
       state_ = STATE_NO_TOKEN;
-      FireRefreshTokensLoaded();
+      device_oauth2_token_service_delegate_->FireRefreshTokensLoaded();
       return;
     }
   }
 
   state_ = STATE_VALIDATION_PENDING;
 
   // If there are pending requests, start a validation.
   if (!pending_requests_.empty())
     StartValidation();
 
   // Announce the token.
-  FireRefreshTokenAvailable(GetRobotAccountId());
-  FireRefreshTokensLoaded();
+  device_oauth2_token_service_delegate_->FireRefreshTokenAvailable(
+      GetRobotAccountId());
+  device_oauth2_token_service_delegate_->FireRefreshTokensLoaded();
 }
 
 void DeviceOAuth2TokenService::CheckRobotAccountId(
     const std::string& gaia_robot_id) {
   // Make sure the value returned by GetRobotAccountId has been validated
   // against current device settings.
   switch (CrosSettings::Get()->PrepareTrustedValues(base::Bind(
               &DeviceOAuth2TokenService::CheckRobotAccountId,
               weak_ptr_factory_.GetWeakPtr(),
               gaia_robot_id))) {
@@ skipping 77 matching lines @@
   for (std::vector<PendingRequest*>::iterator request(requests.begin());
        request != requests.end();
        ++request) {
     scoped_ptr<PendingRequest> scoped_request(*request);
     if (!scoped_request->request)
       continue;
 
     if (token_is_valid) {
       OAuth2TokenService::FetchOAuth2Token(
           scoped_request->request.get(),
-          scoped_request->request->GetAccountId(),
-          GetRequestContext(),
-          scoped_request->client_id,
-          scoped_request->client_secret,
+          scoped_request->request->GetAccountId(), GetRequestContextImpl(),
+          scoped_request->client_id, scoped_request->client_secret,
           scoped_request->scopes);
     } else {
       FailRequest(scoped_request->request.get(), error);
     }
   }
 }
 
 void DeviceOAuth2TokenService::FlushTokenSaveCallbacks(bool result) {
   std::vector<StatusCallback> callbacks;
   callbacks.swap(token_save_callbacks_);
@@ skipping 11 matching lines @@
   GoogleServiceAuthError auth_error(error);
   base::MessageLoop::current()->PostTask(FROM_HERE, base::Bind(
       &RequestImpl::InformConsumer,
       request->AsWeakPtr(),
       auth_error,
       std::string(),
       base::Time()));
 }
 
 }  // namespace chromeos