Cache tokens inside DriveFsHost for remounts

If drivefs needs to restart there is no need to redo all the job for
oauth token minting as the previous one should still be good.

BUG=chromium:897558

Change-Id: I4e1737a94a70476c6312ff11c04b8f3f233e4512
Reviewed-on: https://chromium-review.googlesource.com/c/1293058
Commit-Queue: Sergei Datsenko <dats@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601877}

chrome/browser/chromeos/drive/drive_integration_service.cc

@@ -464,6 +464,8 @@ class DriveIntegrationService::DriveFsHolder
         drivefs_host_(profile_->GetPath(),
                       this,
                       this,
+                      base::DefaultClock::GetInstance(),
+                      chromeos::disks::DiskMountManager::GetInstance(),
                       std::make_unique<base::OneShotTimer>()) {}
 
   drivefs::DriveFsHost* drivefs_host() { return &drivefs_host_; }

chromeos/components/drivefs/drivefs_host.cc

@@ -71,6 +71,122 @@ DriveFsHost::Delegate::CreateMojoConnectionDelegate() {
   return std::make_unique<MojoConnectionDelegateImpl>();
 }
 
+class DriveFsHost::AccountTokenDelegate : public OAuth2MintTokenFlow::Delegate {
+ public:
+  explicit AccountTokenDelegate(DriveFsHost* host) : host_(host) {}
+  ~AccountTokenDelegate() override = default;
+
+  void GetAccessToken(bool use_cached,
+                      const std::string& client_id,
+                      const std::string& app_id,
+                      const std::vector<std::string>& scopes,
+                      mojom::DriveFsDelegate::GetAccessTokenCallback callback) {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(host_->sequence_checker_);
+    if (get_access_token_callback_) {
+      std::move(callback).Run(mojom::AccessTokenStatus::kTransientError, "");
+      return;
+    }
+    DCHECK(!mint_token_flow_);
+    const std::string& token =
+        MaybeGetCachedToken(use_cached, client_id, app_id, scopes);
+    if (!token.empty()) {
+      std::move(callback).Run(mojom::AccessTokenStatus::kSuccess, token);
+      return;
+    }
+    get_access_token_callback_ = std::move(callback);
+    mint_token_flow_ =
+        host_->delegate_->CreateMintTokenFlow(this, client_id, app_id, scopes);
+    DCHECK(mint_token_flow_);
+    GetIdentityManager().GetPrimaryAccountWhenAvailable(base::BindOnce(
+        &AccountTokenDelegate::AccountReady, base::Unretained(this)));
+  }
+
+ private:
+  void AccountReady(const AccountInfo& info,
+                    const identity::AccountState& state) {
+    GetIdentityManager().GetAccessToken(
+        host_->delegate_->GetAccountId().GetUserEmail(), {},
+        kIdentityConsumerId,
+        base::BindOnce(&AccountTokenDelegate::GotChromeAccessToken,
+                       base::Unretained(this)));
+  }
+
+  void GotChromeAccessToken(const base::Optional<std::string>& access_token,
+                            base::Time expiration_time,
+                            const GoogleServiceAuthError& error) {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(host_->sequence_checker_);
+    if (!access_token) {
+      OnMintTokenFailure(error);
+      return;
+    }
+    mint_token_flow_->Start(host_->delegate_->GetURLLoaderFactory(),
+                            *access_token);
+  }
+
+  // OAuth2MintTokenFlow::Delegate:
+  void OnMintTokenSuccess(const std::string& access_token,
+                          int time_to_live) override {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(host_->sequence_checker_);
+    UpdateCachedToken(access_token, base::TimeDelta::FromSeconds(time_to_live));
+    std::move(get_access_token_callback_)
+        .Run(mojom::AccessTokenStatus::kSuccess, access_token);
+    mint_token_flow_.reset();
+  }
+
+  void OnMintTokenFailure(const GoogleServiceAuthError& error) override {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(host_->sequence_checker_);
+    std::move(get_access_token_callback_)
+        .Run(error.IsPersistentError()
+                 ? mojom::AccessTokenStatus::kAuthError
+                 : mojom::AccessTokenStatus::kTransientError,
+             "");
+    mint_token_flow_.reset();
+  }
+
+  const std::string& MaybeGetCachedToken(
+      bool use_cached,
+      const std::string& client_id,
+      const std::string& app_id,
+      const std::vector<std::string>& scopes) {
+    // Return value from cache at most once per mount.
+    if (!use_cached || host_->clock_->Now() >= last_token_expiry_) {
+      last_token_.clear();
+    }
+    return last_token_;
+  }
+
+  void UpdateCachedToken(const std::string& token, const base::TimeDelta& ttl) {
+    last_token_ = token;
+    last_token_expiry_ = host_->clock_->Now() + ttl;
+  }
+
+  identity::mojom::IdentityManager& GetIdentityManager() {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(host_->sequence_checker_);
+    if (!identity_manager_) {
+      host_->delegate_->GetConnector()->BindInterface(
+          identity::mojom::kServiceName, mojo::MakeRequest(&identity_manager_));
+    }
+    return *identity_manager_;
+  }
+
+  // Owns |this|.
+  DriveFsHost* const host_;
+
+  // The connection to the identity service. Access via |GetIdentityManager()|.
+  identity::mojom::IdentityManagerPtr identity_manager_;
+
+  // Pending callback for an in-flight GetAccessToken request.
+  mojom::DriveFsDelegate::GetAccessTokenCallback get_access_token_callback_;
+
+  // The mint token flow, if one is in flight.
+  std::unique_ptr<OAuth2MintTokenFlow> mint_token_flow_;
+
+  std::string last_token_;
+  base::Time last_token_expiry_;
+
+  DISALLOW_COPY_AND_ASSIGN(AccountTokenDelegate);
+};
+
 // A container of state tied to a particular mounting of DriveFS. None of this
 // should be shared between mounts.
 class DriveFsHost::MountState
