Schedule remount of drivefs if it was requested by the service

If drivefs needs to restart itself it will inform the browser and unmount,
and the browser will try to mount again after specified time.

BUG=chromium:845390

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

chrome/browser/chromeos/drive/drive_integration_service.cc

@@ -247,27 +247,56 @@ class DriveIntegrationService::DriveFsHolder
  public:
   DriveFsHolder(Profile* profile,
                 base::RepeatingClosure on_drivefs_mounted,
-                base::RepeatingClosure on_drivefs_unmounted,
+                base::RepeatingCallback<void(base::Optional<base::TimeDelta>)>
+                    on_drivefs_unmounted,
                 DriveFsMojoConnectionDelegateFactory
-                    test_drivefs_mojo_connection_delegate_factory);
+                    test_drivefs_mojo_connection_delegate_factory)
+      : profile_(profile),
+        on_drivefs_mounted_(std::move(on_drivefs_mounted)),
+        on_drivefs_unmounted_(std::move(on_drivefs_unmounted)),
+        test_drivefs_mojo_connection_delegate_factory_(
+            std::move(test_drivefs_mojo_connection_delegate_factory)),
+        drivefs_host_(profile_->GetPath(), this) {}
 
   drivefs::DriveFsHost* drivefs_host() { return &drivefs_host_; }
 
  private:
   // drivefs::DriveFsHost::Delegate:
-  net::URLRequestContextGetter* GetRequestContext() override;
-  service_manager::Connector* GetConnector() override;
-  const AccountId& GetAccountId() override;
-  void OnMounted(const base::FilePath& path) override;
-  void OnUnmounted() override;
+  net::URLRequestContextGetter* GetRequestContext() override {
+    return profile_->GetRequestContext();
+  }
+
+  service_manager::Connector* GetConnector() override {
+    return content::BrowserContext::GetConnectorFor(profile_);
+  }
+
+  const AccountId& GetAccountId() override {
+    return chromeos::ProfileHelper::Get()
+        ->GetUserByProfile(profile_)
+        ->GetAccountId();
+  }
+
+  void OnMounted(const base::FilePath& path) override {
+    on_drivefs_mounted_.Run();
+  }
+
+  void OnUnmounted(base::Optional<base::TimeDelta> remount_delay) override {
+    on_drivefs_unmounted_.Run(std::move(remount_delay));
+  }
+
   std::unique_ptr<drivefs::DriveFsHost::MojoConnectionDelegate>
-  CreateMojoConnectionDelegate() override;
+  CreateMojoConnectionDelegate() override {
+    if (test_drivefs_mojo_connection_delegate_factory_)
+      return test_drivefs_mojo_connection_delegate_factory_.Run();
+    return Delegate::CreateMojoConnectionDelegate();
+  }
 
   Profile* const profile_;
 
   // Invoked when DriveFS mounting is completed.
   const base::RepeatingClosure on_drivefs_mounted_;
-  const base::RepeatingClosure on_drivefs_unmounted_;
+  const base::RepeatingCallback<void(base::Optional<base::TimeDelta>)>
+      on_drivefs_unmounted_;
 
   const DriveFsMojoConnectionDelegateFactory
       test_drivefs_mojo_connection_delegate_factory_;
@@ -277,51 +306,6 @@ class DriveIntegrationService::DriveFsHolder
   DISALLOW_COPY_AND_ASSIGN(DriveFsHolder);
 };
 
