New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 769630 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 612328



Sign in to add a comment

Servicify //device/media_transfer_protocol

Project Member Reported by blundell@chromium.org, Sep 28 2017

Issue description

As part of the transformation of //device into the Device Service, we need to servicify //device/media_transfer_protocol. From a quick inspection, this looks relatively straightforward. Here is my high-level analysis:

- The code is used by //chrome, //chrome/browser/chromeos, //components (entirely browser process clients).
- It looks like it's used on at least Linux and ChromeOS.
- The only public interface of the code is media_transfer_protocol_manager.h.
- Almost all of the methods of that class are already asynchronous, i.e., they take in a callback that gets invoked with the return value. These methods can likely be converted to Mojo in a straightforward way.
- It looks like there is some amount of synchronous methods exposed by that class as well -- I saw at least one synchronous accessor. We'll need to examine how to move those interactions to asynchronous.
 

Comment 1 by ke...@intel.com, Nov 3 2017

Cc: donna...@intel.com blundell@chromium.org
In media_transfer_protocol_manager, there is:
   MtpStorageInfo* GetStorageInfo(const std::string& storage_name)

The MtpStorageInfo is defined in "mtp_storage_info.pb.h' which is generated by 'mtp_storage_info.proto'. We need to re-define a MtpStorageInfo struct in mojom, and convert the native MtpStorageInfo to the mojom::MtpStorageInfo before passing it through mojo interface. 
Cc: roc...@chromium.org
+rockot@

Hey Ken, some APIS to be converted to Mojo currently pass (the C++ generated version of) a proto (mtp_storage_info.proto). It looks like we can't convert the protobuf wholesale to a Mojo struct because it's used on ChromeOS to communicate to daemons over DBus. Are there any patterns/best practices for impedance matching between protobufs and Mojo structs (other than the naive thing of manually defining a Mojo struct that mirrors the proto and converting back/forth as necessary)? 
No, that's about it. Note though that we do now have Chrome OS system
services (midis, for one) which speak Mojo to/from Chrome, using D-Bus only
to facilitate the Mojo IPC bootstrap. Not clear if that's a good option
here but it may be worth considering.
Thanks! That option would require rolling libchrome in ChromeOS to get the mojom in on that side, right?
I'm not sure exactly, but I don't see why the workflow would be any
different from however we maintain a proto for the same purpose.
Components: Internals>Services>Device
Yeah, that makes sense. I don't know what that workflow is and I think that modifying the CrOS system service impl is outside the scope of the work here, so I think that it would be best for us to do as suggested in c#2 and file a bug for changing the CrOS system service to talk Mojo. 
Backing up a sec: do we need to servicify this yet? Is MTP really a device service feature? It kind of feels like it's just another chunk of library code, with Dbus/proto as a chrome OS implementation detail. One option may be to punt this to components/ to get it out of device/.

If course the answers to those questions may in fact be "yes", I'm just not sure.
Cc: thestig@chromium.org satorux@chromium.org
Good question, and I don't know the answer offhand.

+the owners of //device/media_transfer_protocol. satorux@, thestig@: Can you explain exactly what //device/media_transfer is and what its use cases are? Is it implementing this: https://en.wikipedia.org/wiki/Media_Transfer_Protocol?

Thanks!
On ChromeOS, mtpd [1] handles Media Transfer Protocol devices. e.g. Modern Android devices. Chrome, and other potentially clients talk to mtpd over DBus. The code in //device/media_transfer_protocol is the client side code.

The ChromeOS File Manager and the Media Galleries App API care about MTP devices.

The part about Linux is a bit out of date. I was originally using Linux to develop mtpd, but didn't try very hard and gave up on trying to get Linux distros to adopt it.

[1] https://chromium.googlesource.com/chromiumos/platform/mtpd/
Cool, thanks for the info!

Ken: The fact that MTP is "talking to the device" and that it's used by ChromeOS apps makes me think that it *is* a good fit for the Device Service. WDYT?

Lei: Does the latter part of your comment mean that we can stop building this code on Linux and kill whatever Linux-specific code there is, or no?
Makes sense.
I checked my Linux build dir. These files aren't there. The only thing out of date is the #if and #error in device/media_transfer_protocol/*.h. I'll make a pass at that.

Comment 15 by ke...@intel.com, Nov 10 2017

Colin, so we'll go with "defining a Mojo struct that mirrors the proto" solution?
I see you replied "do as suggested in c#2", it actually means "Comment 1", right?
thanks:)
Right :).

Comment 17 Deleted

Comment 18 by donna...@intel.com, Nov 13 2017

Thank you all to make things clear. I'm DonnaWu, a new commer, an engineer in the same team with KeHe. With Ke's introduction, I got the rough picture of the task and I am looking at this issue now.

Comment 19 by ke...@intel.com, Nov 13 2017

Colin, I'm glad to introduce my teammate Donna who is interested to take this issue:)
I found I cannot set her as Owner, could you please help to request the Permission for her? thanks!
Owner: ke...@intel.com
Status: Assigned (was: Available)
Welcome, Donna! Made the request for EditBugs permission for you; I'll let you know when it comes through.

Comment 21 by donna...@intel.com, Nov 15 2017

Thanks blundell@. I'm investigating the current status of MTP and working to the design doc(https://docs.google.com/document/d/1kj6SYCa_oJpcIJ4qU1mniNgtXGcIWzirDkHbmvq8BpA/edit?usp=sharing), will notice all of you when it's ready for review.
Donna, you should have EditBugs permission now; try out taking ownership of this CL and let me know if it doesn't work.

Comment 23 by donna...@intel.com, Nov 21 2017

Cc: -donna...@intel.com -thestig@chromium.org ke...@intel.com
Owner: donna...@intel.com
Status: Started (was: Assigned)

Comment 24 by donna...@intel.com, Nov 21 2017

Thank you very much, blundell@. I took over this issue.

Comment 25 by donna...@intel.com, Nov 29 2017

Hi, blundell@ and all, a draft design doc has been worked out. Would you please help to review it? The public sharing link is:
https://docs.google.com/document/d/1kj6SYCa_oJpcIJ4qU1mniNgtXGcIWzirDkHbmvq8BpA/edit#heading=h.gsaziv51btiv 

Thanks! I reviewed the design doc and left some comments/questions. It's a great start!
Project Member

Comment 27 by bugdroid1@chromium.org, Dec 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/256b482974968727f332676f7194b0bec83d0b00

commit 256b482974968727f332676f7194b0bec83d0b00
Author: Donna Wu <donna.wu@intel.com>
Date: Fri Dec 15 07:59:06 2017

Add mtp_file_entry.mojom file and modify the related code.

Define mojo format MtpFileEntry to mirror the MTP protobuf message.
Use this struct in MediaTransferProtocolManager and related code.

This is a preparation work for MTP servicification.

BUG= 769630 

Change-Id: Ia01f5bc7c00fdcc258f50ae02e7a375caa83d6c9
Reviewed-on: https://chromium-review.googlesource.com/812964
Commit-Queue: Ke He <ke.he@intel.com>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524349}
[modify] https://crrev.com/256b482974968727f332676f7194b0bec83d0b00/chrome/browser/media_galleries/chromeos/mtp_device_object_enumerator.cc
[modify] https://crrev.com/256b482974968727f332676f7194b0bec83d0b00/chrome/browser/media_galleries/chromeos/mtp_device_object_enumerator.h
[modify] https://crrev.com/256b482974968727f332676f7194b0bec83d0b00/chrome/browser/media_galleries/chromeos/mtp_device_object_enumerator_unittest.cc
[modify] https://crrev.com/256b482974968727f332676f7194b0bec83d0b00/chrome/browser/media_galleries/chromeos/mtp_device_task_helper.cc
[modify] https://crrev.com/256b482974968727f332676f7194b0bec83d0b00/chrome/browser/media_galleries/chromeos/mtp_device_task_helper.h
[modify] https://crrev.com/256b482974968727f332676f7194b0bec83d0b00/components/storage_monitor/test_media_transfer_protocol_manager_chromeos.cc
[modify] https://crrev.com/256b482974968727f332676f7194b0bec83d0b00/device/media_transfer_protocol/BUILD.gn
[modify] https://crrev.com/256b482974968727f332676f7194b0bec83d0b00/device/media_transfer_protocol/media_transfer_protocol_daemon_client.cc
[modify] https://crrev.com/256b482974968727f332676f7194b0bec83d0b00/device/media_transfer_protocol/media_transfer_protocol_daemon_client.h
[modify] https://crrev.com/256b482974968727f332676f7194b0bec83d0b00/device/media_transfer_protocol/media_transfer_protocol_manager.cc
[modify] https://crrev.com/256b482974968727f332676f7194b0bec83d0b00/device/media_transfer_protocol/media_transfer_protocol_manager.h
[add] https://crrev.com/256b482974968727f332676f7194b0bec83d0b00/device/media_transfer_protocol/public/interfaces/BUILD.gn
[add] https://crrev.com/256b482974968727f332676f7194b0bec83d0b00/device/media_transfer_protocol/public/interfaces/OWNERS
[add] https://crrev.com/256b482974968727f332676f7194b0bec83d0b00/device/media_transfer_protocol/public/interfaces/mtp_file_entry.mojom

Project Member

