[LINUX] Extract the name and id of the device and send it along the device attach message.

chrome/browser/media_gallery/media_device_notifications_linux.cc

Index: chrome/browser/media_gallery/media_device_notifications_linux.cc
diff --git a/chrome/browser/media_gallery/media_device_notifications_linux.cc b/chrome/browser/media_gallery/media_device_notifications_linux.cc
index e4d61621989cc055d98b4231c146c988f87f2bfd..835ca608e70fbe32c0947216ccef155846895698 100644
--- a/chrome/browser/media_gallery/media_device_notifications_linux.cc
+++ b/chrome/browser/media_gallery/media_device_notifications_linux.cc
@@ -9,11 +9,14 @@
 #include <mntent.h>
 #include <stdio.h>
 
+#include <libudev.h>

[Lei Zhang, 2012/08/09 00:32:41]

not: C header should be grouped with the other C headers.

[kmadhusu, 2012/08/09 02:48:40]

Done.

 #include <vector>
 
 #include "base/bind.h"
 #include "base/file_path.h"
+#include "base/metrics/histogram.h"
 #include "base/stl_util.h"
+#include "base/string_util.h"

[Lei Zhang, 2012/08/09 00:32:41]

nit: alphabetical order.

[kmadhusu, 2012/08/09 02:48:40]

Done.

 #include "base/string_number_conversions.h"
 #include "base/system_monitor/system_monitor.h"
 #include "base/utf_string_conversions.h"
@@ -35,6 +38,128 @@ const char* const kKnownFileSystems[] = {
   "vfat",
 };
 
