Linux removable device notifications: notify about non-dcim devices.

chrome/browser/media_gallery/removable_device_notifications_linux_unittest.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 unit tests.
 
 #include "chrome/browser/media_gallery/removable_device_notifications_linux.h"
 
 #include <mntent.h>
 #include <stdio.h>
@@ skipping 292 matching lines @@
 
   DISALLOW_COPY_AND_ASSIGN(RemovableDeviceNotificationLinuxTest);
 };
 
 // Simple test case where we attach and detach a media device.
 TEST_F(RemovableDeviceNotificationLinuxTest, BasicAttachDetach) {
   testing::Sequence mock_sequence;
   FilePath test_path = CreateMountPointWithDCIMDir(kMountPointA);
   ASSERT_FALSE(test_path.empty());
   MtabTestData test_data[] = {
-    MtabTestData(kDevice1, kInvalidPath, kValidFS),
     MtabTestData(kDevice2, test_path.value(), kValidFS),
+    MtabTestData(kDevice5, kInvalidPath, kValidFS),
   };
   // Only |kDevice2| should be attached, since |kDevice1| has a bad path.

[kmadhusu, 2012/08/29 01:12:02]

nit: kDevice1 => kDevice5

[vandebo (ex-Chrome), 2012/08/29 06:10:13]

Done.

[kmadhusu, 2012/08/29 16:46:19]

Only |kDevice2| should be attached, since |kDevice5| has a bad path.

[vandebo (ex-Chrome), 2012/08/29 17:04:39]

Oops, fixed.

   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(GetDeviceId(kDevice2)))
       .InSequence(mock_sequence);
   WriteEmptyMtabAndRunLoop();
 }
 