-DriveIntegrationService::DriveFsHolder::DriveFsHolder(
-    Profile* profile,
-    base::RepeatingClosure on_drivefs_mounted,
-    base::RepeatingClosure on_drivefs_unmounted,
-    DriveFsMojoConnectionDelegateFactory
-        test_drivefs_mojo_connection_delegate_factory)
-    : profile_(profile),
-      on_drivefs_mounted_(std::move(on_drivefs_mounted)),
-      on_drivefs_unmounted_(std::move(on_drivefs_unmounted)),
-      test_drivefs_mojo_connection_delegate_factory_(
-          std::move(test_drivefs_mojo_connection_delegate_factory)),
-      drivefs_host_(profile_->GetPath(), this) {}
-
-net::URLRequestContextGetter*
-DriveIntegrationService::DriveFsHolder::GetRequestContext() {
-  return profile_->GetRequestContext();
-}
-
-service_manager::Connector*
-DriveIntegrationService::DriveFsHolder::GetConnector() {
-  return content::BrowserContext::GetConnectorFor(profile_);
-}
-
-const AccountId& DriveIntegrationService::DriveFsHolder::GetAccountId() {
-  return chromeos::ProfileHelper::Get()
-      ->GetUserByProfile(profile_)
-      ->GetAccountId();
-}
-
-std::unique_ptr<drivefs::DriveFsHost::MojoConnectionDelegate>
-DriveIntegrationService::DriveFsHolder::CreateMojoConnectionDelegate() {
-  if (test_drivefs_mojo_connection_delegate_factory_)
-    return test_drivefs_mojo_connection_delegate_factory_.Run();
-  return Delegate::CreateMojoConnectionDelegate();
-}
-
-void DriveIntegrationService::DriveFsHolder::OnMounted(
-    const base::FilePath& path) {
-  on_drivefs_mounted_.Run();
-}
-
-void DriveIntegrationService::DriveFsHolder::OnUnmounted() {
-  on_drivefs_unmounted_.Run();
-}
-
 DriveIntegrationService::DriveIntegrationService(
     Profile* profile,
     PreferenceWatcher* preference_watcher,
@@ -347,7 +331,7 @@ DriveIntegrationService::DriveIntegrationService(
                                             AddDriveMountPointAfterMounted,
                                         base::Unretained(this)),
                     base::BindRepeating(
-                        &DriveIntegrationService::RemoveDriveMountPoint,
+                        &DriveIntegrationService::MaybeRemountFileSystem,
                         base::Unretained(this)),
                     std::move(test_drivefs_mojo_connection_delegate_factory))
               : nullptr),
