Make the Linux System Monitor implementation track all devices

chrome/browser/media_gallery/removable_device_notifications_linux.cc

Index: chrome/browser/media_gallery/removable_device_notifications_linux.cc
diff --git a/chrome/browser/media_gallery/media_device_notifications_linux.cc b/chrome/browser/media_gallery/removable_device_notifications_linux.cc
similarity index 43%
rename from chrome/browser/media_gallery/media_device_notifications_linux.cc
rename to chrome/browser/media_gallery/removable_device_notifications_linux.cc
index 012636e5999c1be0ac24c4cca3a341ead52f6abd..03e046025113be791d2a4509a4c9a8d348002c2d 100644
--- a/chrome/browser/media_gallery/media_device_notifications_linux.cc
+++ b/chrome/browser/media_gallery/removable_device_notifications_linux.cc
@@ -2,15 +2,15 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// MediaDeviceNotificationsLinux implementation.
+// RemovableDeviceNotificationsLinux implementation.
 
-#include "chrome/browser/media_gallery/media_device_notifications_linux.h"
+#include "chrome/browser/media_gallery/removable_device_notifications_linux.h"
 
 #include <libudev.h>
 #include <mntent.h>
 #include <stdio.h>
 
-#include <vector>
+#include <list>
 
 #include "base/bind.h"
 #include "base/file_path.h"
