Make the Linux System Monitor implementation track all devices

chrome/browser/media_gallery/removable_device_notifications_linux_unittests.cc

Index: chrome/browser/media_gallery/removable_device_notifications_linux_unittests.cc
diff --git a/chrome/browser/media_gallery/media_device_notifications_linux_unittest.cc b/chrome/browser/media_gallery/removable_device_notifications_linux_unittests.cc
similarity index 50%
rename from chrome/browser/media_gallery/media_device_notifications_linux_unittest.cc
rename to chrome/browser/media_gallery/removable_device_notifications_linux_unittests.cc
index ffba0024482cd0e4003731b92b333a90602d2215..6289f998794fcca8c18b56e781a4b8295d21db69 100644
--- a/chrome/browser/media_gallery/media_device_notifications_linux_unittest.cc
+++ b/chrome/browser/media_gallery/removable_device_notifications_linux_unittests.cc
@@ -2,9 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// MediaDeviceNotificationsLinux unit tests.
+// RemovableDeviceNotificationsLinux unit tests.
 
-#include "chrome/browser/media_gallery/media_device_notifications_linux.h"
+#include "chrome/browser/media_gallery/removable_device_notifications_linux.h"
 
 #include <mntent.h>
 #include <stdio.h>
@@ -37,49 +37,80 @@ const char kInvalidPath[] = "invalid path does not exist";
 const char kDevice1[] = "d1";
 const char kDevice2[] = "d2";
 const char kDevice3[] = "d3";
+const char kDevice4[] = "d4";
+const char kDevice5[] = "d5";
 
-const char kDeviceId1[] = "UUID:FFF0-000F";
-const char kDeviceId2[] = "VendorModelSerial:ComName:Model2010:898989898989";
-const char kDeviceId3[] = "VendorModelSerial:::WEM319X792";
-
-const char kDeviceLabel1[] = "TEST_USB_MODEL_1";
-const char kDeviceLabel2[] = "TEST_USB_MODEL_2";
-const char kDeviceLabel3[] = "TEST_USB_MODEL_3";
+const char kInvalidDevice[] = "invalid_device";
 
 const char kMountPointA[] = "mnt_a";
 const char kMountPointB[] = "mnt_b";
+const char kMountPointC[] = "mnt_c";
 
-std::string GetDCIMDeviceId(std::string unique_id) {
-  return chrome::MediaStorageUtil::MakeDeviceId(
-      chrome::MediaStorageUtil::USB_MASS_STORAGE_WITH_DCIM, unique_id);
-}
+struct TestDeviceData {
+  const char* device_path;
+  const char* unique_id;
+  const char* device_name;
+  MediaStorageUtil::Type type;
+};
+
+const TestDeviceData kTestDeviceData[] = {
+  {kDevice1, "UUID:FFF0-000F", "TEST_USB_MODEL_1",
+   MediaStorageUtil::USB_MASS_STORAGE_WITH_DCIM},
+  {kDevice2, "VendorModelSerial:ComName:Model2010:89898989", "TEST_USB_MODEL_2",
+   MediaStorageUtil::USB_MASS_STORAGE_WITH_DCIM},
+  {kDevice3, "VendorModelSerial:::WEM319X792", "TEST_USB_MODEL_3",
+   MediaStorageUtil::USB_MASS_STORAGE_WITH_DCIM},
+  {kDevice4, "UUID:ABCD-1234", "TEST_USB_MODEL_4",
+   MediaStorageUtil::USB_MASS_STORAGE_NO_DCIM},
+  {kDevice5, "UUID:743A91FD2349", "TEST_USB_MODEL_5",
+   MediaStorageUtil::OTHER_MASS_STORAGE},
+};
 
