Files App: Add Rename to cros disks client.

This CL adds Rename method to the disk client implementation which through dbus invokes cros disks disk renaming functionality. Before renaming happens on the mounted disk, client will save its mount point name for later when re-mounting will occur and try to re-mount the disk to previous mount point name to preserve file paths. BUG=274041 CQ-DEPEND=CL:605108 Change-Id: I40d0522cb5195940b0b949418a99e2eaca1e1041 Reviewed-on: https://chromium-review.googlesource.com/616510 Commit-Queue: Klemen Kozjek <klemenko@google.com> Reviewed-by: Hidehiko Abe <hidehiko@chromium.org> Reviewed-by: Tomasz Mikolajewski <mtomasz@chromium.org> Reviewed-by: Tommy Li <tommycli@chromium.org> Reviewed-by: Naoki Fukino <fukino@chromium.org> Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Reviewed-by: Ilya Sherman <isherman@chromium.org> Reviewed-by: Dan Erat <derat@chromium.org> Cr-Commit-Position: refs/heads/master@{#497518}
Denger-Network · Aug 25, 2017 · bf5610f · bf5610f
1 parent ed5f671
commit bf5610f
Show file tree

Hide file tree

Showing 29 changed files with 553 additions and 112 deletions.
diff --git a/chrome/browser/chromeos/app_mode/kiosk_external_updater.cc b/chrome/browser/chromeos/app_mode/kiosk_external_updater.cc
@@ -180,6 +180,11 @@ void KioskExternalUpdater::OnFormatEvent(
     const std::string& device_path) {
 }
 
+void KioskExternalUpdater::OnRenameEvent(
+    disks::DiskMountManager::RenameEvent event,
+    RenameError error_code,
+    const std::string& device_path) {}
+
 void KioskExternalUpdater::OnExtenalUpdateUnpackSuccess(
     const std::string& app_id,
     const std::string& version,

diff --git a/chrome/browser/chromeos/app_mode/kiosk_external_updater.h b/chrome/browser/chromeos/app_mode/kiosk_external_updater.h
@@ -67,6 +67,9 @@ class KioskExternalUpdater : public disks::DiskMountManager::Observer,
   void OnFormatEvent(disks::DiskMountManager::FormatEvent event,
                      FormatError error_code,
                      const std::string& device_path) override;
+  void OnRenameEvent(disks::DiskMountManager::RenameEvent event,
+                     RenameError error_code,
+                     const std::string& device_path) override;
 
   // KioskExternalUpdateValidatorDelegate overrides:
   void OnExtenalUpdateUnpackSuccess(const std::string& app_id,

diff --git a/chrome/browser/chromeos/extensions/file_manager/device_event_router.cc b/chrome/browser/chromeos/extensions/file_manager/device_event_router.cc
@@ -121,6 +121,24 @@ void DeviceEventRouter::OnFormatCompleted(const std::string& device_path,
                 device_path);
 }
 
+void DeviceEventRouter::OnRenameStarted(const std::string& device_path,
+                                        bool success) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  OnDeviceEvent(success ? file_manager_private::DEVICE_EVENT_TYPE_RENAME_START
+                        : file_manager_private::DEVICE_EVENT_TYPE_RENAME_FAIL,
+                device_path);
+}
+
+void DeviceEventRouter::OnRenameCompleted(const std::string& device_path,
+                                          bool success) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  OnDeviceEvent(success ? file_manager_private::DEVICE_EVENT_TYPE_RENAME_SUCCESS
+                        : file_manager_private::DEVICE_EVENT_TYPE_RENAME_FAIL,
+                device_path);
+}
+
 void DeviceEventRouter::SuspendImminent() {
   DCHECK(thread_checker_.CalledOnValidThread());
   is_resuming_ = true;

diff --git a/chrome/browser/chromeos/extensions/file_manager/device_event_router.h b/chrome/browser/chromeos/extensions/file_manager/device_event_router.h
@@ -54,6 +54,8 @@ class DeviceEventRouter : public VolumeManagerObserver,
                          const Volume& volume) override;
   void OnFormatStarted(const std::string& device_path, bool success) override;
   void OnFormatCompleted(const std::string& device_path, bool success) override;
+  void OnRenameStarted(const std::string& device_path, bool success) override;
+  void OnRenameCompleted(const std::string& device_path, bool success) override;
 
   // PowerManagerClient::Observer overrides.
   void SuspendImminent() override;