Comment 28 by bugdroid1@chromium.org, Dec 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5a9a6ad86770c7e59f445485c19baa3fe8c1e370

commit 5a9a6ad86770c7e59f445485c19baa3fe8c1e370
Author: Donna Wu <donna.wu@intel.com>
Date: Thu Dec 21 22:18:50 2017

Add mtp_storage_info.mojom file and modify the related code.

Define mojo format MtpStorageInfo to mirror the MTP protobuf message.
Use this struct in MediaTransferProtocolManager and related code.

This is a preparation work for MTP servicification.

BUG= 769630 

Change-Id: Ie435600271840e9a26f59dfb53952a2b07642749
Reviewed-on: https://chromium-review.googlesource.com/838372
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525828}
[modify] https://crrev.com/5a9a6ad86770c7e59f445485c19baa3fe8c1e370/chrome/browser/chromeos/extensions/file_manager/private_api_file_system.cc
[modify] https://crrev.com/5a9a6ad86770c7e59f445485c19baa3fe8c1e370/chrome/browser/chromeos/extensions/file_manager/private_api_file_system.h
[modify] https://crrev.com/5a9a6ad86770c7e59f445485c19baa3fe8c1e370/chrome/browser/chromeos/file_manager/volume_manager.cc
[modify] https://crrev.com/5a9a6ad86770c7e59f445485c19baa3fe8c1e370/chrome/browser/chromeos/file_manager/volume_manager.h
[modify] https://crrev.com/5a9a6ad86770c7e59f445485c19baa3fe8c1e370/chrome/browser/chromeos/file_manager/volume_manager_unittest.cc
[modify] https://crrev.com/5a9a6ad86770c7e59f445485c19baa3fe8c1e370/components/storage_monitor/media_transfer_protocol_device_observer_chromeos.cc
[modify] https://crrev.com/5a9a6ad86770c7e59f445485c19baa3fe8c1e370/components/storage_monitor/test_media_transfer_protocol_manager_chromeos.cc
[modify] https://crrev.com/5a9a6ad86770c7e59f445485c19baa3fe8c1e370/components/storage_monitor/test_media_transfer_protocol_manager_chromeos.h
[modify] https://crrev.com/5a9a6ad86770c7e59f445485c19baa3fe8c1e370/device/media_transfer_protocol/media_transfer_protocol_daemon_client.cc
[modify] https://crrev.com/5a9a6ad86770c7e59f445485c19baa3fe8c1e370/device/media_transfer_protocol/media_transfer_protocol_daemon_client.h
[modify] https://crrev.com/5a9a6ad86770c7e59f445485c19baa3fe8c1e370/device/media_transfer_protocol/media_transfer_protocol_manager.cc
[modify] https://crrev.com/5a9a6ad86770c7e59f445485c19baa3fe8c1e370/device/media_transfer_protocol/media_transfer_protocol_manager.h
[modify] https://crrev.com/5a9a6ad86770c7e59f445485c19baa3fe8c1e370/device/media_transfer_protocol/public/interfaces/BUILD.gn
[add] https://crrev.com/5a9a6ad86770c7e59f445485c19baa3fe8c1e370/device/media_transfer_protocol/public/interfaces/mtp_storage_info.mojom

Project Member

Comment 29 by bugdroid1@chromium.org, Jan 11 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5ef2dee5ede63dc6d7d607e42ca40a4668bf77e6

commit 5ef2dee5ede63dc6d7d607e42ca40a4668bf77e6
Author: Donna Wu <donna.wu@intel.com>
Date: Thu Jan 11 22:25:56 2018

Make MediaTransferProtocolManager::GetStorages asynchronous.

This is a preparation work for MTP servicification.

BUG= 769630 

Change-Id: Ie776850f88adb0593b32b7ba701a83aa746a1a74
Reviewed-on: https://chromium-review.googlesource.com/845501
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528783}
[modify] https://crrev.com/5ef2dee5ede63dc6d7d607e42ca40a4668bf77e6/components/storage_monitor/media_transfer_protocol_device_observer_chromeos.cc
[modify] https://crrev.com/5ef2dee5ede63dc6d7d607e42ca40a4668bf77e6/components/storage_monitor/media_transfer_protocol_device_observer_chromeos.h
[modify] https://crrev.com/5ef2dee5ede63dc6d7d607e42ca40a4668bf77e6/components/storage_monitor/test_media_transfer_protocol_manager_chromeos.cc
[modify] https://crrev.com/5ef2dee5ede63dc6d7d607e42ca40a4668bf77e6/components/storage_monitor/test_media_transfer_protocol_manager_chromeos.h
[modify] https://crrev.com/5ef2dee5ede63dc6d7d607e42ca40a4668bf77e6/device/media_transfer_protocol/media_transfer_protocol_manager.cc
[modify] https://crrev.com/5ef2dee5ede63dc6d7d607e42ca40a4668bf77e6/device/media_transfer_protocol/media_transfer_protocol_manager.h

Project Member

Comment 30 by bugdroid1@chromium.org, Jan 18 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6b1bf367493827a13fd9b82da026b44dd7a1bf8f

commit 6b1bf367493827a13fd9b82da026b44dd7a1bf8f
Author: Donna Wu <donna.wu@intel.com>
Date: Thu Jan 18 06:07:42 2018

Test MTP Chrome OS observer on raw MTP storage information.

Introduce a set of raw MtpStorageInfo data, and use it in the unit
tests for MediaTransferProtocolDeviceObserverChromeOS.

This is a preparation work for MTP servicification.

BUG= 769630 

Change-Id: Ib34df5f14994ce75033b280e28437c4f5bb792da
Reviewed-on: https://chromium-review.googlesource.com/864482
Commit-Queue: Donna Wu <donna.wu@intel.com>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530067}
[modify] https://crrev.com/6b1bf367493827a13fd9b82da026b44dd7a1bf8f/components/storage_monitor/media_transfer_protocol_device_observer_chromeos.cc
[modify] https://crrev.com/6b1bf367493827a13fd9b82da026b44dd7a1bf8f/components/storage_monitor/media_transfer_protocol_device_observer_chromeos.h
[modify] https://crrev.com/6b1bf367493827a13fd9b82da026b44dd7a1bf8f/components/storage_monitor/media_transfer_protocol_device_observer_chromeos_unittest.cc

Project Member

Comment 31 by bugdroid1@chromium.org, Jan 18 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a08ccd0c0ba47e351cf77a33c2a2ae712bfa91b9

commit a08ccd0c0ba47e351cf77a33c2a2ae712bfa91b9
Author: Donna Wu <donna.wu@intel.com>
Date: Thu Jan 18 08:51:15 2018

Reverse the if condition for attach and detach MTP storage.

This is a preparation work for MTP servicification.

BUG= 769630 

Change-Id: I53d289d621caf044dd410e98795cbc191ebf9f3c
Reviewed-on: https://chromium-review.googlesource.com/869461
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Commit-Queue: Donna Wu <donna.wu@intel.com>
Cr-Commit-Position: refs/heads/master@{#530092}
[modify] https://crrev.com/a08ccd0c0ba47e351cf77a33c2a2ae712bfa91b9/components/storage_monitor/media_transfer_protocol_device_observer_chromeos.cc

Project Member

Comment 33 by bugdroid1@chromium.org, Jan 29 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d911a0a0018f350a931a3d0048dd9e97536405ef

commit d911a0a0018f350a931a3d0048dd9e97536405ef
Author: Donna Wu <donna.wu@intel.com>
Date: Mon Jan 29 08:52:17 2018

Make MediaTransferProtocolManager::GetStorageInfo callback format.

This is a preparation work for MTP servicification.

BUG= 769630 

Change-Id: I4ff39ddf9968ef35328c17b3ffefd6165b85725b
Reviewed-on: https://chromium-review.googlesource.com/846659
Commit-Queue: Donna Wu <donna.wu@intel.com>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532361}
[modify] https://crrev.com/d911a0a0018f350a931a3d0048dd9e97536405ef/chrome/browser/chromeos/file_manager/volume_manager.cc
[modify] https://crrev.com/d911a0a0018f350a931a3d0048dd9e97536405ef/chrome/browser/chromeos/file_manager/volume_manager.h
[modify] https://crrev.com/d911a0a0018f350a931a3d0048dd9e97536405ef/chrome/browser/chromeos/file_manager/volume_manager_unittest.cc
[modify] https://crrev.com/d911a0a0018f350a931a3d0048dd9e97536405ef/components/storage_monitor/media_transfer_protocol_device_observer_chromeos.cc
[modify] https://crrev.com/d911a0a0018f350a931a3d0048dd9e97536405ef/components/storage_monitor/media_transfer_protocol_device_observer_chromeos.h
[modify] https://crrev.com/d911a0a0018f350a931a3d0048dd9e97536405ef/components/storage_monitor/media_transfer_protocol_device_observer_chromeos_unittest.cc
[modify] https://crrev.com/d911a0a0018f350a931a3d0048dd9e97536405ef/components/storage_monitor/test_media_transfer_protocol_manager_chromeos.cc
[modify] https://crrev.com/d911a0a0018f350a931a3d0048dd9e97536405ef/components/storage_monitor/test_media_transfer_protocol_manager_chromeos.h
[modify] https://crrev.com/d911a0a0018f350a931a3d0048dd9e97536405ef/device/media_transfer_protocol/media_transfer_protocol_manager.cc
[modify] https://crrev.com/d911a0a0018f350a931a3d0048dd9e97536405ef/device/media_transfer_protocol/media_transfer_protocol_manager.h

