Small refactoring in DiskMountManager (event reporting; format method).

Small refactoring in DiskMountManager (event reporting; format method).

* Rename DiskMountManager::Observer methods to On____Event format.

* Break up DiskMountManagerEventType enum.

* Add separate observer method for formatting events.

* Pass formatting event success using bool instead of adding '!' in front of the
path (but only in events from DiskMountManager; will remove it from
CrosDisksClient events in another patch).

* Add unit tests for formatting handling in DiskMountManager.
(TODO: add more tests)

TBR:vandebo@chromium.org
(for mechanical changes in chrome/browser/system_monitor)

BUG=157587, 126815
TEST=manual: formatting works (and Formatting pending notification goes away)
     chromeos_unittests::DiskMountManagerTest.*

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=167799

[tbarzic, 2012-11-08 04:21:17]

next step is to switch CrosDisksClient from using FormatDevice to using Format
method and to listen to FormatCompleted event instead of FormattingFinished.

[satorux1, 2012-11-08 08:35:18]

I don't know enough about DiskMountManager. Please get Ben's feedback first. :)

http://codereview.chromium.org/11365142/diff/13001/chromeos/dbus/cros_disks_c...
File chromeos/dbus/cros_disks_client.h (right):

http://codereview.chromium.org/11365142/diff/13001/chromeos/dbus/cros_disks_c...
chromeos/dbus/cros_disks_client.h:44: // Mount error code used by cros-disks.
I think we should share these error codes with cros-disks via
third_party/cros_system_api. Can you file a bug and add a TODO here?

http://codereview.chromium.org/11365142/diff/13001/chromeos/disks/disk_mount_...
File chromeos/disks/disk_mount_manager.cc (right):

http://codereview.chromium.org/11365142/diff/13001/chromeos/disks/disk_mount_...
chromeos/disks/disk_mount_manager.cc:535: void CrackFormatFinishedPath(const
std::string& received_path,
Crack -> Parse? Crack sounds rather uncivilized. :)

[tbarzic, 2012-11-08 19:23:09]

http://codereview.chromium.org/11365142/diff/13001/chromeos/dbus/cros_disks_c...
File chromeos/dbus/cros_disks_client.h (right):

http://codereview.chromium.org/11365142/diff/13001/chromeos/dbus/cros_disks_c...
chromeos/dbus/cros_disks_client.h:44: // Mount error code used by cros-disks.
On 2012/11/08 08:35:18, satorux1 wrote:
> I think we should share these error codes with cros-disks via
> third_party/cros_system_api. Can you file a bug and add a TODO here?

yep, I agree, I planed to do that in this cl (we already have these enums in
cros_system_api, but there are few values missing:
MOUNT_ERROR_NOT_AUTHENTICATED, MOUNT_TYPE_GDATA which are used for drive only;
so I'll have to update service_contents first).

actually, when I think about it we could use MOUNT_TYPE_NETWORK_STORAGE and
MOUNT_ERROR_INSUFFICIENT_PERMISSIONS instead of those two; these are a bit
broader, but I guess they could work, WDYT

http://codereview.chromium.org/11365142/diff/13001/chromeos/disks/disk_mount_...
File chromeos/disks/disk_mount_manager.cc (right):

http://codereview.chromium.org/11365142/diff/13001/chromeos/disks/disk_mount_...
chromeos/disks/disk_mount_manager.cc:535: void CrackFormatFinishedPath(const
std::string& received_path,
On 2012/11/08 08:35:18, satorux1 wrote:
> Crack -> Parse? Crack sounds rather uncivilized. :)

haha, I think you told me the same thing in another cl some time ago..

I've put crack intentionally, to show how much I dislike this method :) Luckily
it should go away real soon...

But OK, I'l change it to Parse ;)

[Ben Chan, 2012-11-12 23:10:50]

lgtm

https://codereview.chromium.org/11365142/diff/8005/chrome/browser/system_moni...
File chrome/browser/system_monitor/removable_device_notifications_chromeos.cc
(right):

https://codereview.chromium.org/11365142/diff/8005/chrome/browser/system_moni...
chrome/browser/system_monitor/removable_device_notifications_chromeos.cc:177:
const std::string& device_path) {
missing a TODO / NOTIMPLEMENT?

[tbarzic, 2012-11-12 23:14:39]

https://codereview.chromium.org/11365142/diff/8005/chrome/browser/system_moni...
File chrome/browser/system_monitor/removable_device_notifications_chromeos.cc
(right):

https://codereview.chromium.org/11365142/diff/8005/chrome/browser/system_moni...
chrome/browser/system_monitor/removable_device_notifications_chromeos.cc:177:
const std::string& device_path) {
On 2012/11/12 23:10:50, Ben Chan wrote:
> missing a TODO / NOTIMPLEMENT?

not really. RemovableDeviceNotificationsCros is not interested in this event, so
it just ignores it.

[satorux1, 2012-11-13 04:20:26]

LGTM

[tbarzic, 2012-11-14 19:34:48]

tbring vandebo@

[vandebo (ex-Chrome), 2012-11-14 21:54:26]

chrome/browser/system_monitor/* LGTM

[commit-bot: I haz the power, 2012-11-14 22:50:21]

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

[commit-bot: I haz the power, 2012-11-15 00:56:48]

Change committed as 167799