diff --git a/chrome/browser/chromeos/extensions/file_manager/device_event_router_unittest.cc b/chrome/browser/chromeos/extensions/file_manager/device_event_router_unittest.cc
@@ -72,7 +72,7 @@ class DeviceEventRouterTest : public testing::Test {
                       bool is_read_only_hardware) {
     return Disk(device_path, mount_path, false, "", "", "", "", "", "", "", "",
                 "", device_path, chromeos::DEVICE_TYPE_UNKNOWN, 0, false,
-                is_read_only_hardware, false, false, false, false, "vfat");
+                is_read_only_hardware, false, false, false, false, "vfat", "");
   }
 
   std::unique_ptr<DeviceEventRouterImpl> device_event_router;

diff --git a/chrome/browser/chromeos/extensions/file_manager/event_router.cc b/chrome/browser/chromeos/extensions/file_manager/event_router.cc
@@ -1002,6 +1002,18 @@ void EventRouter::OnFormatCompleted(const std::string& device_path,
   // Do nothing.
 }
 
+void EventRouter::OnRenameStarted(const std::string& device_path,
+                                  bool success) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  // Do nothing.
+}
+
+void EventRouter::OnRenameCompleted(const std::string& device_path,
+                                    bool success) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  // Do nothing.
+}
+
 void EventRouter::SetDispatchDirectoryChangeEventImplForTesting(
     const DispatchDirectoryChangeEventImplCallback& callback) {
   dispatch_directory_change_event_impl_ = callback;

diff --git a/chrome/browser/chromeos/extensions/file_manager/event_router.h b/chrome/browser/chromeos/extensions/file_manager/event_router.h
@@ -143,7 +143,8 @@ class EventRouter : public KeyedService,
                          const Volume& volume) override;
   void OnFormatStarted(const std::string& device_path, bool success) override;
   void OnFormatCompleted(const std::string& device_path, bool success) override;
-
+  void OnRenameStarted(const std::string& device_path, bool success) override;
+  void OnRenameCompleted(const std::string& device_path, bool success) override;
   // Set custom dispatch directory change event implementation for testing.
   void SetDispatchDirectoryChangeEventImplForTesting(
       const DispatchDirectoryChangeEventImplCallback& callback);

diff --git a/chrome/browser/chromeos/extensions/file_manager/file_manager_private_apitest.cc b/chrome/browser/chromeos/extensions/file_manager/file_manager_private_apitest.cc
@@ -51,6 +51,7 @@ struct TestDiskInfo {
   bool on_removable_device;
   bool is_hidden;
   const char* file_system_type;
+  const char* base_mount_path;
 };
 
 struct TestMountPoint {
@@ -82,7 +83,8 @@ TestDiskInfo kTestDisks[] = {{"system_path1",
                               false,
                               false,
                               false,
-                              "exfat"},
+                              "exfat",
+                              ""},
                              {"system_path2",
                               "file_path2",
                               false,
@@ -102,7 +104,8 @@ TestDiskInfo kTestDisks[] = {{"system_path1",
                               true,
                               false,
                               false,
-                              "exfat"},
+                              "exfat",
+                              ""},
                              {"system_path3",
                               "file_path3",
                               true,  // write_disabled_by_policy
@@ -122,7 +125,8 @@ TestDiskInfo kTestDisks[] = {{"system_path1",
                               true,
                               false,
                               false,
-                              "exfat"}};
+                              "exfat",
+                              ""}};
 
 void DispatchDirectoryChangeEventImpl(
     int* counter,
@@ -289,7 +293,8 @@ class FileManagerPrivateApiTest : public ExtensionApiTest {
                 kTestDisks[disk_info_index].on_boot_device,
                 kTestDisks[disk_info_index].on_removable_device,
                 kTestDisks[disk_info_index].is_hidden,
-                kTestDisks[disk_info_index].file_system_type)));
+                kTestDisks[disk_info_index].file_system_type,
+                kTestDisks[disk_info_index].base_mount_path)));
       }
     }
   }

diff --git a/chrome/browser/chromeos/extensions/file_manager/private_api_file_system.cc b/chrome/browser/chromeos/extensions/file_manager/private_api_file_system.cc
@@ -625,10 +625,8 @@ bool FileManagerPrivateRenameVolumeFunction::RunAsync() {
   if (!volume)
     return false;
 
-  // TODO(klemenko): Uncomment the code below when RenameMountedDevice is
-  // implemented
-  /*DiskMountManager::GetInstance()->RenameMountedDevice(
-    volume->mount_path().AsUTF8Unsafe(), params->new_name);*/
+  DiskMountManager::GetInstance()->RenameMountedDevice(
+      volume->mount_path().AsUTF8Unsafe(), params->new_name);
   SendResponse(true);
   return true;
 }