Project Member

Comment 34 by bugdroid1@chromium.org, Jan 30 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f162649e2e79c2bbbfd69b78ba3609726104d589

commit f162649e2e79c2bbbfd69b78ba3609726104d589
Author: Donna Wu <donna.wu@intel.com>
Date: Tue Jan 30 06:27:06 2018

Separate StorageInfo utils functions out and add back the unit tests.

This CL separate the general functions for getting needed information
from MtpStorageInfo to storage_monitor::StorageInfo and add the
|id| and |location| test back.

BUG= 769630 

Change-Id: I1d77c1e4ea9471e04706ac9e94778e8bea771a94
Reviewed-on: https://chromium-review.googlesource.com/880487
Commit-Queue: Donna Wu <donna.wu@intel.com>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532794}
[modify] https://crrev.com/f162649e2e79c2bbbfd69b78ba3609726104d589/components/storage_monitor/BUILD.gn
[modify] https://crrev.com/f162649e2e79c2bbbfd69b78ba3609726104d589/components/storage_monitor/media_transfer_protocol_device_observer_chromeos.cc
[modify] https://crrev.com/f162649e2e79c2bbbfd69b78ba3609726104d589/components/storage_monitor/media_transfer_protocol_device_observer_chromeos_unittest.cc
[add] https://crrev.com/f162649e2e79c2bbbfd69b78ba3609726104d589/components/storage_monitor/storage_info_utils.cc
[add] https://crrev.com/f162649e2e79c2bbbfd69b78ba3609726104d589/components/storage_monitor/storage_info_utils.h

Project Member

Comment 35 by bugdroid1@chromium.org, Jan 31 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/907504107b80f99667106fae5dd66a2b9a89b693

commit 907504107b80f99667106fae5dd66a2b9a89b693
Author: Donna Wu <donna.wu@intel.com>
Date: Wed Jan 31 08:38:19 2018

Modify MTP manager observer interfaces to reduce async invokes

This will eliminate the observer needing to make an immediate
async call back to the MediaTransferProtocolManager to obtain
the MtpStorageInfo object from its identifier.

This is a preparation work for MTP servicification.

BUG= 769630 

Change-Id: I239ed96d42c4e496f97d311a689704e34c55edda
Reviewed-on: https://chromium-review.googlesource.com/882494
Commit-Queue: Donna Wu <donna.wu@intel.com>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533232}
[modify] https://crrev.com/907504107b80f99667106fae5dd66a2b9a89b693/components/storage_monitor/media_transfer_protocol_device_observer_chromeos.cc
[modify] https://crrev.com/907504107b80f99667106fae5dd66a2b9a89b693/components/storage_monitor/media_transfer_protocol_device_observer_chromeos.h
[modify] https://crrev.com/907504107b80f99667106fae5dd66a2b9a89b693/components/storage_monitor/media_transfer_protocol_device_observer_chromeos_unittest.cc
[modify] https://crrev.com/907504107b80f99667106fae5dd66a2b9a89b693/device/media_transfer_protocol/media_transfer_protocol_manager.cc
[modify] https://crrev.com/907504107b80f99667106fae5dd66a2b9a89b693/device/media_transfer_protocol/media_transfer_protocol_manager.h

Project Member

Comment 36 by bugdroid1@chromium.org, Feb 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/722e4c97679de29151ce061e7af0fcb0607e9083

commit 722e4c97679de29151ce061e7af0fcb0607e9083
Author: Donna Wu <donna.wu@intel.com>
Date: Thu Feb 08 02:19:18 2018

Add interface AddObserverAndEnumerateStorages() to MTP manager.

When "GetStorages()" and "GetStorageInfo()" are sync, it's cheap for
the observer to call MTP manager's interfaces to get |storage_name|
and |storage_info|. But when these interfaces become async, the same
logic may introduces two issues.

First, frequent async calls may harm the performance, for example in
MediaTransferProtocolDeviceObserverChromeOS constructor, it needs |n+1|
async calls to enumerate storages, it gets the a list of |storage_name|
first, then ask |storage_info| from the manager for each storage.

Second, as the enumerating action is now async, there may be race
condition between enumerating existing storages and the storage
changed events. Consider this case, there are |n| storages available
currently, and during the |n+1| async invokes, a device (No.|i|) has
been removed, MTP manager may notify the observer about the detached
event before the No.|i| device has been initially enumerated.
So the observer looks up on its map and finds there is no No.|i| in
its map and do nothing with the detach event. Then the initial
enumeration |storage_info| returns to the observer, and No.|i| device
will be added to the StorageMonitor's map which cause inconsistency.

So, this CL added a new interface AddObserverAndEnumerateStorages()
to MTP manager to reduce async invokes and the race condition
possibility.

This is a preparation work for MTP servicification.

BUG= 769630 

Change-Id: Idd9a65f81694758958612178d5cfaeb9c63d7ba2
Reviewed-on: https://chromium-review.googlesource.com/858605
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Commit-Queue: Donna Wu <donna.wu@intel.com>
Cr-Commit-Position: refs/heads/master@{#535260}
[modify] https://crrev.com/722e4c97679de29151ce061e7af0fcb0607e9083/components/storage_monitor/media_transfer_protocol_device_observer_chromeos.cc
[modify] https://crrev.com/722e4c97679de29151ce061e7af0fcb0607e9083/components/storage_monitor/media_transfer_protocol_device_observer_chromeos.h
[modify] https://crrev.com/722e4c97679de29151ce061e7af0fcb0607e9083/components/storage_monitor/test_media_transfer_protocol_manager_chromeos.cc
[modify] https://crrev.com/722e4c97679de29151ce061e7af0fcb0607e9083/components/storage_monitor/test_media_transfer_protocol_manager_chromeos.h
[modify] https://crrev.com/722e4c97679de29151ce061e7af0fcb0607e9083/device/media_transfer_protocol/media_transfer_protocol_manager.cc
[modify] https://crrev.com/722e4c97679de29151ce061e7af0fcb0607e9083/device/media_transfer_protocol/media_transfer_protocol_manager.h

Project Member

Comment 37 by bugdroid1@chromium.org, Apr 11 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9e82c21a324d6b46b01f8808b7e5b640187cfc5f

commit 9e82c21a324d6b46b01f8808b7e5b640187cfc5f
Author: Donna Wu <donna.wu@intel.com>
Date: Wed Apr 11 07:55:15 2018

Add MTP Mojo interfaces for talking to MediaTransferProtocolManager.

This CL:
1) Introduces 2 Mojo interfaces: device::mojom::MtpManager and
   device::mojom::MtpManagerClient. The current implementation just
   forwards MTP device access requests to the existing
   MediaTransferProtocolManager..
2) Makes Device Service expose the above 2 Mojo interfaces.
3) Create stubs to connect these 2 interfaces with the main user,
   storage_monitor::StorageMonitor.

More Mojo methods will be added to MtpManager and all users will be
converted to use Mojo interfaces in subsequent CLs.

BUG= 769630 

Change-Id: Ibfffb2a45d13014ec3984ae9e242f49492e8b16a
Reviewed-on: https://chromium-review.googlesource.com/972668
Commit-Queue: Donna Wu <donna.wu@intel.com>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549814}
[modify] https://crrev.com/9e82c21a324d6b46b01f8808b7e5b640187cfc5f/chrome/browser/browser_process_impl.cc
[modify] https://crrev.com/9e82c21a324d6b46b01f8808b7e5b640187cfc5f/components/storage_monitor/BUILD.gn
[modify] https://crrev.com/9e82c21a324d6b46b01f8808b7e5b640187cfc5f/components/storage_monitor/DEPS
[add] https://crrev.com/9e82c21a324d6b46b01f8808b7e5b640187cfc5f/components/storage_monitor/mtp_manager_client_chromeos.cc
[add] https://crrev.com/9e82c21a324d6b46b01f8808b7e5b640187cfc5f/components/storage_monitor/mtp_manager_client_chromeos.h
[modify] https://crrev.com/9e82c21a324d6b46b01f8808b7e5b640187cfc5f/components/storage_monitor/storage_monitor.cc
[modify] https://crrev.com/9e82c21a324d6b46b01f8808b7e5b640187cfc5f/components/storage_monitor/storage_monitor.h
[modify] https://crrev.com/9e82c21a324d6b46b01f8808b7e5b640187cfc5f/components/storage_monitor/storage_monitor_chromeos.cc
[modify] https://crrev.com/9e82c21a324d6b46b01f8808b7e5b640187cfc5f/components/storage_monitor/storage_monitor_chromeos.h
[modify] https://crrev.com/9e82c21a324d6b46b01f8808b7e5b640187cfc5f/content/public/app/mojo/content_browser_manifest.json
[modify] https://crrev.com/9e82c21a324d6b46b01f8808b7e5b640187cfc5f/device/media_transfer_protocol/media_transfer_protocol_manager.cc
[modify] https://crrev.com/9e82c21a324d6b46b01f8808b7e5b640187cfc5f/device/media_transfer_protocol/media_transfer_protocol_manager.h
[modify] https://crrev.com/9e82c21a324d6b46b01f8808b7e5b640187cfc5f/extensions/shell/browser/shell_browser_main_parts.cc
[modify] https://crrev.com/9e82c21a324d6b46b01f8808b7e5b640187cfc5f/services/device/BUILD.gn
[modify] https://crrev.com/9e82c21a324d6b46b01f8808b7e5b640187cfc5f/services/device/device_service.cc
[modify] https://crrev.com/9e82c21a324d6b46b01f8808b7e5b640187cfc5f/services/device/device_service.h
[modify] https://crrev.com/9e82c21a324d6b46b01f8808b7e5b640187cfc5f/services/device/manifest.json
[add] https://crrev.com/9e82c21a324d6b46b01f8808b7e5b640187cfc5f/services/device/media_transfer_protocol/BUILD.gn
[add] https://crrev.com/9e82c21a324d6b46b01f8808b7e5b640187cfc5f/services/device/media_transfer_protocol/DEPS
[add] https://crrev.com/9e82c21a324d6b46b01f8808b7e5b640187cfc5f/services/device/media_transfer_protocol/mtp_device_manager.cc
[add] https://crrev.com/9e82c21a324d6b46b01f8808b7e5b640187cfc5f/services/device/media_transfer_protocol/mtp_device_manager.h
[modify] https://crrev.com/9e82c21a324d6b46b01f8808b7e5b640187cfc5f/services/device/public/mojom/BUILD.gn
[add] https://crrev.com/9e82c21a324d6b46b01f8808b7e5b640187cfc5f/services/device/public/mojom/mtp_manager.mojom

