[CrosDisksClient] Determine device type from DeviceMediaType property sent by cros-disks.

chrome/browser/chromeos/dbus/cros_disks_client.cc

Index: chrome/browser/chromeos/dbus/cros_disks_client.cc
diff --git a/chrome/browser/chromeos/dbus/cros_disks_client.cc b/chrome/browser/chromeos/dbus/cros_disks_client.cc
index 96e5ddebd45ec60c57fcf04897925c878b56aefe..3cd011c55d28fcb0e62607bf15ebb9b024fcc742 100644
--- a/chrome/browser/chromeos/dbus/cros_disks_client.cc
+++ b/chrome/browser/chromeos/dbus/cros_disks_client.cc
@@ -27,13 +27,47 @@ const char* kDefaultUnmountOptions[] = {
   "force",
 };
 
-// Returns the device type from the given arguments.
-DeviceType GetDeviceType(bool is_optical, bool is_rotational) {
-  if (is_optical)
-    return OPTICAL;
-  if (is_rotational)
-    return HDD;
-  return FLASH;
+// Translates enum used in cros-disks to enum used in Chrome.
+// Note that we could just do static_cast, but this is less sensitive to
+// changes in cros-disks.
+DeviceType DeviceMediaTypeToDeviceType(cros_disks::DeviceMediaType type) {
+  switch (type) {
+    case(cros_disks::DEVICE_MEDIA_UNKNOWN):
+      return DEVICE_TYPE_UNKNOWN;
+    case(cros_disks::DEVICE_MEDIA_USB):
+      return DEVICE_TYPE_USB;
+    case(cros_disks::DEVICE_MEDIA_SD):
+      return DEVICE_TYPE_SD;
+    case(cros_disks::DEVICE_MEDIA_OPTICAL_DISC):
+      return DEVICE_TYPE_OPTICAL_DISC;
+    case(cros_disks::DEVICE_MEDIA_MOBILE):
+      return DEVICE_TYPE_MOBILE;
+    default:
+      return DEVICE_TYPE_UNKNOWN;
+  }
+}
+
+// Checks if retrieved media type is in boundaries of DeviceMediaType.
+bool IsValidMediaType(uint32 type) {
+  return type < static_cast<uint32>(cros_disks::DEVICE_MEDIA_NUM_VALUES) &&
+         type >= 0;

[hashimoto, 2012/01/24 05:28:06]

Since |type| is unsigned, type >= 0 does nothing here.
It may be removed.

[tbarzic, 2012/01/24 19:47:33]

Done.

+}
+
+// Pops unit32 value and converts it to DeviceMediaType when |reader| is not
+// NULL. Returns true iff value is popped and converted.
+bool MaybePopDeviceType(dbus::MessageReader* reader, DeviceType* type) {

[hashimoto, 2012/01/24 05:28:06]

Having MaybePopUint32 and doing something like 'device_type_ =
DeviceMediaTypeToDeviceType(media_type_uint32);' in InitializeFromResponse will
be more consistent with other MaybePopXX functions.

[tbarzic, 2012/01/24 19:47:33]

Done.

+  if (!reader)
+    return false;
+
+  uint32 media_type_uint32 = 0;
+  bool success = reader->PopUint32(&media_type_uint32);
+
+  if (!success || !IsValidMediaType(media_type_uint32))
+    return false;
+
+  *type = DeviceMediaTypeToDeviceType(
+              cros_disks::DeviceMediaType(media_type_uint32));
+  return success;
 }
 
 // Pops a bool value when |reader| is not NULL.
@@ -375,7 +409,7 @@ DiskInfo::DiskInfo(const std::string& device_path, dbus::Response* response)
       is_drive_(false),
       has_media_(false),
       on_boot_device_(false),
-      device_type_(UNDEFINED),
+      device_type_(DEVICE_TYPE_UNKNOWN),
       total_size_in_bytes_(0),
       is_read_only_(false),
       is_hidden_(true) {
@@ -501,19 +535,12 @@ void DiskInfo::InitializeFromResponse(dbus::Response* response) {
   MaybePopString(properties[cros_disks::kDriveModel], &drive_model_);
   MaybePopString(properties[cros_disks::kIdLabel], &label_);
   MaybePopUint64(properties[cros_disks::kDeviceSize], &total_size_in_bytes_);
+  MaybePopDeviceType(properties[cros_disks::kDeviceMediaType], &device_type_);
 
   std::vector<std::string> mount_paths;
   if (MaybePopArrayOfStrings(properties[cros_disks::kDeviceMountPaths],
                              &mount_paths) && !mount_paths.empty())
     mount_path_ = mount_paths[0];
-
-  bool is_rotational = false;
-  bool is_optical = false;
-  if (MaybePopBool(properties[cros_disks::kDriveIsRotational],
-                   &is_rotational) &&
-      MaybePopBool(properties[cros_disks::kDeviceIsOpticalDisc],
-                   &is_optical))
-    device_type_ = GetDeviceType(is_optical, is_rotational);
 }
 
 ////////////////////////////////////////////////////////////////////////////////