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}

chrome/browser/media_galleries/chromeos/mtp_device_task_helper.cc

@@ -24,6 +24,10 @@ using storage_monitor::StorageMonitor;
 
 namespace {
 
+// When calling GetFileInfo() with only a single file ID, the offset into the
+// file IDs vector should be 0.
+constexpr size_t kSingleFileIdOffset = 0;
+
 device::mojom::MtpManager* GetMediaTransferProtocolManager() {
   return StorageMonitor::GetInstance()->media_transfer_protocol_manager();
 }
@@ -81,11 +85,11 @@ void MTPDeviceTaskHelper::GetFileInfo(
   if (device_handle_.empty())
     return HandleDeviceError(error_callback, base::File::FILE_ERROR_FAILED);
 
+  const std::vector<uint32_t> file_ids = {file_id};
   GetMediaTransferProtocolManager()->GetFileInfo(
-      device_handle_, file_id,
+      device_handle_, file_ids, kSingleFileIdOffset, file_ids.size(),
       base::Bind(&MTPDeviceTaskHelper::OnGetFileInfo,
-                 weak_ptr_factory_.GetWeakPtr(),
-                 success_callback,
+                 weak_ptr_factory_.GetWeakPtr(), success_callback,
                  error_callback));
 }
 
@@ -144,10 +148,11 @@ void MTPDeviceTaskHelper::ReadBytes(
                              base::File::FILE_ERROR_FAILED);
   }
 