Project Member

Comment 38 by bugdroid1@chromium.org, Apr 11 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f7fd2525a8de64da3f6bf002c5a5642b633637a7

commit f7fd2525a8de64da3f6bf002c5a5642b633637a7
Author: Alan Cutter <alancutter@chromium.org>
Date: Wed Apr 11 09:49:54 2018

Revert "Add MTP Mojo interfaces for talking to MediaTransferProtocolManager."

This reverts commit 9e82c21a324d6b46b01f8808b7e5b640187cfc5f.

Reason for revert: This change causes target_os="chromeos" to crash on start up:
[82494:82494:0411/194350.042525:FATAL:media_transfer_protocol_manager.cc(704)] Check failed: g_media_transfer_protocol_manager.                                             
#0 0x7f8751f192ac base::debug::StackTrace::StackTrace()
#1 0x7f8751f402eb logging::LogMessage::~LogMessage()                 
#2 0x7f87500945b9 device::MediaTransferProtocolManager::GetInstance()      
#3 0x7f87500936a5 device::MtpDeviceManager::EnumerateStoragesAndSetClient()   
#4 0x7f874efb5c3a device::mojom::MtpManagerStubDispatch::AcceptWithResponder()
#5 0x7f87500941b6 device::mojom::MtpManagerStub<>::AcceptWithResponder()  
#6 0x7f87511c5a77 mojo::InterfaceEndpointClient::HandleValidatedMessage()
#7 0x7f87511c53e6 mojo::FilterChain::Accept()                            
#8 0x7f87511c6dd5 mojo::InterfaceEndpointClient::HandleIncomingMessage()    
#9 0x7f87511cd53c mojo::internal::MultiplexRouter::ProcessIncomingMessage()
#10 0x7f87511cc940 mojo::internal::MultiplexRouter::Accept()
#11 0x7f87511c53e6 mojo::FilterChain::Accept()         
#12 0x7f87511c05c3 mojo::Connector::ReadSingleMessage()                           
#13 0x7f87511c0ff1 mojo::Connector::ReadAllAvailableMessages()
#14 0x7f87511c0e99 mojo::Connector::OnHandleReadyInternal()
#15 0x7f87511c1727 mojo::SimpleWatcher::DiscardReadyState()                                               
#16 0x7f8751187e86 mojo::SimpleWatcher::OnHandleReady()                                                              
#17 0x7f8751188391 _ZN4base8internal7InvokerINS0_9BindStateIMN4mojo13SimpleWatcherEFvijRKNS3_18HandleSignalsStateEEJN
S_7WeakPtrIS4_EEijS5_EEEFvvEE7RunImplIRKS9_RKNSt3__15tupleIJSB_ijS5_EEEJLm0ELm1ELm2ELm3EEEEvOT_OT0_NSI_16integer_sequ
enceImJXspT1_EEEE                                                   
#18 0x7f8751f19c45 base::debug::TaskAnnotator::RunTask()        
#19 0x7f8751f4c559 base::internal::IncomingTaskQueue::RunTask()                  
#20 0x7f8751f5009b base::MessageLoop::RunTask()                                 
#21 0x7f8751f5042a base::MessageLoop::DeferOrRunPendingTask()     
#22 0x7f8751f5068c base::MessageLoop::DoWork()     
#23 0x7f8751f52b29 base::MessagePumpLibevent::Run()
#24 0x7f8751f4f9c9 base::MessageLoop::Run()
#25 0x7f8751f86709 base::RunLoop::Run()                        
#26 0x55ecab42c14a ChromeBrowserMainParts::MainMessageLoopRun()       
#27 0x7f874f125a37 content::BrowserMainLoop::RunMainMessageLoopParts()
#28 0x7f874f128b36 content::BrowserMainRunnerImpl::Run()
#29 0x7f874f121cc9 content::BrowserMain()
#30 0x7f874fb09c8d content::ContentMainRunnerImpl::Run()
#31 0x7f875245e292 service_manager::Main()
#32 0x7f874fb08304 content::ContentMain()
#33 0x55ecaa89c263 ChromeMain
#34 0x7f87449822b1 __libc_start_main
#35 0x55ecaa89c0da _start


Original change's description:
> Add MTP Mojo interfaces for talking to MediaTransferProtocolManager.
> 
> This CL:
> 1) Introduces 2 Mojo interfaces: device::mojom::MtpManager and
>    device::mojom::MtpManagerClient. The current implementation just
>    forwards MTP device access requests to the existing
>    MediaTransferProtocolManager..
> 2) Makes Device Service expose the above 2 Mojo interfaces.
> 3) Create stubs to connect these 2 interfaces with the main user,
>    storage_monitor::StorageMonitor.
> 
> More Mojo methods will be added to MtpManager and all users will be
> converted to use Mojo interfaces in subsequent CLs.
> 
> BUG= 769630 
> 
> Change-Id: Ibfffb2a45d13014ec3984ae9e242f49492e8b16a
> Reviewed-on: https://chromium-review.googlesource.com/972668
> Commit-Queue: Donna Wu <donna.wu@intel.com>
> Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: Tom Sepez <tsepez@chromium.org>
> Reviewed-by: Lei Zhang <thestig@chromium.org>
> Reviewed-by: Colin Blundell <blundell@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#549814}

TBR=sky@chromium.org,thestig@chromium.org,blundell@chromium.org,rdevlin.cronin@chromium.org,tsepez@chromium.org,donna.wu@intel.com,leon.han@intel.com,ke.he@intel.com

Change-Id: I9417d3d4be8952819faf638007b3efb21c8eb618
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  769630 
Reviewed-on: https://chromium-review.googlesource.com/1006774
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549842}
[modify] https://crrev.com/f7fd2525a8de64da3f6bf002c5a5642b633637a7/chrome/browser/browser_process_impl.cc
[modify] https://crrev.com/f7fd2525a8de64da3f6bf002c5a5642b633637a7/components/storage_monitor/BUILD.gn
[modify] https://crrev.com/f7fd2525a8de64da3f6bf002c5a5642b633637a7/components/storage_monitor/DEPS
[delete] https://crrev.com/3f610510ee579df6613dd5ad3528d2f56bb5ee27/components/storage_monitor/mtp_manager_client_chromeos.cc
[delete] https://crrev.com/3f610510ee579df6613dd5ad3528d2f56bb5ee27/components/storage_monitor/mtp_manager_client_chromeos.h
[modify] https://crrev.com/f7fd2525a8de64da3f6bf002c5a5642b633637a7/components/storage_monitor/storage_monitor.cc
[modify] https://crrev.com/f7fd2525a8de64da3f6bf002c5a5642b633637a7/components/storage_monitor/storage_monitor.h
[modify] https://crrev.com/f7fd2525a8de64da3f6bf002c5a5642b633637a7/components/storage_monitor/storage_monitor_chromeos.cc
[modify] https://crrev.com/f7fd2525a8de64da3f6bf002c5a5642b633637a7/components/storage_monitor/storage_monitor_chromeos.h
[modify] https://crrev.com/f7fd2525a8de64da3f6bf002c5a5642b633637a7/content/public/app/mojo/content_browser_manifest.json
[modify] https://crrev.com/f7fd2525a8de64da3f6bf002c5a5642b633637a7/device/media_transfer_protocol/media_transfer_protocol_manager.cc
[modify] https://crrev.com/f7fd2525a8de64da3f6bf002c5a5642b633637a7/device/media_transfer_protocol/media_transfer_protocol_manager.h
[modify] https://crrev.com/f7fd2525a8de64da3f6bf002c5a5642b633637a7/extensions/shell/browser/shell_browser_main_parts.cc
[modify] https://crrev.com/f7fd2525a8de64da3f6bf002c5a5642b633637a7/services/device/BUILD.gn
[modify] https://crrev.com/f7fd2525a8de64da3f6bf002c5a5642b633637a7/services/device/device_service.cc
[modify] https://crrev.com/f7fd2525a8de64da3f6bf002c5a5642b633637a7/services/device/device_service.h
[modify] https://crrev.com/f7fd2525a8de64da3f6bf002c5a5642b633637a7/services/device/manifest.json
[delete] https://crrev.com/3f610510ee579df6613dd5ad3528d2f56bb5ee27/services/device/media_transfer_protocol/BUILD.gn
[delete] https://crrev.com/3f610510ee579df6613dd5ad3528d2f56bb5ee27/services/device/media_transfer_protocol/DEPS
[delete] https://crrev.com/3f610510ee579df6613dd5ad3528d2f56bb5ee27/services/device/media_transfer_protocol/mtp_device_manager.cc
[delete] https://crrev.com/3f610510ee579df6613dd5ad3528d2f56bb5ee27/services/device/media_transfer_protocol/mtp_device_manager.h
[modify] https://crrev.com/f7fd2525a8de64da3f6bf002c5a5642b633637a7/services/device/public/mojom/BUILD.gn
[delete] https://crrev.com/3f610510ee579df6613dd5ad3528d2f56bb5ee27/services/device/public/mojom/mtp_manager.mojom