diff --git a/chrome/browser/chromeos/file_manager/fake_disk_mount_manager.cc b/chrome/browser/chromeos/file_manager/fake_disk_mount_manager.cc
@@ -131,6 +131,10 @@ bool FakeDiskMountManager::FinishAllUnmountPathRequests() {
 void FakeDiskMountManager::FormatMountedDevice(const std::string& mount_path) {
 }
 
+void FakeDiskMountManager::RenameMountedDevice(const std::string& mount_path,
+                                               const std::string& volume_name) {
+}
+
 void FakeDiskMountManager::UnmountDeviceRecursively(
     const std::string& device_path,
     const UnmountDeviceRecursivelyCallbackType& callback) {

diff --git a/chrome/browser/chromeos/file_manager/fake_disk_mount_manager.h b/chrome/browser/chromeos/file_manager/fake_disk_mount_manager.h
@@ -90,6 +90,8 @@ class FakeDiskMountManager : public chromeos::disks::DiskMountManager {
   void RemountAllRemovableDrives(
       chromeos::MountAccessMode access_mode) override;
   void FormatMountedDevice(const std::string& mount_path) override;
+  void RenameMountedDevice(const std::string& mount_path,
+                           const std::string& volume_name) override;
   void UnmountDeviceRecursively(
       const std::string& device_path,
       const UnmountDeviceRecursivelyCallbackType& callback) override;

diff --git a/chrome/browser/chromeos/file_manager/volume_manager.cc b/chrome/browser/chromeos/file_manager/volume_manager.cc
@@ -687,6 +687,56 @@ void VolumeManager::OnFormatEvent(
   NOTREACHED();
 }
 
+void VolumeManager::OnRenameEvent(
+    chromeos::disks::DiskMountManager::RenameEvent event,
+    chromeos::RenameError error_code,
+    const std::string& device_path) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DVLOG(1) << "OnDeviceEvent: " << event << ", " << error_code << ", "
+           << device_path;
+
+  switch (event) {
+    case chromeos::disks::DiskMountManager::RENAME_STARTED:
+      for (auto& observer : observers_) {
+        observer.OnRenameStarted(device_path,
+                                 error_code == chromeos::RENAME_ERROR_NONE);
+      }
+      return;
+    case chromeos::disks::DiskMountManager::RENAME_COMPLETED:
+      if (error_code != chromeos::RENAME_ERROR_NONE) {
+        for (auto& observer : observers_)
+          observer.OnRenameCompleted(device_path, false);
+
+        return;
+      }
+
+      // Find previous mount point label if it exists
+      std::string mount_label = "";
+      auto disk_map_iter = disk_mount_manager_->disks().find(device_path);
+      if (disk_map_iter != disk_mount_manager_->disks().end() &&
+          !disk_map_iter->second->base_mount_path().empty()) {
+        mount_label = base::FilePath(disk_map_iter->second->base_mount_path())
+                          .BaseName()
+                          .AsUTF8Unsafe();
+      }
+
+      // If rename is completed successfully, try to mount the device.
+      // MountPath auto-detects filesystem format if second argument is
+      // empty. Third argument is a mount point name of the disk when it was
+      // first time mounted (to preserve mount point regardless of the volume
+      // name).
+      disk_mount_manager_->MountPath(device_path, std::string(), mount_label,
+                                     chromeos::MOUNT_TYPE_DEVICE,
+                                     GetExternalStorageAccessMode(profile_));
+
+      for (auto& observer : observers_)
+        observer.OnRenameCompleted(device_path, true);
+
+      return;
+  }
+  NOTREACHED();
+}
+
 void VolumeManager::OnProvidedFileSystemMount(
     const chromeos::file_system_provider::ProvidedFileSystemInfo&
         file_system_info,

diff --git a/chrome/browser/chromeos/file_manager/volume_manager.h b/chrome/browser/chromeos/file_manager/volume_manager.h
@@ -296,6 +296,9 @@ class VolumeManager : public KeyedService,
   void OnFormatEvent(chromeos::disks::DiskMountManager::FormatEvent event,
                      chromeos::FormatError error_code,
                      const std::string& device_path) override;
+  void OnRenameEvent(chromeos::disks::DiskMountManager::RenameEvent event,
+                     chromeos::RenameError error_code,
+                     const std::string& device_path) override;
 
   // chromeos::file_system_provider::Observer overrides.
   void OnProvidedFileSystemMount(

diff --git a/chrome/browser/chromeos/file_manager/volume_manager_observer.h b/chrome/browser/chromeos/file_manager/volume_manager_observer.h
@@ -48,6 +48,14 @@ class VolumeManagerObserver {
   // Fired when formatting a device is completed (or terminated on error).
   virtual void OnFormatCompleted(
       const std::string& device_path, bool success) = 0;
+
+  // Fired when renaming a device is started (or failed to start).
+  virtual void OnRenameStarted(const std::string& device_path,
+                               bool success) = 0;
+
+  // Fired when renaming a device is completed (or terminated on error).
+  virtual void OnRenameCompleted(const std::string& device_path,
+                                 bool success) = 0;
 };
 
 }  // namespace file_manager

diff --git a/chrome/browser/chromeos/file_manager/volume_manager_unittest.cc b/chrome/browser/chromeos/file_manager/volume_manager_unittest.cc
@@ -43,6 +43,8 @@ class LoggingObserver : public VolumeManagerObserver {
       VOLUME_UNMOUNTED,
       FORMAT_STARTED,
       FORMAT_COMPLETED,
+      RENAME_STARTED,
+      RENAME_COMPLETED
     } type;
 
     // Available on DEVICE_ADDED, DEVICE_REMOVED, VOLUME_MOUNTED,
@@ -131,6 +133,23 @@ class LoggingObserver : public VolumeManagerObserver {
     events_.push_back(event);
   }
 
+  void OnRenameStarted(const std::string& device_path, bool success) override {
+    Event event;
+    event.type = Event::RENAME_STARTED;
+    event.device_path = device_path;
+    event.success = success;
+    events_.push_back(event);
+  }
+
+  void OnRenameCompleted(const std::string& device_path,
+                         bool success) override {
+    Event event;
+    event.type = Event::RENAME_COMPLETED;
+    event.device_path = device_path;
+    event.success = success;
+    events_.push_back(event);
+  }
+
  private:
   std::vector<Event> events_;
 
@@ -243,7 +262,7 @@ TEST_F(VolumeManagerTest, OnDiskEvent_Hidden) {
   const chromeos::disks::DiskMountManager::Disk kDisk(
       "device1", "", false, "", "", "", "", "", "", "", "", "", "",
       chromeos::DEVICE_TYPE_UNKNOWN, 0, false, false, false, false, false,
-      kIsHidden, "");
+      kIsHidden, "", "");
 
   volume_manager()->OnDiskEvent(
       chromeos::disks::DiskMountManager::DISK_ADDED, &kDisk);
@@ -271,7 +290,7 @@ TEST_F(VolumeManagerTest, OnDiskEvent_Added) {
       "",  // empty device path.
       "", false, "", "", "", "", "", "", "", "", "", "",
       chromeos::DEVICE_TYPE_UNKNOWN, 0, false, false, false, false, false,
-      false, "");
+      false, "", "");
   volume_manager()->OnDiskEvent(
       chromeos::disks::DiskMountManager::DISK_ADDED, &kEmptyDevicePathDisk);
   EXPECT_EQ(0U, observer.events().size());
@@ -280,7 +299,7 @@ TEST_F(VolumeManagerTest, OnDiskEvent_Added) {
   const chromeos::disks::DiskMountManager::Disk kMediaDisk(
       "device1", "", false, "", "", "", "", "", "", "", "", "", "",
       chromeos::DEVICE_TYPE_UNKNOWN, 0, false, false, kHasMedia, false, false,
-      false, "");
+      false, "", "");
   volume_manager()->OnDiskEvent(
       chromeos::disks::DiskMountManager::DISK_ADDED, &kMediaDisk);
   ASSERT_EQ(1U, observer.events().size());
@@ -313,7 +332,7 @@ TEST_F(VolumeManagerTest, OnDiskEvent_AddedNonMounting) {
     const chromeos::disks::DiskMountManager::Disk kMountedMediaDisk(
         "device1", "mounted", false, "", "", "", "", "", "", "", "", "", "",
         chromeos::DEVICE_TYPE_UNKNOWN, 0, false, false, kHasMedia, false, false,
-        false, "");
+        false, "", "");
     volume_manager()->OnDiskEvent(
         chromeos::disks::DiskMountManager::DISK_ADDED, &kMountedMediaDisk);
     ASSERT_EQ(1U, observer.events().size());
@@ -336,7 +355,7 @@ TEST_F(VolumeManagerTest, OnDiskEvent_AddedNonMounting) {
     const chromeos::disks::DiskMountManager::Disk kNoMediaDisk(
         "device1", "", false, "", "", "", "", "", "", "", "", "", "",
         chromeos::DEVICE_TYPE_UNKNOWN, 0, false, false, kWithoutMedia, false,
-        false, false, "");
+        false, false, "", "");
     volume_manager()->OnDiskEvent(
         chromeos::disks::DiskMountManager::DISK_ADDED, &kNoMediaDisk);
     ASSERT_EQ(1U, observer.events().size());
@@ -361,7 +380,7 @@ TEST_F(VolumeManagerTest, OnDiskEvent_AddedNonMounting) {
     const chromeos::disks::DiskMountManager::Disk kMediaDisk(
         "device1", "", false, "", "", "", "", "", "", "", "", "", "",
         chromeos::DEVICE_TYPE_UNKNOWN, 0, false, false, kHasMedia, false, false,
-        false, "");
+        false, "", "");
     volume_manager()->OnDiskEvent(
         chromeos::disks::DiskMountManager::DISK_ADDED, &kMediaDisk);
     ASSERT_EQ(1U, observer.events().size());
@@ -383,7 +402,7 @@ TEST_F(VolumeManagerTest, OnDiskEvent_Removed) {
   const chromeos::disks::DiskMountManager::Disk kMountedDisk(
       "device1", "mount_path", false, "", "", "", "", "", "", "", "", "", "",
       chromeos::DEVICE_TYPE_UNKNOWN, 0, false, false, false, false, false,
-      false, "");
+      false, "", "");
   volume_manager()->OnDiskEvent(
       chromeos::disks::DiskMountManager::DISK_REMOVED, &kMountedDisk);
 
@@ -408,7 +427,7 @@ TEST_F(VolumeManagerTest, OnDiskEvent_RemovedNotMounted) {
   const chromeos::disks::DiskMountManager::Disk kNotMountedDisk(
       "device1", "", false, "", "", "", "", "", "", "", "", "", "",
       chromeos::DEVICE_TYPE_UNKNOWN, 0, false, false, false, false, false,
-      false, "");
+      false, "", "");
   volume_manager()->OnDiskEvent(
       chromeos::disks::DiskMountManager::DISK_REMOVED, &kNotMountedDisk);
 
@@ -430,7 +449,7 @@ TEST_F(VolumeManagerTest, OnDiskEvent_Changed) {
   const chromeos::disks::DiskMountManager::Disk kDisk(
       "device1", "", false, "", "", "", "", "", "", "", "", "", "",
       chromeos::DEVICE_TYPE_UNKNOWN, 0, false, false, true, false, false, false,
-      "");
+      "", "");
   volume_manager()->OnDiskEvent(
       chromeos::disks::DiskMountManager::DISK_CHANGED, &kDisk);
 
@@ -454,7 +473,7 @@ TEST_F(VolumeManagerTest, OnDiskEvent_ChangedInReadonly) {
   const chromeos::disks::DiskMountManager::Disk kDisk(
       "device1", "", false, "", "", "", "", "", "", "", "", "", "",
       chromeos::DEVICE_TYPE_UNKNOWN, 0, false, false, true, false, false, false,
-      "");
+      "", "");
   volume_manager()->OnDiskEvent(chromeos::disks::DiskMountManager::DISK_CHANGED,
                                 &kDisk);
 
@@ -549,7 +568,7 @@ TEST_F(VolumeManagerTest, OnMountEvent_Remounting) {
       new chromeos::disks::DiskMountManager::Disk(
           "device1", "", false, "", "", "", "", "", "", "", "", "uuid1", "",
           chromeos::DEVICE_TYPE_UNKNOWN, 0, false, false, false, false, false,
-          false, ""));
+          false, "", ""));
   disk_mount_manager_->AddDiskForTest(std::move(disk));
   disk_mount_manager_->MountPath("device1", "", "", chromeos::MOUNT_TYPE_DEVICE,
                                  chromeos::MOUNT_ACCESS_MODE_READ_WRITE);
@@ -759,7 +778,7 @@ TEST_F(VolumeManagerTest, ExternalStorageDisabledPolicyMultiProfile) {
   const chromeos::disks::DiskMountManager::Disk kMediaDisk(
       "device1", "", false, "", "", "", "", "", "", "", "", "", "",
       chromeos::DEVICE_TYPE_UNKNOWN, 0, false, false, true, false, false, false,
-      "");
+      "", "");
   volume_manager()->OnDiskEvent(
       chromeos::disks::DiskMountManager::DISK_ADDED, &kMediaDisk);
   secondary.volume_manager()->OnDiskEvent(