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}

chrome/browser/media_galleries/chromeos/mtp_device_object_enumerator.cc

@@ -7,7 +7,7 @@
 #include "base/logging.h"
 
 MTPDeviceObjectEnumerator::MTPDeviceObjectEnumerator(
-    const std::vector<MtpFileEntry>& entries)
+    const std::vector<device::mojom::MtpFileEntry>& entries)
     : file_entries_(entries),
       index_(0U),
       is_index_ready_(false) {
@@ -24,25 +24,26 @@ base::FilePath MTPDeviceObjectEnumerator::Next() {
 
   if (!HasMoreEntries())
     return base::FilePath();
-  return base::FilePath(file_entries_[index_].file_name());
+  return base::FilePath(file_entries_[index_].file_name);
 }
 
 int64_t MTPDeviceObjectEnumerator::Size() {
   if (!IsIndexReadyAndInRange())
     return 0;
-  return file_entries_[index_].file_size();
+  return file_entries_[index_].file_size;
 }
 
 bool MTPDeviceObjectEnumerator::IsDirectory() {
   if (!IsIndexReadyAndInRange())
     return false;
-  return file_entries_[index_].file_type() == MtpFileEntry::FILE_TYPE_FOLDER;
+  return file_entries_[index_].file_type ==
+         device::mojom::MtpFileEntry::FileType::FILE_TYPE_FOLDER;
 }
 
 base::Time MTPDeviceObjectEnumerator::LastModifiedTime() {
   if (!IsIndexReadyAndInRange())
     return base::Time();
-  return base::Time::FromTimeT(file_entries_[index_].modification_time());
+  return base::Time::FromTimeT(file_entries_[index_].modification_time);
 }
 
 bool MTPDeviceObjectEnumerator::GetEntryId(uint32_t* entry_id) const {
@@ -50,7 +51,7 @@ bool MTPDeviceObjectEnumerator::GetEntryId(uint32_t* entry_id) const {
   if (!IsIndexReadyAndInRange())
     return false;
 
-  *entry_id = file_entries_[index_].item_id();
+  *entry_id = file_entries_[index_].item_id;
   return true;
 }
 

chrome/browser/media_galleries/chromeos/mtp_device_object_enumerator.h

@@ -13,12 +13,13 @@
 #include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/time/time.h"
-#include "device/media_transfer_protocol/mtp_file_entry.pb.h"
+#include "device/media_transfer_protocol/public/interfaces/mtp_file_entry.mojom.h"
 
 // Used to enumerate top-level files of an media file system.
 class MTPDeviceObjectEnumerator {
  public:
-  explicit MTPDeviceObjectEnumerator(const std::vector<MtpFileEntry>& entries);
+  explicit MTPDeviceObjectEnumerator(
+      const std::vector<device::mojom::MtpFileEntry>& entries);
 
   ~MTPDeviceObjectEnumerator();
 
@@ -41,7 +42,7 @@ class MTPDeviceObjectEnumerator {
   bool IsIndexReadyAndInRange() const;
 
   // List of directory file entries information.
-  const std::vector<MtpFileEntry> file_entries_;
+  const std::vector<device::mojom::MtpFileEntry> file_entries_;
 
   // Index into |file_entries_|.
   // Should only be used when |is_index_ready_| is true.

chrome/browser/media_galleries/chromeos/mtp_device_object_enumerator_unittest.cc

@@ -37,7 +37,7 @@ void TestNextEntryIsEmpty(MTPDeviceObjectEnumerator* enumerator) {
 typedef testing::Test MTPDeviceObjectEnumeratorTest;
 
 TEST_F(MTPDeviceObjectEnumeratorTest, Empty) {
-  std::vector<MtpFileEntry> entries;
+  std::vector<device::mojom::MtpFileEntry> entries;
   MTPDeviceObjectEnumerator enumerator(entries);
   TestEnumeratorIsEmpty(&enumerator);
   TestNextEntryIsEmpty(&enumerator);
@@ -46,15 +46,15 @@ TEST_F(MTPDeviceObjectEnumeratorTest, Empty) {
 }
 
 TEST_F(MTPDeviceObjectEnumeratorTest, Traversal) {
-  std::vector<MtpFileEntry> entries;
+  std::vector<device::mojom::MtpFileEntry> entries;
   for (size_t i = 0; i < arraysize(kTestCases); ++i) {
-    MtpFileEntry entry;
-    entry.set_file_name(kTestCases[i].name);
-    entry.set_file_size(kTestCases[i].size);
-    entry.set_file_type(kTestCases[i].is_directory ?
-        MtpFileEntry::FILE_TYPE_FOLDER :
-        MtpFileEntry::FILE_TYPE_OTHER);
-    entry.set_modification_time(kTestCases[i].modification_time);
+    device::mojom::MtpFileEntry entry;
+    entry.file_name = kTestCases[i].name;
+    entry.file_size = kTestCases[i].size;
+    entry.file_type = kTestCases[i].is_directory
+        ? device::mojom::MtpFileEntry::FileType::FILE_TYPE_FOLDER
+        : device::mojom::MtpFileEntry::FileType::FILE_TYPE_OTHER;
+    entry.modification_time = kTestCases[i].modification_time;
     entries.push_back(entry);
   }
   MTPDeviceObjectEnumerator enumerator(entries);

chrome/browser/media_galleries/chromeos/mtp_device_task_helper.cc

@@ -33,14 +33,16 @@ device::MediaTransferProtocolManager* GetMediaTransferProtocolManager() {
   return StorageMonitor::GetInstance()->media_transfer_protocol_manager();
 }
 
-base::File::Info FileInfoFromMTPFileEntry(const MtpFileEntry& file_entry) {
+base::File::Info FileInfoFromMTPFileEntry(
+    const device::mojom::MtpFileEntry& file_entry) {
   base::File::Info file_entry_info;
-  file_entry_info.size = file_entry.file_size();
+  file_entry_info.size = file_entry.file_size;
   file_entry_info.is_directory =
-      file_entry.file_type() == MtpFileEntry::FILE_TYPE_FOLDER;
+      file_entry.file_type ==
+      device::mojom::MtpFileEntry::FileType::FILE_TYPE_FOLDER;
   file_entry_info.is_symbolic_link = false;
   file_entry_info.last_modified =
-      base::Time::FromTimeT(file_entry.modification_time());
+      base::Time::FromTimeT(file_entry.modification_time);
   file_entry_info.last_accessed = file_entry_info.last_modified;
   file_entry_info.creation_time = base::Time();
   return file_entry_info;
@@ -221,7 +223,7 @@ void MTPDeviceTaskHelper::OnDidOpenStorage(
 void MTPDeviceTaskHelper::OnGetFileInfo(
     const GetFileInfoSuccessCallback& success_callback,
     const ErrorCallback& error_callback,
-    const MtpFileEntry& file_entry,
+    const device::mojom::MtpFileEntry& file_entry,
     bool error) const {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   if (error) {
@@ -254,7 +256,7 @@ void MTPDeviceTaskHelper::OnCreateDirectory(
 void MTPDeviceTaskHelper::OnDidReadDirectory(
     const ReadDirectorySuccessCallback& success_callback,
     const ErrorCallback& error_callback,
-    const std::vector<MtpFileEntry>& file_entries,
+    const std::vector<device::mojom::MtpFileEntry>& file_entries,
     bool has_more,
     bool error) const {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
@@ -282,7 +284,7 @@ void MTPDeviceTaskHelper::OnDidReadDirectory(
 
 void MTPDeviceTaskHelper::OnGetFileInfoToReadBytes(
     const MTPDeviceAsyncDelegate::ReadBytesRequest& request,
-    const MtpFileEntry& file_entry,
+    const device::mojom::MtpFileEntry& file_entry,
     bool error) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(request.buf.get());

chrome/browser/media_galleries/chromeos/mtp_device_task_helper.h

@@ -16,7 +16,7 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/media_galleries/fileapi/mtp_device_async_delegate.h"
-#include "device/media_transfer_protocol/mtp_file_entry.pb.h"
+#include "device/media_transfer_protocol/public/interfaces/mtp_file_entry.mojom.h"
 #include "storage/browser/fileapi/async_file_util.h"
 
 class MTPReadFileWorker;
@@ -169,7 +169,7 @@ class MTPDeviceTaskHelper {
   // caller.
   void OnGetFileInfo(const GetFileInfoSuccessCallback& success_callback,
                      const ErrorCallback& error_callback,
-                     const MtpFileEntry& file_entry,
+                     const device::mojom::MtpFileEntry& file_entry,
                      bool error) const;
 
   // Called when CreateDirectory completes.
@@ -185,16 +185,17 @@ class MTPDeviceTaskHelper {
   //
   // If there is an error, |error| is set to true, |file_entries| is empty
   // and |error_callback| is invoked on the IO thread to notify the caller.
-  void OnDidReadDirectory(const ReadDirectorySuccessCallback& success_callback,
-                          const ErrorCallback& error_callback,
-                          const std::vector<MtpFileEntry>& file_entries,
-                          bool has_more,
-                          bool error) const;
+  void OnDidReadDirectory(
+      const ReadDirectorySuccessCallback& success_callback,
+      const ErrorCallback& error_callback,
+      const std::vector<device::mojom::MtpFileEntry>& file_entries,
+      bool has_more,
+      bool error) const;
 
   // Intermediate step to finish a ReadBytes request.
   void OnGetFileInfoToReadBytes(
       const MTPDeviceAsyncDelegate::ReadBytesRequest& request,
-      const MtpFileEntry& file_entry,
+      const device::mojom::MtpFileEntry& file_entry,
       bool error);
 
   // Query callback for ReadBytes();

components/storage_monitor/test_media_transfer_protocol_manager_chromeos.cc

@@ -63,7 +63,8 @@ void TestMediaTransferProtocolManagerChromeOS::ReadDirectory(
     const uint32_t file_id,
     const size_t max_size,
     const ReadDirectoryCallback& callback) {
-  callback.Run(std::vector<MtpFileEntry>(), false /* no more entries*/,
+  callback.Run(std::vector<device::mojom::MtpFileEntry>(),
+               false /* no more entries*/,
                true /* error */);
 }
 
@@ -80,7 +81,7 @@ void TestMediaTransferProtocolManagerChromeOS::GetFileInfo(
     const std::string& storage_handle,
     uint32_t file_id,
     const GetFileInfoCallback& callback) {
-  callback.Run(MtpFileEntry(), true);
+  callback.Run(device::mojom::MtpFileEntry(), true);
 }
 
 void TestMediaTransferProtocolManagerChromeOS::RenameObject(

device/media_transfer_protocol/BUILD.gn

@@ -34,6 +34,7 @@ static_library("media_transfer_protocol") {
     ":mtp_file_entry_proto",
     ":mtp_storage_info_proto",
     "//base",
+    "//device/media_transfer_protocol/public/interfaces",
   ]
   deps = [
     "//chromeos",

device/media_transfer_protocol/media_transfer_protocol_daemon_client.cc

@@ -13,8 +13,8 @@
 #include "dbus/message.h"
 #include "dbus/object_path.h"
 #include "dbus/object_proxy.h"
-#include "device/media_transfer_protocol/mtp_file_entry.pb.h"
 #include "device/media_transfer_protocol/mtp_storage_info.pb.h"
+#include "device/media_transfer_protocol/mtp_file_entry.pb.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
 namespace device {
@@ -24,6 +24,17 @@ namespace {
 const char kInvalidResponseMsg[] = "Invalid Response: ";
 uint32_t kMaxChunkSize = 1024 * 1024;  // D-Bus has message size limits.
 
+mojom::MtpFileEntry GetMojoMtpFileEntryFromProtobuf(
+    const MtpFileEntry& entry) {
+  return mojom::MtpFileEntry(
+      entry.item_id(),
+      entry.parent_id(),
+      entry.file_name(),
+      entry.file_size(),
+      entry.modification_time(),
+      static_cast<mojom::MtpFileEntry::FileType>(entry.file_type()));
+}
+
 // The MediaTransferProtocolDaemonClient implementation.
 class MediaTransferProtocolDaemonClientImpl
     : public MediaTransferProtocolDaemonClient {
@@ -406,10 +417,13 @@ class MediaTransferProtocolDaemonClientImpl
       return;
     }
 
-    std::vector<MtpFileEntry> file_entries;
+    std::vector<mojom::MtpFileEntry> file_entries;
     file_entries.reserve(entries_protobuf.file_entries_size());
-    for (int i = 0; i < entries_protobuf.file_entries_size(); ++i)
-      file_entries.push_back(entries_protobuf.file_entries(i));
+    for (int i = 0; i < entries_protobuf.file_entries_size(); ++i) {
+      const auto& entry = entries_protobuf.file_entries(i);
+      file_entries.push_back(
+          GetMojoMtpFileEntryFromProtobuf(entry));
+    }
     callback.Run(file_entries);
   }
 

device/media_transfer_protocol/media_transfer_protocol_daemon_client.h

@@ -19,12 +19,12 @@
 #include "base/callback.h"
 #include "base/macros.h"
 #include "build/build_config.h"
+#include "device/media_transfer_protocol/public/interfaces/mtp_file_entry.mojom.h"
 
 #if !defined(OS_CHROMEOS)
 #error "Only used on ChromeOS"
 #endif
 
-class MtpFileEntry;
 class MtpStorageInfo;
 
 namespace dbus {
@@ -69,7 +69,7 @@ class MediaTransferProtocolDaemonClient {
   // A callback to handle the result of GetFileInfo.
   // The argument is a vector of file entries.
   using GetFileInfoCallback =
-      base::Callback<void(const std::vector<MtpFileEntry>& file_entries)>;
+      base::Callback<void(const std::vector<mojom::MtpFileEntry>& file_entries)>;
 
   // A callback to handle the result of ReadFileChunkById.
   // The argument is a string containing the file data.

device/media_transfer_protocol/media_transfer_protocol_manager.cc

@@ -24,7 +24,6 @@
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "dbus/bus.h"
 #include "device/media_transfer_protocol/media_transfer_protocol_daemon_client.h"
-#include "device/media_transfer_protocol/mtp_file_entry.pb.h"
 #include "device/media_transfer_protocol/mtp_storage_info.pb.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
@@ -190,7 +189,7 @@ class MediaTransferProtocolManagerImpl : public MediaTransferProtocolManager {
                      const ReadDirectoryCallback& callback) override {
     DCHECK(thread_checker_.CalledOnValidThread());
     if (!base::ContainsKey(handles_, storage_handle) || !mtp_client_) {
-      callback.Run(std::vector<MtpFileEntry>(),
+      callback.Run(std::vector<mojom::MtpFileEntry>(),
                    false /* no more entries */,
                    true /* error */);
       return;
@@ -230,7 +229,7 @@ class MediaTransferProtocolManagerImpl : public MediaTransferProtocolManager {
                    const GetFileInfoCallback& callback) override {
     DCHECK(thread_checker_.CalledOnValidThread());
     if (!base::ContainsKey(handles_, storage_handle) || !mtp_client_) {
-      callback.Run(MtpFileEntry(), true);
+      callback.Run(mojom::MtpFileEntry(), true);
       return;
     }
     std::vector<uint32_t> file_ids;
@@ -450,7 +449,7 @@ class MediaTransferProtocolManagerImpl : public MediaTransferProtocolManager {
 
     if (file_ids.empty()) {
       OnGotDirectoryEntries(storage_handle, file_ids, kInitialOffset, max_size,
-                            file_ids, std::vector<MtpFileEntry>());
+                            file_ids, std::vector<mojom::MtpFileEntry>());
       return;
     }
 
@@ -470,12 +469,13 @@ class MediaTransferProtocolManagerImpl : public MediaTransferProtocolManager {
                    weak_ptr_factory_.GetWeakPtr()));
   }
 
-  void OnGotDirectoryEntries(const std::string& storage_handle,
-                             const std::vector<uint32_t>& file_ids,
-                             const size_t offset,
-                             const size_t max_size,
-                             const std::vector<uint32_t>& sorted_file_ids,
-                             const std::vector<MtpFileEntry>& file_entries) {
+  void OnGotDirectoryEntries(
+      const std::string& storage_handle,
+      const std::vector<uint32_t>& file_ids,
+      const size_t offset,
+      const size_t max_size,
+      const std::vector<uint32_t>& sorted_file_ids,
+      const std::vector<mojom::MtpFileEntry>& file_entries) {
     DCHECK(thread_checker_.CalledOnValidThread());
     DCHECK_EQ(file_ids.size(), sorted_file_ids.size());
 
@@ -483,7 +483,7 @@ class MediaTransferProtocolManagerImpl : public MediaTransferProtocolManager {
     // subset of the requested file ids.
     for (const auto& entry : file_entries) {
       std::vector<uint32_t>::const_iterator it = std::lower_bound(
-          sorted_file_ids.begin(), sorted_file_ids.end(), entry.item_id());
+          sorted_file_ids.begin(), sorted_file_ids.end(), entry.item_id);
       if (it == sorted_file_ids.end()) {
         OnReadDirectoryError();
         return;
@@ -518,7 +518,7 @@ class MediaTransferProtocolManagerImpl : public MediaTransferProtocolManager {
 
   void OnReadDirectoryError() {
     DCHECK(thread_checker_.CalledOnValidThread());
-    read_directory_callbacks_.front().Run(std::vector<MtpFileEntry>(),
+    read_directory_callbacks_.front().Run(std::vector<mojom::MtpFileEntry>(),
                                           false /* no more entries */,
                                           true /* error */);
     read_directory_callbacks_.pop();
@@ -536,7 +536,7 @@ class MediaTransferProtocolManagerImpl : public MediaTransferProtocolManager {
     read_file_callbacks_.pop();
   }
 
-  void OnGetFileInfo(const std::vector<MtpFileEntry>& entries) {
+  void OnGetFileInfo(const std::vector<mojom::MtpFileEntry>& entries) {
     DCHECK(thread_checker_.CalledOnValidThread());
     if (entries.size() == 1) {
       get_file_info_callbacks_.front().Run(entries[0], false /* no error */);
@@ -548,7 +548,7 @@ class MediaTransferProtocolManagerImpl : public MediaTransferProtocolManager {
 
   void OnGetFileInfoError() {
     DCHECK(thread_checker_.CalledOnValidThread());
-    get_file_info_callbacks_.front().Run(MtpFileEntry(), true);
+    get_file_info_callbacks_.front().Run(mojom::MtpFileEntry(), true);
     get_file_info_callbacks_.pop();
   }
 

device/media_transfer_protocol/media_transfer_protocol_manager.h

@@ -15,12 +15,12 @@
 #include "base/callback.h"
 #include "base/memory/ref_counted.h"
 #include "build/build_config.h"
+#include "device/media_transfer_protocol/public/interfaces/mtp_file_entry.mojom.h"
 
 #if !defined(OS_CHROMEOS)
 #error "Only used on ChromeOS"
 #endif
 
-class MtpFileEntry;
 class MtpStorageInfo;
 
 namespace device {
@@ -55,7 +55,7 @@ class MediaTransferProtocolManager {
   // The second argument is true if there are more file entries.
   // The third argument is true if there was an error.
   using ReadDirectoryCallback =
-      base::Callback<void(const std::vector<MtpFileEntry>& file_entries,
+      base::Callback<void(const std::vector<mojom::MtpFileEntry>& file_entries,
                           bool has_more,
                           bool error)>;
 
@@ -69,7 +69,7 @@ class MediaTransferProtocolManager {
   // The first argument is a file entry.
   // The second argument is true if there was an error.
   using GetFileInfoCallback =
-      base::Callback<void(const MtpFileEntry& file_entry, bool error)>;
+      base::Callback<void(const mojom::MtpFileEntry& file_entry, bool error)>;
 
   // A callback to handle the result of RenameObject.
   // The first argument is true if there was an error.

device/media_transfer_protocol/public/interfaces/BUILD.gn

+# Copyright 2017 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import("//mojo/public/tools/bindings/mojom.gni")
+
+mojom("interfaces") {
+  sources = [
+    "mtp_file_entry.mojom",
+  ]
+}

device/media_transfer_protocol/public/interfaces/OWNERS

+per-file *.mojom=set noparent
+per-file *.mojom=file://ipc/SECURITY_OWNERS

device/media_transfer_protocol/public/interfaces/mtp_file_entry.mojom

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+module device.mojom;
+
+// This is a mojo counterpart of the MtpFileEntry protobuf message from
+// //src/third_party/cros_system_api/dbus/mtp_file_entry.proto
+// See discussion on https://crbug.com/769630.
+struct MtpFileEntry {
+  enum FileType {
+    FILE_TYPE_FOLDER = 0,
+    FILE_TYPE_JPEG = 14,
+    FILE_TYPE_JFIF = 15,
+    FILE_TYPE_TIFF = 16,
+    FILE_TYPE_BMP = 17,
+    FILE_TYPE_GIF = 18,
+    FILE_TYPE_PICT = 19,
+    FILE_TYPE_PNG = 20,
+    FILE_TYPE_WINDOWSIMAGEFORMAT = 25,
+    FILE_TYPE_JP2 = 40,
+    FILE_TYPE_JPX = 41,
+    FILE_TYPE_UNKNOWN = 44,
+    FILE_TYPE_OTHER = 9999
+  };
+
+  uint32 item_id = 0xFFFFFFFF;
+  uint32 parent_id = 0XFFFFFFFF;
+  string file_name;
+  uint64 file_size = 0;
+  int64 modification_time = 0;
+  FileType file_type = FileType.FILE_TYPE_UNKNOWN;
+};