+  const std::vector<uint32_t> file_ids = {request.file_id};
   GetMediaTransferProtocolManager()->GetFileInfo(
-      device_handle_, request.file_id,
-      base::Bind(&MTPDeviceTaskHelper::OnGetFileInfoToReadBytes,
-                 weak_ptr_factory_.GetWeakPtr(), request));
+      device_handle_, file_ids, kSingleFileIdOffset, file_ids.size(),
+      base::BindOnce(&MTPDeviceTaskHelper::OnGetFileInfoToReadBytes,
+                     weak_ptr_factory_.GetWeakPtr(), request));
 }
 
 void MTPDeviceTaskHelper::RenameObject(
@@ -218,18 +223,18 @@ void MTPDeviceTaskHelper::OnDidOpenStorage(
 void MTPDeviceTaskHelper::OnGetFileInfo(
     const GetFileInfoSuccessCallback& success_callback,
     const ErrorCallback& error_callback,
-    device::mojom::MtpFileEntryPtr file_entry,
+    std::vector<device::mojom::MtpFileEntryPtr> entries,
     bool error) const {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  if (error) {
+  if (error || entries.size() != 1) {
     return HandleDeviceError(error_callback,
                              base::File::FILE_ERROR_NOT_FOUND);
   }
 
   content::BrowserThread::PostTask(
       content::BrowserThread::IO, FROM_HERE,
-      base::Bind(success_callback,
-                 FileInfoFromMTPFileEntry(std::move(file_entry))));
+      base::BindOnce(success_callback,
+                     FileInfoFromMTPFileEntry(std::move(entries[0]))));
 }
 
 void MTPDeviceTaskHelper::OnCreateDirectory(
@@ -279,27 +284,29 @@ void MTPDeviceTaskHelper::OnDidReadDirectory(
 
 void MTPDeviceTaskHelper::OnGetFileInfoToReadBytes(
     const MTPDeviceAsyncDelegate::ReadBytesRequest& request,
-    device::mojom::MtpFileEntryPtr file_entry,
+    std::vector<device::mojom::MtpFileEntryPtr> entries,
     bool error) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(request.buf.get());
   DCHECK_GE(request.buf_len, 0);
   DCHECK_GE(request.offset, 0);
-  if (error) {
+  if (error || entries.size() != 1) {
     return HandleDeviceError(request.error_callback,
                              base::File::FILE_ERROR_FAILED);
   }
 
-  base::File::Info file_info = FileInfoFromMTPFileEntry(std::move(file_entry));
+  base::File::Info file_info = FileInfoFromMTPFileEntry(std::move(entries[0]));
   if (file_info.is_directory) {
     return HandleDeviceError(request.error_callback,
                              base::File::FILE_ERROR_NOT_A_FILE);
-  } else if (file_info.size < 0 ||
-             file_info.size > std::numeric_limits<uint32_t>::max() ||
-             request.offset > file_info.size) {
+  }
+  if (file_info.size < 0 ||
+      file_info.size > std::numeric_limits<uint32_t>::max() ||
+      request.offset > file_info.size) {
     return HandleDeviceError(request.error_callback,
                              base::File::FILE_ERROR_FAILED);
-  } else if (request.offset == file_info.size) {
+  }
+  if (request.offset == file_info.size) {
     content::BrowserThread::PostTask(content::BrowserThread::IO,
                                      FROM_HERE,
                                      base::Bind(request.success_callback,

chrome/browser/media_galleries/chromeos/mtp_device_task_helper.h

@@ -160,16 +160,16 @@ class MTPDeviceTaskHelper {
 
   // Query callback for GetFileInfo().
   //
-  // If there is no error, |file_entry| will contain the
+  // If there is no error, |entries| will contain a single element with the
   // requested media device file details and |error| is set to false.
   // |success_callback| is invoked on the IO thread to notify the caller.
   //
-  // If there is an error, |file_entry| is invalid and |error| is
-  // set to true. |error_callback| is invoked on the IO thread to notify the
-  // caller.
+  // When |entries| has a size other than 1, or if |error| is true, then an
+  // error has occurred. In this case, |error_callback| is invoked on the IO
+  // thread to notify the caller.
   void OnGetFileInfo(const GetFileInfoSuccessCallback& success_callback,
                      const ErrorCallback& error_callback,
-                     device::mojom::MtpFileEntryPtr file_entry,
+                     std::vector<device::mojom::MtpFileEntryPtr> entries,
                      bool error) const;
 
   // Called when CreateDirectory completes.
@@ -195,7 +195,7 @@ class MTPDeviceTaskHelper {
   // Intermediate step to finish a ReadBytes request.
   void OnGetFileInfoToReadBytes(
       const MTPDeviceAsyncDelegate::ReadBytesRequest& request,
-      device::mojom::MtpFileEntryPtr file_entry,
+      std::vector<device::mojom::MtpFileEntryPtr> entries,
       bool error);
 
   // Query callback for ReadBytes();

components/storage_monitor/test_media_transfer_protocol_manager_chromeos.cc

@@ -91,9 +91,11 @@ void TestMediaTransferProtocolManagerChromeOS::ReadFileChunk(
 
 void TestMediaTransferProtocolManagerChromeOS::GetFileInfo(
     const std::string& storage_handle,
-    uint32_t file_id,
+    const std::vector<uint32_t>& file_ids,
+    uint32_t offset,
+    uint32_t entries_to_read,
     GetFileInfoCallback callback) {
-  std::move(callback).Run(device::mojom::MtpFileEntry::New(), true);
+  std::move(callback).Run(std::vector<device::mojom::MtpFileEntryPtr>(), true);
 }
 
 void TestMediaTransferProtocolManagerChromeOS::RenameObject(

components/storage_monitor/test_media_transfer_protocol_manager_chromeos.h

@@ -55,7 +55,9 @@ class TestMediaTransferProtocolManagerChromeOS
                      uint32_t count,
                      ReadFileChunkCallback callback) override;
   void GetFileInfo(const std::string& storage_handle,
-                   uint32_t file_id,
+                   const std::vector<uint32_t>& file_ids,
+                   uint32_t offset,
+                   uint32_t entries_to_read,
                    GetFileInfoCallback callback) override;
   void RenameObject(const std::string& storage_handle,
                     uint32_t object_id,

services/device/media_transfer_protocol/media_transfer_protocol_manager.cc

@@ -237,21 +237,19 @@ class MediaTransferProtocolManagerImpl : public MediaTransferProtocolManager {
   }
 
   void GetFileInfo(const std::string& storage_handle,
-                   uint32_t file_id,
+                   const std::vector<uint32_t>& file_ids,
+                   uint32_t offset,
+                   uint32_t entries_to_read,
                    mojom::MtpManager::GetFileInfoCallback callback) override {
     DCHECK(thread_checker_.CalledOnValidThread());
     if (!base::ContainsKey(handles_, storage_handle) || !mtp_client_) {
-      std::move(callback).Run(nullptr, true);
+      std::move(callback).Run(std::vector<device::mojom::MtpFileEntryPtr>(),
+                              /*error=*/true);
       return;
     }
-    std::vector<uint32_t> file_ids;
-    file_ids.push_back(file_id);
     get_file_info_callbacks_.push(std::move(callback));
     mtp_client_->GetFileInfo(
-        storage_handle,
-        file_ids,
-        kInitialOffset,
-        file_ids.size(),
+        storage_handle, file_ids, offset, entries_to_read,
         base::Bind(&MediaTransferProtocolManagerImpl::OnGetFileInfo,
                    weak_ptr_factory_.GetWeakPtr()),
         base::Bind(&MediaTransferProtocolManagerImpl::OnGetFileInfoError,
@@ -552,18 +550,19 @@ class MediaTransferProtocolManagerImpl : public MediaTransferProtocolManager {
 
   void OnGetFileInfo(const std::vector<mojom::MtpFileEntry>& entries) {
     DCHECK(thread_checker_.CalledOnValidThread());
-    if (entries.size() == 1) {
-      std::move(get_file_info_callbacks_.front())
-          .Run(entries[0].Clone(), false /* no error */);
-      get_file_info_callbacks_.pop();
-    } else {
-      OnGetFileInfoError();
-    }
+
+    std::vector<device::mojom::MtpFileEntryPtr> ret(entries.size());
+    for (size_t i = 0; i < entries.size(); ++i)
+      ret[i] = entries[i].Clone();
+    std::move(get_file_info_callbacks_.front())
+        .Run(std::move(ret), /*error=*/false);
+    get_file_info_callbacks_.pop();
   }
 
   void OnGetFileInfoError() {
     DCHECK(thread_checker_.CalledOnValidThread());
-    std::move(get_file_info_callbacks_.front()).Run(nullptr, true);
+    std::move(get_file_info_callbacks_.front())
+        .Run(std::vector<device::mojom::MtpFileEntryPtr>(), /*error=*/true);
     get_file_info_callbacks_.pop();
   }
 

services/device/media_transfer_protocol/media_transfer_protocol_manager.h

@@ -146,12 +146,15 @@ class MediaTransferProtocolManager {
                              uint32_t count,
                              const ReadFileCallback& callback) = 0;
 
-  // Gets the file metadata for |file_id| on |storage_handle| and runs
-  // |callback|.
+  // Gets the metadata for files on |storage_handle| with |file_ids|. Only
+  // considers |entries_to_read| number of entries in |file_ids| starting at
+  // offset|. Runs |callback| with the results.
   // Use mojom::MtpManager::GetFileInfoCallback directly to get prepared for
   // future merge.
   virtual void GetFileInfo(const std::string& storage_handle,
-                           uint32_t file_id,
+                           const std::vector<uint32_t>& file_ids,
+                           uint32_t offset,
+                           uint32_t entries_to_read,
                            mojom::MtpManager::GetFileInfoCallback callback) = 0;
 
   // Renames |object_id| to |new_name|.

services/device/media_transfer_protocol/mtp_device_manager.cc

@@ -136,10 +136,12 @@ void MtpDeviceManager::ReadFileChunk(const std::string& storage_handle,
 }
 
 void MtpDeviceManager::GetFileInfo(const std::string& storage_handle,
-                                   uint32_t file_id,
+                                   const std::vector<uint32_t>& file_ids,
+                                   uint32_t offset,
+                                   uint32_t entries_to_read,
                                    GetFileInfoCallback callback) {
-  media_transfer_protocol_manager_->GetFileInfo(storage_handle, file_id,
-                                                std::move(callback));
+  media_transfer_protocol_manager_->GetFileInfo(
+      storage_handle, file_ids, offset, entries_to_read, std::move(callback));
 }
 
 void MtpDeviceManager::RenameObject(const std::string& storage_handle,

services/device/media_transfer_protocol/mtp_device_manager.h

@@ -56,7 +56,9 @@ class MtpDeviceManager : public mojom::MtpManager {
                      uint32_t count,
                      ReadFileChunkCallback callback) override;
   void GetFileInfo(const std::string& storage_handle,
-                   uint32_t file_id,
+                   const std::vector<uint32_t>& file_ids,
+                   uint32_t offset,
+                   uint32_t entries_to_read,
                    GetFileInfoCallback callback) override;
   void RenameObject(const std::string& storage_handle,
                     uint32_t object_id,

services/device/public/mojom/mtp_manager.mojom

@@ -48,7 +48,6 @@ interface MtpManager {
       uint32 file_id, uint64 max_size) =>
     (array<MtpFileEntry> file_entries, bool has_more, bool error);
 
-
   // Reads file data from |file_id| on |storage_handle|.
   // Reads |count| bytes of data starting at |offset|.
   ReadFileChunk(string storage_handle,
@@ -56,10 +55,14 @@ interface MtpManager {
       uint32 offset,
       uint32 count) => (string data, bool error);
 
-  // Gets the file metadata for |file_id| on |storage_handle|.
+  // Gets the file metadata for entries in |file_ids| on |storage_handle|.
+  // Only do so for the |entries_to_read| number of entries in |file_ids|
+  // starting at 0-based |offset|.
   GetFileInfo(string storage_handle,
-      uint32 file_id) =>
-    (MtpFileEntry file_entry, bool error);
+      array<uint32> file_ids,
+      uint32 offset,
+      uint32 entries_to_read) =>
+    (array<MtpFileEntry> file_entries, bool error);
 
   // Renames |object_id| to |new_name|.
   RenameObject(string storage_handle,