@@ -85,7 +201,7 @@ class DriveFsHost::MountState
             host_->delegate_->CreateMojoConnectionDelegate()),
         pending_token_(base::UnguessableToken::Create()),
         binding_(this) {
-    chromeos::disks::DiskMountManager::GetInstance()->AddObserver(this);
+    host_->disk_mount_manager_->AddObserver(this);
     source_path_ = base::StrCat({kMountScheme, pending_token_.ToString()});
     std::string datadir_option =
         base::StrCat({"datadir=", host_->GetDataPath().value()});
@@ -107,7 +223,7 @@ class DriveFsHost::MountState
         base::BindOnce(&DriveFsHost::MountState::AcceptMojoConnection,
                        base::Unretained(this)));
 
-    chromeos::disks::DiskMountManager::GetInstance()->MountPath(
+    host_->disk_mount_manager_->MountPath(
         source_path_, "",
         base::StrCat({"drivefs-", host_->delegate_->GetObfuscatedAccountId()}),
         {datadir_option}, chromeos::MOUNT_TYPE_NETWORK_STORAGE,
@@ -120,7 +236,7 @@ class DriveFsHost::MountState
 
   ~MountState() override {
     DCHECK_CALLED_ON_VALID_SEQUENCE(host_->sequence_checker_);
-    chromeos::disks::DiskMountManager::GetInstance()->RemoveObserver(this);
+    host_->disk_mount_manager_->RemoveObserver(this);
     host_->delegate_->GetDriveNotificationManager().RemoveObserver(this);
     host_->timer_->Stop();
     if (pending_token_) {
@@ -128,7 +244,7 @@ class DriveFsHost::MountState
           pending_token_);
     }
     if (!mount_path_.empty()) {
-      chromeos::disks::DiskMountManager::GetInstance()->UnmountPath(
+      host_->disk_mount_manager_->UnmountPath(
           mount_path_.value(), chromeos::UNMOUNT_OPTIONS_NONE, {});
     }
     if (mounted()) {
@@ -159,17 +275,10 @@ class DriveFsHost::MountState
                       const std::vector<std::string>& scopes,
                       GetAccessTokenCallback callback) override {
     DCHECK_CALLED_ON_VALID_SEQUENCE(host_->sequence_checker_);
-    if (get_access_token_callback_) {
-      std::move(callback).Run(mojom::AccessTokenStatus::kTransientError, "");
-      return;
-    }
-    DCHECK(!mint_token_flow_);
-    get_access_token_callback_ = std::move(callback);
-    mint_token_flow_ =
-        host_->delegate_->CreateMintTokenFlow(this, client_id, app_id, scopes);
-    DCHECK(mint_token_flow_);
-    GetIdentityManager().GetPrimaryAccountWhenAvailable(base::BindOnce(
-        &DriveFsHost::MountState::AccountReady, base::Unretained(this)));
+    host_->account_token_delegate_->GetAccessToken(!token_fetch_attempted_,
+                                                   client_id, app_id, scopes,
+                                                   std::move(callback));
+    token_fetch_attempted_ = true;
   }
 
   void OnMounted() override {
@@ -262,55 +371,6 @@ class DriveFsHost::MountState
     }
   }
 
-  void AccountReady(const AccountInfo& info,
-                    const identity::AccountState& state) {
-    GetIdentityManager().GetAccessToken(
-        host_->delegate_->GetAccountId().GetUserEmail(), {},
-        kIdentityConsumerId,
-        base::BindOnce(&DriveFsHost::MountState::GotChromeAccessToken,
-                       base::Unretained(this)));
-  }
-
-  void GotChromeAccessToken(const base::Optional<std::string>& access_token,
-                            base::Time expiration_time,
-                            const GoogleServiceAuthError& error) {
-    DCHECK_CALLED_ON_VALID_SEQUENCE(host_->sequence_checker_);
-    if (!access_token) {
-      OnMintTokenFailure(error);
-      return;
-    }
-    mint_token_flow_->Start(host_->delegate_->GetURLLoaderFactory(),
-                            *access_token);
-  }
-
-  // OAuth2MintTokenFlow::Delegate:
-  void OnMintTokenSuccess(const std::string& access_token,
-                          int time_to_live) override {
-    DCHECK_CALLED_ON_VALID_SEQUENCE(host_->sequence_checker_);
-    std::move(get_access_token_callback_)
-        .Run(mojom::AccessTokenStatus::kSuccess, access_token);
-    mint_token_flow_.reset();
-  }
-
-  void OnMintTokenFailure(const GoogleServiceAuthError& error) override {
-    DCHECK_CALLED_ON_VALID_SEQUENCE(host_->sequence_checker_);
-    std::move(get_access_token_callback_)
-        .Run(error.IsPersistentError()
-                 ? mojom::AccessTokenStatus::kAuthError
-                 : mojom::AccessTokenStatus::kTransientError,
-             "");
-    mint_token_flow_.reset();
-  }
-
-  identity::mojom::IdentityManager& GetIdentityManager() {
-    DCHECK_CALLED_ON_VALID_SEQUENCE(host_->sequence_checker_);
-    if (!identity_manager_) {
-      host_->delegate_->GetConnector()->BindInterface(
-          identity::mojom::kServiceName, mojo::MakeRequest(&identity_manager_));
-    }
-    return *identity_manager_;
-  }
-
   // DiskMountManager::Observer:
   void OnMountEvent(chromeos::disks::DiskMountManager::MountEvent event,
                     chromeos::MountError error_code,
@@ -365,21 +425,13 @@ class DriveFsHost::MountState
   // The path where DriveFS is mounted.
   base::FilePath mount_path_;
 
-  // Pending callback for an in-flight GetAccessToken request.
-  GetAccessTokenCallback get_access_token_callback_;
-
-  // The mint token flow, if one is in flight.
-  std::unique_ptr<OAuth2MintTokenFlow> mint_token_flow_;
-
   // Mojo connections to the DriveFS process.
   mojom::DriveFsPtr drivefs_;
   mojo::Binding<mojom::DriveFsDelegate> binding_;
 
   bool drivefs_has_mounted_ = false;
   bool drivefs_has_terminated_ = false;
-
-  // The connection to the identity service. Access via |GetIdentityManager()|.
-  identity::mojom::IdentityManagerPtr identity_manager_;
+  bool token_fetch_attempted_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(MountState);
 };
@@ -387,13 +439,19 @@ class DriveFsHost::MountState
 DriveFsHost::DriveFsHost(const base::FilePath& profile_path,
                          DriveFsHost::Delegate* delegate,
                          DriveFsHost::MountObserver* mount_observer,
+                         const base::Clock* clock,
+                         chromeos::disks::DiskMountManager* disk_mount_manager,
                          std::unique_ptr<base::OneShotTimer> timer)
     : profile_path_(profile_path),
       delegate_(delegate),
       mount_observer_(mount_observer),
-      timer_(std::move(timer)) {
+      clock_(clock),
+      disk_mount_manager_(disk_mount_manager),
+      timer_(std::move(timer)),
+      account_token_delegate_(std::make_unique<AccountTokenDelegate>(this)) {
   DCHECK(delegate_);
   DCHECK(mount_observer_);
+  DCHECK(clock_);
 }
 
 DriveFsHost::~DriveFsHost() {

chromeos/components/drivefs/drivefs_host.h

@@ -13,6 +13,7 @@
 #include "base/files/file_path.h"
 #include "base/files/scoped_file.h"
 #include "base/macros.h"
+#include "base/time/clock.h"
 #include "base/timer/timer.h"
 #include "chromeos/components/drivefs/mojom/drivefs.mojom.h"
 #include "chromeos/disks/disk_mount_manager.h"
@@ -32,6 +33,12 @@ namespace service_manager {
 class Connector;
 }  // namespace service_manager
 
+namespace chromeos {
+namespace disks {
+class DiskMountManager;
+}
+}  // namespace chromeos
+
 namespace drivefs {
 
 class DriveFsHostObserver;
@@ -94,6 +101,8 @@ class COMPONENT_EXPORT(DRIVEFS) DriveFsHost {
   DriveFsHost(const base::FilePath& profile_path,
               Delegate* delegate,
               MountObserver* mount_observer,
+              const base::Clock* clock,
+              chromeos::disks::DiskMountManager* disk_mount_manager,
               std::unique_ptr<base::OneShotTimer> timer);
   ~DriveFsHost();
 
@@ -118,6 +127,7 @@ class COMPONENT_EXPORT(DRIVEFS) DriveFsHost {
   mojom::DriveFs* GetDriveFsInterface() const;
 
  private:
+  class AccountTokenDelegate;
   class MountState;
 
   SEQUENCE_CHECKER(sequence_checker_);
@@ -127,9 +137,12 @@ class COMPONENT_EXPORT(DRIVEFS) DriveFsHost {
 
   Delegate* const delegate_;
   MountObserver* const mount_observer_;
-
+  const base::Clock* const clock_;
+  chromeos::disks::DiskMountManager* const disk_mount_manager_;
   std::unique_ptr<base::OneShotTimer> timer_;
 
+  std::unique_ptr<AccountTokenDelegate> account_token_delegate_;
+
   // State specific to the current mount, or null if not mounted.
   std::unique_ptr<MountState> mount_state_;
 

chromeos/components/drivefs/drivefs_host_unittest.cc

@@ -15,6 +15,7 @@
 #include "base/strings/string_split.h"
 #include "base/test/bind_test_util.h"
 #include "base/test/scoped_task_environment.h"
+#include "base/test/simple_test_clock.h"
 #include "base/timer/mock_timer.h"
 #include "chromeos/components/drivefs/drivefs_host_observer.h"
 #include "chromeos/components/drivefs/pending_connection_manager.h"
@@ -57,8 +58,10 @@ class ForwardingOAuth2MintTokenFlow;
 
 ACTION_P(SucceedMintToken, token) {
   base::SequencedTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE,
-      base::BindLambdaForTesting([=] { arg0->OnMintTokenSuccess(token, 0); }));
+      FROM_HERE, base::BindLambdaForTesting([=] {
+        arg0->OnMintTokenSuccess(token,
+                                 base::TimeDelta::FromHours(1).InSeconds());
+      }));
 }
 
 ACTION_P(FailMintToken, error) {
@@ -85,12 +88,13 @@ class MockOAuth2MintTokenFlow {
 
   void ExpectNoStartCalls() { EXPECT_CALL(*this, Start(_, _)).Times(0); }
 
- private:
-  friend class ForwardingOAuth2MintTokenFlow;
   MOCK_METHOD2(Start,
                void(OAuth2MintTokenFlow::Delegate* delegate,
                     const std::string& access_token));
 
+ private:
+  friend class ForwardingOAuth2MintTokenFlow;
+
   DISALLOW_COPY_AND_ASSIGN(MockOAuth2MintTokenFlow);
 };
 
@@ -221,7 +225,9 @@ class FakeIdentityService
     : public identity::mojom::IdentityManagerInterceptorForTesting,
       public service_manager::Service {
  public:
-  explicit FakeIdentityService(MockIdentityManager* mock) : mock_(mock) {
+  explicit FakeIdentityService(MockIdentityManager* mock,
+                               const base::Clock* clock)
+      : mock_(mock), clock_(clock) {
     binder_registry_.AddInterface(
         base::BindRepeating(&FakeIdentityService::BindIdentityManagerRequest,
                             base::Unretained(this)));
@@ -258,7 +264,7 @@ class FakeIdentityService
                       const std::string& consumer_id,
                       GetAccessTokenCallback callback) override {
     auto result = mock_->GetAccessToken(account_id, scopes, consumer_id);
-    std::move(callback).Run(std::move(result.first), base::Time::Now(),
+    std::move(callback).Run(std::move(result.first), clock_->Now(),
                             GoogleServiceAuthError(result.second));
   }
 
@@ -268,6 +274,7 @@ class FakeIdentityService
   }
 
   MockIdentityManager* const mock_;
+  const base::Clock* const clock_;
   service_manager::BinderRegistry binder_registry_;
   mojo::BindingSet<identity::mojom::IdentityManager> bindings_;
 
@@ -297,25 +304,23 @@ class DriveFsHostTest : public ::testing::Test, public mojom::DriveFsBootstrap {
     profile_path_ = base::FilePath(FILE_PATH_LITERAL("/path/to/profile"));
     account_id_ = AccountId::FromUserEmailGaiaId("test@example.com", "ID");
 
-    disk_manager_ = new chromeos::disks::MockDiskMountManager;
-    // Takes ownership of |disk_manager_|.
-    chromeos::disks::DiskMountManager::InitializeForTesting(disk_manager_);
+    disk_manager_ = std::make_unique<chromeos::disks::MockDiskMountManager>();
     connector_factory_ =
         service_manager::TestConnectorFactory::CreateForUniqueService(
-            std::make_unique<FakeIdentityService>(&mock_identity_manager_));
+            std::make_unique<FakeIdentityService>(&mock_identity_manager_,
+                                                  &clock_));
     host_delegate_ = std::make_unique<TestingDriveFsHostDelegate>(
         connector_factory_->CreateConnector(), account_id_);
     auto timer = std::make_unique<base::MockOneShotTimer>();
     timer_ = timer.get();
-    host_ =
-        std::make_unique<DriveFsHost>(profile_path_, host_delegate_.get(),
-                                      host_delegate_.get(), std::move(timer));
+    host_ = std::make_unique<DriveFsHost>(
+        profile_path_, host_delegate_.get(), host_delegate_.get(), &clock_,
+        disk_manager_.get(), std::move(timer));
   }
 
   void TearDown() override {
     host_.reset();
-    disk_manager_ = nullptr;
-    chromeos::disks::DiskMountManager::Shutdown();
+    disk_manager_.reset();
   }
 
   void DispatchMountEvent(
@@ -384,11 +389,38 @@ class DriveFsHostTest : public ::testing::Test, public mojom::DriveFsBootstrap {
     EXPECT_CALL(*host_delegate_,
                 OnMounted(base::FilePath("/media/drivefsroot/salt-g-ID")))
         .WillOnce(RunQuitClosure(&quit_closure));
+    // Eventually we must attempt unmount.
+    EXPECT_CALL(*disk_manager_, UnmountPath("/media/drivefsroot/salt-g-ID",
+                                            chromeos::UNMOUNT_OPTIONS_NONE, _));
     SendOnMounted();
     run_loop.Run();
     ASSERT_TRUE(host_->IsMounted());
   }
 
+  void DoUnmount() {
+    host_->Unmount();
+    binding_.Unbind();
+    bootstrap_binding_.Unbind();
+    delegate_ptr_.reset();
+    base::RunLoop().RunUntilIdle();
+    testing::Mock::VerifyAndClearExpectations(disk_manager_.get());
+  }
+
+  void ExpectAccessToken(mojom::AccessTokenStatus expected_status,
+                         const std::string& expected_token) {
+    base::RunLoop run_loop;
+    auto quit_closure = run_loop.QuitClosure();
+    delegate_ptr_->GetAccessToken(
+        "client ID", "app ID", {"scope1", "scope2"},
+        base::BindLambdaForTesting(
+            [&](mojom::AccessTokenStatus status, const std::string& token) {
+              EXPECT_EQ(expected_status, status);
+              EXPECT_EQ(expected_token, token);
+              std::move(quit_closure).Run();
+            }));
+    run_loop.Run();
+  }
+
   void Init(mojom::DriveFsConfigurationPtr config,
             mojom::DriveFsRequest drive_fs_request,
             mojom::DriveFsDelegatePtr delegate) override {
@@ -401,7 +433,8 @@ class DriveFsHostTest : public ::testing::Test, public mojom::DriveFsBootstrap {
   base::FilePath profile_path_;
   base::test::ScopedTaskEnvironment task_environment_;
   AccountId account_id_;
-  chromeos::disks::MockDiskMountManager* disk_manager_;
+  std::unique_ptr<chromeos::disks::MockDiskMountManager> disk_manager_;
+  base::SimpleTestClock clock_;
   MockIdentityManager mock_identity_manager_;
   std::unique_ptr<service_manager::TestConnectorFactory> connector_factory_;
   std::unique_ptr<TestingDriveFsHostDelegate> host_delegate_;
@@ -428,6 +461,8 @@ TEST_F(DriveFsHostTest, Basic) {
 
   EXPECT_EQ(base::FilePath("/media/drivefsroot/salt-g-ID"),
             host_->GetMountPath());
+
+  DoUnmount();
 }
 
 TEST_F(DriveFsHostTest, GetMountPathWhileUnmounted) {
@@ -448,12 +483,16 @@ TEST_F(DriveFsHostTest, OnMountedBeforeMountEvent) {
 
   EXPECT_CALL(*host_delegate_,
               OnMounted(base::FilePath("/media/drivefsroot/salt-g-ID")));
+  EXPECT_CALL(*disk_manager_, UnmountPath("/media/drivefsroot/salt-g-ID",
+                                          chromeos::UNMOUNT_OPTIONS_NONE, _));
 
   DispatchMountSuccessEvent(token);
 
   ASSERT_TRUE(host_->IsMounted());
   EXPECT_EQ(base::FilePath("/media/drivefsroot/salt-g-ID"),
             host_->GetMountPath());
+
+  DoUnmount();
 }
 
 TEST_F(DriveFsHostTest, OnMountFailedFromMojo) {
@@ -519,8 +558,6 @@ TEST_F(DriveFsHostTest, UnmountAfterMountComplete) {
 
   ASSERT_NO_FATAL_FAILURE(DoMount());
 
-  EXPECT_CALL(*disk_manager_, UnmountPath("/media/drivefsroot/salt-g-ID",
-                                          chromeos::UNMOUNT_OPTIONS_NONE, _));
   EXPECT_CALL(observer, OnUnmounted());
   base::RunLoop run_loop;
   delegate_ptr_.set_connection_error_handler(run_loop.QuitClosure());
@@ -598,6 +635,7 @@ TEST_F(DriveFsHostTest, ObserveOtherMount) {
 TEST_F(DriveFsHostTest, MountError) {
   auto token = StartMount();
   EXPECT_CALL(*disk_manager_, UnmountPath(_, _, _)).Times(0);
+  EXPECT_CALL(*host_delegate_, OnMountFailed(_));
 
   DispatchMountEvent(chromeos::disks::DiskMountManager::MOUNTING,
                      chromeos::MOUNT_ERROR_DIRECTORY_CREATION_FAILED,
@@ -662,8 +700,8 @@ TEST_F(DriveFsHostTest, UnsupportedAccountTypes) {
     host_delegate_ = std::make_unique<TestingDriveFsHostDelegate>(
         connector_factory_->CreateConnector(), account);
     host_ = std::make_unique<DriveFsHost>(
-        profile_path_, host_delegate_.get(), host_delegate_.get(),
-        std::make_unique<base::MockOneShotTimer>());
+        profile_path_, host_delegate_.get(), host_delegate_.get(), &clock_,
+        disk_manager_.get(), std::make_unique<base::MockOneShotTimer>());
     EXPECT_FALSE(host_->Mount());
     EXPECT_FALSE(host_->IsMounted());
   }
@@ -679,17 +717,7 @@ TEST_F(DriveFsHostTest, GetAccessToken_Success) {
   host_delegate_->mock_flow().ExpectStartAndSucceed("chrome token",
                                                     "auth token");
 
-  base::RunLoop run_loop;
-  auto quit_closure = run_loop.QuitClosure();
-  delegate_ptr_->GetAccessToken(
-      "client ID", "app ID", {"scope1", "scope2"},
-      base::BindLambdaForTesting(
-          [&](mojom::AccessTokenStatus status, const std::string& token) {
-            EXPECT_EQ(mojom::AccessTokenStatus::kSuccess, status);
-            EXPECT_EQ("auth token", token);
-            std::move(quit_closure).Run();
-          }));
-  run_loop.Run();
+  ExpectAccessToken(mojom::AccessTokenStatus::kSuccess, "auth token");
 }
 
 TEST_F(DriveFsHostTest, GetAccessToken_ParallelRequests) {
@@ -724,18 +752,7 @@ TEST_F(DriveFsHostTest, GetAccessToken_SequentialRequests) {
             std::make_pair("chrome token", GoogleServiceAuthError::NONE)));
     host_delegate_->mock_flow().ExpectStartAndSucceed("chrome token",
                                                       "auth token");
-
-    base::RunLoop run_loop;
-    auto quit_closure = run_loop.QuitClosure();
-    delegate_ptr_->GetAccessToken(
-        "client ID", "app ID", {"scope1", "scope2"},
-        base::BindLambdaForTesting(
-            [&](mojom::AccessTokenStatus status, const std::string& token) {
-              EXPECT_EQ(mojom::AccessTokenStatus::kSuccess, status);
-              EXPECT_EQ("auth token", token);
-              std::move(quit_closure).Run();
-            }));
-    run_loop.Run();
+    ExpectAccessToken(mojom::AccessTokenStatus::kSuccess, "auth token");
   }
   for (int i = 0; i < 3; ++i) {
     EXPECT_CALL(mock_identity_manager_,
@@ -743,18 +760,7 @@ TEST_F(DriveFsHostTest, GetAccessToken_SequentialRequests) {
         .WillOnce(testing::Return(std::make_pair(
             base::nullopt, GoogleServiceAuthError::ACCOUNT_DISABLED)));
     host_delegate_->mock_flow().ExpectNoStartCalls();
-
-    base::RunLoop run_loop;
-    auto quit_closure = run_loop.QuitClosure();
-    delegate_ptr_->GetAccessToken(
-        "client ID", "app ID", {"scope1", "scope2"},
-        base::BindLambdaForTesting(
-            [&](mojom::AccessTokenStatus status, const std::string& token) {
-              EXPECT_EQ(mojom::AccessTokenStatus::kAuthError, status);
-              EXPECT_TRUE(token.empty());
-              std::move(quit_closure).Run();
-            }));
-    run_loop.Run();
+    ExpectAccessToken(mojom::AccessTokenStatus::kAuthError, "");
   }
 }
 
@@ -766,18 +772,7 @@ TEST_F(DriveFsHostTest, GetAccessToken_GetAccessTokenFailure_Permanent) {
       .WillOnce(testing::Return(std::make_pair(
           base::nullopt, GoogleServiceAuthError::ACCOUNT_DISABLED)));
   host_delegate_->mock_flow().ExpectNoStartCalls();
-
-  base::RunLoop run_loop;
-  auto quit_closure = run_loop.QuitClosure();
-  delegate_ptr_->GetAccessToken(
-      "client ID", "app ID", {"scope1", "scope2"},
-      base::BindLambdaForTesting(
-          [&](mojom::AccessTokenStatus status, const std::string& token) {
-            EXPECT_EQ(mojom::AccessTokenStatus::kAuthError, status);
-            EXPECT_TRUE(token.empty());
-            std::move(quit_closure).Run();
-          }));
-  run_loop.Run();
+  ExpectAccessToken(mojom::AccessTokenStatus::kAuthError, "");
 }
 
 TEST_F(DriveFsHostTest, GetAccessToken_GetAccessTokenFailure_Transient) {
@@ -788,18 +783,7 @@ TEST_F(DriveFsHostTest, GetAccessToken_GetAccessTokenFailure_Transient) {
       .WillOnce(testing::Return(std::make_pair(
           base::nullopt, GoogleServiceAuthError::SERVICE_UNAVAILABLE)));
   host_delegate_->mock_flow().ExpectNoStartCalls();
-
-  base::RunLoop run_loop;
-  auto quit_closure = run_loop.QuitClosure();
-  delegate_ptr_->GetAccessToken(
-      "client ID", "app ID", {"scope1", "scope2"},
-      base::BindLambdaForTesting(
-          [&](mojom::AccessTokenStatus status, const std::string& token) {
-            EXPECT_EQ(mojom::AccessTokenStatus::kTransientError, status);
-            EXPECT_TRUE(token.empty());
-            std::move(quit_closure).Run();
-          }));
-  run_loop.Run();
+  ExpectAccessToken(mojom::AccessTokenStatus::kTransientError, "");
 }
 
 TEST_F(DriveFsHostTest, GetAccessToken_MintTokenFailure_Permanent) {
@@ -811,18 +795,7 @@ TEST_F(DriveFsHostTest, GetAccessToken_MintTokenFailure_Permanent) {
           std::make_pair("chrome token", GoogleServiceAuthError::NONE)));
   host_delegate_->mock_flow().ExpectStartAndFail(
       "chrome token", GoogleServiceAuthError::ACCOUNT_DISABLED);
-
-  base::RunLoop run_loop;
-  auto quit_closure = run_loop.QuitClosure();
-  delegate_ptr_->GetAccessToken(
-      "client ID", "app ID", {"scope1", "scope2"},
-      base::BindLambdaForTesting(
-          [&](mojom::AccessTokenStatus status, const std::string& token) {
-            EXPECT_EQ(mojom::AccessTokenStatus::kAuthError, status);
-            EXPECT_TRUE(token.empty());
-            std::move(quit_closure).Run();
-          }));
-  run_loop.Run();
+  ExpectAccessToken(mojom::AccessTokenStatus::kAuthError, "");
 }
 
 TEST_F(DriveFsHostTest, GetAccessToken_MintTokenFailure_Transient) {
@@ -834,18 +807,7 @@ TEST_F(DriveFsHostTest, GetAccessToken_MintTokenFailure_Transient) {
           std::make_pair("chrome token", GoogleServiceAuthError::NONE)));
   host_delegate_->mock_flow().ExpectStartAndFail(
       "chrome token", GoogleServiceAuthError::SERVICE_UNAVAILABLE);
-
-  base::RunLoop run_loop;
-  auto quit_closure = run_loop.QuitClosure();
-  delegate_ptr_->GetAccessToken(
-      "client ID", "app ID", {"scope1", "scope2"},
-      base::BindLambdaForTesting(
-          [&](mojom::AccessTokenStatus status, const std::string& token) {
-            EXPECT_EQ(mojom::AccessTokenStatus::kTransientError, status);
-            EXPECT_TRUE(token.empty());
-            std::move(quit_closure).Run();
-          }));
-  run_loop.Run();
+  ExpectAccessToken(mojom::AccessTokenStatus::kTransientError, "");
 }
 
 TEST_F(DriveFsHostTest, GetAccessToken_UnmountDuringMojoRequest) {
@@ -1026,5 +988,124 @@ TEST_F(DriveFsHostTest, RemoveDriveNotificationObserver) {
                    .might_have_observers());
 }
 
+TEST_F(DriveFsHostTest, Remount_Cached) {
+  ASSERT_NO_FATAL_FAILURE(DoMount());
+
+  EXPECT_CALL(mock_identity_manager_,
+              GetAccessToken("test@example.com", _, "drivefs"))
+      .WillOnce(testing::Return(
+          std::make_pair("chrome token", GoogleServiceAuthError::NONE)));
+
+  host_delegate_->mock_flow().ExpectStartAndSucceed("chrome token",
+                                                    "auth token");
+
+  ExpectAccessToken(mojom::AccessTokenStatus::kSuccess, "auth token");
+
+  base::Optional<base::TimeDelta> delay = base::TimeDelta::FromSeconds(5);
+  EXPECT_CALL(*host_delegate_, OnUnmounted(delay));
+  SendOnUnmounted(delay);
+  base::RunLoop().RunUntilIdle();
+  ASSERT_NO_FATAL_FAILURE(DoUnmount());
+
+  // Second mount attempt should reuse already available token.
+  ASSERT_NO_FATAL_FAILURE(DoMount());
+  ExpectAccessToken(mojom::AccessTokenStatus::kSuccess, "auth token");
+}
+
+TEST_F(DriveFsHostTest, Remount_CachedOnceOnly) {
+  ASSERT_NO_FATAL_FAILURE(DoMount());
+
+  EXPECT_CALL(mock_identity_manager_,
+              GetAccessToken("test@example.com", _, "drivefs"))
+      .WillOnce(testing::Return(
+          std::make_pair("chrome token", GoogleServiceAuthError::NONE)))
+      .WillOnce(testing::Return(
+          std::make_pair("chrome token 2", GoogleServiceAuthError::NONE)));
+
+  host_delegate_->mock_flow().ExpectStartAndSucceed("chrome token",
+                                                    "auth token");
+
+  ExpectAccessToken(mojom::AccessTokenStatus::kSuccess, "auth token");
+
+  base::Optional<base::TimeDelta> delay = base::TimeDelta::FromSeconds(5);
+  EXPECT_CALL(*host_delegate_, OnUnmounted(delay));
+  SendOnUnmounted(delay);
+  base::RunLoop().RunUntilIdle();
+  ASSERT_NO_FATAL_FAILURE(DoUnmount());
+
+  // Second mount attempt should reuse already available token.
+  ASSERT_NO_FATAL_FAILURE(DoMount());
+  ExpectAccessToken(mojom::AccessTokenStatus::kSuccess, "auth token");
+
+  // But if it asks for token more than once it goes straight to identity.
+  host_delegate_->mock_flow().ExpectStartAndSucceed("chrome token 2",
+                                                    "auth token 2");
+
+  ExpectAccessToken(mojom::AccessTokenStatus::kSuccess, "auth token 2");
+}
+
+TEST_F(DriveFsHostTest, Remount_CacheExpired) {
+  ASSERT_NO_FATAL_FAILURE(DoMount());
+
+  EXPECT_CALL(mock_identity_manager_,
+              GetAccessToken("test@example.com", _, "drivefs"))
+      .WillOnce(testing::Return(
+          std::make_pair("chrome token", GoogleServiceAuthError::NONE)))
+      .WillOnce(testing::Return(
+          std::make_pair("chrome token 2", GoogleServiceAuthError::NONE)));
+
+  host_delegate_->mock_flow().ExpectStartAndSucceed("chrome token",
+                                                    "auth token");
+
+  ExpectAccessToken(mojom::AccessTokenStatus::kSuccess, "auth token");
+
+  base::Optional<base::TimeDelta> delay = base::TimeDelta::FromSeconds(5);
+  EXPECT_CALL(*host_delegate_, OnUnmounted(delay));
+  SendOnUnmounted(delay);
+  base::RunLoop().RunUntilIdle();
+  ASSERT_NO_FATAL_FAILURE(DoUnmount());
+
+  clock_.Advance(base::TimeDelta::FromHours(2));
+
+  // As the token expired second mount should go to identity.
+  ASSERT_NO_FATAL_FAILURE(DoMount());
+  host_delegate_->mock_flow().ExpectStartAndSucceed("chrome token 2",
+                                                    "auth token 2");
+
+  ExpectAccessToken(mojom::AccessTokenStatus::kSuccess, "auth token 2");
+}
+
+TEST_F(DriveFsHostTest, Remount_RequestInflight) {
+  ASSERT_NO_FATAL_FAILURE(DoMount());
+
+  EXPECT_CALL(mock_identity_manager_,
+              GetAccessToken("test@example.com", _, "drivefs"))
+      .WillOnce(testing::Return(
+          std::make_pair("chrome token", GoogleServiceAuthError::NONE)));
+  OAuth2MintTokenFlow::Delegate* delegate = nullptr;
+  EXPECT_CALL(host_delegate_->mock_flow(), Start(_, "chrome token"))
+      .WillOnce(testing::SaveArg<0>(&delegate));
+
+  delegate_ptr_->GetAccessToken(
+      "client ID", "app ID", {"scope1", "scope2"},
+      base::BindLambdaForTesting([&](mojom::AccessTokenStatus status,
+                                     const std::string& token) { FAIL(); }));
+
+  base::Optional<base::TimeDelta> delay = base::TimeDelta::FromSeconds(5);
+  EXPECT_CALL(*host_delegate_, OnUnmounted(delay));
+  SendOnUnmounted(delay);
+  base::RunLoop().RunUntilIdle();
+  ASSERT_NO_FATAL_FAILURE(DoUnmount());
+
+  // Now the response is ready.
+  delegate->OnMintTokenSuccess("auth token",
+                               base::TimeDelta::FromHours(1).InSeconds());
+
+  // Second mount will reuse previous token.
+  ASSERT_NO_FATAL_FAILURE(DoMount());
+
+  ExpectAccessToken(mojom::AccessTokenStatus::kSuccess, "auth token");
+}
+
 }  // namespace
 }  // namespace drivefs

chromeos/disks/mock_disk_mount_manager.cc

@@ -56,22 +56,16 @@ std::unique_ptr<Disk::Builder> MakeDiskBuilder() {
 
 }  // namespace
 
-void MockDiskMountManager::AddObserverInternal(
-    DiskMountManager::Observer* observer) {
+void MockDiskMountManager::AddObserver(DiskMountManager::Observer* observer) {
   observers_.AddObserver(observer);
 }
 
-void MockDiskMountManager::RemoveObserverInternal(
+void MockDiskMountManager::RemoveObserver(
     DiskMountManager::Observer* observer) {
   observers_.RemoveObserver(observer);
 }
 
 MockDiskMountManager::MockDiskMountManager() {
-  ON_CALL(*this, AddObserver(_))
-      .WillByDefault(Invoke(this, &MockDiskMountManager::AddObserverInternal));
-  ON_CALL(*this, RemoveObserver(_))
-      .WillByDefault(Invoke(this,
-                            &MockDiskMountManager::RemoveObserverInternal));
   ON_CALL(*this, disks())
       .WillByDefault(Invoke(this, &MockDiskMountManager::disksInternal));
   ON_CALL(*this, mount_points())
@@ -137,10 +131,6 @@ void MockDiskMountManager::NotifyMountEvent(MountEvent event,
 }
 
 void MockDiskMountManager::SetupDefaultReplies() {
-  EXPECT_CALL(*this, AddObserver(_))
-      .Times(AnyNumber());
-  EXPECT_CALL(*this, RemoveObserver(_))
-      .Times(AnyNumber());
   EXPECT_CALL(*this, disks())
       .WillRepeatedly(ReturnRef(disks_));
   EXPECT_CALL(*this, mount_points())

chromeos/disks/mock_disk_mount_manager.h

@@ -28,8 +28,8 @@ class MockDiskMountManager : public DiskMountManager {
 
   // DiskMountManager override.
   MOCK_METHOD0(Init, void(void));
-  MOCK_METHOD1(AddObserver, void(DiskMountManager::Observer*));
-  MOCK_METHOD1(RemoveObserver, void(DiskMountManager::Observer*));
+  void AddObserver(DiskMountManager::Observer*) override;
+  void RemoveObserver(DiskMountManager::Observer*) override;
   MOCK_CONST_METHOD0(disks, const DiskMountManager::DiskMap&(void));
   MOCK_CONST_METHOD1(FindDiskBySourcePath, const Disk*(const std::string&));
   MOCK_CONST_METHOD0(mount_points,