CrOS: Convert MediaTransferProtocolDaemonClient to use protobufs.

BUG=chromium-os:29557

Review URL: https://chromiumcodereview.appspot.com/10913048

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@156468 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 59 insertions, 350 deletions

diff --git a/chromeos/chromeos.gyp b/chromeos/chromeos.gyp
index fbba17c..48014f4 100644
--- a/chromeos/chromeos.gyp
+++ b/chromeos/chromeos.gyp
@@ -16,6 +16,8 @@
         '../dbus/dbus.gyp:dbus',
         '../net/net.gyp:net',
         '../third_party/libxml/libxml.gyp:libxml',
+        'mtp_file_entry_proto',
+        'mtp_storage_info_proto',
         'power_state_control_proto',
         'power_supply_properties_proto',
         'video_activity_update_proto',
@@ -138,6 +140,7 @@
         '../build/linux/system.gyp:dbus',
         '../testing/gmock.gyp:gmock',
         'chromeos',
+        'mtp_file_entry_proto',
       ],
       'sources': [
         'cryptohome/mock_async_method_caller.cc',
@@ -278,6 +281,37 @@
       ],
     },
     {
+      # Protobuf compiler / generator for the MtpFileEntry and MtpFileEntries
+      # protocol buffers.
+      'target_name': 'mtp_file_entry_proto',
+      'type': 'static_library',
+      'sources': [
+        '../third_party/cros_system_api/dbus/mtp_file_entry.proto',
+      ],
+      'variables': {
+        'proto_in_dir': '../third_party/cros_system_api/dbus',
+        'proto_out_dir': 'chromeos/dbus',
+        'cc_generator_options': 'dllexport_decl=CHROMEOS_EXPORT:',
+        'cc_include': 'chromeos/chromeos_export.h',
+      },
+      'includes': ['../build/protoc.gypi'],
+    },
+    {
+      # Protobuf compiler / generator for the MtpStorageInfo protocol buffers.
+      'target_name': 'mtp_storage_info_proto',
+      'type': 'static_library',
+      'sources': [
+        '../third_party/cros_system_api/dbus/mtp_storage_info.proto',
+      ],
+      'variables': {
+        'proto_in_dir': '../third_party/cros_system_api/dbus',
+        'proto_out_dir': 'chromeos/dbus',
+        'cc_generator_options': 'dllexport_decl=CHROMEOS_EXPORT:',
+        'cc_include': 'chromeos/chromeos_export.h',
+      },
+      'includes': ['../build/protoc.gypi'],
+    },
+    {
       # Protobuf compiler / generator for the PowerSupplyProperties protocol
       # buffer.
       'target_name': 'power_state_control_proto',
@@ -286,7 +320,7 @@
         '../third_party/cros_system_api/dbus/power_state_control.proto',
       ],
       'variables': {
-        'proto_in_dir': '../third_party/cros_system_api/dbus/',
+        'proto_in_dir': '../third_party/cros_system_api/dbus',
         'proto_out_dir': 'chromeos/dbus',
       },
       'includes': ['../build/protoc.gypi'],
@@ -300,7 +334,7 @@
         '../third_party/cros_system_api/dbus/power_supply_properties.proto',
       ],
       'variables': {
-        'proto_in_dir': '../third_party/cros_system_api/dbus/',
+        'proto_in_dir': '../third_party/cros_system_api/dbus',
         'proto_out_dir': 'chromeos/dbus',
       },
       'includes': ['../build/protoc.gypi'],
diff --git a/chromeos/dbus/media_transfer_protocol_daemon_client.cc b/chromeos/dbus/media_transfer_protocol_daemon_client.cc
index 954a59a..9ed81a0 100644
--- a/chromeos/dbus/media_transfer_protocol_daemon_client.cc
+++ b/chromeos/dbus/media_transfer_protocol_daemon_client.cc
@@ -4,11 +4,9 @@
 
 #include "chromeos/dbus/media_transfer_protocol_daemon_client.h"
 
-#include <map>
-
 #include "base/bind.h"
-#include "base/stl_util.h"
-#include "base/stringprintf.h"
+#include "chromeos/dbus/mtp_file_entry.pb.h"
+#include "chromeos/dbus/mtp_storage_info.pb.h"
 #include "dbus/bus.h"
 #include "dbus/message.h"
 #include "dbus/object_path.h"