Project Member

Comment 39 by bugdroid1@chromium.org, Apr 20 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aee2f66f67f0cfaab9f5d3284081a19c5cfe9bcd

commit aee2f66f67f0cfaab9f5d3284081a19c5cfe9bcd
Author: Donna Wu <donna.wu@intel.com>
Date: Fri Apr 20 06:25:15 2018

Reland: Introduce MTP Mojo interface talking to MediaTransferProtocolManager.

The original CL has been reverted because it caused a crash on start up.

In previous CL, MediaTransferProtocolManager was built as a static library
which was depended by both component::StorageMonitor(a static library),
and device::mojom::MtpManager which will be included in "content" shared
library for component build. So the singleton MediaTransferProtocolManager
instance actually was not identical in two places as the shared library
would keep a copy in its own package which caused the crash.

This reland CL build the MediaTransferProtocolManager as a "component" to
keep the singleton instance identical for component build.

The original CL's description:
> This CL:
> 1) introduces 2 Mojo interfaces device::mojom::MtpManager and
>    device::mojom::MtpManagerClient which expose MTP device accessing
>    by forwarding invokes to existing MediaTransferProtocolManager.
> 2) makes Device Service expose the above 2 Mojo interfaces.
> 3) create stubs to connect these 2 interfaces with the main user
>    component::StorageMonitor.

> More Mojo methods will be added to MtpManager and all users will be
> converted to use Mojo interfaces in the succeeding CLs.

> BUG= 769630 

> Change-Id: I4680de2b9307cd40fb660c8e115e03f08cd3da10

Change-Id: I4680de2b9307cd40fb660c8e115e03f08cd3da10
Reviewed-on: https://chromium-review.googlesource.com/1014576
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Commit-Queue: Donna Wu <donna.wu@intel.com>
Cr-Commit-Position: refs/heads/master@{#552274}
[modify] https://crrev.com/aee2f66f67f0cfaab9f5d3284081a19c5cfe9bcd/chrome/browser/browser_process_impl.cc
[modify] https://crrev.com/aee2f66f67f0cfaab9f5d3284081a19c5cfe9bcd/components/storage_monitor/BUILD.gn
[modify] https://crrev.com/aee2f66f67f0cfaab9f5d3284081a19c5cfe9bcd/components/storage_monitor/DEPS
[add] https://crrev.com/aee2f66f67f0cfaab9f5d3284081a19c5cfe9bcd/components/storage_monitor/mtp_manager_client_chromeos.cc
[add] https://crrev.com/aee2f66f67f0cfaab9f5d3284081a19c5cfe9bcd/components/storage_monitor/mtp_manager_client_chromeos.h
[modify] https://crrev.com/aee2f66f67f0cfaab9f5d3284081a19c5cfe9bcd/components/storage_monitor/storage_monitor.cc
[modify] https://crrev.com/aee2f66f67f0cfaab9f5d3284081a19c5cfe9bcd/components/storage_monitor/storage_monitor.h
[modify] https://crrev.com/aee2f66f67f0cfaab9f5d3284081a19c5cfe9bcd/components/storage_monitor/storage_monitor_chromeos.cc
[modify] https://crrev.com/aee2f66f67f0cfaab9f5d3284081a19c5cfe9bcd/components/storage_monitor/storage_monitor_chromeos.h
[modify] https://crrev.com/aee2f66f67f0cfaab9f5d3284081a19c5cfe9bcd/content/public/app/mojo/content_browser_manifest.json
[modify] https://crrev.com/aee2f66f67f0cfaab9f5d3284081a19c5cfe9bcd/device/media_transfer_protocol/BUILD.gn
[modify] https://crrev.com/aee2f66f67f0cfaab9f5d3284081a19c5cfe9bcd/device/media_transfer_protocol/media_transfer_protocol_manager.cc
[modify] https://crrev.com/aee2f66f67f0cfaab9f5d3284081a19c5cfe9bcd/device/media_transfer_protocol/media_transfer_protocol_manager.h
[modify] https://crrev.com/aee2f66f67f0cfaab9f5d3284081a19c5cfe9bcd/extensions/shell/browser/shell_browser_main_parts.cc
[modify] https://crrev.com/aee2f66f67f0cfaab9f5d3284081a19c5cfe9bcd/services/device/BUILD.gn
[modify] https://crrev.com/aee2f66f67f0cfaab9f5d3284081a19c5cfe9bcd/services/device/device_service.cc
[modify] https://crrev.com/aee2f66f67f0cfaab9f5d3284081a19c5cfe9bcd/services/device/device_service.h
[modify] https://crrev.com/aee2f66f67f0cfaab9f5d3284081a19c5cfe9bcd/services/device/manifest.json
[add] https://crrev.com/aee2f66f67f0cfaab9f5d3284081a19c5cfe9bcd/services/device/media_transfer_protocol/BUILD.gn
[add] https://crrev.com/aee2f66f67f0cfaab9f5d3284081a19c5cfe9bcd/services/device/media_transfer_protocol/DEPS
[add] https://crrev.com/aee2f66f67f0cfaab9f5d3284081a19c5cfe9bcd/services/device/media_transfer_protocol/mtp_device_manager.cc
[add] https://crrev.com/aee2f66f67f0cfaab9f5d3284081a19c5cfe9bcd/services/device/media_transfer_protocol/mtp_device_manager.h
[modify] https://crrev.com/aee2f66f67f0cfaab9f5d3284081a19c5cfe9bcd/services/device/public/mojom/BUILD.gn
[add] https://crrev.com/aee2f66f67f0cfaab9f5d3284081a19c5cfe9bcd/services/device/public/mojom/mtp_manager.mojom

Project Member

Comment 40 by bugdroid1@chromium.org, Apr 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5d01e7d6448d8bc11f270facabf274b25227f7ed

commit 5d01e7d6448d8bc11f270facabf274b25227f7ed
Author: Donna Wu <donna.wu@intel.com>
Date: Mon Apr 23 08:28:32 2018

Add remaining mojo methods to device::mojom::MtpManager.

This CL makes device::mojom::MtpManager interface complete with
remaining methods. Clients will be migrated to use MTP mojo
interfaces in the subsequent CL.

BUG= 769630 

Change-Id: Ic0c443873f7645c4d221fb8892a322c2d2162bfe
Reviewed-on: https://chromium-review.googlesource.com/1002572
Commit-Queue: Donna Wu <donna.wu@intel.com>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552652}
[modify] https://crrev.com/5d01e7d6448d8bc11f270facabf274b25227f7ed/services/device/media_transfer_protocol/mtp_device_manager.cc
[modify] https://crrev.com/5d01e7d6448d8bc11f270facabf274b25227f7ed/services/device/media_transfer_protocol/mtp_device_manager.h
[modify] https://crrev.com/5d01e7d6448d8bc11f270facabf274b25227f7ed/services/device/public/mojom/mtp_manager.mojom

Project Member

Comment 41 by bugdroid1@chromium.org, May 11 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6547fc81718ae2da3750725863841b8005a9fb5e

commit 6547fc81718ae2da3750725863841b8005a9fb5e
Author: Donna Wu <donna.wu@intel.com>
Date: Fri May 11 09:47:45 2018

Implement MTPManagerClientChromeOS to replace existing MTP observer.

Implementation of MTPManagerClientChromeOS is mostly copied from
media_transfer_protocol_device_observer_chromeos.h/cc files which
will be replaced in subsequent CLs.

This is a preparation CL for the conversion of MTP mojo interfaces'
usage in the client side without any functional changes for the
notifications to upper level clients have been blocked.

BUG= 769630 

Change-Id: I28076cc4a4e30783426c6ccb5af1805ef913d7ae
Reviewed-on: https://chromium-review.googlesource.com/1046376
Commit-Queue: Donna Wu <donna.wu@intel.com>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557827}
[modify] https://crrev.com/6547fc81718ae2da3750725863841b8005a9fb5e/components/storage_monitor/mtp_manager_client_chromeos.cc
[modify] https://crrev.com/6547fc81718ae2da3750725863841b8005a9fb5e/components/storage_monitor/mtp_manager_client_chromeos.h