-// Only mount points with DCIM directories are recognized.
-TEST_F(RemovableDeviceNotificationLinuxTest, DCIM) {
+// Only removable devices are recognized.
+TEST_F(RemovableDeviceNotificationLinuxTest, Removable) {
   testing::Sequence mock_sequence;
   FilePath test_path_a = CreateMountPointWithDCIMDir(kMountPointA);
   ASSERT_FALSE(test_path_a.empty());
   MtabTestData test_data1[] = {
     MtabTestData(kDevice1, test_path_a.value(), kValidFS),
   };
   // |kDevice1| should be attached as expected.
   EXPECT_CALL(observer(), OnRemovableStorageAttached(GetDeviceId(kDevice1),
                                                      GetDeviceName(kDevice1),
                                                      test_path_a.value()))
       .InSequence(mock_sequence);
   AppendToMtabAndRunLoop(test_data1, arraysize(test_data1));
 
-  // This should do nothing, since |kMountPointB| does not have a DCIM dir.
+  // This should do nothing, since |kDevice5| is not removable.
   FilePath test_path_b = CreateMountPointWithoutDCIMDir(kMountPointB);
   ASSERT_FALSE(test_path_b.empty());
   MtabTestData test_data2[] = {
-    MtabTestData(kDevice2, test_path_b.value(), kValidFS),
+    MtabTestData(kDevice5, test_path_b.value(), kValidFS),
   };
   AppendToMtabAndRunLoop(test_data2, arraysize(test_data2));
 
   // |kDevice1| should be detached as expected.
   EXPECT_CALL(observer(), OnRemovableStorageDetached(GetDeviceId(kDevice1)))
       .InSequence(mock_sequence);

[kmadhusu, 2012/08/29 01:12:02]

We will also receive an attach and detach for kDevice5. Can you add those
checks?

[vandebo (ex-Chrome), 2012/08/29 06:10:13]

No we won't, device5 is not removable.

[kmadhusu, 2012/08/29 16:46:19]

For readability, we should have better names for these test devices. You can do
that change in a separate CL. 

[vandebo (ex-Chrome), 2012/08/29 17:04:39]

Done.

   WriteEmptyMtabAndRunLoop();
+
+  // |kDevice4| should be attached as expected.
+  EXPECT_CALL(observer(), OnRemovableStorageAttached(GetDeviceId(kDevice4),
+                                                     GetDeviceName(kDevice4),
+                                                     test_path_b.value()))
+      .InSequence(mock_sequence);
+  MtabTestData test_data3[] = {
+    MtabTestData(kDevice4, test_path_b.value(), kValidFS),
+  };
+  AppendToMtabAndRunLoop(test_data3, arraysize(test_data3));
+
+  // |kDevice4| should be detached as expected.
+  EXPECT_CALL(observer(), OnRemovableStorageDetached(GetDeviceId(kDevice4)))
+      .InSequence(mock_sequence);
+  WriteEmptyMtabAndRunLoop();
 }
 
 // More complicated test case with multiple devices on multiple mount points.
 TEST_F(RemovableDeviceNotificationLinuxTest, SwapMountPoints) {
   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 *

[kmadhusu, 2012/08/29 16:46:19]

nit: It took a while for me to understand what "*" means in the mount point
info. May be you want to add a note somewhere.

[vandebo (ex-Chrome), 2012/08/29 17:04:39]

Done.

   MtabTestData test_data1[] = {
     MtabTestData(kDevice1, test_path_a.value(), kValidFS),
     MtabTestData(kDevice2, test_path_b.value(), kValidFS),
   };
   EXPECT_CALL(observer(), OnRemovableStorageAttached(_, _, _)).Times(2);
   EXPECT_CALL(observer(), OnRemovableStorageDetached(_)).Times(0);
   AppendToMtabAndRunLoop(test_data1, arraysize(test_data1));
 
   // Detach two devices from old mount points and attach the devices at new
   // mount points.
-  // kDevice1 -> kMountPointB
-  // kDevice2 -> kMountPointA
+  // kDevice1 -> kMountPointB *
+  // kDevice2 -> kMountPointA *
   MtabTestData test_data2[] = {
     MtabTestData(kDevice1, test_path_b.value(), kValidFS),
     MtabTestData(kDevice2, test_path_a.value(), kValidFS),
   };
   EXPECT_CALL(observer(), OnRemovableStorageAttached(_, _, _)).Times(2);
   EXPECT_CALL(observer(), OnRemovableStorageDetached(_)).Times(2);
   OverwriteMtabAndRunLoop(test_data2, arraysize(test_data2));
 
   // Detach all devices.
   EXPECT_CALL(observer(), OnRemovableStorageAttached(_, _, _)).Times(0);
@@ skipping 86 matching lines @@
   // Attach |kDevice1| to |kMountPointB|.
   // kDevice1 -> kMountPointA *
   // kDevice1 -> kMountPointB
   MtabTestData test_data2[] = {
     MtabTestData(kDevice1, test_path_b.value(), kValidFS),
   };
   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|.
+  // Attach |kDevice5| (a non-removable device) to |kMountPointA|.
   // kDevice1 -> kMountPointA
   // kDevice1 -> kMountPointB *
-  // kDevice4 -> kMountPointA
+  // kDevice5 -> kMountPointA
   MtabTestData test_data3[] = {
-    MtabTestData(kDevice4, test_path_a.value(), kValidFS),
+    MtabTestData(kDevice5, test_path_a.value(), kValidFS),
   };
   EXPECT_CALL(observer(), OnRemovableStorageAttached(_, _, _)).Times(1);
   EXPECT_CALL(observer(), OnRemovableStorageDetached(_)).Times(1);
   RemoveDCIMDirFromMountPoint(kMountPointA);
   AppendToMtabAndRunLoop(test_data3, arraysize(test_data3));
 
-  // Detach |kDevice4|.
+  // Detach |kDevice5|.
   // 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);
+  CreateMountPointWithDCIMDir(kMountPointA);
   OverwriteMtabAndRunLoop(test_data4, arraysize(test_data4));
 
-  // Detach |kDevice1| from |kMountPointA|.
-  // kDevice1 -> kMountPointB *
+  // Attach |kDevice4| (a non-DCIM device) to |kMountPointB|.
+  // kDevice1 -> kMountPointA *
+  // kDevice1 -> kMountPointB
+  // kDevice4 -> kMountPointB *
+  MtabTestData test_data5[] = {
+    MtabTestData(kDevice4, test_path_b.value(), kValidFS),
+  };
+  EXPECT_CALL(observer(), OnRemovableStorageAttached(_, _, _)).Times(2);
+  EXPECT_CALL(observer(), OnRemovableStorageDetached(_)).Times(1);
+  file_util::Delete(test_path_b.Append("DCIM"), false);
+  AppendToMtabAndRunLoop(test_data5, arraysize(test_data5));
+
+  // Detach |kDevice4|.
+  // kDevice1 -> kMountPointA *
+  // kDevice1 -> kMountPointB
+  MtabTestData test_data6[] = {
+    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(1);
+  CreateMountPointWithDCIMDir(kMountPointB);
+  OverwriteMtabAndRunLoop(test_data6, arraysize(test_data6));
+
+  // Detach |kDevice1| from |kMountPointB|.
+  // kDevice1 -> kMountPointA *
   EXPECT_CALL(observer(), OnRemovableStorageAttached(_, _, _)).Times(0);
   EXPECT_CALL(observer(), OnRemovableStorageDetached(_)).Times(0);
-  OverwriteMtabAndRunLoop(test_data2, arraysize(test_data2));
+  OverwriteMtabAndRunLoop(test_data1, arraysize(test_data1));
 
   // Detach all devices.
   EXPECT_CALL(observer(), OnRemovableStorageAttached(_, _, _)).Times(0);
   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
+  // kDevice4 -> kMountPointB *
   // kDevice5 -> kMountPointC

[kmadhusu, 2012/08/29 01:12:02]

Why did you add a * in kMountPointB but not to kMountPointC?

[vandebo (ex-Chrome), 2012/08/29 06:10:13]

The mount points with *'s are the ones we've notified system monitor about.

   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(), OnRemovableStorageAttached(_, _, _)).Times(2);
   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);
+  EXPECT_CALL(observer(), OnRemovableStorageDetached(_)).Times(1);
   // 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 *
@@ skipping 35 matching lines @@
 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
+  // kDevice4 -> kMountPointB *
   // kDevice5 -> kMountPointC

[kmadhusu, 2012/08/29 01:12:02]

nit: 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(), OnRemovableStorageAttached(_, _, _)).Times(2);
   EXPECT_CALL(observer(), OnRemovableStorageDetached(_)).Times(0);
   AppendToMtabAndRunLoop(test_data1, arraysize(test_data1));
 
   FilePath mount_point;
   EXPECT_EQ(GetDeviceId(kDevice1),
             notifier()->GetDeviceIdForPath(test_path_a, &mount_point));
   EXPECT_EQ(test_path_a.value(), mount_point.value());
   EXPECT_EQ(GetDeviceId(kDevice4),
             notifier()->GetDeviceIdForPath(test_path_b, &mount_point));
   EXPECT_EQ(test_path_b.value(), mount_point.value());
@@ skipping 12 matching lines @@
   // 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);
+  EXPECT_CALL(observer(), OnRemovableStorageDetached(_)).Times(1);
   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