Eject support for ChromeOS

Eject support for ChromeOS

R=thestig@chromium.org,tbarzic@chromium.org
BUG=NONE


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

[tbarzic, 2013-03-13 18:57:03]

https://codereview.chromium.org/12596010/diff/3001/chrome/browser/storage_mon...
File chrome/browser/storage_monitor/storage_monitor_chromeos.cc (right):

https://codereview.chromium.org/12596010/diff/3001/chrome/browser/storage_mon...
chrome/browser/storage_monitor/storage_monitor_chromeos.cc:241:
manager->UnmountPath(mount_path, chromeos::UNMOUNT_OPTIONS_NONE);
unmount path may actually fail, but the success/failure is reported using
DiskMountManager's Observer interface (OnMountEvent)

I guess it would make sense to add a callback param to UnmountPath in addition
to broadcasting the result.
Would it be a problem to wait for a day with this cl (until I add callback to
unmount method)

https://codereview.chromium.org/12596010/diff/3001/chrome/browser/storage_mon...
File chrome/browser/storage_monitor/storage_monitor_chromeos.h (right):

https://codereview.chromium.org/12596010/diff/3001/chrome/browser/storage_mon...
chrome/browser/storage_monitor/storage_monitor_chromeos.h:56: virtual void
EjectDevice(
can you add a method comment.

[Lei Zhang, 2013-03-13 22:39:27]

https://codereview.chromium.org/12596010/diff/3001/chrome/browser/storage_mon...
File chrome/browser/storage_monitor/storage_monitor_chromeos.cc (right):

https://codereview.chromium.org/12596010/diff/3001/chrome/browser/storage_mon...
chrome/browser/storage_monitor/storage_monitor_chromeos.cc:225: info_it !=
mount_map_.end(); info_it++) {
nit: ++info_it

https://codereview.chromium.org/12596010/diff/3001/chrome/browser/storage_mon...
chrome/browser/storage_monitor/storage_monitor_chromeos.cc:245:
base::Bind(&NotifyEjectSuccess, callback));
You probably don't need NotifyEjectSuccess().

BrowserThread::PostTask(
    BrowserThread::UI, FROM_HERE,
    base::Bind(callback, chrome::StorageMonitor::EJECT_OK));

[Greg Billock, 2013-03-18 19:38:39]

https://codereview.chromium.org/12596010/diff/3001/chrome/browser/storage_mon...
File chrome/browser/storage_monitor/storage_monitor_chromeos.cc (right):

https://codereview.chromium.org/12596010/diff/3001/chrome/browser/storage_mon...
chrome/browser/storage_monitor/storage_monitor_chromeos.cc:225: info_it !=
mount_map_.end(); info_it++) {
On 2013/03/13 22:39:27, Lei Zhang wrote:
> nit: ++info_it

Done.

https://codereview.chromium.org/12596010/diff/3001/chrome/browser/storage_mon...
chrome/browser/storage_monitor/storage_monitor_chromeos.cc:241:
manager->UnmountPath(mount_path, chromeos::UNMOUNT_OPTIONS_NONE);
On 2013/03/13 18:57:04, tbarzic wrote:
> unmount path may actually fail, but the success/failure is reported using
> DiskMountManager's Observer interface (OnMountEvent)
> 
> I guess it would make sense to add a callback param to UnmountPath in addition
> to broadcasting the result.
> Would it be a problem to wait for a day with this cl (until I add callback to
> unmount method)

Done.

https://codereview.chromium.org/12596010/diff/3001/chrome/browser/storage_mon...
chrome/browser/storage_monitor/storage_monitor_chromeos.cc:245:
base::Bind(&NotifyEjectSuccess, callback));
On 2013/03/13 22:39:27, Lei Zhang wrote:
> You probably don't need NotifyEjectSuccess().
> 
> BrowserThread::PostTask(
>     BrowserThread::UI, FROM_HERE,
>     base::Bind(callback, chrome::StorageMonitor::EJECT_OK));

Replaced with trampoline to convert parameter from DiskMountManager now.

[tbarzic, 2013-03-18 19:48:26]

lgtm

https://codereview.chromium.org/12596010/diff/17001/chrome/browser/storage_mo...
File chrome/browser/storage_monitor/storage_monitor_chromeos.cc (right):

https://codereview.chromium.org/12596010/diff/17001/chrome/browser/storage_mo...
chrome/browser/storage_monitor/storage_monitor_chromeos.cc:248:
base::Bind(NotifyEjectSuccess, callback));
nit: how about NotifyEjectResult

[Greg Billock, 2013-03-18 21:44:44]

https://codereview.chromium.org/12596010/diff/17001/chrome/browser/storage_mo...
File chrome/browser/storage_monitor/storage_monitor_chromeos.cc (right):

https://codereview.chromium.org/12596010/diff/17001/chrome/browser/storage_mo...
chrome/browser/storage_monitor/storage_monitor_chromeos.cc:248:
base::Bind(NotifyEjectSuccess, callback));
On 2013/03/18 19:48:26, tbarzic wrote:
> nit: how about NotifyEjectResult

Good point. Changed to NotifyUnmountResult.

[commit-bot: I haz the power, 2013-03-19 16:47:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gbillock@chromium.org/12596010/30001

[Lei Zhang, 2013-03-19 18:58:57]

lgtm++

[commit-bot: I haz the power, 2013-03-19 19:52:18]

Change committed as 189068