Add callback to DiskMountManager::UnmountPath

chromeos/disks/disk_mount_manager.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROMEOS_DISKS_DISK_MOUNT_MANAGER_H_
 #define CHROMEOS_DISKS_DISK_MOUNT_MANAGER_H_
 
 #include <map>
 
 #include "base/callback_forward.h"
@@ skipping 177 matching lines @@
     }
   };
 
   // MountPointMap key is mount_path.
   typedef std::map<std::string, MountPointInfo> MountPointMap;
 
   // A callback function type which is called after UnmountDeviceRecursively
   // finishes.
   typedef base::Callback<void(bool)> UnmountDeviceRecursivelyCallbackType;
 
+  // A callback type for UnmountPath method.
+  typedef base::Callback<void(MountError error_code)> UnmountPathCallback;
+
   // Implement this interface to be notified about disk/mount related events.
   class Observer {
    public:
     virtual ~Observer() {}
 
     // Called when disk mount status is changed.
     virtual void OnDiskEvent(DiskEvent event, const Disk* disk) = 0;
     // Called when device status is changed.
     virtual void OnDeviceEvent(DeviceEvent event,
                                const std::string& device_path) = 0;
@@ skipping 22 matching lines @@
   virtual const Disk* FindDiskBySourcePath(
       const std::string& source_path) const = 0;
 
   // Gets the list of mount points.
   virtual const MountPointMap& mount_points() const = 0;
 
   // Requests refreshing all the information about mounted disks.
   virtual void RequestMountInfoRefresh() = 0;
 
   // Mounts a device.
+  // Note that the mount operation may fail. To find out the result, one should
+  // observe DiskMountManager for |Observer::OnMountEvent| event, which will be
+  // raised upon the mount operation completion.
   virtual void MountPath(const std::string& source_path,
                          const std::string& source_format,
                          const std::string& mount_label,
                          MountType type) = 0;
 
   // Unmounts a mounted disk.
   // |UnmountOptions| enum defined in chromeos/dbus/cros_disks_client.h.
+  // When the method is complete, |callback| will be called and observers'
+  // |OnMountEvent| will be raised.

[satorux1, 2013/03/14 00:58:43]

Please comment that |callback| can be optional (can be empty)

I personally find that making |callback| parameter mandatory, as checking
callback.is_null() before Run() is error-prone.

However, if the parameter is not used in most callers, making it optional makes
sense.

[tbarzic, 2013/03/14 01:10:03]

I agree, I don't like optional parameters in general (not just with callbacks),
but in this case the most callers don't use it (yet).

comment updated

   virtual void UnmountPath(const std::string& mount_path,
-                           UnmountOptions options) = 0;
+                           UnmountOptions options,
+                           const UnmountPathCallback& callback) = 0;
 
   // Formats Device given its mount path. Unmounts the device.
   // Example: mount_path: /media/VOLUME_LABEL
   virtual void FormatMountedDevice(const std::string& mount_path) = 0;
 
   // Unmounts device_path and all of its known children.
   virtual void UnmountDeviceRecursively(
       const std::string& device_path,
       const UnmountDeviceRecursivelyCallbackType& callback) = 0;
 
@@ skipping 28 matching lines @@
 
   // Returns a pointer to the global DiskMountManager instance.
   // Initialize() should already have been called.
   static DiskMountManager* GetInstance();
 };
 
 }  // namespace disks
 }  // namespace chromeos
 
 #endif  // CHROMEOS_DISKS_DISK_MOUNT_MANAGER_H_