@@ -19,46 +17,6 @@ namespace chromeos {
 
 namespace {
 
-// Pops a string value when |reader| is not NULL.
-// Returns true when a value is popped, false otherwise.
-bool MaybePopString(dbus::MessageReader* reader, std::string* value) {
-  if (!reader)
-    return false;
-  return reader->PopString(value);
-}
-
-// Pops a uint16 value when |reader| is not NULL.
-// Returns true when a value is popped, false otherwise.
-bool MaybePopUint16(dbus::MessageReader* reader, uint16* value) {
-  if (!reader)
-    return false;
-  return reader->PopUint16(value);
-}
-
-// Pops a uint32 value when |reader| is not NULL.
-// Returns true when a value is popped, false otherwise.
-bool MaybePopUint32(dbus::MessageReader* reader, uint32* value) {
-  if (!reader)
-    return false;
-  return reader->PopUint32(value);
-}
-
-// Pops a uint64 value when |reader| is not NULL.
-// Returns true when a value is popped, false otherwise.
-bool MaybePopUint64(dbus::MessageReader* reader, uint64* value) {
-  if (!reader)
-    return false;
-  return reader->PopUint64(value);
-}
-
-// Pops a int64 value when |reader| is not NULL.
-// Returns true when a value is popped, false otherwise.
-bool MaybePopInt64(dbus::MessageReader* reader, int64* value) {
-  if (!reader)
-    return false;
-  return reader->PopInt64(value);
-}
-
 // The MediaTransferProtocolDaemonClient implementation.
 class MediaTransferProtocolDaemonClientImpl
     : public MediaTransferProtocolDaemonClient {
@@ -296,8 +254,11 @@ class MediaTransferProtocolDaemonClientImpl
       error_callback.Run();
       return;
     }
-    StorageInfo storage_info(storage_name, response);
-    callback.Run(storage_info);
+
+    dbus::MessageReader reader(response);
+    MtpStorageInfo protobuf;
+    reader.PopArrayOfBytesAsProto(&protobuf);
+    callback.Run(protobuf);
   }
 
   // Handles the result of OpenStorage and calls |callback| or |error_callback|.
@@ -340,18 +301,12 @@ class MediaTransferProtocolDaemonClientImpl
       return;
     }
 
-    std::vector<FileEntry> file_entries;
-    dbus::MessageReader response_reader(response);
-    dbus::MessageReader array_reader(response);
-    if (!response_reader.PopArray(&array_reader)) {
-      LOG(ERROR) << "Invalid response: " << response->ToString();
-      error_callback.Run();
-      return;
-    }
-    while (array_reader.HasMoreData()) {
-      FileEntry entry(response);
-      file_entries.push_back(entry);
-    }
+    std::vector<MtpFileEntry> file_entries;
+    dbus::MessageReader reader(response);
+    MtpFileEntries entries_protobuf;
+    reader.PopArrayOfBytesAsProto(&entries_protobuf);
+    for (int i = 0; i < entries_protobuf.file_entries_size(); ++i)
+      file_entries.push_back(entries_protobuf.file_entries(i));
     callback.Run(file_entries);
   }
 
@@ -386,8 +341,10 @@ class MediaTransferProtocolDaemonClientImpl
       return;
     }
 
-    FileEntry file_entry(response);
-    callback.Run(file_entry);
+    dbus::MessageReader reader(response);
+    MtpFileEntry protobuf;
+    reader.PopArrayOfBytesAsProto(&protobuf);
+    callback.Run(protobuf);
   }
 
   // Handles MTPStorageAttached/Dettached signals and calls |handler|.
