[Media Galleries] Populate volume metadata in ChromeOS/Linux

chrome/browser/storage_monitor/removable_device_notifications_linux.h

 // 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 listens for mount point changes, notifies
 // listeners about the addition and deletion of media devices, and
 // answers queries about mounted devices.
 
 #ifndef CHROME_BROWSER_STORAGE_MONITOR_REMOVABLE_DEVICE_NOTIFICATIONS_LINUX_H_
 #define CHROME_BROWSER_STORAGE_MONITOR_REMOVABLE_DEVICE_NOTIFICATIONS_LINUX_H_
@@ skipping 11 matching lines @@
 #include "base/compiler_specific.h"
 #include "base/files/file_path_watcher.h"
 #include "base/memory/ref_counted.h"
 #include "chrome/browser/storage_monitor/storage_monitor.h"
 #include "content/public/browser/browser_thread.h"
 
 namespace base {
 class FilePath;
 }
 
-// Gets device information given a |device_path|. On success, fills in
-// |unique_id|, |name|, |removable| and |partition_size_in_bytes|.
+// Gets device information given a |device_path| and |mount_point|.
+// On success, fills in metadata fields.
 typedef void (*GetDeviceInfoFunc)(const base::FilePath& device_path,

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

So I think the signature of this function is pretty consisten across platforms. 
I'm not sure how I feel about changing that.  Also, this is a function that gets
changed out for tests.  So the less code we have in here the better.

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

We are in the position now where we can use inheritance for the tests instead of
the swappable function. I've changed a couple to use the mocks, I think the rest
can follow, or perhaps delegate to a protected method (which we are also doing
in a couple spots...)

This really should take a StorageInfo*.

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

Inheritance for the test sounds good.  The swappable functions are ugly.  But
every bit of code that is in a function with a OS specific APIs that we can't
mock is code that can't be tested.  As much as possible, I'd like to keep the OS
calls in isolated functions that do as little as possible in order to increase
our testability. 

[Greg Billock, 2013/03/06 23:24:25]

I think we have a plan for this now.

-                                  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* volume_label,
+                                  string16* vendor_name,
+                                  string16* model_name);
 
 namespace chrome {
 
 class RemovableDeviceNotificationsLinux
     : public StorageMonitor,
       public base::RefCountedThreadSafe<RemovableDeviceNotificationsLinux,
           content::BrowserThread::DeleteOnFileThread> {
  public:
   // Should only be called by browser start up code.  Use GetInstance() instead.
   explicit RemovableDeviceNotificationsLinux(const base::FilePath& path);
@@ skipping 23 matching lines @@
   virtual ~RemovableDeviceNotificationsLinux();
 
   virtual void OnFilePathChanged(const base::FilePath& path, bool error);
 
  private:
   friend class base::RefCountedThreadSafe<RemovableDeviceNotificationsLinux>;
   friend class base::DeleteHelper<RemovableDeviceNotificationsLinux>;
   friend struct content::BrowserThread::DeleteOnThread<
       content::BrowserThread::FILE>;
 
-  // Structure to save mounted device information such as device path, unique
-  // identifier, device name and partition size.
-  struct MountPointInfo {
-    MountPointInfo();
+  base::FilePath GetDeviceForMountPoint(const base::FilePath& mount_point);
 
-    base::FilePath mount_device;
-    std::string device_id;
-    string16 device_name;
-    uint64 partition_size_in_bytes;
-  };
-
-  // Mapping of mount points to MountPointInfo.
-  typedef std::map<base::FilePath, MountPointInfo> MountMap;
+  // Mapping of mount points to StorageInfo.
+  typedef std::map<base::FilePath, StorageInfo> MountMap;
 
   // (mount point, priority)
   // For devices that are mounted to multiple mount points, this helps us track
   // which one we've notified system monitor about.
   typedef std::map<base::FilePath, bool> ReferencedMountPoint;
 
   // (mount device, map of known mount points)
   // For each mount device, track the places it is mounted and which one (if
   // any) we have notified system monitor about.
   typedef std::map<base::FilePath, ReferencedMountPoint> MountPriorityMap;
@@ skipping 35 matching lines @@
   // to notify about it's departure and notify about another one of it's mount
   // points.
   MountPriorityMap mount_priority_map_;
 
   DISALLOW_COPY_AND_ASSIGN(RemovableDeviceNotificationsLinux);
 };
 
 }  // namespace chrome
 
 #endif  // CHROME_BROWSER_STORAGE_MONITOR_REMOVABLE_DEVICE_NOTIFICATIONS_LINUX_H_