[Media Galleries] Populate volume metadata in ChromeOS/Linux

chrome/browser/storage_monitor/removable_device_notifications_linux.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // RemovableDeviceNotificationsLinux implementation.
 
 #include "chrome/browser/storage_monitor/removable_device_notifications_linux.h"
 
 #include <mntent.h>
 #include <stdio.h>
@@ skipping 117 matching lines @@
 
   uint64 total_size_in_bytes = 0;
   if (!base::StringToUint64(partition_size, &total_size_in_bytes))
     return 0;
   return (total_size_in_bytes <= kuint64max / 512) ?
       total_size_in_bytes * 512 : 0;
 }
 
 // Constructs the device name from the device properties. If the device details
 // are unavailable, returns an empty string.
-string16 GetDeviceName(struct udev_device* device) {
+string16 GetDeviceName(struct udev_device* device,
+                       string16* out_volume_label,
+                       string16* out_vendor_name,
+                       string16* out_model_name) {
   std::string device_label = GetUdevDevicePropertyValue(device, kLabel);
+  std::string vendor_name = GetUdevDevicePropertyValue(device, kVendor);
+  std::string model_name = GetUdevDevicePropertyValue(device, kModel);
+  if (out_volume_label)
+    *out_volume_label = UTF8ToUTF16(device_label);
+  if (out_vendor_name)
+    *out_vendor_name = UTF8ToUTF16(vendor_name);
+  if (out_model_name)
+    *out_model_name = UTF8ToUTF16(model_name);
+
   if (!device_label.empty() && IsStringUTF8(device_label))
     return UTF8ToUTF16(device_label);
 
   device_label = GetUdevDevicePropertyValue(device, kFsUUID);
   // Keep track of device uuid, to see how often we receive empty uuid values.
   UMA_HISTOGRAM_BOOLEAN(
       "RemovableDeviceNotificationsLinux.device_file_system_uuid_available",
       !device_label.empty());
 
-  const string16 name = GetFullProductName(
-      GetUdevDevicePropertyValue(device, kVendor),
-      GetUdevDevicePropertyValue(device, kModel));
+  const string16 name = GetFullProductName(vendor_name, model_name);
 
   const string16 device_label_utf16 =
       (!device_label.empty() && IsStringUTF8(device_label)) ?
           UTF8ToUTF16(device_label) : string16();
   if (!name.empty() && !device_label_utf16.empty())
     return device_label_utf16 + ASCIIToUTF16(" ") + name;
   return name.empty() ? device_label_utf16 : name;
 }
 
 // Get the device information using udev library.
 // On success, returns true and fill in |unique_id|, |name|, |removable| and
 // |partition_size_in_bytes|.
 void GetDeviceInfo(const base::FilePath& device_path,
-                   std::string* unique_id,
+                   const base::FilePath& mount_point,
+                   std::string* device_id,
                    string16* name,
                    bool* removable,
-                   uint64* partition_size_in_bytes) {
+                   uint64* partition_size_in_bytes,
+                   string16* out_volume_label,
+                   string16* out_vendor_name,
+                   string16* out_model_name) {
   DCHECK(!device_path.empty());
   ScopedUdevObject udev_obj(udev_new());
   if (!udev_obj.get()) {
     RecordGetDeviceInfoResult(false);
     return;
   }
 
   struct stat device_stat;
   if (stat(device_path.value().c_str(), &device_stat) < 0) {
     RecordGetDeviceInfoResult(false);
@@ skipping 10 matching lines @@
     return;  // Not a supported type.
   }
 
   ScopedUdevDeviceObject device(
       udev_device_new_from_devnum(udev_obj, device_type, device_stat.st_rdev));
   if (!device.get()) {
     RecordGetDeviceInfoResult(false);
     return;
   }
 
+  string16 volume_label;
+  string16 vendor_name;
+  string16 model_name;
+  string16 device_name = GetDeviceName(device, &volume_label,
+                                       &vendor_name, &model_name);
   if (name)
-    *name = GetDeviceName(device);
+    *name = device_name;
 
-  if (unique_id)
-    *unique_id = MakeDeviceUniqueId(device);
+  std::string unique_id = MakeDeviceUniqueId(device);
+  MediaStorageUtil::RecordDeviceInfoHistogram(true, unique_id, device_name);
 
+  const char* value = udev_device_get_sysattr_value(device,
+                                                    kRemovableSysAttr);
+  if (!value) {
+    // |parent_device| is owned by |device| and does not need to be cleaned
+    // up.
+    struct udev_device* parent_device =
+        udev_device_get_parent_with_subsystem_devtype(device,
+                                                      kBlockSubsystemKey,
+                                                      kDiskDeviceTypeKey);
+    value = udev_device_get_sysattr_value(parent_device, kRemovableSysAttr);
+  }
+  bool is_removable = (value && atoi(value) == 1);
   if (removable) {
-    const char* value = udev_device_get_sysattr_value(device,
-                                                      kRemovableSysAttr);
-    if (!value) {
-      // |parent_device| is owned by |device| and does not need to be cleaned
-      // up.
-      struct udev_device* parent_device =
-          udev_device_get_parent_with_subsystem_devtype(device,
-                                                        kBlockSubsystemKey,
-                                                        kDiskDeviceTypeKey);
-      value = udev_device_get_sysattr_value(parent_device, kRemovableSysAttr);
-    }
-    *removable = (value && atoi(value) == 1);
+    *removable = is_removable;
   }
 
+  bool has_dcim = IsMediaDevice(mount_point.value());

[vandebo (ex-Chrome), 2013/03/01 22:21:48]

This block should all be in if(device_id) {

[Greg Billock, 2013/03/05 19:20:55]

Done.

+  MediaStorageUtil::Type type = MediaStorageUtil::FIXED_MASS_STORAGE;
+  if (is_removable) {
+    if (has_dcim)
+      type = MediaStorageUtil::REMOVABLE_MASS_STORAGE_WITH_DCIM;
+    else
+      type = MediaStorageUtil::REMOVABLE_MASS_STORAGE_NO_DCIM;
+  }
+  if (device_id)
+    *device_id = MediaStorageUtil::MakeDeviceId(type, unique_id);
+
   if (partition_size_in_bytes)
     *partition_size_in_bytes = GetDeviceStorageSize(device_path, device);
   RecordGetDeviceInfoResult(true);
 }
 
 }  // namespace
 
 RemovableDeviceNotificationsLinux::RemovableDeviceNotificationsLinux(
     const base::FilePath& path)
     : initialized_(false),
@@ skipping 31 matching lines @@
   if (!path.IsAbsolute())
     return false;
 
   base::FilePath current = path;
   while (!ContainsKey(mount_info_map_, current) && current != current.DirName())
     current = current.DirName();
 
   MountMap::const_iterator mount_info = mount_info_map_.find(current);
   if (mount_info == mount_info_map_.end())
     return false;
-
-  if (device_info) {
-    device_info->device_id = mount_info->second.device_id;
-    device_info->name = mount_info->second.device_name;
-    device_info->location = current.value();
-  }
+  if (device_info)
+    *device_info = mount_info->second;
   return true;
 }
 
 uint64 RemovableDeviceNotificationsLinux::GetStorageSize(
     const std::string& location) const {
   MountMap::const_iterator mount_info = mount_info_map_.find(
       base::FilePath(location));
   return (mount_info != mount_info_map_.end()) ?
-      mount_info->second.partition_size_in_bytes : 0;
+      mount_info->second.total_size_in_bytes : 0;
 }
 
 void RemovableDeviceNotificationsLinux::OnFilePathChanged(
     const base::FilePath& path,
     bool error) {
   if (path != mtab_path_) {
     // This cannot happen unless FilePathWatcher is buggy. Just ignore this
     // notification and do nothing.
     NOTREACHED();
     return;
   }
   if (error) {
     LOG(ERROR) << "Error watching " << mtab_path_.value();
     return;
   }
 
   UpdateMtab();
 }
 
