Rewrite DeviceOAuth2TokenService.

The old code handled asynchronous system salt loading in the factory
code, which made the Service a PITA to use. This change rewrites
DeviceOauth2TokenService to fold all asynchronous operations into
regular OAuth2TokenService implementation behavior, which is already
set up with Observers and callbacks already. Also, this change
improves the code to only perform validation once, even if there are
multiple concurrent token requests.

BUG=chromium:269455
TEST=Existing unit tests.
TBR=rogerta@chromium.org

Review URL: https://codereview.chromium.org/159773003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@253453 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/chromeos/login/kiosk_browsertest.cc

@@ -125,12 +125,6 @@ void OnNetworkWaitTimedOut(const base::Closure& runner_quit_task) {
   runner_quit_task.Run();
 }
 
-// Helper function for DeviceOAuth2TokenServiceFactory::Get().
-void CopyTokenService(DeviceOAuth2TokenService** out_token_service,
-                      DeviceOAuth2TokenService* in_token_service) {
-  *out_token_service = in_token_service;
-}
-
 // Helper functions for CanConfigureNetwork mock.
 class ScopedCanConfigureNetwork {
  public:
@@ -1039,12 +1033,11 @@ class KioskEnterpriseTest : public KioskTest {
     access_token_info.email = kTestEnterpriseServiceAccountId;
     fake_gaia_->IssueOAuthToken(kTestLoginToken, access_token_info);
 
-    DeviceOAuth2TokenService* token_service = NULL;
-    DeviceOAuth2TokenServiceFactory::Get(
-        base::Bind(&CopyTokenService, &token_service));
+    DeviceOAuth2TokenService* token_service =
+        DeviceOAuth2TokenServiceFactory::Get();
+    token_service->SetAndSaveRefreshToken(
+        kTestRefreshToken, DeviceOAuth2TokenService::StatusCallback());
     base::RunLoop().RunUntilIdle();
-    ASSERT_TRUE(token_service);
-    token_service->SetAndSaveRefreshToken(kTestRefreshToken);
   }
 
   static void StorePolicyCallback(bool result) {

chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos_unittest.cc

@@ -24,6 +24,8 @@
 #include "chromeos/cryptohome/system_salt_getter.h"
 #include "chromeos/dbus/dbus_client_implementation_type.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/fake_cryptohome_client.h"
+#include "chromeos/dbus/fake_dbus_thread_manager.h"
 #include "chromeos/system/mock_statistics_provider.h"
 #include "chromeos/system/statistics_provider.h"
 #include "components/policy/core/common/cloud/cloud_policy_client.h"
@@ -59,11 +61,6 @@ void CopyLockResult(base::RunLoop* loop,
   loop->Quit();
 }
 
-void CopyTokenService(chromeos::DeviceOAuth2TokenService** out_token_service,
-                      chromeos::DeviceOAuth2TokenService* in_token_service) {
-  *out_token_service = in_token_service;
-}
-
 class DeviceCloudPolicyManagerChromeOSTest
     : public chromeos::DeviceSettingsTestBase {
  protected:
@@ -109,7 +106,7 @@ class DeviceCloudPolicyManagerChromeOSTest
     TestingBrowserProcess::GetGlobal()->SetSystemRequestContext(
         request_context_getter_.get());
     TestingBrowserProcess::GetGlobal()->SetLocalState(&local_state_);
-    // SystemSaltGetter is used in DeviceOAuth2TokenServiceFactory.
+    // SystemSaltGetter is used in DeviceOAuth2TokenService.
     chromeos::SystemSaltGetter::Initialize();
     chromeos::DeviceOAuth2TokenServiceFactory::Initialize();
     url_fetcher_response_code_ = 200;
@@ -421,12 +418,10 @@ class DeviceCloudPolicyManagerChromeOSEnrollmentTest
       return;
 
     // Process robot refresh token store.
-    chromeos::DeviceOAuth2TokenService* token_service = NULL;
-    chromeos::DeviceOAuth2TokenServiceFactory::Get(
-        base::Bind(&CopyTokenService, &token_service));
-    base::RunLoop().RunUntilIdle();
-    ASSERT_TRUE(token_service);
-    EXPECT_EQ("refreshToken4Test", token_service->GetRefreshToken(""));
+    chromeos::DeviceOAuth2TokenService* token_service =
+        chromeos::DeviceOAuth2TokenServiceFactory::Get();
+    EXPECT_TRUE(token_service->RefreshTokenIsAvailable(
+        token_service->GetRobotAccountId()));
 
     // Process policy store.
     device_settings_test_helper_.set_store_result(store_result_);
@@ -518,13 +513,6 @@ TEST_F(DeviceCloudPolicyManagerChromeOSEnrollmentTest,
   ExpectFailedEnrollment(EnrollmentStatus::STATUS_ROBOT_REFRESH_FETCH_FAILED);
 }
 
-TEST_F(DeviceCloudPolicyManagerChromeOSEnrollmentTest, RobotRefreshSaveFailed) {
-  // Without a DeviceOAuth2TokenService, the refresh token can't be saved.
-  chromeos::DeviceOAuth2TokenServiceFactory::Shutdown();
-  RunTest();
-  ExpectFailedEnrollment(EnrollmentStatus::STATUS_ROBOT_REFRESH_STORE_FAILED);
-}
-
 TEST_F(DeviceCloudPolicyManagerChromeOSEnrollmentTest,
        RobotRefreshEncryptionFailed) {
   // The encryption lib is a noop for tests, but empty results from encryption
@@ -571,5 +559,26 @@ TEST_F(DeviceCloudPolicyManagerChromeOSEnrollmentTest, LoadError) {
             status_.store_status());
 }
 
+// A subclass that runs with a blank system salt.
+class DeviceCloudPolicyManagerChromeOSEnrollmentBlankSystemSaltTest
+    : public DeviceCloudPolicyManagerChromeOSEnrollmentTest {
+ protected:
+  DeviceCloudPolicyManagerChromeOSEnrollmentBlankSystemSaltTest() {
+    // Set up a FakeCryptohomeClient with a blank system salt.
+    scoped_ptr<chromeos::FakeCryptohomeClient> fake_cryptohome_client(
+        new chromeos::FakeCryptohomeClient());
+    fake_cryptohome_client->set_system_salt(std::vector<uint8>());
+    fake_dbus_thread_manager_->SetCryptohomeClient(
+        fake_cryptohome_client.PassAs<chromeos::CryptohomeClient>());
+  }
+};
+
+TEST_F(DeviceCloudPolicyManagerChromeOSEnrollmentBlankSystemSaltTest,
+       RobotRefreshSaveFailed) {
+  // Without the system salt, the robot token can't be stored.
+  RunTest();
+  ExpectFailedEnrollment(EnrollmentStatus::STATUS_ROBOT_REFRESH_STORE_FAILED);
+}
+
 }  // namespace
 }  // namespace policy

chrome/browser/chromeos/policy/enrollment_handler_chromeos.cc

@@ -311,8 +311,9 @@ void EnrollmentHandlerChromeOS::HandleLockDeviceResult(
     case EnterpriseInstallAttributes::LOCK_SUCCESS:
       // Get the token service so we can store our robot refresh token.
       enrollment_step_ = STEP_STORE_ROBOT_AUTH;
-      chromeos::DeviceOAuth2TokenServiceFactory::Get(
-          base::Bind(&EnrollmentHandlerChromeOS::DidGetTokenService,
+      chromeos::DeviceOAuth2TokenServiceFactory::Get()->SetAndSaveRefreshToken(
+          refresh_token_,
+          base::Bind(&EnrollmentHandlerChromeOS::HandleRobotAuthTokenStored,
                      weak_ptr_factory_.GetWeakPtr()));
       return;
     case EnterpriseInstallAttributes::LOCK_NOT_READY:
@@ -351,18 +352,10 @@ void EnrollmentHandlerChromeOS::HandleLockDeviceResult(
       EnrollmentStatus::STATUS_LOCK_ERROR));
 }
 
-void EnrollmentHandlerChromeOS::DidGetTokenService(
-    chromeos::DeviceOAuth2TokenService* token_service) {
+void EnrollmentHandlerChromeOS::HandleRobotAuthTokenStored(bool result) {
   CHECK_EQ(STEP_STORE_ROBOT_AUTH, enrollment_step_);
-  // Store the robot API auth refresh token.
-  if (!token_service) {
-    LOG(ERROR) << "Failed to store API refresh token (no token service).";
-    ReportResult(EnrollmentStatus::ForStatus(
-        EnrollmentStatus::STATUS_ROBOT_REFRESH_STORE_FAILED));
-    return;
-  }
 
-  if (!token_service->SetAndSaveRefreshToken(refresh_token_)) {
+  if (!result) {
     LOG(ERROR) << "Failed to store API refresh token.";
     ReportResult(EnrollmentStatus::ForStatus(
         EnrollmentStatus::STATUS_ROBOT_REFRESH_STORE_FAILED));

chrome/browser/chromeos/policy/enrollment_handler_chromeos.h

@@ -23,10 +23,6 @@ namespace base {
 class SequencedTaskRunner;
 }
 
-namespace chromeos {
-class DeviceOAuth2TokenService;
-}
-
 namespace enterprise_management {
 class PolicyFetchResponse;
 }
@@ -136,15 +132,15 @@ class EnrollmentHandlerChromeOS : public CloudPolicyClient::Observer,
       const std::string& device_id,
       EnterpriseInstallAttributes::LockResult lock_result);
 
+  // Handles completion of the robot token store operation.
+  void HandleRobotAuthTokenStored(bool result);
+
   // Drops any ongoing actions.
   void Stop();
 
   // Reports the result of the enrollment process to the initiator.
   void ReportResult(EnrollmentStatus status);
 
-  // Continuation of OnStoreLoaded().
-  void DidGetTokenService(chromeos::DeviceOAuth2TokenService* token_service);
-
   DeviceCloudPolicyStoreChromeOS* store_;
   EnterpriseInstallAttributes* install_attributes_;
   scoped_ptr<CloudPolicyClient> client_;
@@ -174,7 +170,7 @@ class EnrollmentHandlerChromeOS : public CloudPolicyClient::Observer,
   // initialization.
   int lockbox_init_duration_;
 
-  // Used for locking the device and getting the OAuth2 token service.
+  // Used for locking the device.
   base::WeakPtrFactory<EnrollmentHandlerChromeOS> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(EnrollmentHandlerChromeOS);