Handle drivefs refusing to start.

If feature flags has changed drivefs may need a restart before it's able
to serve requests. In this case mounting will fail. Make browser handle
this situation by retrying after some time.

BUG=chromium:845390,chromium:845388

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

chrome/browser/chromeos/drive/drive_integration_service.cc

@@ -276,6 +276,10 @@ class DriveIntegrationService::DriveFsHolder
         ->GetAccountId();
   }
 
+  void OnMountFailed(base::Optional<base::TimeDelta> remount_delay) override {
+    on_drivefs_unmounted_.Run(std::move(remount_delay));
+  }
+
   void OnMounted(const base::FilePath& path) override {
     on_drivefs_mounted_.Run();
   }

chromeos/components/drivefs/drivefs_host.cc

@@ -148,13 +148,12 @@ class DriveFsHost::MountState : public mojom::DriveFsDelegate,
       return true;
     }
     if (error_code != chromeos::MOUNT_ERROR_NONE) {
+      host_->delegate_->OnMountFailed({});
       return false;
     }
-    mount_path_ = base::FilePath(mount_info.mount_path);
     DCHECK(!mount_info.mount_path.empty());
-    if (mounted()) {
-      NotifyDelegateOnMounted();
-    }
+    mount_path_ = base::FilePath(mount_info.mount_path);
+    MaybeNotifyDelegateOnMounted();
     return true;
   }
 
@@ -181,21 +180,25 @@ class DriveFsHost::MountState : public mojom::DriveFsDelegate,
   void OnMounted() override {
     DCHECK_CALLED_ON_VALID_SEQUENCE(host_->sequence_checker_);
     drivefs_has_mounted_ = true;
-    if (mounted()) {
-      NotifyDelegateOnMounted();
-    }
+    MaybeNotifyDelegateOnMounted();
   }
 
-  void OnSyncingStatusUpdate(mojom::SyncingStatusPtr status) override {
-    for (auto& observer : host_->observers_) {
-      observer.OnSyncingStatusUpdate(*status);
-    }
+  void OnMountFailed(base::Optional<base::TimeDelta> remount_delay) override {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(host_->sequence_checker_);
+    drivefs_has_mounted_ = false;
+    host_->delegate_->OnMountFailed(std::move(remount_delay));
   }
 
   void OnUnmounted(base::Optional<base::TimeDelta> remount_delay) override {
     DCHECK_CALLED_ON_VALID_SEQUENCE(host_->sequence_checker_);
     drivefs_has_mounted_ = false;
-    NotifyDelegateOnUnmounted(std::move(remount_delay));
+    host_->delegate_->OnUnmounted(std::move(remount_delay));
+  }
+
+  void OnSyncingStatusUpdate(mojom::SyncingStatusPtr status) override {
+    for (auto& observer : host_->observers_) {
+      observer.OnSyncingStatusUpdate(*status);
+    }
   }
 
   void OnFilesChanged(std::vector<mojom::FileChangePtr> changes) override {
@@ -209,11 +212,10 @@ class DriveFsHost::MountState : public mojom::DriveFsDelegate,
     }
   }
 
