[StorageMonitor] Handle EjectDevice call for MTP devices

[StorageMonitor] Handle EjectDevice call for MTP devices

This change includes intercept code to dispatch eject calls on
MTP devices for all platforms. By and large, MTP devices don't need
specific ejection APIs to be called -- the devices are safe to remove
without notice. So the implementations mainly just simulate their
removal and signal OK to the caller. When the device is then actually
removed, the removal will be swallowed without additional notification.

R=thestig@chromium.org
BUG=257179
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=222112

[Lei Zhang, 2013-08-21 21:37:53]

https://codereview.chromium.org/23383009/diff/1/chrome/browser/storage_monito...
File
chrome/browser/storage_monitor/media_transfer_protocol_device_observer_linux.cc
(right):

https://codereview.chromium.org/23383009/diff/1/chrome/browser/storage_monito...
chrome/browser/storage_monitor/media_transfer_protocol_device_observer_linux.cc:177:
StorageChanged(false, location);
We may be able to do more here, and communicate with mtpd to say "eject device
foo". Feel free to put a TODO here for me.

https://codereview.chromium.org/23383009/diff/1/chrome/browser/storage_monito...
chrome/browser/storage_monitor/media_transfer_protocol_device_observer_linux.cc:227:
bool MediaTransferProtocolDeviceObserverLinux::GetLocationForDeviceId(
Can't you just return a std::string, with an empty string being the failure
case?

https://codereview.chromium.org/23383009/diff/1/chrome/browser/storage_monito...
chrome/browser/storage_monitor/media_transfer_protocol_device_observer_linux.cc:227:
bool MediaTransferProtocolDeviceObserverLinux::GetLocationForDeviceId(
nit: const method

https://codereview.chromium.org/23383009/diff/1/chrome/browser/storage_monito...
File chrome/browser/storage_monitor/portable_device_watcher_win.cc (right):

https://codereview.chromium.org/23383009/diff/1/chrome/browser/storage_monito...
chrome/browser/storage_monitor/portable_device_watcher_win.cc:558: // MTP
devices on windows don't have a detach API needed -- signal
nit: Windows

[Greg Billock, 2013-08-23 20:17:13]

https://codereview.chromium.org/23383009/diff/1/chrome/browser/storage_monito...
File
chrome/browser/storage_monitor/media_transfer_protocol_device_observer_linux.cc
(right):

https://codereview.chromium.org/23383009/diff/1/chrome/browser/storage_monito...
chrome/browser/storage_monitor/media_transfer_protocol_device_observer_linux.cc:177:
StorageChanged(false, location);
On 2013/08/21 21:37:53, Lei Zhang wrote:
> We may be able to do more here, and communicate with mtpd to say "eject device
> foo". Feel free to put a TODO here for me.

Done.

https://codereview.chromium.org/23383009/diff/1/chrome/browser/storage_monito...
chrome/browser/storage_monitor/media_transfer_protocol_device_observer_linux.cc:227:
bool MediaTransferProtocolDeviceObserverLinux::GetLocationForDeviceId(
On 2013/08/21 21:37:53, Lei Zhang wrote:
> Can't you just return a std::string, with an empty string being the failure
> case?

I actually had that. :-) Switched to this as I think its more in line with the
rest of the code, and probably better form than overloading a string value, even
though an empty device id is illegal...

https://codereview.chromium.org/23383009/diff/1/chrome/browser/storage_monito...
chrome/browser/storage_monitor/media_transfer_protocol_device_observer_linux.cc:227:
bool MediaTransferProtocolDeviceObserverLinux::GetLocationForDeviceId(
On 2013/08/21 21:37:53, Lei Zhang wrote:
> nit: const method

Done.

https://codereview.chromium.org/23383009/diff/1/chrome/browser/storage_monito...
File chrome/browser/storage_monitor/portable_device_watcher_win.cc (right):

https://codereview.chromium.org/23383009/diff/1/chrome/browser/storage_monito...
chrome/browser/storage_monitor/portable_device_watcher_win.cc:558: // MTP
devices on windows don't have a detach API needed -- signal
On 2013/08/21 21:37:53, Lei Zhang wrote:
> nit: Windows

Done.

[Lei Zhang, 2013-08-23 22:22:18]

lgtm

[Greg Billock, 2013-09-06 20:22:13]

On 2013/08/23 22:22:18, Lei Zhang wrote:
> lgtm

Sailesh, any comment on the mac portions of this?

[commit-bot: I haz the power, 2013-09-09 20:05:59]

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

[sail, 2013-09-09 21:26:12]

sorry for the slow response, mac code LGTM

https://codereview.chromium.org/23383009/diff/20001/chrome/browser/storage_mo...
File chrome/browser/storage_monitor/image_capture_device_manager.mm (right):

https://codereview.chromium.org/23383009/diff/20001/chrome/browser/storage_mo...
chrome/browser/storage_monitor/image_capture_device_manager.mm:152:
base::scoped_nsobject<ImageCaptureDevice> camera_device(
Is there a reason you need to keep a strong reference to this here? If there's
not then you could just do:
  ImageCaptureDevice* camera_device = [device_browser_ deviceForUUID:uuid];

[commit-bot: I haz the power, 2013-09-09 22:32:12]

Change committed as 222112

[Greg Billock, 2013-09-12 17:43:58]

https://codereview.chromium.org/23383009/diff/20001/chrome/browser/storage_mo...
File chrome/browser/storage_monitor/image_capture_device_manager.mm (right):

https://codereview.chromium.org/23383009/diff/20001/chrome/browser/storage_mo...
chrome/browser/storage_monitor/image_capture_device_manager.mm:152:
base::scoped_nsobject<ImageCaptureDevice> camera_device(
On 2013/09/09 21:26:13, sail wrote:
> Is there a reason you need to keep a strong reference to this here? If there's
> not then you could just do:
>   ImageCaptureDevice* camera_device = [device_browser_ deviceForUUID:uuid];

No, not really. This is how we did it elsewhere, so I was just following that
basically.