[Media Gallery][Linux] Improve device media gallery names.

chrome/browser/system_monitor/removable_device_notifications_linux.cc

Index: chrome/browser/system_monitor/removable_device_notifications_linux.cc
diff --git a/chrome/browser/system_monitor/removable_device_notifications_linux.cc b/chrome/browser/system_monitor/removable_device_notifications_linux.cc
index db68742d6ebe584cd6091975a53a0c0a4abe27ea..be153081354cfed15af317356553dde57868b382 100644
--- a/chrome/browser/system_monitor/removable_device_notifications_linux.cc
+++ b/chrome/browser/system_monitor/removable_device_notifications_linux.cc
@@ -24,10 +24,10 @@
 #include "chrome/browser/system_monitor/media_device_notifications_utils.h"
 #include "chrome/browser/system_monitor/removable_device_constants.h"
 #include "chrome/browser/system_monitor/media_storage_util.h"
+#include "ui/base/text/bytes_formatting.h"
 
 namespace chrome {
 
-using base::SystemMonitor;
 using content::BrowserThread;
 
 namespace {
@@ -51,15 +51,14 @@ const char* const kKnownFileSystems[] = {
 
 // udev device property constants.
 const char kBlockSubsystemKey[] = "block";
-const char kDevName[] = "DEVNAME";
 const char kDiskDeviceTypeKey[] = "disk";
 const char kFsUUID[] = "ID_FS_UUID";
 const char kLabel[] = "ID_FS_LABEL";
 const char kModel[] = "ID_MODEL";
 const char kModelID[] = "ID_MODEL_ID";
 const char kRemovableSysAttr[] = "removable";
-const char kSerial[] = "ID_SERIAL";
 const char kSerialShort[] = "ID_SERIAL_SHORT";
+const char kSizeSysAttr[] = "size";
 const char kVendor[] = "ID_VENDOR";
 const char kVendorID[] = "ID_VENDOR_ID";
 
@@ -148,12 +147,44 @@ void RecordGetDeviceInfoResult(bool result) {
   UMA_HISTOGRAM_BOOLEAN("MediaDeviceNotification.UdevRequestSuccess", result);
 }
 
+// Returns the storage size information of the device specified by
+// |device_path|. If the requested information is unavailable, returns an
+// empty string.
+string16 GetSizeInfo(const FilePath& device_path, struct udev_device* device) {
+  // sysfs provides the device size value, which is the actual size in bytes
+  // divided by 512.
+  const std::string partition_size = udev_device_get_sysattr_value(
+      device, kSizeSysAttr);
+  uint64 total_size_in_bytes = 0;
+  if (partition_size.empty() ||
+      !base::StringToUint64(partition_size, &total_size_in_bytes))
+    return string16();
+  return ui::FormatBytes(total_size_in_bytes * 512);
+}
+
+// Constructs the device name from the device properties. If the device details
+// are unavailable, returns an empty string.
+string16 GetDeviceName(struct udev_device* device) {
+  std::string device_label = GetUdevDevicePropertyValue(device, kLabel);
+  if (!device_label.empty() && IsStringUTF8(device_label))
+    return UTF8ToUTF16(device_label);
+
+  device_label = GetUdevDevicePropertyValue(device, kFsUUID);

[Lei Zhang, 2012/11/12 08:01:39]

What are the chances that a partition will not have a UUID?

[Lei Zhang, 2012/11/12 08:01:39]

Also, why do we put the UUID in the display name on Linux, but not on CrOS?

[kmadhusu, 2012/11/12 19:56:19]

On Linux, When the user opens a device in a file dialog, they can easily map the
UUID to the device. On CrOS, there is no easy way of mapping the UUID with the
actual device (user has to open the terminal and navigate to the mount path to
map the UUID value).

[kmadhusu, 2012/11/12 19:56:19]

Chances are very less. Added a UMA to track this.

+  std::string manufacturer_name = GetDeviceManufacturerName(
+      GetUdevDevicePropertyValue(device, kVendor),
+      GetUdevDevicePropertyValue(device, kModel));
+  if (!manufacturer_name.empty())
+    device_label += " " + manufacturer_name;
+  return IsStringUTF8(device_label) ? UTF8ToUTF16(device_label) : string16();
+}
+
 // Get the device information using udev library.
-// On success, returns true and fill in |unique_id|, |name|, and |removable|.
+// On success, returns true and fill in |unique_id|, |name|, |removable| and
+// |storage_size|.
 void GetDeviceInfo(const FilePath& device_path, std::string* unique_id,
-                   string16* name, bool* removable) {
+                   string16* name, bool* removable, string16* storage_size) {
   DCHECK(!device_path.empty());
-
+  DCHECK(storage_size);

[Lei Zhang, 2012/11/12 08:01:39]

It's weird to make |storage_size| required, but the other output parameters are
optional.

[kmadhusu, 2012/11/12 19:56:19]

Made |storage_size| as a optional param just to be consistent with other output
params.

   ScopedUdevObject udev_obj(udev_new());
   if (!udev_obj.get()) {
     RecordGetDeviceInfoResult(false);
@@ -183,29 +214,11 @@ void GetDeviceInfo(const FilePath& device_path, std::string* unique_id,
     return;
   }
 
-  // Construct a device name using label or manufacturer (vendor and model)
-  // details.
-  if (name) {
-    std::string device_label = GetUdevDevicePropertyValue(device, kLabel);
-    if (device_label.empty())
-      device_label = GetUdevDevicePropertyValue(device, kSerial);
-    if (device_label.empty()) {
-      // Format: VendorInfo ModelInfo
-      // E.g.: KnCompany Model2010
-      device_label = GetUdevDevicePropertyValue(device, kVendor);
-      std::string model_name = GetUdevDevicePropertyValue(device, kModel);
-      if (device_label.empty())
-        device_label = model_name;
-      else if (!model_name.empty())
-        device_label += " " + model_name;
-    }
-    if (IsStringUTF8(device_label))
-      *name = UTF8ToUTF16(device_label);
-  }
+  if (name)
+    *name = GetDeviceName(device);
 
-  if (unique_id) {
+  if (unique_id)
     *unique_id = MakeDeviceUniqueId(device);
-  }
 
   if (removable) {
     const char* value = udev_device_get_sysattr_value(device,
@@ -221,6 +234,7 @@ void GetDeviceInfo(const FilePath& device_path, std::string* unique_id,
     }
     *removable = (value && atoi(value) == 1);
   }
+  *storage_size = GetSizeInfo(device_path, device);
   RecordGetDeviceInfoResult(true);
 }
 
@@ -272,7 +286,7 @@ void RemovableDeviceNotificationsLinux::Init() {
 
 bool RemovableDeviceNotificationsLinux::GetDeviceInfoForPath(
     const FilePath& path,
-    SystemMonitor::RemovableStorageInfo* device_info) const {
+    base::SystemMonitor::RemovableStorageInfo* device_info) const {
   if (!path.IsAbsolute())
     return false;
 
@@ -292,6 +306,14 @@ bool RemovableDeviceNotificationsLinux::GetDeviceInfoForPath(
   return true;
 }
 
+string16 RemovableDeviceNotificationsLinux::GetStorageSize(
+    const std::string& location) {
+  MountMap::const_iterator mount_info = mount_info_map_.find(
+      FilePath(location));
+  return (mount_info != mount_info_map_.end()) ?
+      mount_info->second.device_partition_size : string16();
+}
+
 void RemovableDeviceNotificationsLinux::OnFilePathChanged(const FilePath& path,
                                                           bool error) {
   if (path != mtab_path_) {
@@ -354,7 +376,7 @@ void RemovableDeviceNotificationsLinux::UpdateMtab() {
       if (MediaStorageUtil::IsRemovableDevice(old_iter->second.device_id)) {
         DCHECK(has_priority != priority->second.end());
         if (has_priority->second) {
-          SystemMonitor::Get()->ProcessRemovableStorageDetached(
+          base::SystemMonitor::Get()->ProcessRemovableStorageDetached(
               old_iter->second.device_id);
         }
         if (priority->second.size() > 1)
@@ -425,7 +447,9 @@ void RemovableDeviceNotificationsLinux::AddNewMount(
   std::string unique_id;
   string16 name;
   bool removable;
-  get_device_info_func_(mount_device, &unique_id, &name, &removable);
+  string16 device_partition_size;
+  get_device_info_func_(mount_device, &unique_id, &name, &removable,
+                        &device_partition_size);
 
   // Keep track of device info details to see how often we get invalid values.
   MediaStorageUtil::RecordDeviceInfoHistogram(true, unique_id, name);
@@ -449,13 +473,15 @@ void RemovableDeviceNotificationsLinux::AddNewMount(
   mount_point_info.mount_device = mount_device;
   mount_point_info.device_id = device_id;
   mount_point_info.device_name = name;
+  mount_point_info.device_partition_size = device_partition_size;
 
   mount_info_map_[mount_point] = mount_point_info;
   mount_priority_map_[mount_device][mount_point] = removable;
 
   if (removable) {
-    SystemMonitor::Get()->ProcessRemovableStorageAttached(device_id, name,
-                                                          mount_point.value());
+    base::SystemMonitor::Get()->ProcessRemovableStorageAttached(
+        device_id, GetDisplayNameForDevice(device_partition_size, name),
+        mount_point.value());
   }
 }