Project Member

Comment 42 by bugdroid1@chromium.org, May 14 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec

commit aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec
Author: Donna Wu <donna.wu@intel.com>
Date: Mon May 14 04:33:24 2018

Convert clients and testing code to use device::mojom::MtpManager.

With this CL, the singleton device::MediaTransferProtocolManager
is owned by device::mojom::MtpManager, and usage of the former MTP
manager has also been converted to Mojo MTP interfaces.

Next, we will merge device/media_transfer_protocol/ into the
implementation of device::mojom::MtpManager.

BUG= 769630 

Change-Id: I6a2b5d0bf388152e953bb7af3d13e85c2272e570
Reviewed-on: https://chromium-review.googlesource.com/1032237
Commit-Queue: Donna Wu <donna.wu@intel.com>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558192}
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/chrome/browser/BUILD.gn
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/chrome/browser/chromeos/extensions/file_manager/private_api_file_system.cc
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/chrome/browser/chromeos/extensions/file_manager/private_api_file_system.h
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/chrome/browser/chromeos/file_manager/volume_manager.cc
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/chrome/browser/chromeos/file_manager/volume_manager.h
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/chrome/browser/chromeos/file_manager/volume_manager_factory.cc
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/chrome/browser/chromeos/file_manager/volume_manager_unittest.cc
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/chrome/browser/media_galleries/chromeos/mtp_device_object_enumerator.cc
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/chrome/browser/media_galleries/chromeos/mtp_device_object_enumerator.h
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/chrome/browser/media_galleries/chromeos/mtp_device_object_enumerator_unittest.cc
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/chrome/browser/media_galleries/chromeos/mtp_device_task_helper.cc
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/chrome/browser/media_galleries/chromeos/mtp_device_task_helper.h
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/chrome/browser/media_galleries/chromeos/mtp_read_file_worker.cc
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/chrome/browser/media_galleries/media_galleries_preferences_unittest.cc
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/components/storage_monitor/BUILD.gn
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/components/storage_monitor/mtp_manager_client_chromeos.cc
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/components/storage_monitor/mtp_manager_client_chromeos.h
[rename] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/components/storage_monitor/mtp_manager_client_chromeos_unittest.cc
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/components/storage_monitor/storage_monitor.h
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/components/storage_monitor/storage_monitor_chromeos.cc
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/components/storage_monitor/storage_monitor_chromeos.h
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/components/storage_monitor/storage_monitor_chromeos_unittest.cc
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/components/storage_monitor/storage_monitor_unittest.cc
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/components/storage_monitor/test_media_transfer_protocol_manager_chromeos.cc
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/components/storage_monitor/test_media_transfer_protocol_manager_chromeos.h
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/components/storage_monitor/test_storage_monitor.cc
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/components/storage_monitor/test_storage_monitor.h
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/device/media_transfer_protocol/BUILD.gn
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/device/media_transfer_protocol/media_transfer_protocol_manager.cc
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/device/media_transfer_protocol/media_transfer_protocol_manager.h
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/services/device/media_transfer_protocol/BUILD.gn
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/services/device/media_transfer_protocol/mtp_device_manager.cc
[modify] https://crrev.com/aaa4d5aa673b0e9f9973bc38386f95f59e2fb3ec/services/device/media_transfer_protocol/mtp_device_manager.h

Project Member

Comment 43 by bugdroid1@chromium.org, May 15 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cb728bef621d221f70c0b8e08af4b8e90f8f3b90

commit cb728bef621d221f70c0b8e08af4b8e90f8f3b90
Author: Donna Wu <donna.wu@intel.com>
Date: Tue May 15 02:18:46 2018

Delete useless class MediaTransferProtocolDeviceObserverChromeOS.

This class has been replaced by MTPManagerClientChromeOS introduced
by MTP mojofication process.

BUG= 769630 

Change-Id: I27a83181ebbb464ab37308d85fec10832edf06fa
Reviewed-on: https://chromium-review.googlesource.com/1046377
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Commit-Queue: Donna Wu <donna.wu@intel.com>
Cr-Commit-Position: refs/heads/master@{#558589}
[delete] https://crrev.com/435a3a234cf75db27ba33c136c8b245071c2ef20/components/storage_monitor/media_transfer_protocol_device_observer_chromeos.cc
[delete] https://crrev.com/435a3a234cf75db27ba33c136c8b245071c2ef20/components/storage_monitor/media_transfer_protocol_device_observer_chromeos.h

Project Member

Comment 44 by bugdroid1@chromium.org, May 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e5a8310ffa85f78f366a212cd8bdbe3237f8b05b

commit e5a8310ffa85f78f366a212cd8bdbe3237f8b05b
Author: Donna Wu <donna.wu@intel.com>
Date: Wed May 23 05:35:09 2018

Merge device/media_transfer_protocol to services folder.

BUG= 769630 

Change-Id: If9caefc4a503430648bd6cfccb56adcc066b1f62
Reviewed-on: https://chromium-review.googlesource.com/1063554
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Donna Wu <donna.wu@intel.com>
Cr-Commit-Position: refs/heads/master@{#560955}
[modify] https://crrev.com/e5a8310ffa85f78f366a212cd8bdbe3237f8b05b/chrome/browser/DEPS
[modify] https://crrev.com/e5a8310ffa85f78f366a212cd8bdbe3237f8b05b/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/e5a8310ffa85f78f366a212cd8bdbe3237f8b05b/chrome/browser/chromeos/extensions/file_manager/private_api_file_system.h
[modify] https://crrev.com/e5a8310ffa85f78f366a212cd8bdbe3237f8b05b/chrome/browser/media_galleries/chromeos/mtp_device_object_enumerator.h
[modify] https://crrev.com/e5a8310ffa85f78f366a212cd8bdbe3237f8b05b/chrome/browser/media_galleries/chromeos/mtp_device_task_helper.h
[modify] https://crrev.com/e5a8310ffa85f78f366a212cd8bdbe3237f8b05b/components/storage_monitor/BUILD.gn
[modify] https://crrev.com/e5a8310ffa85f78f366a212cd8bdbe3237f8b05b/components/storage_monitor/DEPS
[modify] https://crrev.com/e5a8310ffa85f78f366a212cd8bdbe3237f8b05b/components/storage_monitor/storage_info_utils.h
[modify] https://crrev.com/e5a8310ffa85f78f366a212cd8bdbe3237f8b05b/components/storage_monitor/storage_monitor_chromeos.cc
[delete] https://crrev.com/9abbe58fbf6c886744187fb134158d8fc097f8fa/device/media_transfer_protocol/BUILD.gn
[delete] https://crrev.com/9abbe58fbf6c886744187fb134158d8fc097f8fa/device/media_transfer_protocol/DEPS
[delete] https://crrev.com/9abbe58fbf6c886744187fb134158d8fc097f8fa/device/media_transfer_protocol/OWNERS
[delete] https://crrev.com/9abbe58fbf6c886744187fb134158d8fc097f8fa/device/media_transfer_protocol/public/mojom/BUILD.gn
[delete] https://crrev.com/9abbe58fbf6c886744187fb134158d8fc097f8fa/device/media_transfer_protocol/public/mojom/OWNERS
[modify] https://crrev.com/e5a8310ffa85f78f366a212cd8bdbe3237f8b05b/services/device/media_transfer_protocol/BUILD.gn
[modify] https://crrev.com/e5a8310ffa85f78f366a212cd8bdbe3237f8b05b/services/device/media_transfer_protocol/DEPS
[rename] https://crrev.com/e5a8310ffa85f78f366a212cd8bdbe3237f8b05b/services/device/media_transfer_protocol/media_transfer_protocol_daemon_client.cc
[rename] https://crrev.com/e5a8310ffa85f78f366a212cd8bdbe3237f8b05b/services/device/media_transfer_protocol/media_transfer_protocol_daemon_client.h
[rename] https://crrev.com/e5a8310ffa85f78f366a212cd8bdbe3237f8b05b/services/device/media_transfer_protocol/media_transfer_protocol_manager.cc
[rename] https://crrev.com/e5a8310ffa85f78f366a212cd8bdbe3237f8b05b/services/device/media_transfer_protocol/media_transfer_protocol_manager.h
[modify] https://crrev.com/e5a8310ffa85f78f366a212cd8bdbe3237f8b05b/services/device/media_transfer_protocol/mtp_device_manager.h
[modify] https://crrev.com/e5a8310ffa85f78f366a212cd8bdbe3237f8b05b/services/device/public/mojom/BUILD.gn
[rename] https://crrev.com/e5a8310ffa85f78f366a212cd8bdbe3237f8b05b/services/device/public/mojom/mtp_file_entry.mojom
[modify] https://crrev.com/e5a8310ffa85f78f366a212cd8bdbe3237f8b05b/services/device/public/mojom/mtp_manager.mojom
[rename] https://crrev.com/e5a8310ffa85f78f366a212cd8bdbe3237f8b05b/services/device/public/mojom/mtp_storage_info.mojom

Project Member

Comment 48 by bugdroid1@chromium.org, Jun 7 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3d7f15c742e7aff293b9b2065837bf98cb3a042b

commit 3d7f15c742e7aff293b9b2065837bf98cb3a042b
Author: Lei Zhang <thestig@chromium.org>
Date: Thu Jun 07 21:18:17 2018

Align GetFileInfo() between MtpManager and MtpDaemonClient.

MediaTransferProtocolDaemonClient currently provides a low-level
GetFileInfo() method, and MtpManager does the translation to the
high-level GetFileInfo() method that MTPDeviceTaskHelper understands.

Change MtpManager's GetFileInfo() from the high-level method to the
low-level method, so it acts more like a pass-through. Make
MTPDeviceTaskHelper handle the translation.

BUG= 769630 

Change-Id: I21e20b71d333a1ee7173071f04aaf2e37dd61844
Reviewed-on: https://chromium-review.googlesource.com/1080212
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565413}
[modify] https://crrev.com/3d7f15c742e7aff293b9b2065837bf98cb3a042b/chrome/browser/media_galleries/chromeos/mtp_device_task_helper.cc
[modify] https://crrev.com/3d7f15c742e7aff293b9b2065837bf98cb3a042b/chrome/browser/media_galleries/chromeos/mtp_device_task_helper.h
[modify] https://crrev.com/3d7f15c742e7aff293b9b2065837bf98cb3a042b/components/storage_monitor/test_media_transfer_protocol_manager_chromeos.cc
[modify] https://crrev.com/3d7f15c742e7aff293b9b2065837bf98cb3a042b/components/storage_monitor/test_media_transfer_protocol_manager_chromeos.h
[modify] https://crrev.com/3d7f15c742e7aff293b9b2065837bf98cb3a042b/services/device/media_transfer_protocol/media_transfer_protocol_manager.cc
[modify] https://crrev.com/3d7f15c742e7aff293b9b2065837bf98cb3a042b/services/device/media_transfer_protocol/media_transfer_protocol_manager.h
[modify] https://crrev.com/3d7f15c742e7aff293b9b2065837bf98cb3a042b/services/device/media_transfer_protocol/mtp_device_manager.cc
[modify] https://crrev.com/3d7f15c742e7aff293b9b2065837bf98cb3a042b/services/device/media_transfer_protocol/mtp_device_manager.h
[modify] https://crrev.com/3d7f15c742e7aff293b9b2065837bf98cb3a042b/services/device/public/mojom/mtp_manager.mojom

Project Member

Comment 49 by bugdroid1@chromium.org, Jun 7 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/44663e8f02729bf01d296ce86087f87168420713

commit 44663e8f02729bf01d296ce86087f87168420713
Author: Lei Zhang <thestig@chromium.org>
Date: Thu Jun 07 22:24:46 2018

Replace MtpManager's ReadDirectory() with ReadDirectoryEntryIds().

MediaTransferProtocolDaemonClient currently provides a low-level
ReadDirectoryEntryIds() method, and MtpManager does the translation to
the high-level ReadDirectory() method that MTPDeviceTaskHelper
understands.

Change MtpManager's ReadDirectory() method to ReadDirectoryEntryIds(),
so it acts more like a pass-through. Make MTPDeviceTaskHelper handle the
translation.

BUG= 769630 

Change-Id: I5154bd4beac0b6ab15579f166cdf8b8add6a7dac
Reviewed-on: https://chromium-review.googlesource.com/1080214
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565441}
[modify] https://crrev.com/44663e8f02729bf01d296ce86087f87168420713/chrome/browser/media_galleries/chromeos/mtp_device_task_helper.cc
[modify] https://crrev.com/44663e8f02729bf01d296ce86087f87168420713/chrome/browser/media_galleries/chromeos/mtp_device_task_helper.h
[modify] https://crrev.com/44663e8f02729bf01d296ce86087f87168420713/components/storage_monitor/test_media_transfer_protocol_manager_chromeos.cc
[modify] https://crrev.com/44663e8f02729bf01d296ce86087f87168420713/components/storage_monitor/test_media_transfer_protocol_manager_chromeos.h
[modify] https://crrev.com/44663e8f02729bf01d296ce86087f87168420713/services/device/media_transfer_protocol/media_transfer_protocol_manager.cc
[modify] https://crrev.com/44663e8f02729bf01d296ce86087f87168420713/services/device/media_transfer_protocol/media_transfer_protocol_manager.h
[modify] https://crrev.com/44663e8f02729bf01d296ce86087f87168420713/services/device/media_transfer_protocol/mtp_device_manager.cc
[modify] https://crrev.com/44663e8f02729bf01d296ce86087f87168420713/services/device/media_transfer_protocol/mtp_device_manager.h
[modify] https://crrev.com/44663e8f02729bf01d296ce86087f87168420713/services/device/public/mojom/mtp_manager.mojom

