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}

components/storage_monitor/media_transfer_protocol_device_observer_chromeos.cc

@@ -78,33 +78,27 @@ void MediaTransferProtocolDeviceObserverChromeOS::EjectDevice(
 
   // TODO(thestig): Change this to tell the mtp manager to eject the device.
 
-  StorageChanged(false, location);
+  StorageDetached(location);
   callback.Run(StorageMonitor::EJECT_OK);
 }
 
 // device::MediaTransferProtocolManager::Observer override.
-void MediaTransferProtocolDeviceObserverChromeOS::StorageChanged(
-    bool is_attached,
+void MediaTransferProtocolDeviceObserverChromeOS::StorageAttached(
+    const device::mojom::MtpStorageInfo& mtp_storage_info) {
+  DoAttachStorage(&mtp_storage_info);
+}
+
+// device::MediaTransferProtocolManager::Observer override.
+void MediaTransferProtocolDeviceObserverChromeOS::StorageDetached(
     const std::string& storage_name) {
   DCHECK(!storage_name.empty());
 
-  // Existing storage is detached.
-  if (!is_attached) {
-    StorageLocationToInfoMap::iterator it =
-        storage_map_.find(GetDeviceLocationFromStorageName(storage_name));
-    if (it == storage_map_.end())
-      return;
-    notifications_->ProcessDetach(it->second.device_id());
-    storage_map_.erase(it);
+  StorageLocationToInfoMap::iterator it =
+      storage_map_.find(GetDeviceLocationFromStorageName(storage_name));
+  if (it == storage_map_.end())
     return;
-  }
-
-  // New storage is attached.
-  get_mtp_storage_info_cb_.Run(
-      storage_name,
-      base::BindOnce(
-          &MediaTransferProtocolDeviceObserverChromeOS::DoAttachStorage,
-          weak_ptr_factory_.GetWeakPtr()));
+  notifications_->ProcessDetach(it->second.device_id());
+  storage_map_.erase(it);
 }
 
 void MediaTransferProtocolDeviceObserverChromeOS::DoAttachStorage(
@@ -142,7 +136,11 @@ void MediaTransferProtocolDeviceObserverChromeOS::OnReceivedStorages(
   typedef std::vector<std::string> StorageList;
   for (StorageList::const_iterator storage_iter = storages.begin();
        storage_iter != storages.end(); ++storage_iter) {
-    StorageChanged(true, *storage_iter);
+    get_mtp_storage_info_cb_.Run(
+        *storage_iter,
+        base::BindOnce(
+            &MediaTransferProtocolDeviceObserverChromeOS::DoAttachStorage,
+            weak_ptr_factory_.GetWeakPtr()));
   }
 }
 

components/storage_monitor/media_transfer_protocol_device_observer_chromeos.h

@@ -52,8 +52,9 @@ class MediaTransferProtocolDeviceObserverChromeOS
 
   // device::MediaTransferProtocolManager::Observer implementation.
   // Exposed for unit tests.
-  void StorageChanged(bool is_attached,
-                      const std::string& storage_name) override;
+  void StorageAttached(
+      const device::mojom::MtpStorageInfo& storage_info) override;
+  void StorageDetached(const std::string& storage_name) override;
 
  private:
   // Mapping of storage location and mtp storage info object.

components/storage_monitor/media_transfer_protocol_device_observer_chromeos_unittest.cc

@@ -93,7 +93,10 @@ class TestMediaTransferProtocolDeviceObserverChromeOS
   // of
   // mtp storage device given the |storage_name|.
   void MtpStorageAttached(const std::string& storage_name) {
-    StorageChanged(true, storage_name);
+    auto* storage_info = GetFakeMtpStorageInfoSync(storage_name);
+    DCHECK(storage_info);
+
+    StorageAttached(*storage_info);
     base::RunLoop().RunUntilIdle();
   }
 
@@ -101,7 +104,7 @@ class TestMediaTransferProtocolDeviceObserverChromeOS
   // of
   // mtp storage device given the |storage_name|.
   void MtpStorageDetached(const std::string& storage_name) {
-    StorageChanged(false, storage_name);
+    StorageDetached(storage_name);
     base::RunLoop().RunUntilIdle();
   }
 

device/media_transfer_protocol/media_transfer_protocol_manager.cc

@@ -338,7 +338,7 @@ class MediaTransferProtocolManagerImpl : public MediaTransferProtocolManager {
       return;
     }
     for (auto& observer : observers_)
-      observer.StorageChanged(false /* detach */, storage_name);
+      observer.StorageDetached(storage_name);
   }
 
   void OnStorageChanged(bool is_attach, const std::string& storage_name) {
@@ -379,8 +379,9 @@ class MediaTransferProtocolManagerImpl : public MediaTransferProtocolManager {
 
     // New storage. Add it and let the observers know.
     storage_info_map_.insert(std::make_pair(storage_name, storage_info));
+
     for (auto& observer : observers_)
-      observer.StorageChanged(true /* is attach */, storage_name);
+      observer.StorageAttached(storage_info);
   }
 
   void OnGetStorageInfoFromDevice(const mojom::MtpStorageInfo& storage_info) {

device/media_transfer_protocol/media_transfer_protocol_manager.h

@@ -99,9 +99,10 @@ class MediaTransferProtocolManager {
    public:
     virtual ~Observer() {}
 
-    // A function called after a MTP storage has been attached / detached.
-    virtual void StorageChanged(bool is_attached,
-                                const std::string& storage_name) = 0;
+    // Functions called after a MTP storage has been attached / detached.
+    virtual void StorageAttached(
+        const device::mojom::MtpStorageInfo& storage_info) = 0;
+    virtual void StorageDetached(const std::string& storage_name) = 0;
   };
 
   virtual ~MediaTransferProtocolManager() {}