@@ -480,159 +437,6 @@ class MediaTransferProtocolDaemonClientStubImpl
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
-// StorageInfo
-
-StorageInfo::StorageInfo()
-    : vendor_id_(0),
-      product_id_(0),
-      device_flags_(0),
-      storage_type_(0),
-      filesystem_type_(0),
-      access_capability_(0),
-      max_capacity_(0),
-      free_space_in_bytes_(0),
-      free_space_in_objects_(0) {
-}
-
-StorageInfo::StorageInfo(const std::string& storage_name,
-                         dbus::Response* response)
-    : vendor_id_(0),
-      product_id_(0),
-      device_flags_(0),
-      storage_name_(storage_name),
-      storage_type_(0),
-      filesystem_type_(0),
-      access_capability_(0),
-      max_capacity_(0),
-      free_space_in_bytes_(0),
-      free_space_in_objects_(0) {
-  InitializeFromResponse(response);
-}
-
-StorageInfo::~StorageInfo() {
-}
-
-// Initializes |this| from |response| given by the mtpd service.
-void StorageInfo::InitializeFromResponse(dbus::Response* response) {
-  dbus::MessageReader response_reader(response);
-  dbus::MessageReader array_reader(response);
-  if (!response_reader.PopArray(&array_reader)) {
-    LOG(ERROR) << "Invalid response: " << response->ToString();
-    return;
-  }
-  // TODO(thestig): Rework this code using Protocol Buffers. crosbug.com/22626
-  typedef std::map<std::string, dbus::MessageReader*> PropertiesMap;
-  PropertiesMap properties;
-  STLValueDeleter<PropertiesMap> properties_value_deleter(&properties);
-  while (array_reader.HasMoreData()) {
-    dbus::MessageReader* value_reader = new dbus::MessageReader(response);
-    dbus::MessageReader dict_entry_reader(response);
-    std::string key;
-    if (!array_reader.PopDictEntry(&dict_entry_reader) ||
-        !dict_entry_reader.PopString(&key) ||
-        !dict_entry_reader.PopVariant(value_reader)) {
-      LOG(ERROR) << "Invalid response: " << response->ToString();
-      return;
-    }
-    properties[key] = value_reader;
-  }
-  // TODO(thestig) Add enums for fields below as appropriate.
-  MaybePopString(properties[mtpd::kVendor], &vendor_);
-  MaybePopString(properties[mtpd::kProduct], &product_);
-  MaybePopString(properties[mtpd::kStorageDescription], &storage_description_);
-  MaybePopString(properties[mtpd::kVolumeIdentifier], &volume_identifier_);
-  MaybePopUint16(properties[mtpd::kVendorId], &vendor_id_);
-  MaybePopUint16(properties[mtpd::kProductId], &product_id_);
-  MaybePopUint16(properties[mtpd::kStorageType], &storage_type_);
-  MaybePopUint16(properties[mtpd::kFilesystemType], &filesystem_type_);
-  MaybePopUint16(properties[mtpd::kAccessCapability], &access_capability_);
-  MaybePopUint32(properties[mtpd::kDeviceFlags], &device_flags_);
-  MaybePopUint64(properties[mtpd::kMaxCapacity], &max_capacity_);
-  MaybePopUint64(properties[mtpd::kFreeSpaceInBytes], &free_space_in_bytes_);
-  MaybePopUint64(properties[mtpd::kFreeSpaceInObjects],
-                 &free_space_in_objects_);
-}
-
-////////////////////////////////////////////////////////////////////////////////
-// FileEntry
-
-FileEntry::FileEntry()
-    : item_id_(0),
-      parent_id_(0),
-      file_size_(0),
-      file_type_(FILE_TYPE_UNKNOWN) {
-}
-
-FileEntry::FileEntry(dbus::Response* response)
-    : item_id_(0),
-      parent_id_(0),
-      file_size_(0),
-      file_type_(FILE_TYPE_UNKNOWN) {
-  InitializeFromResponse(response);
-}
-
-FileEntry::~FileEntry() {
-}
-
-// Initializes |this| from |response| given by the mtpd service.
-void FileEntry::InitializeFromResponse(dbus::Response* response) {
-  dbus::MessageReader response_reader(response);
-  dbus::MessageReader array_reader(response);
-  if (!response_reader.PopArray(&array_reader)) {
-    LOG(ERROR) << "Invalid response: " << response->ToString();
-    return;
-  }
-  // TODO(thestig): Rework this code using Protocol Buffers. crosbug.com/22626
-  typedef std::map<std::string, dbus::MessageReader*> PropertiesMap;
-  PropertiesMap properties;
-  STLValueDeleter<PropertiesMap> properties_value_deleter(&properties);
-  while (array_reader.HasMoreData()) {
-    dbus::MessageReader* value_reader = new dbus::MessageReader(response);
-    dbus::MessageReader dict_entry_reader(response);
-    std::string key;
-    if (!array_reader.PopDictEntry(&dict_entry_reader) ||
-        !dict_entry_reader.PopString(&key) ||
-        !dict_entry_reader.PopVariant(value_reader)) {
-      LOG(ERROR) << "Invalid response: " << response->ToString();
-      return;
-    }
-    properties[key] = value_reader;
-  }
-
-  MaybePopString(properties[mtpd::kFileName], &file_name_);
-  MaybePopUint32(properties[mtpd::kItemId], &item_id_);
-  MaybePopUint32(properties[mtpd::kParentId], &parent_id_);
-  MaybePopUint64(properties[mtpd::kFileSize], &file_size_);
-
-  int64 modification_date = -1;
-  if (MaybePopInt64(properties[mtpd::kModificationDate], &modification_date))
-    modification_date_ = base::Time::FromTimeT(modification_date);
-
-  uint16 file_type = FILE_TYPE_OTHER;
-  if (MaybePopUint16(properties[mtpd::kFileType], &file_type)) {
-    switch (file_type) {
-      case FILE_TYPE_FOLDER:
-      case FILE_TYPE_JPEG:
-      case FILE_TYPE_JFIF:
-      case FILE_TYPE_TIFF:
-      case FILE_TYPE_BMP:
-      case FILE_TYPE_GIF:
-      case FILE_TYPE_PICT:
-      case FILE_TYPE_PNG:
-      case FILE_TYPE_WINDOWSIMAGEFORMAT:
-      case FILE_TYPE_JP2:
-      case FILE_TYPE_JPX:
-      case FILE_TYPE_UNKNOWN:
-        file_type_ = static_cast<FileType>(file_type);
-        break;
-      default:
-        file_type_ = FILE_TYPE_OTHER;
-        break;
-    }
-  }
-}
-
-////////////////////////////////////////////////////////////////////////////////
 // MediaTransferProtocolDaemonClient
 
 MediaTransferProtocolDaemonClient::MediaTransferProtocolDaemonClient() {}