@@ -33,6 +33,9 @@ using content::BrowserThread;
 
 namespace {
 
+static RemovableDeviceNotificationsLinux*
+    g_removable_device_notifications_linux = NULL;
+
 // List of file systems we care about.
 const char* const kKnownFileSystems[] = {
   "ext2",
@@ -48,21 +51,48 @@ 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 kVendor[] = "ID_VENDOR";
 const char kVendorID[] = "ID_VENDOR_ID";
 
-// Device mount point details.
-struct MountPointEntryInfo {
-  std::string mount_point;
-  int entry_pos;
-};
+// (mount point, mount device)
+// A mapping from mount point to mount device, as extracted from the mtab
+// file.
+typedef std::map<FilePath, FilePath> MountPointDeviceMap;
+
+// Reads mtab file entries into |mtab|.
+void ReadMtab(const FilePath& mtab_path,
+              const std::set<std::string>& interesting_file_systems,
+              MountPointDeviceMap* mtab) {
+  mtab->clear();
+
+  FILE* fp = setmntent(mtab_path.value().c_str(), "r");
+  if (!fp)
+    return;
+
+  mntent entry;
+  char buf[512];
+
+  // We return the same device mounted to multiple locations, but hide
+  // devices that have been mounted over.
+  while (getmntent_r(fp, &entry, buf, sizeof(buf))) {
+    // We only care about real file systems.
+    if (!ContainsKey(interesting_file_systems, entry.mnt_type))
+      continue;
+
+    (*mtab)[FilePath(entry.mnt_dir)] = FilePath(entry.mnt_fsname);
+  }
+  endmntent(fp);
+}
 
 // ScopedGenericObj functor for UdevObjectRelease().
 class ScopedReleaseUdevObject {
@@ -94,10 +124,34 @@ std::string GetUdevDevicePropertyValue(struct udev_device* udev_device,
   return (strlen(value) > 0) ? value : std::string();
 }
 
+// Construct a device id using label or manufacturer (vendor and model) details.
+std::string MakeDeviceUniqueId(struct udev_device* device) {
+  std::string uuid = GetUdevDevicePropertyValue(device, kFsUUID);
+  if (!uuid.empty())
+    return kFSUniqueIdPrefix + uuid;
+
+  // If one of the vendor, model, serial information is missing, its value
+  // in the string is empty.
+  // Format: VendorModelSerial:VendorInfo:ModelInfo:SerialShortInfo
+  // E.g.: VendorModelSerial:Kn:DataTravel_12.10:8000000000006CB02CDB
+  std::string vendor = GetUdevDevicePropertyValue(device, kVendorID);
+  std::string model = GetUdevDevicePropertyValue(device, kModelID);
+  std::string serial_short = GetUdevDevicePropertyValue(device,
+                                                        kSerialShort);
+  if (vendor.empty() && model.empty() && serial_short.empty())
+    return std::string();
+
+  return base::StringPrintf("%s%s%s%s%s%s",
+                            kVendorModelSerialPrefix,
+                            vendor.c_str(), kNonSpaceDelim,
+                            model.c_str(), kNonSpaceDelim,
+                            serial_short.c_str());
+}
+
 // Get the device information using udev library.
-// On success, returns true and fill in |id| and |name|.
-bool GetDeviceInfo(const std::string& device_path, std::string* id,
-                   string16* name) {
+// On success, returns true and fill in |unique_id|, |name|, and |removable|.
+bool GetDeviceInfo(const FilePath& device_path, std::string* unique_id,
+                   string16* name, bool* removable) {
   DCHECK(!device_path.empty());
 
   ScopedUdevObject udev_obj(udev_new());
@@ -105,7 +159,7 @@ bool GetDeviceInfo(const std::string& device_path, std::string* id,
     return false;
 
   struct stat device_stat;
-  if (stat(device_path.c_str(), &device_stat) < 0)
+  if (stat(device_path.value().c_str(), &device_stat) < 0)
     return false;
 
   char device_type;
@@ -147,60 +201,59 @@ bool GetDeviceInfo(const std::string& device_path, std::string* id,
       *name = UTF8ToUTF16(device_label);
   }
 
-  // Construct a device id using label or manufacturer (vendor and model)
-  // details.
-  if (id) {
-    std::string unique_id = GetUdevDevicePropertyValue(device, kFsUUID);
-    if (unique_id.empty()) {
-      // If one of the vendor, model, serial information is missing, its value
-      // in the string is empty.
-      // Format: VendorModelSerial:VendorInfo:ModelInfo:SerialShortInfo
-      // E.g.: VendorModelSerial:Kn:DataTravel_12.10:8000000000006CB02CDB
-      std::string vendor = GetUdevDevicePropertyValue(device, kVendorID);
-      std::string model = GetUdevDevicePropertyValue(device, kModelID);
-      std::string serial_short = GetUdevDevicePropertyValue(device,
-                                                            kSerialShort);
-      if (vendor.empty() && model.empty() && serial_short.empty())
-        return false;
+  if (unique_id) {
+    *unique_id = MakeDeviceUniqueId(device);
+    if (unique_id->empty())
+      return false;
+  }
 
-      unique_id = base::StringPrintf("%s%s%s%s%s%s",
-                                     kVendorModelSerialPrefix,
-                                     vendor.c_str(),
-                                     kNonSpaceDelim,
-                                     model.c_str(),
-                                     kNonSpaceDelim,
-                                     serial_short.c_str());
-    } else {
-      unique_id = kFSUniqueIdPrefix + unique_id;
-    }
-    *id = MediaStorageUtil::MakeDeviceId(
-        MediaStorageUtil::USB_MASS_STORAGE_WITH_DCIM, unique_id);
+  if (removable) {
+    // |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);
+    const char* value = udev_device_get_sysattr_value(parent_device,

[Lei Zhang, 2012/08/28 07:55:30]

Try this on |device| first, in case |device| is /dev/sdX.

[vandebo (ex-Chrome), 2012/08/28 18:59:30]

Done.

+                                                      kRemovableSysAttr);
+    *removable = (value && atoi(value) == 1);
   }
   return true;
 }
 
 }  // namespace
 
-MediaDeviceNotificationsLinux::MediaDeviceNotificationsLinux(
+RemovableDeviceNotificationsLinux::RemovableDeviceNotificationsLinux(
     const FilePath& path)
     : initialized_(false),
       mtab_path_(path),
       get_device_info_func_(&GetDeviceInfo) {
+  DCHECK(!g_removable_device_notifications_linux);
+  g_removable_device_notifications_linux = this;
 }
 
-MediaDeviceNotificationsLinux::MediaDeviceNotificationsLinux(
+RemovableDeviceNotificationsLinux::RemovableDeviceNotificationsLinux(
     const FilePath& path,
     GetDeviceInfoFunc get_device_info_func)
     : initialized_(false),
       mtab_path_(path),
       get_device_info_func_(get_device_info_func) {
+  DCHECK(!g_removable_device_notifications_linux);
+  g_removable_device_notifications_linux = this;
 }
 
-MediaDeviceNotificationsLinux::~MediaDeviceNotificationsLinux() {
+RemovableDeviceNotificationsLinux::~RemovableDeviceNotificationsLinux() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  DCHECK_EQ(this, g_removable_device_notifications_linux);
+  g_removable_device_notifications_linux = NULL;
 }
 
-void MediaDeviceNotificationsLinux::Init() {
+// static
+RemovableDeviceNotificationsLinux*
+RemovableDeviceNotificationsLinux::GetInstance() {
+  return g_removable_device_notifications_linux;
+}
+
+void RemovableDeviceNotificationsLinux::Init() {
   DCHECK(!mtab_path_.empty());
 
   // Put |kKnownFileSystems| in std::set to get O(log N) access time.
@@ -209,11 +262,66 @@ void MediaDeviceNotificationsLinux::Init() {
 
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
-      base::Bind(&MediaDeviceNotificationsLinux::InitOnFileThread, this));
+      base::Bind(&RemovableDeviceNotificationsLinux::InitOnFileThread, this));
+}
+
+FilePath RemovableDeviceNotificationsLinux::GetDeviceMountPoint(
+    const std::string& device_id) const {
+
+  MediaStorageUtil::Type type;
+  MediaStorageUtil::CrackDeviceId(device_id, &type, NULL);
+  if (type == MediaStorageUtil::USB_MTP)

[Lei Zhang, 2012/08/28 07:55:30]

Do you think we'll be adding more storage types in the future? Perhaps change
this to a switch statement so we get an compiler error when we do add a new
type?

[vandebo (ex-Chrome), 2012/08/28 18:59:30]

Added a DCHECK

[Lei Zhang, 2012/08/28 20:49:04]

I was thinking of:

switch (type) {
  case MediaStorageUtil::MTP_OR_PTP:
    return FilePath();
  case MediaStorageUtil::REMOVABLE_MASS_STORAGE_WITH_DCIM:
  case MediaStorageUtil::REMOVABLE_MASS_STORAGE_NO_DCIM:
  case MediaStorageUtil::FIXED_MASS_STORAGE:
    break;  // continue
}

Which will cause a failure at compile time rather than maybe failing at run
time.

[vandebo (ex-Chrome), 2012/08/28 21:25:26]

It seems a bit overkill for the protection that we're trying to provide at this
point.  In all likelihood, the caller won't even call this function if it has a
non-mass storage device.  The check for MTP is just a safety net.  DCHECKS to
ensure the safety net isn't outdated seem fine whereas runtime overhead in
release builds seems overkill to me.

+    return FilePath();
+
+  FilePath mount_device;
+  for (MountMap::const_iterator it = mount_info_map_.begin();
+       it != mount_info_map_.end();
+       ++it) {
+    if (it->second.device_id == device_id) {
+      mount_device = it->second.mount_device;
+      break;
+    }
+  }
+  if (mount_device.empty())
+    return mount_device;
+
+  const ReferencedMountPoint& referenced_info =
+      mount_priority_map_.find(mount_device)->second;
+  for (ReferencedMountPoint::const_iterator it = referenced_info.begin();
+       it != referenced_info.end();
+       ++it) {
+    if (it->second)
+      return it->first;
+  }
+  // If none of them are default, just return the first.
+  return FilePath(
+      mount_priority_map_.find(mount_device)->second.begin()->first);

[Lei Zhang, 2012/08/28 07:55:30]

Isn't this just: referenced_info.begin()->first ?

[vandebo (ex-Chrome), 2012/08/28 18:59:30]

Done.

 }
 
-void MediaDeviceNotificationsLinux::OnFilePathChanged(const FilePath& path,
-                                                      bool error) {
+std::string RemovableDeviceNotificationsLinux::GetDeviceIdForPath(
+    const FilePath& path, FilePath* mount_point) const {
+  if (!path.IsAbsolute())
+    return std::string();
+
+  FilePath current = path;
+  for (current = path;

[Lei Zhang, 2012/08/28 07:55:30]

I think you want a while loop here.

[vandebo (ex-Chrome), 2012/08/28 18:59:30]

Done.

+       mount_info_map_.find(current) == mount_info_map_.end() &&
+          current != current.DirName();
+       current = current.DirName()) {}
+
+  if (mount_info_map_.find(current) == mount_info_map_.end())

[Lei Zhang, 2012/08/28 07:55:30]

Save the iterator so you don't have to call find() again on line 320.

[vandebo (ex-Chrome), 2012/08/28 18:59:30]

Done.

+    return std::string();
+
+  if (mount_point) {
+    *mount_point = FilePath();
+    current.AppendRelativePath(path, mount_point);

[Lei Zhang, 2012/08/28 07:55:30]

I may be understanding this wrong, but this doesn't seem to match the comment in
the header.

If |current| is /home and |path| is /home/foo, then we are setting |mount_point|
to "foo" here, whereas the mount point is "/home".

[vandebo (ex-Chrome), 2012/08/28 18:59:30]

Indeed.  The caller will also want the relative path, but returning the mount
point will let them compute both.  Fixed.

+  }
+
+  return mount_info_map_.find(current)->second.device_id;
+}
+
+void RemovableDeviceNotificationsLinux::OnFilePathChanged(const FilePath& path,
+                                                          bool error) {
   if (path != mtab_path_) {
     // This cannot happen unless FilePathWatcher is buggy. Just ignore this
     // notification and do nothing.
@@ -228,17 +336,17 @@ void MediaDeviceNotificationsLinux::OnFilePathChanged(const FilePath& path,
   UpdateMtab();
 }
 
-void MediaDeviceNotificationsLinux::InitOnFileThread() {
+void RemovableDeviceNotificationsLinux::InitOnFileThread() {
   DCHECK(!initialized_);
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   initialized_ = true;
 
   // The callback passed to Watch() has to be unretained. Otherwise
-  // MediaDeviceNotificationsLinux will live longer than expected, and
+  // RemovableDeviceNotificationsLinux will live longer than expected, and
   // FilePathWatcher will get in trouble at shutdown time.
   bool ret = file_watcher_.Watch(
       mtab_path_,
-      base::Bind(&MediaDeviceNotificationsLinux::OnFilePathChanged,
+      base::Bind(&RemovableDeviceNotificationsLinux::OnFilePathChanged,
                  base::Unretained(this)));
   if (!ret) {
     LOG(ERROR) << "Adding watch for " << mtab_path_.value() << " failed";
@@ -248,116 +356,98 @@ void MediaDeviceNotificationsLinux::InitOnFileThread() {
   UpdateMtab();
 }
 
-void MediaDeviceNotificationsLinux::UpdateMtab() {
+void RemovableDeviceNotificationsLinux::UpdateMtab() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
   MountPointDeviceMap new_mtab;
-  ReadMtab(&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::vector<std::string> mount_points_to_erase;
+  std::list<FilePath> mount_points_to_erase;
+  std::list<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 std::string& mount_point = old_iter->first;
-    const std::string& mount_device = old_iter->second.mount_device;
+    const FilePath& mount_point = old_iter->first;
+    const FilePath& mount_device = old_iter->second.mount_device;
     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)) {
-      RemoveOldDevice(old_iter->second.device_id);
+      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 (old_iter->second.has_dcim) {
+        DCHECK(has_priority != priority->second.end());
+        if (has_priority->second)
+          RemoveMediaMount(old_iter->second.device_id);
+        if (mount_priority_map_.find(mount_device)->second.size() > 1)

[Lei Zhang, 2012/08/28 07:55:30]

no need to call find() again? ditto below.

[vandebo (ex-Chrome), 2012/08/28 18:59:30]

Done.

+          multiple_mounted_devices_needing_reattachment.push_back(mount_device);
+      }
+      priority->second.erase(mount_point);
+      if (mount_priority_map_.find(mount_device)->second.empty())
+        mount_priority_map_.erase(mount_device);
       mount_points_to_erase.push_back(mount_point);
     }
   }
+
   // Erase the |mount_info_map_| entries afterwards. Erasing in the loop above
   // using the iterator is slightly more efficient, but more tricky, since
   // calling std::map::erase() on an iterator invalidates it.
-  for (size_t i = 0; i < mount_points_to_erase.size(); ++i)
-    mount_info_map_.erase(mount_points_to_erase[i]);
+  for (std::list<FilePath>::const_iterator it = mount_points_to_erase.begin();
+       it != mount_points_to_erase.end();
+       ++it) {
+    mount_info_map_.erase(*it);
+  }
+
+  // For any multiply mounted device where the mount that we had notified
+  // got detached, send a notification of attachment for one of the other
+  // mount points.
+  for (std::list<FilePath>::const_iterator it =
+           multiple_mounted_devices_needing_reattachment.begin();
+       it != multiple_mounted_devices_needing_reattachment.end();
+       ++it) {
+    const FilePath& mount_point = mount_priority_map_[*it].begin()->first;

[Lei Zhang, 2012/08/28 07:55:30]

If there was ever a bug in the code, you may end up setting |mount_point| to an
empty FilePath.

[vandebo (ex-Chrome), 2012/08/28 18:59:30]

I can see how this might be a little fragile.  Added a DCHECK to catch that if a
bug is introduced in the future.

[Lei Zhang, 2012/08/28 20:49:04]

Reading from a map using operator[] still sucks, because you're actually trying
to do an insert() and you are constructing an object to insert even though the
insert will likely fail. See [3] in {1} for what operator [] really does.

OTOH, for writing to a map, using operator [] is only slightly inefficient. You
insert a default constructed object, return the reference to it in the map, and
then write the rvalue into the reference. Since we don't have map-util.h, using
map::insert() is rather tedious. Which is probably why a lot of Chromium's map
usage don't use insert() as you mentioned in the previous CL.

{1} http://www.sgi.com/tech/stl/Map.html#3

[vandebo (ex-Chrome), 2012/08/28 21:25:26]

I mostly agree with what you've said.  But what would you like me to do about
it?  I think this needs to be addressed on the whole for chrome.  Either we
decide that reading from a map with [] is ok.  Or we add helper utilities to
make things less painful.

[Lei Zhang, 2012/08/28 23:22:33]

We can't fix everyone's code, but we can at least write our own code to not use
operator [] when reading. I'll try to put map-util.h in base when I have a
chance.

MountPriorityMap::iterator prio_it = mount_priority_map_.find(*it);
DCHECK_NE(prio_it, mount_priority_map_.end());
const FilePath& mount_point = prio_it->begin()->first;
prio_it->begin()->second = true;

MountInfoMap::const_iterator info_it = mount_info_map_.find(mount_point);
DCHECK_NE(info_it, mount_info_map_.end());
DCHECK(info_it->has_dcim);
base::SystemMonitor::Get()->ProcessRemovableStorageAttached(
  info_it->device_id
  ...

[vandebo (ex-Chrome), 2012/08/29 00:20:12]

Changed all instances (here and AddNewMount) of operator[ lookup to find.

+    mount_priority_map_[*it].begin()->second = true;
+    DCHECK(mount_info_map_[mount_point].has_dcim);
+    base::SystemMonitor::Get()->ProcessRemovableStorageAttached(
+        mount_info_map_[mount_point].device_id,
+        mount_info_map_[mount_point].device_name,
+        mount_point.value());
+  }
 
   // Check new mtab entries against existing ones.
   for (MountPointDeviceMap::iterator new_iter = new_mtab.begin();
        new_iter != new_mtab.end(); ++new_iter) {
-    const std::string& mount_point = new_iter->first;
-    const std::string& mount_device = new_iter->second;
+    const FilePath& mount_point = new_iter->first;
+    const 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) {
       // New mount point found or an existing mount point found with a new
       // device.
-      CheckAndAddMediaDevice(mount_device, mount_point);
+      AddNewMount(mount_device, mount_point);
     }
   }
 }
 
-void MediaDeviceNotificationsLinux::ReadMtab(MountPointDeviceMap* mtab) {
-  FILE* fp = setmntent(mtab_path_.value().c_str(), "r");
-  if (!fp)
+void RemovableDeviceNotificationsLinux::AddNewMount(
+    const FilePath& mount_device, const FilePath& mount_point) {
+  if (mount_priority_map_.find(mount_device) != mount_priority_map_.end()) {

[Lei Zhang, 2012/08/28 07:55:30]

You can easily save the iterator from find() and not have to use
mount_priority_map_[mount_device] below.

[vandebo (ex-Chrome), 2012/08/28 18:59:30]

Done.

+    const FilePath& other_mount_point =
+        mount_priority_map_[mount_device].begin()->first;
+    mount_priority_map_[mount_device][mount_point] = false;
+    mount_info_map_[mount_point] = mount_info_map_[other_mount_point];
     return;
-
-  mntent entry;
-  char buf[512];
-
-  // Keep track of mount point entry positions in mtab file.
-  int entry_pos = 0;
-
-  // Helper map to store the device mount point details.
-  // (mount device, MountPointEntryInfo)
-  typedef std::map<std::string, MountPointEntryInfo> DeviceMap;
-  DeviceMap device_map;
-  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 |entry_info| and use that below to resolve
-    // multiple devices mounted at the same mount point.
-    MountPointEntryInfo entry_info;
-    entry_info.mount_point = entry.mnt_dir;
-    entry_info.entry_pos = entry_pos++;
-    device_map[entry.mnt_fsname] = entry_info;
   }
-  endmntent(fp);
-
-  // Helper map to store mount point entries.
-  // (mount point, entry position in mtab file)
-  typedef std::map<std::string, int> MountPointsInfoMap;
-  MountPointsInfoMap mount_points_info_map;
-  MountPointDeviceMap& new_mtab = *mtab;
-  for (DeviceMap::const_iterator device_it = device_map.begin();
-       device_it != device_map.end();
-       ++device_it) {
-    // Add entries, but overwrite entries for the same mount point. Keep track
-    // of the entry positions and use that information to resolve multiple
-    // devices mounted at the same mount point.
-    const std::string& mount_device = device_it->first;
-    const std::string& mount_point = device_it->second.mount_point;
-    const int entry_pos = device_it->second.entry_pos;
-    MountPointDeviceMap::iterator new_it = new_mtab.find(mount_point);
-    MountPointsInfoMap::iterator mount_point_info_map_it =
-        mount_points_info_map.find(mount_point);
-
-    // New mount point entry found or there is already a device mounted at
-    // |mount_point| and the existing mount entry is older than the current one.
-    if (new_it == new_mtab.end() ||
-        (mount_point_info_map_it != mount_points_info_map.end() &&
-         mount_point_info_map_it->second < entry_pos)) {
-      new_mtab[mount_point] = mount_device;
-      mount_points_info_map[mount_point] = entry_pos;
-    }
-  }
-}
 
-void MediaDeviceNotificationsLinux::CheckAndAddMediaDevice(
-    const std::string& mount_device,
-    const std::string& mount_point) {
-  if (!IsMediaDevice(mount_point))
-    return;
-
-  std::string id;
+  std::string unique_id;
   string16 name;
-  bool result = get_device_info_func_(mount_device, &id, &name);
+  bool removable;
+  bool result = get_device_info_func_(mount_device, &unique_id, &name,
+                                      &removable);
 
   // Keep track of GetDeviceInfo result, to see how often we fail to get device
   // details.
@@ -368,22 +458,42 @@ void MediaDeviceNotificationsLinux::CheckAndAddMediaDevice(
 
   // Keep track of device uuid, to see how often we receive empty values.
   UMA_HISTOGRAM_BOOLEAN("MediaDeviceNotification.device_uuid_available",
-                        !id.empty());
+                        !unique_id.empty());
   UMA_HISTOGRAM_BOOLEAN("MediaDeviceNotification.device_name_available",
                         !name.empty());
 
-  if (id.empty() || name.empty())
+  if (unique_id.empty() || name.empty())
     return;
 
-  MountDeviceAndId mount_device_and_id;
-  mount_device_and_id.mount_device = mount_device;
-  mount_device_and_id.device_id = id;
-  mount_info_map_[mount_point] = mount_device_and_id;
+  bool has_dcim = IsMediaDevice(mount_point.value());
+  MediaStorageUtil::Type type;
+  if (removable) {
+    if (has_dcim) {
+      type = MediaStorageUtil::USB_MASS_STORAGE_WITH_DCIM;
+    } else {
+      type = MediaStorageUtil::USB_MASS_STORAGE_NO_DCIM;
+    }
+  } else {
+    type = MediaStorageUtil::OTHER_MASS_STORAGE;
+  }
+  std::string device_id = MediaStorageUtil::MakeDeviceId(type, unique_id);
+
+  MountPointInfo mount_point_info;
+  mount_point_info.mount_device = mount_device;
+  mount_point_info.device_id = device_id;
+  mount_point_info.device_name = name;
+  mount_point_info.has_dcim = has_dcim;
 
-  SystemMonitor::Get()->ProcessRemovableStorageAttached(id, name, mount_point);
+  mount_info_map_[mount_point] = mount_point_info;
+  mount_priority_map_[mount_device][mount_point] = has_dcim;
+
+  if (mount_point_info.has_dcim) {
+    SystemMonitor::Get()->ProcessRemovableStorageAttached(device_id, name,
+                                                          mount_point.value());
+  }
 }
 
-void MediaDeviceNotificationsLinux::RemoveOldDevice(
+void RemovableDeviceNotificationsLinux::RemoveMediaMount(
     const std::string& device_id) {
   SystemMonitor::Get()->ProcessRemovableStorageDetached(device_id);
 }