Project Member

Comment 50 by bugdroid1@chromium.org, Jun 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/55954d13eb38560ea783795422432cf372fff7fd

commit 55954d13eb38560ea783795422432cf372fff7fd
Author: Lei Zhang <thestig@chromium.org>
Date: Fri Jun 08 04:57:51 2018

Implement MTPDeviceTaskHelper::CheckDirectoryEmpty().

This is what the information the caller really wants to know about
directories on MTP devices before deleting them. By implementing this,
the caller no longer have to use MTPDeviceTaskHelper::ReadDirectory().
As a result, ReadDirectory() can be simplified in a follow-up CL.

BUG= 769630 

Change-Id: I29c1b40f7682cb68c271ea4817fbf32c776f33ab
Reviewed-on: https://chromium-review.googlesource.com/1090221
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565544}
[modify] https://crrev.com/55954d13eb38560ea783795422432cf372fff7fd/chrome/browser/media_galleries/chromeos/mtp_device_delegate_impl_chromeos.cc
[modify] https://crrev.com/55954d13eb38560ea783795422432cf372fff7fd/chrome/browser/media_galleries/chromeos/mtp_device_delegate_impl_chromeos.h
[modify] https://crrev.com/55954d13eb38560ea783795422432cf372fff7fd/chrome/browser/media_galleries/chromeos/mtp_device_task_helper.cc
[modify] https://crrev.com/55954d13eb38560ea783795422432cf372fff7fd/chrome/browser/media_galleries/chromeos/mtp_device_task_helper.h

Project Member

Comment 51 by bugdroid1@chromium.org, Jun 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4727d0ccdbe79e948ef8c0288099f1acf5a37eb1

commit 4727d0ccdbe79e948ef8c0288099f1acf5a37eb1
Author: Lei Zhang <thestig@chromium.org>
Date: Fri Jun 08 05:03:29 2018

Simplify MTPDeviceTaskHelper::ReadDirectory().

Remove the |max_size| parameter, which is now always 0.

BUG= 769630 

Change-Id: Id44b41f4c783119aa9069f474e47e567e5cda6a2
Reviewed-on: https://chromium-review.googlesource.com/1090222
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565552}
[modify] https://crrev.com/4727d0ccdbe79e948ef8c0288099f1acf5a37eb1/chrome/browser/media_galleries/chromeos/mtp_device_delegate_impl_chromeos.cc
[modify] https://crrev.com/4727d0ccdbe79e948ef8c0288099f1acf5a37eb1/chrome/browser/media_galleries/chromeos/mtp_device_task_helper.cc
[modify] https://crrev.com/4727d0ccdbe79e948ef8c0288099f1acf5a37eb1/chrome/browser/media_galleries/chromeos/mtp_device_task_helper.h

Project Member

Comment 52 by bugdroid1@chromium.org, Jun 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5606a804316cbf68d722c345341f9122ee40d3a9

commit 5606a804316cbf68d722c345341f9122ee40d3a9
Author: Lei Zhang <thestig@chromium.org>
Date: Fri Jun 08 05:15:00 2018

Simplify MtpManager::GetFileInfo().

Currently it takes an array of IDs, an offset into the array, and a
count. This CL changes the method to just take a list of IDs, so its
implementation is more straight forward. Callers that used to keep track
of offsets into a fixed array now does the work of splitting the array
into smaller chunks.

BUG= 769630 

Change-Id: I7661bac4a965ce0734ebc5e89828b6b10b11572a
Reviewed-on: https://chromium-review.googlesource.com/1080219
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565553}
[modify] https://crrev.com/5606a804316cbf68d722c345341f9122ee40d3a9/chrome/browser/media_galleries/chromeos/mtp_device_task_helper.cc
[modify] https://crrev.com/5606a804316cbf68d722c345341f9122ee40d3a9/chrome/browser/media_galleries/chromeos/mtp_device_task_helper.h
[modify] https://crrev.com/5606a804316cbf68d722c345341f9122ee40d3a9/components/storage_monitor/test_media_transfer_protocol_manager_chromeos.cc
[modify] https://crrev.com/5606a804316cbf68d722c345341f9122ee40d3a9/components/storage_monitor/test_media_transfer_protocol_manager_chromeos.h
[modify] https://crrev.com/5606a804316cbf68d722c345341f9122ee40d3a9/services/device/media_transfer_protocol/media_transfer_protocol_daemon_client.cc
[modify] https://crrev.com/5606a804316cbf68d722c345341f9122ee40d3a9/services/device/media_transfer_protocol/media_transfer_protocol_daemon_client.h
[modify] https://crrev.com/5606a804316cbf68d722c345341f9122ee40d3a9/services/device/media_transfer_protocol/media_transfer_protocol_manager.cc
[modify] https://crrev.com/5606a804316cbf68d722c345341f9122ee40d3a9/services/device/media_transfer_protocol/media_transfer_protocol_manager.h
[modify] https://crrev.com/5606a804316cbf68d722c345341f9122ee40d3a9/services/device/media_transfer_protocol/mtp_device_manager.cc
[modify] https://crrev.com/5606a804316cbf68d722c345341f9122ee40d3a9/services/device/media_transfer_protocol/mtp_device_manager.h
[modify] https://crrev.com/5606a804316cbf68d722c345341f9122ee40d3a9/services/device/public/mojom/mtp_manager.mojom

