Add callback to DiskMountManager::UnmountPath

Add callback to DiskMountManager::UnmountPath

The callback makes it easier to handle failures (in comparison to observing
mount manager for MountEvents).

TEST=existing
BUG=tested formatting still works


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=188161

[tbarzic, 2013-03-14 00:47:00]

Please take a look.

I guess we did not really need this before, but I've noticed a need for this
here: https://codereview.chromium.org/12596010/

This also simplifies formatting handling, and is a good step towards cleaning up
unmount recursive..

[tbarzic, 2013-03-14 00:47:36]

+ gbillock (fyi)

[satorux1, 2013-03-14 00:58:42]

LGTM with a suggestion.

https://codereview.chromium.org/12537016/diff/7001/chromeos/disks/disk_mount_...
File chromeos/disks/disk_mount_manager.h (right):

https://codereview.chromium.org/12537016/diff/7001/chromeos/disks/disk_mount_...
chromeos/disks/disk_mount_manager.h:254: // |OnMountEvent| will be raised.
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]

https://codereview.chromium.org/12537016/diff/7001/chromeos/disks/disk_mount_...
File chromeos/disks/disk_mount_manager.h (right):

https://codereview.chromium.org/12537016/diff/7001/chromeos/disks/disk_mount_...
chromeos/disks/disk_mount_manager.h:254: // |OnMountEvent| will be raised.
On 2013/03/14 00:58:43, satorux1 wrote:
> 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.

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

[commit-bot: I haz the power, 2013-03-14 01:38:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tbarzic@chromium.org/12537016/11001

[Greg Billock, 2013-03-14 01:56:20]

On 2013/03/14 01:38:53, I haz the power (commit-bot) wrote:
> CQ is trying da patch. Follow status at
> https://chromium-status.appspot.com/cq/tbarzic%40chromium.org/12537016/11001

FYI Thanks. I'll wait for this to land and use it in mine.

[commit-bot: I haz the power, 2013-03-14 08:14:12]

Failed to trigger a try job on win_x64_rel
HTTP Error 400: Bad Request

[commit-bot: I haz the power, 2013-03-14 08:15:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tbarzic@chromium.org/12537016/33001

[commit-bot: I haz the power, 2013-03-14 19:53:33]

Change committed as 188161