-RemovableDeviceNotificationsLinux::MountPointInfo::MountPointInfo()
-    : partition_size_in_bytes(0) {
+base::FilePath RemovableDeviceNotificationsLinux::GetDeviceForMountPoint(

[vandebo (ex-Chrome), 2013/03/01 22:21:48]

Not a fan of this.  Yes all the information is here, but it's complicated to get
at.  How about changing MountPointInfo to:

struct MountPointInfo {
  StorageInfo storage_info;
  base::FilePath mount_device;
};

[Greg Billock, 2013/03/05 19:20:55]

I think it's complicated because of the priority map, but I don't have a
suggested simplification for that -- tracking multiple mount points is just
complex.

One thing to like about this is that the two data structures are now storing
disjoint information -- I think even better would be to rekey mount_info_map_,
but I didn't do that here because it's kind of hard to follow.

[vandebo (ex-Chrome), 2013/03/06 01:34:54]

So the way it was before was storing some redundant information, but that made
it fast to access.  At the least, this seems like an unrelated change that
should go into its own CL.

+    const base::FilePath& mount_point) {
+  for (MountPriorityMap::const_iterator iter = mount_priority_map_.begin();
+       iter != mount_priority_map_.end(); ++iter) {
+    for (ReferencedMountPoint::const_iterator itermp = iter->second.begin();
+         itermp != iter->second.end(); ++itermp) {
+      if (itermp->first == mount_point)
+        return iter->first;
+    }
+  }
+  return base::FilePath();
 }
 
 void RemovableDeviceNotificationsLinux::InitOnFileThread() {
   DCHECK(!initialized_);
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   initialized_ = true;
 
   // The callback passed to Watch() has to be unretained. Otherwise
   // RemovableDeviceNotificationsLinux will live longer than expected, and
   // FilePathWatcher will get in trouble at shutdown time.
@@ skipping 15 matching lines @@
   MountPointDeviceMap new_mtab;
   ReadMtab(mtab_path_, known_file_systems_, &new_mtab);
 
   // Check existing mtab entries for unaccounted mount points.
   // These mount points must have been removed in the new mtab.
   std::list<base::FilePath> mount_points_to_erase;
   std::list<base::FilePath> multiple_mounted_devices_needing_reattachment;
   for (MountMap::const_iterator old_iter = mount_info_map_.begin();
        old_iter != mount_info_map_.end(); ++old_iter) {
     const base::FilePath& mount_point = old_iter->first;
-    const base::FilePath& mount_device = old_iter->second.mount_device;
+    const base::FilePath& mount_device = GetDeviceForMountPoint(mount_point);
     MountPointDeviceMap::iterator new_iter = new_mtab.find(mount_point);
     // |mount_point| not in |new_mtab| or |mount_device| is no longer mounted at
     // |mount_point|.
     if (new_iter == new_mtab.end() || (new_iter->second != mount_device)) {
       MountPriorityMap::iterator priority =
           mount_priority_map_.find(mount_device);
       DCHECK(priority != mount_priority_map_.end());
       ReferencedMountPoint::const_iterator has_priority =
           priority->second.find(mount_point);
       if (MediaStorageUtil::IsRemovableDevice(old_iter->second.device_id)) {
@@ skipping 26 matching lines @@
   // mount points.
   for (std::list<base::FilePath>::const_iterator it =
            multiple_mounted_devices_needing_reattachment.begin();
        it != multiple_mounted_devices_needing_reattachment.end();
        ++it) {
     ReferencedMountPoint::iterator first_mount_point_info =
         mount_priority_map_.find(*it)->second.begin();
     const base::FilePath& mount_point = first_mount_point_info->first;
     first_mount_point_info->second = true;
 
-    const MountPointInfo& mount_info =
+    const StorageInfo& mount_info =
         mount_info_map_.find(mount_point)->second;
     DCHECK(MediaStorageUtil::IsRemovableDevice(mount_info.device_id));
-    receiver()->ProcessAttach(StorageInfo(
-        mount_info.device_id, mount_info.device_name, mount_point.value()));
+    receiver()->ProcessAttach(mount_info);
   }
 
   // Check new mtab entries against existing ones.
   for (MountPointDeviceMap::iterator new_iter = new_mtab.begin();
        new_iter != new_mtab.end(); ++new_iter) {
     const base::FilePath& mount_point = new_iter->first;
     const base::FilePath& mount_device = new_iter->second;
     MountMap::iterator old_iter = mount_info_map_.find(mount_point);
     if (old_iter == mount_info_map_.end() ||
-        old_iter->second.mount_device != mount_device) {
+        GetDeviceForMountPoint(old_iter->first) != mount_device) {
       // New mount point found or an existing mount point found with a new
       // device.
       AddNewMount(mount_device, mount_point);
     }
   }
 }
 
 void RemovableDeviceNotificationsLinux::AddNewMount(
     const base::FilePath& mount_device, const base::FilePath& mount_point) {
   MountPriorityMap::iterator priority =
       mount_priority_map_.find(mount_device);
   if (priority != mount_priority_map_.end()) {
     const base::FilePath& other_mount_point = priority->second.begin()->first;
     priority->second[mount_point] = false;
     mount_info_map_[mount_point] =
         mount_info_map_.find(other_mount_point)->second;
     return;
   }
 
-  std::string unique_id;
+  std::string device_id;
   string16 name;
   bool removable;
   uint64 partition_size_in_bytes;
-  get_device_info_func_(mount_device, &unique_id, &name, &removable,
-                        &partition_size_in_bytes);
+  string16 volume_label;
+  string16 vendor_name;
+  string16 model_name;
+  get_device_info_func_(mount_device, mount_point, &device_id, &name,
+                        &removable, &partition_size_in_bytes,
+                        &volume_label, &vendor_name, &model_name);
 
   // Keep track of device info details to see how often we get invalid values.
-  MediaStorageUtil::RecordDeviceInfoHistogram(true, unique_id, name);
-  if (unique_id.empty() || name.empty())
+  if (device_id.empty() || name.empty())
     return;
 
-  bool has_dcim = IsMediaDevice(mount_point.value());
-  MediaStorageUtil::Type type;
-  if (removable) {
-    if (has_dcim) {
-      type = MediaStorageUtil::REMOVABLE_MASS_STORAGE_WITH_DCIM;
-    } else {
-      type = MediaStorageUtil::REMOVABLE_MASS_STORAGE_NO_DCIM;
-    }
-  } else {
-    type = MediaStorageUtil::FIXED_MASS_STORAGE;
-  }
-  std::string device_id = MediaStorageUtil::MakeDeviceId(type, unique_id);
-
-  MountPointInfo mount_point_info;
-  mount_point_info.mount_device = mount_device;
+  StorageInfo mount_point_info;

[vandebo (ex-Chrome), 2013/03/01 22:21:48]

use the constructor?

[Greg Billock, 2013/03/05 19:20:55]

Done.

   mount_point_info.device_id = device_id;
-  mount_point_info.device_name = name;
-  mount_point_info.partition_size_in_bytes = partition_size_in_bytes;
+  mount_point_info.location = mount_point.value();
+  mount_point_info.name = name;
+  mount_point_info.total_size_in_bytes = partition_size_in_bytes;
+  mount_point_info.storage_label = volume_label;
+  mount_point_info.vendor_name = vendor_name;
+  mount_point_info.model_name = model_name;
 
   mount_info_map_[mount_point] = mount_point_info;
   mount_priority_map_[mount_device][mount_point] = removable;
 
   if (removable) {
     receiver()->ProcessAttach(StorageInfo(
         device_id, GetDisplayNameForDevice(partition_size_in_bytes, name),
         mount_point.value()));
   }
 }
 
 }  // namespace chrome