diff --git a/chromeos/dbus/media_transfer_protocol_daemon_client.h b/chromeos/dbus/media_transfer_protocol_daemon_client.h
index a577937..2417912 100644
--- a/chromeos/dbus/media_transfer_protocol_daemon_client.h
+++ b/chromeos/dbus/media_transfer_protocol_daemon_client.h
@@ -18,9 +18,11 @@
 #include "chromeos/chromeos_export.h"
 #include "chromeos/dbus/dbus_client_implementation_type.h"
 
+class MtpFileEntry;
+class MtpStorageInfo;
+
 namespace dbus {
 class Bus;
-class Response;
 }
 
 namespace chromeos {
@@ -30,137 +32,6 @@ enum OpenStorageMode {
   OPEN_STORAGE_MODE_READ_ONLY,
 };
 
-// Values match libmtp values unless noted below.
-// TODO(thestig) See if we can do better than this.
-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,
-  // Truly unknown file type.
-  FILE_TYPE_UNKNOWN = 44,
-  // There's more file types to map to, but right now they are not interesting.
-  // Just assign a dummy value for now.
-  FILE_TYPE_OTHER = 9999
-};
-
-// A class to represent information about a storage sent from mtpd.
-class CHROMEOS_EXPORT StorageInfo {
- public:
-  StorageInfo();
-  StorageInfo(const std::string& storage_name, dbus::Response* response);
-  ~StorageInfo();
-
-  // Storage name. (e.g. usb:1,5:65537)
-  const std::string& storage_name() const { return storage_name_; }
-
-  // Vendor. (e.g. Kodak)
-  const std::string& vendor() const { return vendor_; }
-
-  // Vendor ID. (e.g. 0x040a)
-  uint16 vendor_id() const { return vendor_id_; }
-
-  // Product. (e.g. DC4800)
-  const std::string& product() const { return product_; }
-
-  // Vendor ID. (e.g. 0x0160)
-  uint16 product_id() const { return product_id_; }
-
-  // Device flags as defined by libmtp.
-  uint32 device_flags() const { return device_flags_; }
-
-  // Storage type as defined in libmtp. (e.g. PTP_ST_FixedROM)
-  uint16 storage_type() const { return storage_type_; }
-
-  // File system type as defined in libmtp. (e.g. PTP_FST_DCF)
-  uint16 filesystem_type() const { return filesystem_type_; }
-
-  // Access capability as defined in libmtp. (e.g. PTP_AC_ReadWrite)
-  uint16 access_capability() const { return access_capability_; }
-
-  // Max capacity in bytes.
-  uint64 max_capacity() const { return max_capacity_; }
-
-  // Free space in byte.
-  uint64 free_space_in_bytes() const { return free_space_in_bytes_; }
-
-  // Free space in number of objects.
-  uint64 free_space_in_objects() const { return free_space_in_objects_; }
-
-  // Storage description. (e.g. internal memory)
-  const std::string& storage_description() const {
-    return storage_description_;
-  }
-
-  // Volume identifier. (e.g. the serial number, should be unique)
-  const std::string& volume_identifier() const { return volume_identifier_; }
-
- private:
-  void InitializeFromResponse(dbus::Response* response);
-
-  // Device info. (A device can have multiple storages)
-  std::string vendor_;
-  uint16 vendor_id_;
-  std::string product_;
-  uint16 product_id_;
-  uint32 device_flags_;
-
-  // Storage info.
-  std::string storage_name_;
-  uint16 storage_type_;
-  uint16 filesystem_type_;
-  uint16 access_capability_;
-  uint64 max_capacity_;
-  uint64 free_space_in_bytes_;
-  uint64 free_space_in_objects_;
-  std::string storage_description_;
-  std::string volume_identifier_;
-};
-
-// A class to represent information about a file entry sent from mtpd.
-class CHROMEOS_EXPORT FileEntry {
- public:
-  FileEntry();
-  explicit FileEntry(dbus::Response* response);
-  ~FileEntry();
-
-  // ID for the file.
-  uint32 item_id() const { return item_id_; }
-
-  // ID for the file's parent.
-  uint32 parent_id() const { return parent_id_; }
-
-  // Name of the file.
-  const std::string& file_name() const { return file_name_; }
-
-  // Size of the file.
-  uint64 file_size() const { return file_size_; }
-
-  // Modification time of the file.
-  base::Time modification_date() const { return modification_date_; }
-
-  // File type.
-  FileType file_type() const { return file_type_; }
-
- private:
-  void InitializeFromResponse(dbus::Response* response);
-
-  // Storage info.
-  uint32 item_id_;
-  uint32 parent_id_;
-  std::string file_name_;
-  uint64 file_size_;
-  base::Time modification_date_;
-  FileType file_type_;
-};
-
 // A class to make the actual DBus calls for mtpd service.
 // This class only makes calls, result/error handling should be done
 // by callbacks.