-bool GetDeviceInfo(const std::string& dev_path, std::string* id,
-                   string16* name) {
-  std::string device_name;
-  if (dev_path == kDevice1) {
-    *id = GetDCIMDeviceId(kDeviceId1);
-    device_name = kDeviceLabel1;
-  } else if (dev_path == kDevice2) {
-    *id = GetDCIMDeviceId(kDeviceId2);
-    device_name = kDeviceLabel2;
-  } else if (dev_path == kDevice3) {
-    *id = GetDCIMDeviceId(kDeviceId3);
-    device_name = kDeviceLabel3;
-  } else {
-    return false;
+bool GetDeviceInfo(const FilePath& device_path, std::string* id,
+                   string16* name, bool* removable) {
+  for (size_t i = 0; i < arraysize(kTestDeviceData); i++) {
+    if (device_path.value() == kTestDeviceData[i].device_path) {
+      if (id)
+        *id = kTestDeviceData[i].unique_id;
+      if (name)
+        *name = ASCIIToUTF16(kTestDeviceData[i].device_name);
+      if (removable) {
+        MediaStorageUtil::Type type = kTestDeviceData[i].type;
+        *removable = (type == MediaStorageUtil::USB_MASS_STORAGE_WITH_DCIM) ||
+                     (type == MediaStorageUtil::USB_MASS_STORAGE_NO_DCIM);
+      }
+      return true;
+    }
   }
+  return false;
+}
 
-  *name = ASCIIToUTF16(device_name);
-  return true;
+std::string GetDeviceId(const std::string& device) {
+  for (size_t i = 0; i < arraysize(kTestDeviceData); i++) {
+    if (device == kTestDeviceData[i].device_path) {
+      return MediaStorageUtil::MakeDeviceId(kTestDeviceData[i].type,
+                                            kTestDeviceData[i].unique_id);
+    }
+  }
+  if (device == kInvalidDevice)

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

nit: curly brackets

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

Done.

+    return MediaStorageUtil::MakeDeviceId(MediaStorageUtil::OTHER_MASS_STORAGE,
+                                          kInvalidDevice);
+  return std::string();
+}
+string16 GetDeviceName(const std::string& device) {
+  for (size_t i = 0; i < arraysize(kTestDeviceData); i++) {
+    if (device == kTestDeviceData[i].device_path)
+      return ASCIIToUTF16(kTestDeviceData[i].device_name);
+  }
+  return string16();
 }
 
-class MediaDeviceNotificationsLinuxTestWrapper
-    : public MediaDeviceNotificationsLinux {
+class RemovableDeviceNotificationsLinuxTestWrapper
+   : public RemovableDeviceNotificationsLinux {
  public:
-  MediaDeviceNotificationsLinuxTestWrapper(const FilePath& path,
-                                           MessageLoop* message_loop)
-      : MediaDeviceNotificationsLinux(path, &GetDeviceInfo),
+  RemovableDeviceNotificationsLinuxTestWrapper(const FilePath& path,
+                                               MessageLoop* message_loop)
+      : RemovableDeviceNotificationsLinux(path, &GetDeviceInfo),
         message_loop_(message_loop) {
   }
 
@@ -87,19 +118,19 @@ class MediaDeviceNotificationsLinuxTestWrapper
   // Avoids code deleting the object while there are references to it.
   // Aside from the base::RefCountedThreadSafe friend class, any attempts to
   // call this dtor will result in a compile-time error.
-  ~MediaDeviceNotificationsLinuxTestWrapper() {}
+  ~RemovableDeviceNotificationsLinuxTestWrapper() {}
 
   virtual void OnFilePathChanged(const FilePath& path, bool error) OVERRIDE {
-    MediaDeviceNotificationsLinux::OnFilePathChanged(path, error);
+    RemovableDeviceNotificationsLinux::OnFilePathChanged(path, error);
     message_loop_->PostTask(FROM_HERE, MessageLoop::QuitClosure());
   }
 
   MessageLoop* message_loop_;
 
-  DISALLOW_COPY_AND_ASSIGN(MediaDeviceNotificationsLinuxTestWrapper);
+  DISALLOW_COPY_AND_ASSIGN(RemovableDeviceNotificationsLinuxTestWrapper);
 };
 
-class MediaDeviceNotificationsLinuxTest : public testing::Test {
+class RemovableDeviceNotificationLinuxTest : public testing::Test {
  public:
   struct MtabTestData {
     MtabTestData(const std::string& mount_device,
@@ -115,11 +146,11 @@ class MediaDeviceNotificationsLinuxTest : public testing::Test {
     const std::string mount_type;
   };
 
-  MediaDeviceNotificationsLinuxTest()
+  RemovableDeviceNotificationLinuxTest()
       : message_loop_(MessageLoop::TYPE_IO),
         file_thread_(content::BrowserThread::FILE, &message_loop_) {
   }
-  virtual ~MediaDeviceNotificationsLinuxTest() {}
+  virtual ~RemovableDeviceNotificationLinuxTest() {}
 
  protected:
   virtual void SetUp() OVERRIDE {
@@ -140,9 +171,8 @@ class MediaDeviceNotificationsLinuxTest : public testing::Test {
                 true  /* overwrite */);
 
     // Initialize the test subject.
-    notifications_ =
-        new MediaDeviceNotificationsLinuxTestWrapper(mtab_file_,
-                                                     &message_loop_);
+    notifications_ = new RemovableDeviceNotificationsLinuxTestWrapper(
+        mtab_file_, &message_loop_);
     notifications_->Init();
     message_loop_.RunAllPending();
   }
@@ -176,15 +206,15 @@ class MediaDeviceNotificationsLinuxTest : public testing::Test {
   }
 
   // Create a directory named |dir| relative to the test directory.
-  // It has a DCIM directory, so MediaDeviceNotificationsLinux recognizes it as
-  // a media directory.
+  // It has a DCIM directory, so RemovableDeviceNotificationsLinux recognizes it
+  // as a media directory.
   FilePath CreateMountPointWithDCIMDir(const std::string& dir) {
     return CreateMountPoint(dir, true  /* create DCIM dir */);
   }
 
   // Create a directory named |dir| relative to the test directory.
-  // It does not have a DCIM directory, so MediaDeviceNotificationsLinux does
-  // not recognizes it as a media directory.
+  // It does not have a DCIM directory, so RemovableDeviceNotificationsLinux
+  // does not recognizes it as a media directory.
   FilePath CreateMountPointWithoutDCIMDir(const std::string& dir) {
     return CreateMountPoint(dir, false  /* do not create DCIM dir */);
   }
@@ -193,6 +223,10 @@ class MediaDeviceNotificationsLinuxTest : public testing::Test {
     return *mock_devices_changed_observer_;
   }
 
+  RemovableDeviceNotificationsLinux* notifier() {
+    return notifications_.get();
+  }
+
  private:
   // Create a directory named |dir| relative to the test directory.
   // Set |with_dcim_dir| to true if the created directory will have a "DCIM"
@@ -256,13 +290,13 @@ class MediaDeviceNotificationsLinuxTest : public testing::Test {
   // Path to the test mtab file.
   FilePath mtab_file_;
 
-  scoped_refptr<MediaDeviceNotificationsLinuxTestWrapper> notifications_;
+  scoped_refptr<RemovableDeviceNotificationsLinuxTestWrapper> notifications_;
 
-  DISALLOW_COPY_AND_ASSIGN(MediaDeviceNotificationsLinuxTest);
+  DISALLOW_COPY_AND_ASSIGN(RemovableDeviceNotificationLinuxTest);
 };
 
 // Simple test case where we attach and detach a media device.
-TEST_F(MediaDeviceNotificationsLinuxTest, BasicAttachDetach) {
+TEST_F(RemovableDeviceNotificationLinuxTest, BasicAttachDetach) {
   testing::Sequence mock_sequence;
   FilePath test_path = CreateMountPointWithDCIMDir(kMountPointA);
   ASSERT_FALSE(test_path.empty());
@@ -271,22 +305,20 @@ TEST_F(MediaDeviceNotificationsLinuxTest, BasicAttachDetach) {
     MtabTestData(kDevice2, test_path.value(), kValidFS),
   };
   // Only |kDevice2| should be attached, since |kDevice1| has a bad path.
-  EXPECT_CALL(observer(),
-              OnRemovableStorageAttached(GetDCIMDeviceId(kDeviceId2),
-                                         ASCIIToUTF16(kDeviceLabel2),
-                                         test_path.value()))
+  EXPECT_CALL(observer(), OnRemovableStorageAttached(GetDeviceId(kDevice2),
+                                                     GetDeviceName(kDevice2),
+                                                     test_path.value()))
       .InSequence(mock_sequence);
   AppendToMtabAndRunLoop(test_data, arraysize(test_data));
 
   // |kDevice2| should be detached here.
-  EXPECT_CALL(observer(),
-              OnRemovableStorageDetached(GetDCIMDeviceId(kDeviceId2)))
+  EXPECT_CALL(observer(), OnRemovableStorageDetached(GetDeviceId(kDevice2)))
       .InSequence(mock_sequence);
   WriteEmptyMtabAndRunLoop();
 }
 
 // Only mount points with DCIM directories are recognized.
-TEST_F(MediaDeviceNotificationsLinuxTest, DCIM) {
+TEST_F(RemovableDeviceNotificationLinuxTest, DCIM) {
   testing::Sequence mock_sequence;
   FilePath test_path_a = CreateMountPointWithDCIMDir(kMountPointA);
   ASSERT_FALSE(test_path_a.empty());
@@ -294,10 +326,9 @@ TEST_F(MediaDeviceNotificationsLinuxTest, DCIM) {
     MtabTestData(kDevice1, test_path_a.value(), kValidFS),
   };
   // |kDevice1| should be attached as expected.
-  EXPECT_CALL(observer(),
-              OnRemovableStorageAttached(GetDCIMDeviceId(kDeviceId1),
-                                         ASCIIToUTF16(kDeviceLabel1),
-                                         test_path_a.value()))
+  EXPECT_CALL(observer(), OnRemovableStorageAttached(GetDeviceId(kDevice1),
+                                                     GetDeviceName(kDevice1),
+                                                     test_path_a.value()))
       .InSequence(mock_sequence);
   AppendToMtabAndRunLoop(test_data1, arraysize(test_data1));
 
@@ -310,14 +341,13 @@ TEST_F(MediaDeviceNotificationsLinuxTest, DCIM) {
   AppendToMtabAndRunLoop(test_data2, arraysize(test_data2));
 
   // |kDevice1| should be detached as expected.
-  EXPECT_CALL(observer(),
-              OnRemovableStorageDetached(GetDCIMDeviceId(kDeviceId1)))
+  EXPECT_CALL(observer(), OnRemovableStorageDetached(GetDeviceId(kDevice1)))
       .InSequence(mock_sequence);
   WriteEmptyMtabAndRunLoop();
 }
 
 // More complicated test case with multiple devices on multiple mount points.
-TEST_F(MediaDeviceNotificationsLinuxTest, SwapMountPoints) {
+TEST_F(RemovableDeviceNotificationLinuxTest, SwapMountPoints) {
   FilePath test_path_a = CreateMountPointWithDCIMDir(kMountPointA);
   FilePath test_path_b = CreateMountPointWithDCIMDir(kMountPointB);
   ASSERT_FALSE(test_path_a.empty());
@@ -353,15 +383,15 @@ TEST_F(MediaDeviceNotificationsLinuxTest, SwapMountPoints) {
 }
 
 // More complicated test case with multiple devices on multiple mount points.
-TEST_F(MediaDeviceNotificationsLinuxTest, MultiDevicesMultiMountPoints) {
+TEST_F(RemovableDeviceNotificationLinuxTest, MultiDevicesMultiMountPoints) {
   FilePath test_path_a = CreateMountPointWithDCIMDir(kMountPointA);
   FilePath test_path_b = CreateMountPointWithDCIMDir(kMountPointB);
   ASSERT_FALSE(test_path_a.empty());
   ASSERT_FALSE(test_path_b.empty());
 
   // Attach two devices.
-  // kDevice1 -> kMountPointA
-  // kDevice2 -> kMountPointB
+  // kDevice1 -> kMountPointA *
+  // kDevice2 -> kMountPointB *
   MtabTestData test_data1[] = {
     MtabTestData(kDevice1, test_path_a.value(), kValidFS),
     MtabTestData(kDevice2, test_path_b.value(), kValidFS),
@@ -372,45 +402,42 @@ TEST_F(MediaDeviceNotificationsLinuxTest, MultiDevicesMultiMountPoints) {
 
   // Attach |kDevice1| to |kMountPointB|.
   // |kDevice2| is inaccessible, so it is detached. |kDevice1| has been
-  // re-attached at |kMountPointB|, so it is 'detached' from kMountPointA.
-  // kDevice1 -> kMountPointA
+  // attached at |kMountPointB|, but is still accessible from |kMountPointA|.
+  // kDevice1 -> kMountPointA *

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

When I first wrote the test, I thought we came to the conclusion that if the
user mounts a device at mount point A, and then mounts it again at mount point
B, then mount point B is the one we show, since the user's most recent decision
is to put it there.

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

But if we've already notified consumers of system monitor that it's a mount
point a, then we'd have to detach and reattach it at mount point b.  While it's
still at mount point a, there's no need to do that.

   // kDevice2 -> kMountPointB
   // kDevice1 -> kMountPointB
   MtabTestData test_data2[] = {
     MtabTestData(kDevice1, test_path_b.value(), kValidFS),
   };
-  EXPECT_CALL(observer(), OnRemovableStorageAttached(_, _, _)).Times(1);
-  EXPECT_CALL(observer(), OnRemovableStorageDetached(_)).Times(2);
+  EXPECT_CALL(observer(), OnRemovableStorageAttached(_, _, _)).Times(0);
+  EXPECT_CALL(observer(), OnRemovableStorageDetached(_)).Times(1);
   AppendToMtabAndRunLoop(test_data2, arraysize(test_data2));
 
-  // Attach |kDevice2| to |kMountPointA|.
-  // kDevice1 -> kMountPointA
+  // Detach |kDevice1| from |kMountPointA|, causing a detach and attach event.
   // kDevice2 -> kMountPointB
-  // kDevice1 -> kMountPointB
-  // kDevice2 -> kMountPointA
+  // kDevice1 -> kMountPointB *
   MtabTestData test_data3[] = {
-    MtabTestData(kDevice2, test_path_a.value(), kValidFS),
+    MtabTestData(kDevice2, test_path_b.value(), kValidFS),
+    MtabTestData(kDevice1, test_path_b.value(), kValidFS),
   };
   EXPECT_CALL(observer(), OnRemovableStorageAttached(_, _, _)).Times(1);
-  EXPECT_CALL(observer(), OnRemovableStorageDetached(_)).Times(0);
-  AppendToMtabAndRunLoop(test_data3, arraysize(test_data3));
+  EXPECT_CALL(observer(), OnRemovableStorageDetached(_)).Times(1);
+  OverwriteMtabAndRunLoop(test_data3, arraysize(test_data3));
 
-  // Detach |kDevice2| from |kMountPointA|.
-  // kDevice1 -> kMountPointA
+  // Attach |kDevice1| to |kMountPointA|.
   // kDevice2 -> kMountPointB
-  // kDevice1 -> kMountPointB
+  // kDevice1 -> kMountPointB *
+  // kDevice1 -> kMountPointA
   MtabTestData test_data4[] = {
     MtabTestData(kDevice1, test_path_a.value(), kValidFS),
-    MtabTestData(kDevice2, test_path_b.value(), kValidFS),
-    MtabTestData(kDevice1, test_path_b.value(), kValidFS),
   };
   EXPECT_CALL(observer(), OnRemovableStorageAttached(_, _, _)).Times(0);
-  EXPECT_CALL(observer(), OnRemovableStorageDetached(_)).Times(1);
-  OverwriteMtabAndRunLoop(test_data4, arraysize(test_data4));
+  EXPECT_CALL(observer(), OnRemovableStorageDetached(_)).Times(0);
+  AppendToMtabAndRunLoop(test_data4, arraysize(test_data4));
 
   // Detach |kDevice1| from |kMountPointB|.
-  // kDevice1 -> kMountPointA
-  // kDevice2 -> kMountPointB
+  // kDevice1 -> kMountPointA *
+  // kDevice2 -> kMountPointB *
   EXPECT_CALL(observer(), OnRemovableStorageAttached(_, _, _)).Times(2);
   EXPECT_CALL(observer(), OnRemovableStorageDetached(_)).Times(1);
   OverwriteMtabAndRunLoop(test_data1, arraysize(test_data1));
@@ -421,59 +448,216 @@ TEST_F(MediaDeviceNotificationsLinuxTest, MultiDevicesMultiMountPoints) {
   WriteEmptyMtabAndRunLoop();
 }
 
-// More complicated test case with multiple devices on one mount point.
-TEST_F(MediaDeviceNotificationsLinuxTest, MultiDevicesOneMountPoint) {
+TEST_F(RemovableDeviceNotificationLinuxTest,
+       MultipleMountPointsWithNonDCIMDevices) {
   FilePath test_path_a = CreateMountPointWithDCIMDir(kMountPointA);
   FilePath test_path_b = CreateMountPointWithDCIMDir(kMountPointB);
   ASSERT_FALSE(test_path_a.empty());
   ASSERT_FALSE(test_path_b.empty());
 
-  // |kDevice1| is most recently mounted at |kMountPointB|.
-  // kDevice1 -> kMountPointA
-  // kDevice2 -> kMountPointB
-  // kDevice1 -> kMountPointB
+  // Attach to one first.
+  // kDevice1 -> kMountPointA *
   MtabTestData test_data1[] = {
     MtabTestData(kDevice1, test_path_a.value(), kValidFS),
-    MtabTestData(kDevice2, test_path_b.value(), kValidFS),
-    MtabTestData(kDevice1, test_path_b.value(), kValidFS),
   };
-  EXPECT_CALL(observer(),
-              OnRemovableStorageAttached(GetDCIMDeviceId(kDeviceId1),
-                                         ASCIIToUTF16(kDeviceLabel1),
-                                         test_path_b.value()))
-      .Times(1);
+  EXPECT_CALL(observer(), OnRemovableStorageAttached(_, _, _)).Times(1);
   EXPECT_CALL(observer(), OnRemovableStorageDetached(_)).Times(0);
-  OverwriteMtabAndRunLoop(test_data1, arraysize(test_data1));
+  AppendToMtabAndRunLoop(test_data1, arraysize(test_data1));
 
-  // Attach |kDevice3| to |kMountPointB|.
-  // |kDevice1| is inaccessible at its most recent mount point, so it is
-  // detached and unavailable, even though it is still accessible via
-  // |kMountPointA|.
-  // kDevice1 -> kMountPointA
-  // kDevice2 -> kMountPointB
+  // Attach |kDevice1| to |kMountPointB|.
+  // kDevice1 -> kMountPointA *
   // kDevice1 -> kMountPointB
-  // kDevice3 -> kMountPointB
   MtabTestData test_data2[] = {
-    MtabTestData(kDevice3, test_path_b.value(), kValidFS),
+    MtabTestData(kDevice1, test_path_b.value(), kValidFS),
   };
-  EXPECT_CALL(observer(),
-              OnRemovableStorageDetached(GetDCIMDeviceId(kDeviceId1)))
-      .Times(1);
-  EXPECT_CALL(observer(),
-              OnRemovableStorageAttached(GetDCIMDeviceId(kDeviceId3),
-                                         ASCIIToUTF16(kDeviceLabel3),
-                                         test_path_b.value()))
-      .Times(1);
+  EXPECT_CALL(observer(), OnRemovableStorageAttached(_, _, _)).Times(0);
+  EXPECT_CALL(observer(), OnRemovableStorageDetached(_)).Times(0);
   AppendToMtabAndRunLoop(test_data2, arraysize(test_data2));
 
+  // Attach |kDevice4| (a non-DCIM device) to |kMountPointA|.
+  // kDevice1 -> kMountPointA
+  // kDevice1 -> kMountPointB *
+  // kDevice4 -> kMountPointA
+  MtabTestData test_data3[] = {
+    MtabTestData(kDevice4, test_path_a.value(), kValidFS),
+  };
+  EXPECT_CALL(observer(), OnRemovableStorageAttached(_, _, _)).Times(1);
+  EXPECT_CALL(observer(), OnRemovableStorageDetached(_)).Times(1);
+  file_util::Delete(test_path_a.Append("DCIM"), false);
+  AppendToMtabAndRunLoop(test_data3, arraysize(test_data3));
+
+  // Detach |kDevice4|.
+  // kDevice1 -> kMountPointA
+  // kDevice1 -> kMountPointB *
+  MtabTestData test_data4[] = {
+    MtabTestData(kDevice1, test_path_a.value(), kValidFS),
+    MtabTestData(kDevice1, test_path_b.value(), kValidFS),
+  };
+  EXPECT_CALL(observer(), OnRemovableStorageAttached(_, _, _)).Times(0);
+  EXPECT_CALL(observer(), OnRemovableStorageDetached(_)).Times(0);
+  OverwriteMtabAndRunLoop(test_data4, arraysize(test_data4));
+
+  // Detach |kDevice1| from |kMountPointA|.
+  // kDevice1 -> kMountPointB *
+  EXPECT_CALL(observer(), OnRemovableStorageAttached(_, _, _)).Times(0);
+  EXPECT_CALL(observer(), OnRemovableStorageDetached(_)).Times(0);
+  OverwriteMtabAndRunLoop(test_data2, arraysize(test_data2));
+
   // Detach all devices.
   EXPECT_CALL(observer(), OnRemovableStorageAttached(_, _, _)).Times(0);
-  EXPECT_CALL(observer(),
-              OnRemovableStorageDetached(GetDCIMDeviceId(kDeviceId3)))
-      .Times(1);
+  EXPECT_CALL(observer(), OnRemovableStorageDetached(_)).Times(1);
   WriteEmptyMtabAndRunLoop();
 }
 
+TEST_F(RemovableDeviceNotificationLinuxTest, MountLookUp) {
+  FilePath test_path_a = CreateMountPointWithDCIMDir(kMountPointA);
+  FilePath test_path_b = CreateMountPointWithoutDCIMDir(kMountPointB);
+  FilePath test_path_c = CreateMountPointWithoutDCIMDir(kMountPointC);
+  ASSERT_FALSE(test_path_a.empty());
+  ASSERT_FALSE(test_path_b.empty());
+  ASSERT_FALSE(test_path_c.empty());
+
+  // Attach to one first.
+  // kDevice1 -> kMountPointA *
+  // kDevice4 -> kMountPointB
+  // kDevice5 -> kMountPointC
+  MtabTestData test_data1[] = {
+    MtabTestData(kDevice1, test_path_a.value(), kValidFS),
+    MtabTestData(kDevice4, test_path_b.value(), kValidFS),
+    MtabTestData(kDevice5, test_path_c.value(), kValidFS),
+  };
+  EXPECT_CALL(observer(), OnRemovableStorageAttached(_, _, _)).Times(1);
+  EXPECT_CALL(observer(), OnRemovableStorageDetached(_)).Times(0);
+  AppendToMtabAndRunLoop(test_data1, arraysize(test_data1));
+
+  EXPECT_EQ(test_path_a,
+            notifier()->GetDeviceMountPoint(GetDeviceId(kDevice1)));
+  EXPECT_EQ(test_path_b,
+            notifier()->GetDeviceMountPoint(GetDeviceId(kDevice4)));
+  EXPECT_EQ(test_path_c,
+            notifier()->GetDeviceMountPoint(GetDeviceId(kDevice5)));
+  EXPECT_EQ(FilePath(),
+            notifier()->GetDeviceMountPoint(GetDeviceId(kInvalidDevice)));
+
+  // Make |kDevice1| mounted in multiple places.
+  // kDevice1 -> kMountPointA *
+  // kDevice1 -> kMountPointB
+  // kDevice1 -> kMountPointC
+  MtabTestData test_data2[] = {
+    MtabTestData(kDevice1, test_path_a.value(), kValidFS),
+    MtabTestData(kDevice1, test_path_b.value(), kValidFS),
+    MtabTestData(kDevice1, test_path_c.value(), kValidFS),
+  };
+  EXPECT_CALL(observer(), OnRemovableStorageAttached(_, _, _)).Times(0);
+  EXPECT_CALL(observer(), OnRemovableStorageDetached(_)).Times(0);
+  // Add DCIM dirs.
+  CreateMountPointWithDCIMDir(kMountPointB);
+  CreateMountPointWithDCIMDir(kMountPointC);
+  OverwriteMtabAndRunLoop(test_data2, arraysize(test_data2));
+
+  EXPECT_EQ(test_path_a,
+            notifier()->GetDeviceMountPoint(GetDeviceId(kDevice1)));
+
+  // Unmount |kDevice1| from |kMountPointA|.
+  // kDevice1 -> kMountPointB *
+  // kDevice1 -> kMountPointC
+  // Either |kMountPointB| or |kMountPointC| is a valid new default, but
+  // it turns out that it will be B.
+  MtabTestData test_data3[] = {
+    MtabTestData(kDevice1, test_path_b.value(), kValidFS),
+  };
+  EXPECT_CALL(observer(), OnRemovableStorageAttached(_, _, _)).Times(1);
+  EXPECT_CALL(observer(), OnRemovableStorageDetached(_)).Times(1);
+  OverwriteMtabAndRunLoop(test_data3, arraysize(test_data3));
+
+  EXPECT_EQ(test_path_b,
+            notifier()->GetDeviceMountPoint(GetDeviceId(kDevice1)));
+
+  // Mount a non-removable device in multiple places.
+  // kDevice5 -> kMountPointA
+  // kDevice5 -> kMountPointB
+  // kDevice5 -> kMountPointC
+  MtabTestData test_data4[] = {
+    MtabTestData(kDevice5, test_path_a.value(), kValidFS),
+    MtabTestData(kDevice5, test_path_b.value(), kValidFS),
+    MtabTestData(kDevice5, test_path_c.value(), kValidFS),
+  };
+  EXPECT_CALL(observer(), OnRemovableStorageAttached(_, _, _)).Times(0);
+  EXPECT_CALL(observer(), OnRemovableStorageDetached(_)).Times(1);
+  // Remove DCIM dirs.
+  file_util::Delete(test_path_a.Append("DCIM"), false);

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

DCIM should be a constant.

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

Done.

+  file_util::Delete(test_path_b.Append("DCIM"), false);
+  file_util::Delete(test_path_c.Append("DCIM"), false);
+  OverwriteMtabAndRunLoop(test_data4, arraysize(test_data4));
+
+  // Any of the mount points would be valid.
+  EXPECT_EQ(test_path_a.value(),
+            notifier()->GetDeviceMountPoint(GetDeviceId(kDevice5)).value());
+}
+
+TEST_F(RemovableDeviceNotificationLinuxTest, DeviceLookUp) {
+  FilePath test_path_a = CreateMountPointWithDCIMDir(kMountPointA);
+  FilePath test_path_b = CreateMountPointWithoutDCIMDir(kMountPointB);
+  FilePath test_path_c = CreateMountPointWithoutDCIMDir(kMountPointC);
+  ASSERT_FALSE(test_path_a.empty());
+  ASSERT_FALSE(test_path_b.empty());
+  ASSERT_FALSE(test_path_c.empty());
+
+  // Attach to one first.
+  // kDevice1 -> kMountPointA *
+  // kDevice4 -> kMountPointB
+  // kDevice5 -> kMountPointC
+  MtabTestData test_data1[] = {
+    MtabTestData(kDevice1, test_path_a.value(), kValidFS),
+    MtabTestData(kDevice4, test_path_b.value(), kValidFS),
+    MtabTestData(kDevice5, test_path_c.value(), kValidFS),
+  };
+  EXPECT_CALL(observer(), OnRemovableStorageAttached(_, _, _)).Times(1);
+  EXPECT_CALL(observer(), OnRemovableStorageDetached(_)).Times(0);
+  AppendToMtabAndRunLoop(test_data1, arraysize(test_data1));
+
+  FilePath relative_path;
+  EXPECT_EQ(GetDeviceId(kDevice1),
+            notifier()->GetDeviceIdForPath(test_path_a, &relative_path));
+  EXPECT_EQ(FilePath().value(), relative_path.value());

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

I think you can just compare FilePaths directly.

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

Nope, it compares the pointer values instead.

+  EXPECT_EQ(GetDeviceId(kDevice4),
+            notifier()->GetDeviceIdForPath(test_path_b, &relative_path));
+  EXPECT_EQ(FilePath().value(), relative_path.value());
+  EXPECT_EQ(GetDeviceId(kDevice5),
+            notifier()->GetDeviceIdForPath(test_path_c, NULL));
+
+  // An invalid path.
+  EXPECT_EQ(std::string(),
+            notifier()->GetDeviceIdForPath(FilePath(kInvalidPath), NULL));
+
+  // Test filling in of the relative path.
+  FilePath extra_path("some/other/path");
+  EXPECT_EQ(GetDeviceId(kDevice1),
+            notifier()->GetDeviceIdForPath(test_path_a.Append(extra_path),
+                                           &relative_path));
+  EXPECT_EQ(extra_path.value(), relative_path.value());
+
+  // One device attached at multiple points.
+  // kDevice1 -> kMountPointA *
+  // kDevice5 -> kMountPointB
+  // kDevice5 -> kMountPointC
+  MtabTestData test_data2[] = {
+    MtabTestData(kDevice1, test_path_a.value(), kValidFS),
+    MtabTestData(kDevice5, test_path_b.value(), kValidFS),
+    MtabTestData(kDevice5, test_path_c.value(), kValidFS),
+  };
+  EXPECT_CALL(observer(), OnRemovableStorageAttached(_, _, _)).Times(0);
+  EXPECT_CALL(observer(), OnRemovableStorageDetached(_)).Times(0);
+  AppendToMtabAndRunLoop(test_data2, arraysize(test_data2));
+
+  EXPECT_EQ(GetDeviceId(kDevice1),
+            notifier()->GetDeviceIdForPath(test_path_a, NULL));
+  EXPECT_EQ(GetDeviceId(kDevice5),
+            notifier()->GetDeviceIdForPath(test_path_b, NULL));
+  EXPECT_EQ(GetDeviceId(kDevice5),
+            notifier()->GetDeviceIdForPath(test_path_c, NULL));
+}
+
 }  // namespace
 
 }  // namespace chrome