+// Device property constants.
+const char kDevName[] = "DEVNAME";
+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 kSeperator[] = "_";
+const char kSerial[] = "ID_SERIAL";
+const char kSerialShort[] = "ID_SERIAL_SHORT";
+const char kVendor[] = "ID_VENDOR";
+const char kVendorID[] = "ID_VENDOR_ID";
+
+// Return syspath from devpath or an empty string on failure.
+std::string GetSysPathFromDevPath(struct udev* udev,
+                                  const std::string& dev_path) {
+  DCHECK(!dev_path.empty());
+
+  udev_enumerate* enumerate = udev_enumerate_new(udev);
+  if (!enumerate)

[Lei Zhang, 2012/08/09 00:32:41]

Just CHECK(enumerate). It should never fail, and if it does, we would like to
know about it.

[kmadhusu, 2012/08/09 02:48:40]

Done.

+    return std::string();
+
+  if (udev_enumerate_add_match_property(enumerate, kDevName,
+                                        dev_path.c_str()) < 0 ||
+      udev_enumerate_scan_devices(enumerate) < 0) {
+    udev_enumerate_unref(enumerate);
+    return std::string();
+  }
+
+  // Enumerate the devices that matches the specified device name filter.
+  udev_list_entry* devs = udev_enumerate_get_list_entry(enumerate);
+  if (!devs) {
+    udev_enumerate_unref(enumerate);
+    return std::string();
+  }
+
+  udev_list_entry *cur = NULL;

[Lei Zhang, 2012/08/09 00:32:41]

nit: foo_struct* varname; Same elsewhere.

[kmadhusu, 2012/08/09 02:48:40]

Done.

+  std::string sys_path;
+  udev_list_entry_foreach(cur, devs) {

[Lei Zhang, 2012/08/09 00:32:41]

Why not just call udev_list_entry_get_next()? Can this list ever contain more
than 1 entry?

[kmadhusu, 2012/08/09 02:48:40]

I don't expect more than one entry. Just to be on the safer side, I am breaking
the loop control.

+    // Get the first entry and return.
+    sys_path.assign(udev_list_entry_get_name(cur));

[Lei Zhang, 2012/08/09 00:32:41]

Is there a reason you use std_string_var.assign() instead of just '=' ?

[kmadhusu, 2012/08/09 02:48:40]

Done.

+    break;
+  }
+
+  udev_enumerate_unref(enumerate);
+  return sys_path;
+}
+
+// Get the device information using udev library.
+// Returns true on success, false on failure.
+bool GetDeviceInfoHelper(const std::string& dev_path,
+                         std::string* id,
+                         string16* name) {
+  DCHECK(!dev_path.empty());
+
+  // libudev-related items.
+  udev* udev_ptr;
+  udev_ptr = udev_new();
+  if (!udev_ptr)

[Lei Zhang, 2012/08/09 00:32:41]

Just CHECK() instead.

[kmadhusu, 2012/08/09 02:48:40]

Done.

+    return false;
+
+  // Get sys path from dev path.
+  std::string sys_path = GetSysPathFromDevPath(udev_ptr, dev_path);
+  if (sys_path.empty()) {
+    udev_unref(udev_ptr);
+    return false;
+  }
+
+  // Create a new udev_device object from sys path.
+  udev_device* dev = udev_device_new_from_syspath(udev_ptr, sys_path.c_str());

[Lei Zhang, 2012/08/09 00:32:41]

I think you may be able to just stat(dev_path) and use
udev_device_new_from_devnum(). Then you don't even need GetSysPathFromDevPath().

[kmadhusu, 2012/08/09 02:48:40]

Thanks for the pointer. That worked.

+  if (!dev) {
+    udev_unref(udev_ptr);
+    return false;
+  }
+
+  // Construct a device name using label or vendor and model information.

[Lei Zhang, 2012/08/09 00:32:41]

From the comment, I can't tell if this means

(label | vendor) & model
or
label | (vendor & model)

stupid natural languages.

[kmadhusu, 2012/08/09 02:48:40]

Rephrased the comment to "Construct a device name using label or
manufacturer(vendor and model) details."

+  std::string device_name;
+  const char* dev_name = NULL;
+  if ((dev_name = udev_device_get_property_value(dev, kLabel)) ||
+      (dev_name = udev_device_get_property_value(dev, kSerial))) {
+    device_name.assign(dev_name);
+  } else {
+    // Format: VendorInfo_ModelInfo
+    // Eg: KnCompany_Model2010
+    const char *vendor_name = NULL, *model_name = NULL;
+    if ((vendor_name = udev_device_get_property_value(dev, kVendor)))
+      device_name.assign(vendor_name);
+    if ((model_name = udev_device_get_property_value(dev, kModel))) {
+      if (!device_name.empty())
+        device_name.append(kSeperator);
+      device_name.append(model_name);
+    }
+  }
+  *name = UTF8ToUTF16(device_name);
+
+  // Construct a unique id using fs uuid or vendor and model information.
+  const char* uuid  = NULL;
+  if ((uuid = udev_device_get_property_value(dev, kFsUUID))) {
+    id->assign(uuid);
+  } else {
+    // Format: VendorInfo_ModelInfo_SerialShortInfo
+    // Eg: Kn_DataTravel_12.10_8000000000006CB02CDB
+    const char *vendor = NULL, *model = NULL, *serial_short = NULL;
+    if ((vendor = udev_device_get_property_value(dev, kVendorID)))
+      id->assign(vendor);
+
+    if ((model = udev_device_get_property_value(dev, kModelID))) {
+      if (!id->empty())
+        id->append(kSeperator);
+      id->append(model);
+    }
+    if ((serial_short = udev_device_get_property_value(dev, kSerialShort))) {
+      if (!id->empty())
+        id->append(kSeperator);
+      id->append(serial_short);
+    }
+  }
+
+  udev_unref(udev_ptr);
+  udev_device_unref(dev);
+  return true;
+}
+
 }  // namespace
 
 namespace chrome {
@@ -45,8 +170,7 @@ using content::BrowserThread;
 MediaDeviceNotificationsLinux::MediaDeviceNotificationsLinux(
     const FilePath& path)
     : initialized_(false),
-      mtab_path_(path),
-      current_device_id_(0U) {
+      mtab_path_(path) {
   CHECK(!path.empty());
 
   // Put |kKnownFileSystems| in std::set to get O(log N) access time.
@@ -81,6 +205,15 @@ void MediaDeviceNotificationsLinux::OnFilePathChanged(const FilePath& path,
   UpdateMtab();
 }
 
+bool MediaDeviceNotificationsLinux::GetDeviceInfo(const std::string& dev_path,

[Lei Zhang, 2012/08/09 00:32:41]

Is it possible to make this static, get rid of GetDeviceInfoHelper() ? Not sure
if you can override this as a static.

[kmadhusu, 2012/08/09 02:48:40]

I don't want to add udev function calls in this class. That is the reason, I
added a helper function. 
I cannot override a static function.

+                                                  std::string* id,
+                                                  string16* name) {
+  if (dev_path.empty())
+    return false;
+
+  return GetDeviceInfoHelper(dev_path, id, name);
+}
+
 void MediaDeviceNotificationsLinux::InitOnFileThread() {
   DCHECK(!initialized_);
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
@@ -104,7 +237,7 @@ void MediaDeviceNotificationsLinux::InitOnFileThread() {
 void MediaDeviceNotificationsLinux::UpdateMtab() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
-  MountMap new_mtab;
+  MountPointDeviceMap new_mtab;
   ReadMtab(&new_mtab);
 
   // Check existing mtab entries for unaccounted mount points.
@@ -112,8 +245,13 @@ void MediaDeviceNotificationsLinux::UpdateMtab() {
   std::vector<std::string> mount_points_to_erase;
   for (MountMap::const_iterator it = mtab_.begin(); it != mtab_.end(); ++it) {
     const std::string& mount_point = it->first;
-    // |mount_point| not in |new_mtab|.
-    if (!ContainsKey(new_mtab, mount_point)) {
+    const std::string& mount_device = it->second.first;
+    MountPointDeviceMap::iterator newiter = new_mtab.find(mount_point);
+    // |mount_point| not in |new_mtab| or |mount_device| is no longer mounted in
+    // |mount_point|.
+    if ((!ContainsKey(new_mtab, mount_point)) ||
+        (base::strcasecmp(newiter->second.c_str(),

[Lei Zhang, 2012/08/09 00:32:41]

Can't you just compare |newiter->second| to |mount_device| with == ?

[kmadhusu, 2012/08/09 02:48:40]

Done.

+                          mount_device.c_str()) != 0)) {
       const std::string& device_id = it->second.second;
       RemoveOldDevice(device_id);
       mount_points_to_erase.push_back(mount_point);
@@ -126,63 +264,76 @@ void MediaDeviceNotificationsLinux::UpdateMtab() {
     mtab_.erase(mount_points_to_erase[i]);
 
   // Check new mtab entries against existing ones.
-  for (MountMap::iterator newiter = new_mtab.begin();
+  for (MountPointDeviceMap::iterator newiter = new_mtab.begin();
        newiter != new_mtab.end();
        ++newiter) {
+    std::string device_id;
     const std::string& mount_point = newiter->first;
-    const MountDeviceAndId& mount_device_and_id = newiter->second;
-    const std::string& mount_device = mount_device_and_id.first;
-    std::string& id = newiter->second.second;
+    const std::string& mount_device = newiter->second;
     MountMap::iterator olditer = mtab_.find(mount_point);
     // Check to see if it is a new mount point.
     if (olditer == mtab_.end()) {
       if (IsMediaDevice(mount_point)) {
-        AddNewDevice(mount_device, mount_point, &id);
-        mtab_.insert(std::make_pair(mount_point, mount_device_and_id));
+        AddNewDevice(mount_device, mount_point, &device_id);
+        mtab_.insert(std::make_pair(mount_point,
+                                    std::make_pair(mount_device, device_id)));
       }
       continue;
     }
 
     // Existing mount point. Check to see if a new device is mounted there.
-    const MountDeviceAndId& old_mount_device_and_id = olditer->second;
-    if (mount_device == old_mount_device_and_id.first)
+    if (mount_device == olditer->second.first)
       continue;
 
-    // New device mounted.
-    RemoveOldDevice(old_mount_device_and_id.second);
     if (IsMediaDevice(mount_point)) {
-      AddNewDevice(mount_device, mount_point, &id);
-      olditer->second = mount_device_and_id;
+      AddNewDevice(mount_device, mount_point, &device_id);
+      mtab_[mount_point] = std::make_pair(mount_device, device_id);
     }
   }
 }
 
-void MediaDeviceNotificationsLinux::ReadMtab(MountMap* mtab) {
+void MediaDeviceNotificationsLinux::ReadMtab(MountPointDeviceMap* mtab) {
   FILE* fp = setmntent(mtab_path_.value().c_str(), "r");
   if (!fp)
     return;
 
-  MountMap& new_mtab = *mtab;
+  // Mount point entry position.
+  typedef int EntryPos;

[Lei Zhang, 2012/08/09 00:32:41]

just use int.

[kmadhusu, 2012/08/09 02:48:40]

Done.

+
+  // (mount point, entry position in mtab file)
+  typedef std::pair<std::string, EntryPos> MountEntryInfo;
+
+  // (mount device, MountEntryInfo)
+  typedef std::map<std::string, MountEntryInfo> DeviceMap;
+
+  // (mount point, entry position in mtab file)
+  typedef std::map<std::string, EntryPos> MountPointsInfoMap;
+
+  // Helper maps to store the device mount point details and mount point
+  // entries.
+  DeviceMap device_map;
+  MountPointsInfoMap mount_points_info_map;
+
+  MountPointDeviceMap& new_mtab = *mtab;
   mntent entry;
   char buf[512];
-  int mount_position = 0;
-  typedef std::pair<std::string, std::string> MountPointAndId;
-  typedef std::map<std::string, MountPointAndId> DeviceMap;
-  DeviceMap device_map;
+
+  // Keep track of mount point entry positions in mtab file.
+  EntryPos entry_pos = 0;
+
   while (getmntent_r(fp, &entry, buf, sizeof(buf))) {
     // We only care about real file systems.
     if (!ContainsKey(known_file_systems_, entry.mnt_type))
       continue;
-    // Add entries, but overwrite entries for the same mount device. Keep track
-    // of the entry positions in the device id field and use that below to
-    // resolve multiple devices mounted at the same mount point.
-    MountPointAndId mount_point_and_id =
-        std::make_pair(entry.mnt_dir, base::IntToString(mount_position++));
-    DeviceMap::iterator it = device_map.find(entry.mnt_fsname);
+    const std::string mount_device = entry.mnt_fsname;
+    const std::string mount_point = entry.mnt_dir;
+    DeviceMap::iterator it = device_map.find(mount_device);
     if (it == device_map.end()) {
-      device_map.insert(std::make_pair(entry.mnt_fsname, mount_point_and_id));
+      device_map.insert(std::make_pair(mount_device,
+                                       std::make_pair(mount_point,
+                                                      entry_pos++)));
     } else {
-      it->second = mount_point_and_id;
+      device_map[mount_device] = std::make_pair(mount_point, entry_pos++);
     }
   }
   endmntent(fp);
@@ -190,28 +341,24 @@ void MediaDeviceNotificationsLinux::ReadMtab(MountMap* mtab) {
   for (DeviceMap::const_iterator device_it = device_map.begin();
        device_it != device_map.end();
        ++device_it) {
-    const std::string& device = device_it->first;
-    const std::string& mount_point = device_it->second.first;
-    const std::string& position = device_it->second.second;
-
-    // No device at |mount_point|, save |device| to it.
-    MountMap::iterator mount_it = new_mtab.find(mount_point);
-    if (mount_it == new_mtab.end()) {
-      new_mtab.insert(std::make_pair(mount_point,
-                                     std::make_pair(device, position)));
-      continue;
+    const std::string mount_device = device_it->first;
+    const std::string mount_point = device_it->second.first;
+    const EntryPos entry_pos = device_it->second.second;
+    MountPointDeviceMap::iterator new_it = new_mtab.find(mount_point);
+    if (new_it == new_mtab.end()) {
+      new_mtab.insert(std::make_pair(mount_point, mount_device));
+      mount_points_info_map.insert(std::make_pair(mount_point, entry_pos));
+    } else {
+      MountPointsInfoMap::iterator it = mount_points_info_map.find(mount_point);
+      DCHECK(it != mount_points_info_map.end());
+      // There is already a device mounted at |mount_point|. Check to see if
+      // the existing mount entry is newer than the current one.
+      if (it->second < entry_pos) {
+        // The current entry is newer, update the mount point entry.
+        it->second = entry_pos;
+        new_mtab[mount_point] = mount_device;
+      }
     }
-
-    // There is already a device mounted at |mount_point|. Check to see if
-    // the existing mount entry is newer than the current one.
-    std::string& existing_device = mount_it->second.first;
-    std::string& existing_position = mount_it->second.second;
-    if (existing_position > position)
-      continue;
-
-    // The current entry is newer, update the mount point entry.
-    existing_device = device;
-    existing_position = position;
   }
 }
 
@@ -219,10 +366,22 @@ void MediaDeviceNotificationsLinux::AddNewDevice(
     const std::string& mount_device,
     const std::string& mount_point,
     std::string* device_id) {
-  *device_id = base::IntToString(current_device_id_++);
+  string16 device_name;
+  if (!GetDeviceInfo(mount_device, device_id, &device_name))
+    return;
+
+  // Keep track of device uuid, to see how often we receive empty values.
+  UMA_HISTOGRAM_BOOLEAN("MediaDeviceNotification.device_uuid_available",
+                        !device_id->empty());
+  UMA_HISTOGRAM_BOOLEAN("MediaDeviceNotification.device_name_available",
+                        !device_name.empty());
+
+  if (device_id->empty() || device_name.empty())
+    return;
+
   base::SystemMonitor* system_monitor = base::SystemMonitor::Get();
   system_monitor->ProcessMediaDeviceAttached(*device_id,
-                                             UTF8ToUTF16(mount_device),
+                                             device_name,
                                              SystemMonitor::TYPE_PATH,
                                              mount_point);
 }