@@ -176,7 +47,7 @@ class CHROMEOS_EXPORT MediaTransferProtocolDaemonClient {
 
   // A callback to handle the result of GetStorageInfo.
   // The argument is the information about the specified storage.
-  typedef base::Callback<void(const StorageInfo& storage_info)
+  typedef base::Callback<void(const MtpStorageInfo& storage_info)
                          > GetStorageInfoCallback;
 
   // A callback to handle the result of OpenStorage.
@@ -188,7 +59,7 @@ class CHROMEOS_EXPORT MediaTransferProtocolDaemonClient {
 
   // A callback to handle the result of ReadDirectoryByPath/Id.
   // The argument is a vector of file entries.
-  typedef base::Callback<void(const std::vector<FileEntry>& file_entries)
+  typedef base::Callback<void(const std::vector<MtpFileEntry>& file_entries)
                          > ReadDirectoryCallback;
 
   // A callback to handle the result of ReadFileByPath/Id.
@@ -198,7 +69,7 @@ class CHROMEOS_EXPORT MediaTransferProtocolDaemonClient {
 
   // A callback to handle the result of GetFileInfoByPath/Id.
   // The argument is a file entry.
-  typedef base::Callback<void(const FileEntry& file_entry)
+  typedef base::Callback<void(const MtpFileEntry& file_entry)
                          > GetFileInfoCallback;
 
   // A callback to handle storage attach/detach events.