@@ -610,6 +594,8 @@ void DriveIntegrationService::AddDriveMountPoint() {
   DCHECK_EQ(INITIALIZED, state_);
   DCHECK(enabled_);
 
+  weak_ptr_factory_.InvalidateWeakPtrs();
+
   if (drivefs_holder_ && !drivefs_holder_->drivefs_host()->IsMounted()) {
     drivefs_holder_->drivefs_host()->Mount();
     return;
@@ -641,6 +627,8 @@ void DriveIntegrationService::AddDriveMountPointAfterMounted() {
 void DriveIntegrationService::RemoveDriveMountPoint() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
+  weak_ptr_factory_.InvalidateWeakPtrs();
+
   if (!mount_point_name_.empty()) {
     if (!drivefs_holder_)
       job_list()->CancelAllJobs();
@@ -659,6 +647,21 @@ void DriveIntegrationService::RemoveDriveMountPoint() {
     drivefs_holder_->drivefs_host()->Unmount();
 }
 
+void DriveIntegrationService::MaybeRemountFileSystem(
+    base::Optional<base::TimeDelta> remount_delay) {
+  DCHECK_EQ(INITIALIZED, state_);
+
+  RemoveDriveMountPoint();
+
+  if (remount_delay) {
+    base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
+        FROM_HERE,
+        base::BindOnce(&DriveIntegrationService::AddDriveMountPoint,
+                       weak_ptr_factory_.GetWeakPtr()),
+        remount_delay.value());
+  }
+}
+
 void DriveIntegrationService::Initialize() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK_EQ(NOT_INITIALIZED, state_);

chrome/browser/chromeos/drive/drive_integration_service.h

@@ -183,6 +183,10 @@ class DriveIntegrationService : public KeyedService,
   void AddBackDriveMountPoint(const base::Callback<void(bool)>& callback,
                               FileError error);
 
+  // Unregisters drive mount point, and if |remount_delay| is specified
+  // then tries to add it back after that delay.
+  void MaybeRemountFileSystem(base::Optional<base::TimeDelta> remount_delay);
+
   // Initializes the object. This function should be called before any
   // other functions.
   void Initialize();

chromeos/components/drivefs/drivefs_host.cc

@@ -192,13 +192,10 @@ class DriveFsHost::MountState : public mojom::DriveFsDelegate,
     }
   }
 
-  void OnUnmounted(base::Optional<base::TimeDelta> retry_delay) override {
+  void OnUnmounted(base::Optional<base::TimeDelta> remount_delay) override {
     DCHECK_CALLED_ON_VALID_SEQUENCE(host_->sequence_checker_);
     drivefs_has_mounted_ = false;
-    NotifyDelegateOnUnmounted();
-    if (retry_delay) {
-      // TODO(crbug.com/845390): Schedule restart.
-    }
+    NotifyDelegateOnUnmounted(std::move(remount_delay));
   }
 
   void OnFilesChanged(std::vector<mojom::FileChangePtr> changes) override {
@@ -214,7 +211,10 @@ class DriveFsHost::MountState : public mojom::DriveFsDelegate,
 
   void NotifyDelegateOnMounted() { host_->delegate_->OnMounted(mount_path()); }
 
-  void NotifyDelegateOnUnmounted() { host_->delegate_->OnUnmounted(); }
+  void NotifyDelegateOnUnmounted(
+      base::Optional<base::TimeDelta> remount_delay) {
+    host_->delegate_->OnUnmounted(std::move(remount_delay));
+  }
 
   void AccountReady(const AccountInfo& info,
                     const identity::AccountState& state) {
@@ -328,7 +328,7 @@ bool DriveFsHost::IsMounted() const {
 }
 
 const base::FilePath& DriveFsHost::GetMountPath() const {
-  DCHECK(IsMounted());
+  DCHECK(mount_state_);
   return mount_state_->mount_path();
 }
 

chromeos/components/drivefs/drivefs_host.h

@@ -68,7 +68,7 @@ class COMPONENT_EXPORT(DRIVEFS) DriveFsHost
     CreateMojoConnectionDelegate();
 
     virtual void OnMounted(const base::FilePath& mount_path) = 0;
-    virtual void OnUnmounted() = 0;
+    virtual void OnUnmounted(base::Optional<base::TimeDelta> remount_delay) = 0;
 
    private:
     DISALLOW_COPY_AND_ASSIGN(Delegate);

chromeos/components/drivefs/drivefs_host_unittest.cc

@@ -119,7 +119,7 @@ class TestingDriveFsHostDelegate : public DriveFsHost::Delegate {
 
   // DriveFsHost::Delegate:
   MOCK_METHOD1(OnMounted, void(const base::FilePath&));
-  MOCK_METHOD0(OnUnmounted, void());
+  MOCK_METHOD1(OnUnmounted, void(base::Optional<base::TimeDelta>));
 
  private:
   // DriveFsHost::Delegate:
@@ -296,7 +296,9 @@ class DriveFsHostTest : public ::testing::Test, public mojom::DriveFsBootstrap {
 
   void SendOnMounted() { delegate_ptr_->OnMounted(); }
 
-  void SendOnUnmounted() { delegate_ptr_->OnUnmounted({}); }
+  void SendOnUnmounted(base::Optional<base::TimeDelta> delay) {
+    delegate_ptr_->OnUnmounted(std::move(delay));
+  }
 
   void DoMount() {
     auto token = StartMount();
@@ -477,8 +479,9 @@ TEST_F(DriveFsHostTest, MountWhileAlreadyMounted) {
 
 TEST_F(DriveFsHostTest, UnmountByRemote) {
   ASSERT_NO_FATAL_FAILURE(DoMount());
-  EXPECT_CALL(*host_delegate_, OnUnmounted());
-  SendOnUnmounted();
+  base::Optional<base::TimeDelta> delay = base::TimeDelta::FromSeconds(5);
+  EXPECT_CALL(*host_delegate_, OnUnmounted(delay));
+  SendOnUnmounted(delay);
   base::RunLoop().RunUntilIdle();
 }