Use cros-disks Format DBus call instead of deprecated FormatDevice.

The cros-disks FormatDevice DBus call has been long deprecated (back in 2011).
This CL modifies CrosDisksClient and DiskMountManager to use the Format DBus
call instead.

BUG=369877
TEST=Tested the following:
1. Run chromeos_unittests.
2. Insert a USB drive and format the mounted partition via Files.app.
   Verify that Files.app notifies the successful completion of the
   format operation.
3. Insert a write-protected SD card and format the mounted partition via
   Files.app. Verify that Files.app reports an error.

R=stevenjb@chromium.org, tbarzic@chromium.org

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

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

6 files changed, 105 insertions, 195 deletions

diff --git a/chromeos/dbus/cros_disks_client.cc b/chromeos/dbus/cros_disks_client.cc
index 39bdb6f..cca2b17 100644
--- a/chromeos/dbus/cros_disks_client.cc
+++ b/chromeos/dbus/cros_disks_client.cc
@@ -148,17 +148,21 @@ class CrosDisksClientImpl : public CrosDisksClient {
   }
 
   // CrosDisksClient override.
-  virtual void FormatDevice(const std::string& device_path,
-                            const std::string& filesystem,
-                            const FormatDeviceCallback& callback,
-                            const base::Closure& error_callback) OVERRIDE {
+  virtual void Format(const std::string& device_path,
+                      const std::string& filesystem,
+                      const base::Closure& callback,
+                      const base::Closure& error_callback) OVERRIDE {
     dbus::MethodCall method_call(cros_disks::kCrosDisksInterface,
-                                 cros_disks::kFormatDevice);
+                                 cros_disks::kFormat);
     dbus::MessageWriter writer(&method_call);
     writer.AppendString(device_path);
     writer.AppendString(filesystem);
+    // No format option is currently specified, but we can later use this
+    // argument to specify options for the format operation.
+    std::vector<std::string> format_options;
+    writer.AppendArrayOfStrings(format_options);
     proxy_->CallMethod(&method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
-                       base::Bind(&CrosDisksClientImpl::OnFormatDevice,
+                       base::Bind(&CrosDisksClientImpl::OnFormat,
                                   weak_ptr_factory_.GetWeakPtr(),
                                   callback,
                                   error_callback));
@@ -308,23 +312,15 @@ class CrosDisksClientImpl : public CrosDisksClient {
     callback.Run(device_paths);
   }
 
-  // Handles the result of FormatDevice and calls |callback| or
-  // |error_callback|.
-  void OnFormatDevice(const FormatDeviceCallback& callback,
-                      const base::Closure& error_callback,
-                      dbus::Response* response) {
+  // Handles the result of Format and calls |callback| or |error_callback|.
+  void OnFormat(const base::Closure& callback,
+                const base::Closure& error_callback,
+                dbus::Response* response) {
     if (!response) {
       error_callback.Run();
       return;
     }
-    dbus::MessageReader reader(response);
-    bool success = false;
-    if (!reader.PopBool(&success)) {
-      LOG(ERROR) << "Invalid response: " << response->ToString();
-      error_callback.Run();
-      return;
-    }
-    callback.Run(success);
+    callback.Run();
   }
 
   // Handles the result of GetDeviceProperties and calls |callback| or
@@ -472,10 +468,10 @@ class CrosDisksClientStubImpl : public CrosDisksClient {
         FROM_HERE, base::Bind(callback, device_paths));
   }
 
-  virtual void FormatDevice(const std::string& device_path,
-                            const std::string& filesystem,
-                            const FormatDeviceCallback& callback,
-                            const base::Closure& error_callback) OVERRIDE {
+  virtual void Format(const std::string& device_path,
+                      const std::string& filesystem,
+                      const base::Closure& callback,
+                      const base::Closure& error_callback) OVERRIDE {
     base::MessageLoopProxy::current()->PostTask(FROM_HERE, error_callback);
   }
 
diff --git a/chromeos/dbus/cros_disks_client.h b/chromeos/dbus/cros_disks_client.h
index 3cd9ee9..095566f 100644
--- a/chromeos/dbus/cros_disks_client.h
+++ b/chromeos/dbus/cros_disks_client.h
@@ -194,10 +194,6 @@ class CHROMEOS_EXPORT CrosDisksClient : public DBusClient {
   typedef base::Callback<void(const std::vector<std::string>& device_paths)>
       EnumerateAutoMountableDevicesCallback;
 
-  // A callback to handle the result of FormatDevice.
-  // The argument is true when formatting succeeded.
-  typedef base::Callback<void(bool format_succeeded)> FormatDeviceCallback;
-
   // A callback to handle the result of GetDeviceProperties.
   // The argument is the information about the specified device.
   typedef base::Callback<void(const DiskInfo& disk_info)>
@@ -258,12 +254,12 @@ class CHROMEOS_EXPORT CrosDisksClient : public DBusClient {
       const EnumerateAutoMountableDevicesCallback& callback,
       const base::Closure& error_callback) = 0;
 
-  // Calls FormatDevice method.  |callback| is called after the method call
-  // succeeds, otherwise, |error_callback| is called.
-  virtual void FormatDevice(const std::string& device_path,
-                            const std::string& filesystem,
-                            const FormatDeviceCallback& callback,
-                            const base::Closure& error_callback) = 0;
+  // Calls Format method.  |callback| is called after the method call succeeds,
+  // otherwise, |error_callback| is called.
+  virtual void Format(const std::string& device_path,
+                      const std::string& filesystem,
+                      const base::Closure& callback,
+                      const base::Closure& error_callback) = 0;
 
   // Calls GetDeviceProperties method.  |callback| is called after the method
   // call succeeds, otherwise, |error_callback| is called.
diff --git a/chromeos/dbus/fake_cros_disks_client.cc b/chromeos/dbus/fake_cros_disks_client.cc
index 8e96bf2..344a98e 100644
--- a/chromeos/dbus/fake_cros_disks_client.cc
+++ b/chromeos/dbus/fake_cros_disks_client.cc
@@ -13,8 +13,8 @@ namespace chromeos {
 FakeCrosDisksClient::FakeCrosDisksClient()
     : unmount_call_count_(0),
       unmount_success_(true),
-      format_device_call_count_(0),
-      format_device_success_(true) {
+      format_call_count_(0),
+      format_success_(true) {
 }
 
 FakeCrosDisksClient::~FakeCrosDisksClient() {
@@ -51,19 +51,18 @@ void FakeCrosDisksClient::EnumerateAutoMountableDevices(
     const base::Closure& error_callback) {
 }
 
-void FakeCrosDisksClient::FormatDevice(const std::string& device_path,
-                                       const std::string& filesystem,
-                                       const FormatDeviceCallback& callback,
-                                       const base::Closure& error_callback) {
+void FakeCrosDisksClient::Format(const std::string& device_path,
+                                 const std::string& filesystem,
+                                 const base::Closure& callback,
+                                 const base::Closure& error_callback) {
   DCHECK(!callback.is_null());
   DCHECK(!error_callback.is_null());
 
-  format_device_call_count_++;
-  last_format_device_device_path_ = device_path;
-  last_format_device_filesystem_ = filesystem;
-  if (format_device_success_) {
-    base::MessageLoopProxy::current()->PostTask(FROM_HERE,
-                                                base::Bind(callback, true));
+  format_call_count_++;
+  last_format_device_path_ = device_path;
+  last_format_filesystem_ = filesystem;
+  if (format_success_) {
+    base::MessageLoopProxy::current()->PostTask(FROM_HERE, callback);
   } else {
     base::MessageLoopProxy::current()->PostTask(FROM_HERE, error_callback);
   }
diff --git a/chromeos/dbus/fake_cros_disks_client.h b/chromeos/dbus/fake_cros_disks_client.h
index f51a4be..23e4c36 100644
--- a/chromeos/dbus/fake_cros_disks_client.h
+++ b/chromeos/dbus/fake_cros_disks_client.h
@@ -33,10 +33,10 @@ class FakeCrosDisksClient : public CrosDisksClient {
   virtual void EnumerateAutoMountableDevices(
       const EnumerateAutoMountableDevicesCallback& callback,
       const base::Closure& error_callback) OVERRIDE;
-  virtual void FormatDevice(const std::string& device_path,
-                            const std::string& filesystem,
-                            const FormatDeviceCallback& callback,
-                            const base::Closure& error_callback) OVERRIDE;
+  virtual void Format(const std::string& device_path,
+                      const std::string& filesystem,
+                      const base::Closure& callback,
+                      const base::Closure& error_callback) OVERRIDE;
   virtual void GetDeviceProperties(
       const std::string& device_path,
       const GetDevicePropertiesCallback& callback,
@@ -85,27 +85,24 @@ class FakeCrosDisksClient : public CrosDisksClient {
     unmount_listener_ = listener;
   }
 
-  // Returns how many times FormatDevice() was called.
-  int format_device_call_count() const {
-    return format_device_call_count_;
+  // Returns how many times Format() was called.
+  int format_call_count() const {
+    return format_call_count_;
   }
 
-  // Returns the |device_path| parameter from the last invocation of
-  // FormatDevice().
-  const std::string& last_format_device_device_path() const {
-    return last_format_device_device_path_;
+  // Returns the |device_path| parameter from the last invocation of Format().
+  const std::string& last_format_device_path() const {
+    return last_format_device_path_;
   }
 
-  // Returns the |filesystem| parameter from the last invocation of
-  // FormatDevice().
-  const std::string& last_format_device_filesystem() const {
-    return last_format_device_filesystem_;
+  // Returns the |filesystem| parameter from the last invocation of Format().
+  const std::string& last_format_filesystem() const {
+    return last_format_filesystem_;
   }
 
-  // Makes the subsequent FormatDevice() calls fail. FormatDevice() succeeds by
-  // default.
-  void MakeFormatDeviceFail() {
-    format_device_success_ = false;
+  // Makes the subsequent Format() calls fail. Format() succeeds by default.
+  void MakeFormatFail() {
+    format_success_ = false;
   }
 
  private:
@@ -118,10 +115,10 @@ class FakeCrosDisksClient : public CrosDisksClient {
   UnmountOptions last_unmount_options_;
   bool unmount_success_;
   base::Closure unmount_listener_;
-  int format_device_call_count_;
-  std::string last_format_device_device_path_;
-  std::string last_format_device_filesystem_;
-  bool format_device_success_;
+  int format_call_count_;
+  std::string last_format_device_path_;
+  std::string last_format_filesystem_;
+  bool format_success_;
 };
 
 }  // namespace chromeos
diff --git a/chromeos/disks/disk_mount_manager.cc b/chromeos/disks/disk_mount_manager.cc
index 75ab642..58f1608 100644
--- a/chromeos/disks/disk_mount_manager.cc
+++ b/chromeos/disks/disk_mount_manager.cc
@@ -107,7 +107,7 @@ class DiskMountManagerImpl : public DiskMountManager {
     MountPointMap::const_iterator mount_point = mount_points_.find(mount_path);
     if (mount_point == mount_points_.end()) {
       LOG(ERROR) << "Mount point with path \"" << mount_path << "\" not found.";
-      OnFormatDevice(mount_path, false);
+      OnFormatCompleted(FORMAT_ERROR_UNKNOWN, mount_path);
       return;
     }
 
@@ -115,7 +115,7 @@ class DiskMountManagerImpl : public DiskMountManager {
     DiskMap::const_iterator disk = disks_.find(device_path);
     if (disk == disks_.end()) {
       LOG(ERROR) << "Device with path \"" << device_path << "\" not found.";
-      OnFormatDevice(device_path, false);
+      OnFormatCompleted(FORMAT_ERROR_UNKNOWN, device_path);
       return;
     }
 
@@ -382,7 +382,7 @@ class DiskMountManagerImpl : public DiskMountManager {
         disks_.find(device_path) != disks_.end()) {
       FormatUnmountedDevice(device_path);
     } else {
-      OnFormatDevice(device_path, false);
+      OnFormatCompleted(FORMAT_ERROR_UNKNOWN, device_path);
     }
   }
 
@@ -392,50 +392,27 @@ class DiskMountManagerImpl : public DiskMountManager {
     DCHECK(disk != disks_.end() && disk->second->mount_path().empty());
 
     const char kFormatVFAT[] = "vfat";
-    cros_disks_client_->FormatDevice(
+    cros_disks_client_->Format(
         device_path,
         kFormatVFAT,
-        base::Bind(&DiskMountManagerImpl::OnFormatDevice,
+        base::Bind(&DiskMountManagerImpl::OnFormatStarted,
                    weak_ptr_factory_.GetWeakPtr(),
                    device_path),
-        base::Bind(&DiskMountManagerImpl::OnFormatDevice,
+        base::Bind(&DiskMountManagerImpl::OnFormatCompleted,
                    weak_ptr_factory_.GetWeakPtr(),
-                   device_path,
-                   false));
+                   FORMAT_ERROR_UNKNOWN,
+                   device_path));
   }
 
-  // Callback for FormatDevice.
-  // TODO(tbarzic): Pass FormatError instead of bool.
-  void OnFormatDevice(const std::string& device_path, bool success) {
-    FormatError error_code = success ? FORMAT_ERROR_NONE : FORMAT_ERROR_UNKNOWN;
-    NotifyFormatStatusUpdate(FORMAT_STARTED, error_code, device_path);
+  // Callback for Format.
+  void OnFormatStarted(const std::string& device_path) {
+    NotifyFormatStatusUpdate(FORMAT_STARTED, FORMAT_ERROR_NONE, device_path);
   }
 
   // Callback to handle FormatCompleted signal and Format method call failure.
   void OnFormatCompleted(FormatError error_code,
-                        const std::string& device_path) {
-    std::string actual_device_path;
-    // Depending on cros-disks implementation the event may return either file
-    // path or device path. We want to use device path.
-    // TODO(benchan): This shouldn't be necessary. Find out the issue and ensure
-    // that cros-disks simply returns the path format that Chrome expects, then
-    // we should remove the following code.
-    for (DiskMountManager::DiskMap::const_iterator it = disks_.begin();
-         it != disks_.end(); ++it) {
-      if (it->second->file_path() == device_path ||
-          it->second->device_path() == device_path) {
-        actual_device_path = it->second->device_path();
-        break;
-      }
-    }
-
-    if (actual_device_path.empty()) {
-      LOG(ERROR) << "Error while handling disks metadata. Cannot find "
-                 << "device that is being formatted.";
-      return;
-    }
-
-    NotifyFormatStatusUpdate(FORMAT_COMPLETED, error_code, actual_device_path);
+                         const std::string& device_path) {
+    NotifyFormatStatusUpdate(FORMAT_COMPLETED, error_code, device_path);
   }
 
   // Callbcak for GetDeviceProperties.
diff --git a/chromeos/disks/disk_mount_manager_unittest.cc b/chromeos/disks/disk_mount_manager_unittest.cc
index 3786dcd3..fd2fab0 100644
--- a/chromeos/disks/disk_mount_manager_unittest.cc
+++ b/chromeos/disks/disk_mount_manager_unittest.cc
@@ -207,7 +207,7 @@ class DiskMountManagerTest : public testing::Test {
 // Tests that the observer gets notified on attempt to format non existent mount
 // point.
 TEST_F(DiskMountManagerTest, Format_NotMounted) {
-  EXPECT_CALL(observer_, OnFormatEvent(DiskMountManager::FORMAT_STARTED,
+  EXPECT_CALL(observer_, OnFormatEvent(DiskMountManager::FORMAT_COMPLETED,
                                        chromeos::FORMAT_ERROR_UNKNOWN,
                                        "/mount/non_existent"))
       .Times(1);
@@ -216,7 +216,7 @@ TEST_F(DiskMountManagerTest, Format_NotMounted) {
 
 // Tests that it is not possible to format archive mount point.
 TEST_F(DiskMountManagerTest, Format_Archive) {
-  EXPECT_CALL(observer_, OnFormatEvent(DiskMountManager::FORMAT_STARTED,
+  EXPECT_CALL(observer_, OnFormatEvent(DiskMountManager::FORMAT_COMPLETED,
                                        chromeos::FORMAT_ERROR_UNKNOWN,
                                        "/archive/source_path"))
       .Times(1);
@@ -243,7 +243,7 @@ TEST_F(DiskMountManagerTest, Format_FailToUnmount) {
                            "/device/mount_path")))
         .Times(1);
 
-    EXPECT_CALL(observer_, OnFormatEvent(DiskMountManager::FORMAT_STARTED,
+    EXPECT_CALL(observer_, OnFormatEvent(DiskMountManager::FORMAT_COMPLETED,
                                          chromeos::FORMAT_ERROR_UNKNOWN,
                                          "/device/source_path"))
         .Times(1);
@@ -261,7 +261,7 @@ TEST_F(DiskMountManagerTest, Format_FailToUnmount) {
             fake_cros_disks_client_->last_unmount_device_path());
   EXPECT_EQ(chromeos::UNMOUNT_OPTIONS_NONE,
             fake_cros_disks_client_->last_unmount_options());
-  EXPECT_EQ(0, fake_cros_disks_client_->format_device_call_count());
+  EXPECT_EQ(0, fake_cros_disks_client_->format_call_count());
 
   // The device mount should still be here.
   EXPECT_TRUE(HasMountPoint("/device/mount_path"));
@@ -271,7 +271,7 @@ TEST_F(DiskMountManagerTest, Format_FailToUnmount) {
 // process.
 TEST_F(DiskMountManagerTest, Format_FormatFailsToStart) {
   // Before formatting mounted device, the device should be unmounted.
-  // In this test, unmount will succeed, but call to FormatDevice method will
+  // In this test, unmount will succeed, but call to Format method will
   // fail.
 
   // Set up expectations for observer mock.
@@ -287,13 +287,13 @@ TEST_F(DiskMountManagerTest, Format_FormatFailsToStart) {
                            "/device/mount_path")))
         .Times(1);
 
-    EXPECT_CALL(observer_, OnFormatEvent(DiskMountManager::FORMAT_STARTED,
+    EXPECT_CALL(observer_, OnFormatEvent(DiskMountManager::FORMAT_COMPLETED,
                                          chromeos::FORMAT_ERROR_UNKNOWN,
                                          "/device/source_path"))
         .Times(1);
   }
 
-  fake_cros_disks_client_->MakeFormatDeviceFail();
+  fake_cros_disks_client_->MakeFormatFail();
   // Start the test.
   DiskMountManager::GetInstance()->FormatMountedDevice("/device/mount_path");
 
@@ -305,11 +305,10 @@ TEST_F(DiskMountManagerTest, Format_FormatFailsToStart) {
             fake_cros_disks_client_->last_unmount_device_path());
   EXPECT_EQ(chromeos::UNMOUNT_OPTIONS_NONE,
             fake_cros_disks_client_->last_unmount_options());
-  EXPECT_EQ(1, fake_cros_disks_client_->format_device_call_count());
+  EXPECT_EQ(1, fake_cros_disks_client_->format_call_count());
   EXPECT_EQ("/device/source_path",
-            fake_cros_disks_client_->last_format_device_device_path());
-  EXPECT_EQ("vfat",
-            fake_cros_disks_client_->last_format_device_filesystem());
+            fake_cros_disks_client_->last_format_device_path());
+  EXPECT_EQ("vfat", fake_cros_disks_client_->last_format_filesystem());
 
   // The device mount should be gone.
   EXPECT_FALSE(HasMountPoint("/device/mount_path"));
@@ -323,9 +322,9 @@ TEST_F(DiskMountManagerTest, Format_ConcurrentFormatCalls) {
   // is successfully started.
 
   // Set up expectations for observer mock.
-  // The observer should get two FORMAT_STARTED events, one for each format
-  // request, but with different error codes (the formatting will be started
-  // only for the first request).
+  // The observer should get a FORMAT_STARTED event for one format request and a
+  // FORMAT_COMPLETED with an error code for the other format request. The
+  // formatting will be started only for the first request.
   // There should be only one UNMOUNTING event. The result of the second one
   // should not be reported as the mount point will go away after the first
   // request.
@@ -342,7 +341,7 @@ TEST_F(DiskMountManagerTest, Format_ConcurrentFormatCalls) {
                            "/device/mount_path")))
         .Times(1);
 
-    EXPECT_CALL(observer_, OnFormatEvent(DiskMountManager::FORMAT_STARTED,
+    EXPECT_CALL(observer_, OnFormatEvent(DiskMountManager::FORMAT_COMPLETED,
                                          chromeos::FORMAT_ERROR_UNKNOWN,
                                          "/device/source_path"))
         .Times(1);
@@ -368,11 +367,11 @@ TEST_F(DiskMountManagerTest, Format_ConcurrentFormatCalls) {
             fake_cros_disks_client_->last_unmount_device_path());
   EXPECT_EQ(chromeos::UNMOUNT_OPTIONS_NONE,
             fake_cros_disks_client_->last_unmount_options());
-  EXPECT_EQ(1, fake_cros_disks_client_->format_device_call_count());
+  EXPECT_EQ(1, fake_cros_disks_client_->format_call_count());
   EXPECT_EQ("/device/source_path",
-            fake_cros_disks_client_->last_format_device_device_path());
+            fake_cros_disks_client_->last_format_device_path());
   EXPECT_EQ("vfat",
-            fake_cros_disks_client_->last_format_device_filesystem());
+            fake_cros_disks_client_->last_format_filesystem());
 
   // The device mount should be gone.
   EXPECT_FALSE(HasMountPoint("/device/mount_path"));
@@ -380,7 +379,7 @@ TEST_F(DiskMountManagerTest, Format_ConcurrentFormatCalls) {
 
 // Tests the case when the format process actually starts and fails.
 TEST_F(DiskMountManagerTest, Format_FormatFails) {
-  // Both unmount and format device cals are successfull in this test.
+  // Both unmount and format device cals are successful in this test.
 
   // Set up expectations for observer mock.
   // The observer should get notified that the device was unmounted and that
@@ -412,7 +411,7 @@ TEST_F(DiskMountManagerTest, Format_FormatFails) {
   // Start the test.
   DiskMountManager::GetInstance()->FormatMountedDevice("/device/mount_path");
 
-  // Wait for Unmount and FormatDevice calls to end.
+  // Wait for Unmount and Format calls to end.
   message_loop_.RunUntilIdle();
 
   EXPECT_EQ(1, fake_cros_disks_client_->unmount_call_count());
@@ -420,11 +419,10 @@ TEST_F(DiskMountManagerTest, Format_FormatFails) {
             fake_cros_disks_client_->last_unmount_device_path());
   EXPECT_EQ(chromeos::UNMOUNT_OPTIONS_NONE,
             fake_cros_disks_client_->last_unmount_options());
-  EXPECT_EQ(1, fake_cros_disks_client_->format_device_call_count());
+  EXPECT_EQ(1, fake_cros_disks_client_->format_call_count());
   EXPECT_EQ("/device/source_path",
-            fake_cros_disks_client_->last_format_device_device_path());
-  EXPECT_EQ("vfat",
-            fake_cros_disks_client_->last_format_device_filesystem());
+            fake_cros_disks_client_->last_format_device_path());
+  EXPECT_EQ("vfat", fake_cros_disks_client_->last_format_filesystem());
 
   // The device should be unmounted by now.
   EXPECT_FALSE(HasMountPoint("/device/mount_path"));
@@ -436,60 +434,10 @@ TEST_F(DiskMountManagerTest, Format_FormatFails) {
       chromeos::FORMAT_ERROR_UNKNOWN, "/device/source_path");
 }
 
-// Tests the same case as Format_FormatFails, but the FORMAT_COMPLETED event
-// is sent with file_path of the formatted device (instead of its device path).
-TEST_F(DiskMountManagerTest, Format_FormatFailsAndReturnFilePath) {
-  // Set up expectations for observer mock.
-  {
-    InSequence s;
-
-    EXPECT_CALL(observer_,
-        OnMountEvent(DiskMountManager::UNMOUNTING,
-                     chromeos::MOUNT_ERROR_NONE,
-                     Field(&DiskMountManager::MountPointInfo::mount_path,
-                           "/device/mount_path")))
-        .Times(1);
-
-    EXPECT_CALL(observer_, OnFormatEvent(DiskMountManager::FORMAT_STARTED,
-                                         chromeos::FORMAT_ERROR_NONE,
-                                         "/device/source_path"))
-        .Times(1);
-
-    EXPECT_CALL(observer_, OnFormatEvent(DiskMountManager::FORMAT_COMPLETED,
-                                         chromeos::FORMAT_ERROR_UNKNOWN,
-                                         "/device/source_path"))
-        .Times(1);
-  }
-
-  // Start test.
-  DiskMountManager::GetInstance()->FormatMountedDevice("/device/mount_path");
-
-  // Wait for Unmount and FormatDevice calls to end.
-  message_loop_.RunUntilIdle();
-
-  EXPECT_EQ(1, fake_cros_disks_client_->unmount_call_count());
-  EXPECT_EQ("/device/mount_path",
-            fake_cros_disks_client_->last_unmount_device_path());
-  EXPECT_EQ(chromeos::UNMOUNT_OPTIONS_NONE,
-            fake_cros_disks_client_->last_unmount_options());
-  EXPECT_EQ(1, fake_cros_disks_client_->format_device_call_count());
-  EXPECT_EQ("/device/source_path",
-            fake_cros_disks_client_->last_format_device_device_path());
-  EXPECT_EQ("vfat",
-            fake_cros_disks_client_->last_format_device_filesystem());
-
-  // The device should be unmounted by now.
-  EXPECT_FALSE(HasMountPoint("/device/mount_path"));
-
-  // Send failing FORMAT_COMPLETED signal with the device's file path.
-  fake_cros_disks_client_->SendFormatCompletedEvent(
-      chromeos::FORMAT_ERROR_UNKNOWN, "/device/file_path");
-}
-
 // Tests the case when formatting completes successfully.
 TEST_F(DiskMountManagerTest, Format_FormatSuccess) {
   // Set up cros disks client mocks.
-  // Both unmount and format device cals are successfull in this test.
+  // Both unmount and format device cals are successful in this test.
 
   // Set up expectations for observer mock.
   // The observer should receive UNMOUNTING, FORMAT_STARTED and FORMAT_COMPLETED
@@ -518,7 +466,7 @@ TEST_F(DiskMountManagerTest, Format_FormatSuccess) {
   // Start the test.
   DiskMountManager::GetInstance()->FormatMountedDevice("/device/mount_path");
 
-  // Wait for Unmount and FormatDevice calls to end.
+  // Wait for Unmount and Format calls to end.
   message_loop_.RunUntilIdle();
 
   EXPECT_EQ(1, fake_cros_disks_client_->unmount_call_count());
@@ -526,11 +474,10 @@ TEST_F(DiskMountManagerTest, Format_FormatSuccess) {
             fake_cros_disks_client_->last_unmount_device_path());
   EXPECT_EQ(chromeos::UNMOUNT_OPTIONS_NONE,
             fake_cros_disks_client_->last_unmount_options());
-  EXPECT_EQ(1, fake_cros_disks_client_->format_device_call_count());
+  EXPECT_EQ(1, fake_cros_disks_client_->format_call_count());
   EXPECT_EQ("/device/source_path",
-            fake_cros_disks_client_->last_format_device_device_path());
-  EXPECT_EQ("vfat",
-            fake_cros_disks_client_->last_format_device_filesystem());
+            fake_cros_disks_client_->last_format_device_path());
+  EXPECT_EQ("vfat", fake_cros_disks_client_->last_format_filesystem());
 
   // The device should be unmounted by now.
   EXPECT_FALSE(HasMountPoint("/device/mount_path"));
@@ -543,7 +490,7 @@ TEST_F(DiskMountManagerTest, Format_FormatSuccess) {
 // Tests that it's possible to format the device twice in a row (this may not be
 // true if the list of pending formats is not properly cleared).
 TEST_F(DiskMountManagerTest, Format_ConsecutiveFormatCalls) {
-  // All unmount and format device cals are successfull in this test.
+  // All unmount and format device cals are successful in this test.
   // Each of the should be made twice (once for each formatting task).
 
   // Set up expectations for observer mock.
@@ -579,7 +526,7 @@ TEST_F(DiskMountManagerTest, Format_ConsecutiveFormatCalls) {
   // Start the test.
   DiskMountManager::GetInstance()->FormatMountedDevice("/device/mount_path");
 
-  // Wait for Unmount and FormatDevice calls to end.
+  // Wait for Unmount and Format calls to end.
   message_loop_.RunUntilIdle();
 
   EXPECT_EQ(1, fake_cros_disks_client_->unmount_call_count());
@@ -587,11 +534,10 @@ TEST_F(DiskMountManagerTest, Format_ConsecutiveFormatCalls) {
             fake_cros_disks_client_->last_unmount_device_path());
   EXPECT_EQ(chromeos::UNMOUNT_OPTIONS_NONE,
             fake_cros_disks_client_->last_unmount_options());
-  EXPECT_EQ(1, fake_cros_disks_client_->format_device_call_count());
+  EXPECT_EQ(1, fake_cros_disks_client_->format_call_count());
   EXPECT_EQ("/device/source_path",
-            fake_cros_disks_client_->last_format_device_device_path());
-  EXPECT_EQ("vfat",
-            fake_cros_disks_client_->last_format_device_filesystem());
+            fake_cros_disks_client_->last_format_device_path());
+  EXPECT_EQ("vfat", fake_cros_disks_client_->last_format_filesystem());
 
   // The device should be unmounted by now.
   EXPECT_FALSE(HasMountPoint("/device/mount_path"));
@@ -612,7 +558,7 @@ TEST_F(DiskMountManagerTest, Format_ConsecutiveFormatCalls) {
   // Try formatting again.
   DiskMountManager::GetInstance()->FormatMountedDevice("/device/mount_path");
 
-  // Wait for Unmount and FormatDevice calls to end.
+  // Wait for Unmount and Format calls to end.
   message_loop_.RunUntilIdle();
 
   EXPECT_EQ(2, fake_cros_disks_client_->unmount_call_count());
@@ -620,11 +566,10 @@ TEST_F(DiskMountManagerTest, Format_ConsecutiveFormatCalls) {
             fake_cros_disks_client_->last_unmount_device_path());
   EXPECT_EQ(chromeos::UNMOUNT_OPTIONS_NONE,
             fake_cros_disks_client_->last_unmount_options());
-  EXPECT_EQ(2, fake_cros_disks_client_->format_device_call_count());
+  EXPECT_EQ(2, fake_cros_disks_client_->format_call_count());
   EXPECT_EQ("/device/source_path",
-            fake_cros_disks_client_->last_format_device_device_path());
-  EXPECT_EQ("vfat",
-            fake_cros_disks_client_->last_format_device_filesystem());
+            fake_cros_disks_client_->last_format_device_path());
+  EXPECT_EQ("vfat", fake_cros_disks_client_->last_format_filesystem());
 
   // Simulate cros_disks reporting success.
   fake_cros_disks_client_->SendFormatCompletedEvent(