Project Member

Comment 54 by bugdroid1@chromium.org, Jun 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9d0cafb59a153b49e528d6da9db1214abf0c5fd8

commit 9d0cafb59a153b49e528d6da9db1214abf0c5fd8
Author: Donna Wu <donna.wu@intel.com>
Date: Fri Jun 22 02:36:17 2018

Merge MediaTransferProtocolManager to MtpDeviceManager.

This CL changes MtpDeviceManager from forwarding to
MediaTransferProtocolManagerImpl to directly having the
implementations of the latter in the methods that previously
forwarded. As MediaTransferProtocolManagerImpl was previously an
implementation detail hidden in media_transfer_protocol_manager.cc,
the relevant parts of the MediaTransferProtocolManagerImpl declaration
are moved from that cc file to mtp_device_manager.h, while the
relevant parts of the impl are inlined into the corresponding
MtpDeviceManager method implementations in mtp_device_manager.cc.

BUG= 769630 

Change-Id: Ia3ce06a69b8b4574b666711c2d8811d71101f579
Reviewed-on: https://chromium-review.googlesource.com/1096656
Commit-Queue: Donna Wu <donna.wu@intel.com>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569515}
[modify] https://crrev.com/9d0cafb59a153b49e528d6da9db1214abf0c5fd8/services/device/device_service.cc
[modify] https://crrev.com/9d0cafb59a153b49e528d6da9db1214abf0c5fd8/services/device/media_transfer_protocol/BUILD.gn
[delete] https://crrev.com/1aed13d794de1fc3ac6b074047148d69b8e96c57/services/device/media_transfer_protocol/media_transfer_protocol_manager.cc
[delete] https://crrev.com/1aed13d794de1fc3ac6b074047148d69b8e96c57/services/device/media_transfer_protocol/media_transfer_protocol_manager.h
[modify] https://crrev.com/9d0cafb59a153b49e528d6da9db1214abf0c5fd8/services/device/media_transfer_protocol/mtp_device_manager.cc
[modify] https://crrev.com/9d0cafb59a153b49e528d6da9db1214abf0c5fd8/services/device/media_transfer_protocol/mtp_device_manager.h

Comment 55 by donna...@intel.com, Jun 22 2018

Cc: thestig@chromium.org
Status: Fixed (was: Started)
@Colin and @Lei I'd like to mark it as "Fixed", let me know if there are other concerns.
blundell:src(port_gcm_account_tracker) $  git grep -l "services/device/media_transfer_protocol"
services/device/BUILD.gn
services/device/device_service.h
services/device/media_transfer_protocol/BUILD.gn
services/device/media_transfer_protocol/media_transfer_protocol_daemon_client.cc
services/device/media_transfer_protocol/mtp_device_manager.cc
services/device/media_transfer_protocol/mtp_device_manager.h

LGTM! Thank you for all of the effort here -- it was a lot of work to get to this point!

Project Member

Comment 57 by bugdroid1@chromium.org, Jul 10

Labels: merge-merged-3440
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/99180f2481b702ae4928a92b1dd39231996ede2d

commit 99180f2481b702ae4928a92b1dd39231996ede2d
Author: Lei Zhang <thestig@chromium.org>
Date: Tue Jul 10 00:24:16 2018

[M68] Align GetFileInfo() between MtpManager and MtpDaemonClient.

TBR=thestig@chromium.org
BUG= 858899 

Align GetFileInfo() between MtpManager and MtpDaemonClient.

MediaTransferProtocolDaemonClient currently provides a low-level
GetFileInfo() method, and MtpManager does the translation to the
high-level GetFileInfo() method that MTPDeviceTaskHelper understands.

Change MtpManager's GetFileInfo() from the high-level method to the
low-level method, so it acts more like a pass-through. Make
MTPDeviceTaskHelper handle the translation.

BUG= 769630 

Change-Id: I21e20b71d333a1ee7173071f04aaf2e37dd61844
Reviewed-on: https://chromium-review.googlesource.com/1080212
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#565413}(cherry picked from commit 3d7f15c742e7aff293b9b2065837bf98cb3a042b)
Reviewed-on: https://chromium-review.googlesource.com/1130715
Reviewed-by: Anand Mistry <amistry@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#624}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/99180f2481b702ae4928a92b1dd39231996ede2d/chrome/browser/media_galleries/chromeos/mtp_device_task_helper.cc
[modify] https://crrev.com/99180f2481b702ae4928a92b1dd39231996ede2d/chrome/browser/media_galleries/chromeos/mtp_device_task_helper.h
[modify] https://crrev.com/99180f2481b702ae4928a92b1dd39231996ede2d/components/storage_monitor/test_media_transfer_protocol_manager_chromeos.cc
[modify] https://crrev.com/99180f2481b702ae4928a92b1dd39231996ede2d/components/storage_monitor/test_media_transfer_protocol_manager_chromeos.h
[modify] https://crrev.com/99180f2481b702ae4928a92b1dd39231996ede2d/services/device/media_transfer_protocol/media_transfer_protocol_manager.cc
[modify] https://crrev.com/99180f2481b702ae4928a92b1dd39231996ede2d/services/device/media_transfer_protocol/media_transfer_protocol_manager.h
[modify] https://crrev.com/99180f2481b702ae4928a92b1dd39231996ede2d/services/device/media_transfer_protocol/mtp_device_manager.cc
[modify] https://crrev.com/99180f2481b702ae4928a92b1dd39231996ede2d/services/device/media_transfer_protocol/mtp_device_manager.h
[modify] https://crrev.com/99180f2481b702ae4928a92b1dd39231996ede2d/services/device/public/mojom/mtp_manager.mojom

Project Member

Comment 58 by bugdroid1@chromium.org, Jul 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1530ae01b4687f98b62ee816762ac1042ea2b270

commit 1530ae01b4687f98b62ee816762ac1042ea2b270
Author: Lei Zhang <thestig@chromium.org>
Date: Tue Jul 10 00:27:57 2018

[M68] Replace MtpManager's ReadDirectory() with ReadDirectoryEntryIds().

TBR=thestig@chromium.org
BUG= 858899 

Replace MtpManager's ReadDirectory() with ReadDirectoryEntryIds().

MediaTransferProtocolDaemonClient currently provides a low-level
ReadDirectoryEntryIds() method, and MtpManager does the translation to
the high-level ReadDirectory() method that MTPDeviceTaskHelper
understands.

Change MtpManager's ReadDirectory() method to ReadDirectoryEntryIds(),
so it acts more like a pass-through. Make MTPDeviceTaskHelper handle the
translation.

BUG= 769630 

Change-Id: I5154bd4beac0b6ab15579f166cdf8b8add6a7dac
Reviewed-on: https://chromium-review.googlesource.com/1080214
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#565441}(cherry picked from commit 44663e8f02729bf01d296ce86087f87168420713)
Reviewed-on: https://chromium-review.googlesource.com/1130735
Reviewed-by: Anand Mistry <amistry@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#625}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/1530ae01b4687f98b62ee816762ac1042ea2b270/chrome/browser/media_galleries/chromeos/mtp_device_task_helper.cc
[modify] https://crrev.com/1530ae01b4687f98b62ee816762ac1042ea2b270/chrome/browser/media_galleries/chromeos/mtp_device_task_helper.h
[modify] https://crrev.com/1530ae01b4687f98b62ee816762ac1042ea2b270/components/storage_monitor/test_media_transfer_protocol_manager_chromeos.cc
[modify] https://crrev.com/1530ae01b4687f98b62ee816762ac1042ea2b270/components/storage_monitor/test_media_transfer_protocol_manager_chromeos.h
[modify] https://crrev.com/1530ae01b4687f98b62ee816762ac1042ea2b270/services/device/media_transfer_protocol/media_transfer_protocol_manager.cc
[modify] https://crrev.com/1530ae01b4687f98b62ee816762ac1042ea2b270/services/device/media_transfer_protocol/media_transfer_protocol_manager.h
[modify] https://crrev.com/1530ae01b4687f98b62ee816762ac1042ea2b270/services/device/media_transfer_protocol/mtp_device_manager.cc
[modify] https://crrev.com/1530ae01b4687f98b62ee816762ac1042ea2b270/services/device/media_transfer_protocol/mtp_device_manager.h
[modify] https://crrev.com/1530ae01b4687f98b62ee816762ac1042ea2b270/services/device/public/mojom/mtp_manager.mojom

Sign in to add a comment