[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..ab2038d96d9fa86506218ee5b43e0f7dff95aaae 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;
+// Translate enum used in cros-disks to enum used in Chrome.

[satorux1, 2012/01/23 22:18:55]

"Translates" per C++ style guide.

[tbarzic, 2012/01/23 22:42:40]

Done.

+// 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;

[satorux1, 2012/01/23 22:18:55]

DEVICE_TYPE_OPTICAL_DISC would be better for consistency.

[tbarzic, 2012/01/23 22:42:40]

Done.

+    case(cros_disks::DEVICE_MEDIA_MOBILE):
+      return DEVICE_TYPE_MOBILE;
+    default:
+      return DEVICE_TYPE_UNKNOWN;
+  }
+}
+
+// Check if retrieved media type is in boundaries of DeviceMediaType.

[satorux1, 2012/01/23 22:18:55]

Checks

[tbarzic, 2012/01/23 22:42:40]

Done.

+bool MediaTypeInBoundaries(uint32 type) {

[satorux1, 2012/01/23 22:18:55]

maybe IsValidMediaType() would be a bit better name?

[tbarzic, 2012/01/23 22:42:40]

Done.

+  return type < static_cast<uint32>(cros_disks::DEVICE_MEDIA_NUM_VALUES) &&
+         type >= 0;
+}
+
+// Pop unit32 value and converts it to DeviceMediaType when |reader| is not

[satorux1, 2012/01/23 22:18:55]

Pops

[tbarzic, 2012/01/23 22:42:40]

Done.

+// NULL. Returns true iff value is popped and converted.
+bool MaybePopDeviceType(dbus::MessageReader* reader, DeviceType* type) {
+  if (!reader)
+    return false;
+
+  uint32 media_type_uint32;

[satorux1, 2012/01/23 22:18:55]

uint32 media_type_uint32 = 0;

just in case.

[tbarzic, 2012/01/23 22:42:40]

Done.

+  bool success = reader->PopUint32(&media_type_uint32);
+
+  if (!success || !MediaTypeInBoundaries(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);
 }
 
 ////////////////////////////////////////////////////////////////////////////////