-  void NotifyDelegateOnMounted() { host_->delegate_->OnMounted(mount_path()); }
-
-  void NotifyDelegateOnUnmounted(
-      base::Optional<base::TimeDelta> remount_delay) {
-    host_->delegate_->OnUnmounted(std::move(remount_delay));
+  void MaybeNotifyDelegateOnMounted() {
+    if (mounted()) {
+      host_->delegate_->OnMounted(mount_path());
+    }
   }
 
   void AccountReady(const AccountInfo& info,
@@ -346,6 +348,7 @@ void DriveFsHost::OnMountEvent(
   if (!mount_state_) {
     return;
   }
+
   if (!mount_state_->OnMountEvent(event, error_code, mount_info)) {
     Unmount();
   }

chromeos/components/drivefs/drivefs_host.h

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

chromeos/components/drivefs/drivefs_host_unittest.cc

@@ -119,6 +119,7 @@ class TestingDriveFsHostDelegate : public DriveFsHost::Delegate {
 
   // DriveFsHost::Delegate:
   MOCK_METHOD1(OnMounted, void(const base::FilePath&));
+  MOCK_METHOD1(OnMountFailed, void(base::Optional<base::TimeDelta>));
   MOCK_METHOD1(OnUnmounted, void(base::Optional<base::TimeDelta>));
 
  private:
@@ -300,7 +301,11 @@ class DriveFsHostTest : public ::testing::Test, public mojom::DriveFsBootstrap {
     delegate_ptr_->OnUnmounted(std::move(delay));
   }
 
-  void DoMount() {
+  void SendMountFailed(base::Optional<base::TimeDelta> delay) {
+    delegate_ptr_->OnMountFailed(std::move(delay));
+  }
+
+  void EstablishConnection() {
     auto token = StartMount();
     DispatchMountSuccessEvent(token);
 
@@ -310,6 +315,10 @@ class DriveFsHostTest : public ::testing::Test, public mojom::DriveFsBootstrap {
       bootstrap_binding_.set_connection_error_handler(run_loop.QuitClosure());
       run_loop.Run();
     }
+  }
+
+  void DoMount() {
+    EstablishConnection();
     base::RunLoop run_loop;
     base::OnceClosure quit_closure = run_loop.QuitClosure();
     EXPECT_CALL(*host_delegate_,
@@ -375,6 +384,39 @@ TEST_F(DriveFsHostTest, OnMountedBeforeMountEvent) {
   EXPECT_EQ(base::FilePath("/media/drivefsroot/g-ID"), host_->GetMountPath());
 }
 
+TEST_F(DriveFsHostTest, OnMountFailedFromMojo) {
+  ASSERT_FALSE(host_->IsMounted());
+
+  ASSERT_NO_FATAL_FAILURE(EstablishConnection());
+  base::RunLoop run_loop;
+  base::OnceClosure quit_closure = run_loop.QuitClosure();
+  EXPECT_CALL(*host_delegate_, OnMountFailed(_))
+      .WillOnce(RunQuitClosure(&quit_closure));
+  SendMountFailed({});
+  run_loop.Run();
+  ASSERT_FALSE(host_->IsMounted());
+}
+
+TEST_F(DriveFsHostTest, OnMountFailedFromDbus) {
+  ASSERT_FALSE(host_->IsMounted());
+
+  auto token = StartMount();
+
+  base::RunLoop run_loop;
+  base::OnceClosure quit_closure = run_loop.QuitClosure();
+  EXPECT_CALL(*host_delegate_, OnMountFailed(_))
+      .WillOnce(RunQuitClosure(&quit_closure));
+  DispatchMountEvent(chromeos::disks::DiskMountManager::MOUNTING,
+                     chromeos::MOUNT_ERROR_INVALID_MOUNT_OPTIONS,
+                     {base::StrCat({"drivefs://", token}),
+                      "/media/drivefsroot/g-ID",
+                      chromeos::MOUNT_TYPE_NETWORK_STORAGE,
+                      {}});
+  run_loop.Run();
+
+  ASSERT_FALSE(host_->IsMounted());
+}
+
 TEST_F(DriveFsHostTest, UnmountAfterMountComplete) {
   MockDriveFsHostObserver observer;
   ScopedObserver<DriveFsHost, DriveFsHostObserver> observer_scoper(&observer);

chromeos/components/drivefs/mojom/drivefs.mojom

@@ -41,13 +41,17 @@ interface DriveFsDelegate {
   // Invoked when the mount is ready for use.
   OnMounted();
 
-  // Invoked when the syncing status changes.
-  OnSyncingStatusUpdate(SyncingStatus status);
+  // Invoked if mounting has failed. If retry_delay is present the
+  // browser should try to mount again after the specified interval.
+  OnMountFailed(mojo_base.mojom.TimeDelta? retry_delay);
 
   // Invoked when the mount is going away. If retry_delay is present the
-  // browser should try to mount DriveFs again after the specified interval.
+  // browser should try to mount again after the specified interval.
   OnUnmounted(mojo_base.mojom.TimeDelta? retry_delay);
 
+  // Invoked when the syncing status changes.
+  OnSyncingStatusUpdate(SyncingStatus status);
+
   // Invoked when server-side file changes are received.
   OnFilesChanged(array<FileChange> changes);
 };