[Win, MediaGallery] Enumerate and handle mtp device attach/detach events.

chrome/browser/system_monitor/removable_device_notifications_window_win_unittest.cc

Index: chrome/browser/system_monitor/removable_device_notifications_window_win_unittest.cc
diff --git a/chrome/browser/system_monitor/removable_device_notifications_window_win_unittest.cc b/chrome/browser/system_monitor/removable_device_notifications_window_win_unittest.cc
index 1417b1b28a4705d8ba1947abd50efec6eb918a69..02d3e00b0ed45a76e7cd4acb6b168dad5f703804 100644
--- a/chrome/browser/system_monitor/removable_device_notifications_window_win_unittest.cc
+++ b/chrome/browser/system_monitor/removable_device_notifications_window_win_unittest.cc
@@ -15,7 +15,9 @@
 #include "base/scoped_temp_dir.h"
 #include "base/system_monitor/system_monitor.h"
 #include "base/test/mock_devices_changed_observer.h"
+#include "base/threading/sequenced_worker_pool.h"
 #include "chrome/browser/system_monitor/media_storage_util.h"
+#include "chrome/browser/system_monitor/removable_device_constants.h"
 #include "chrome/browser/system_monitor/volume_mount_watcher_win.h"
 #include "content/public/test/test_browser_thread.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -26,9 +28,29 @@ namespace {
 using content::BrowserThread;
 using chrome::RemovableDeviceNotificationsWindowWin;
 
+// Mtp device interface path constants.
+const char16 kMtpDeviceWithInvalidInfo[] =
+    L"\\?\usb#vid_00&pid_00#0&2&1#{0000-0000-0000-0000-0000})";
+const char16 kMtpDeviceWithValidInfo[] =
+    L"\\?\usb#vid_ff&pid_000f#32&2&1#{abcd-1234-ffde-1112-9172})";
+const char16 kMtpDeviceWithMultipleStorage[] =
+    L"\\?\usb#vid_ff&pid_18#32&2&1#{ab33-1de4-f22e-1882-9724})";
+
+// Sample mtp device storage information.
+const char16 kMtpDeviceFriendlyName[] = L"Camera V1.1";
+const char16 kStorageLabelA[] = L"Camera V1.1 (s10001)";
+const char16 kStorageLabelB[] = L"Camera V1.1 (s20001)";
+const char16 kStorageObjectIdA[] = L"s10001";
+const char16 kStorageObjectIdB[] = L"s20001";
+const char kStorageUniqueIdA[] =
+    "mtp:StorageSerial:SID-{s10001, D, 12378}:123123";
+const char kStorageUniqueIdB[] =
+    "mtp:StorageSerial:SID-{s20001, S, 2238}:123123";
+
 // Inputs of 'A:\' - 'Z:\' are valid. 'N:\' is not removable.
-bool GetDeviceDetails(const FilePath& device_path, string16* device_location,
-                      std::string* unique_id, string16* name, bool* removable) {
+bool GetMassStorageDeviceDetails(const FilePath& device_path,
+    string16* device_location, std::string* unique_id, string16* name,
+    bool* removable) {

[Peter Kasting, 2012/10/19 21:31:12]

Nit: One arg per line, indented even (several places)

[kmadhusu, 2012/10/23 23:44:17]

Done.

   if (device_path.value().length() != 3 || device_path.value()[0] < L'A' ||
       device_path.value()[0] > L'Z') {
     return false;
@@ -65,10 +87,118 @@ std::vector<FilePath> GetTestAttachedDevices() {
   return result;
 }
 
+// Returns the persistent storage unique id of the device specified by the
+// |pnp_device_id|. |storage_object_id| specifies the temporary object
+// identifier that uniquely identifies the object on the device.
+std::string GetMtpStorageUniqueId(const string16& pnp_device_id,
+                                  const string16& storage_object_id)  {
+  if (pnp_device_id == string16(kMtpDeviceWithInvalidInfo))

[Peter Kasting, 2012/10/19 21:31:12]

Nit: No need for explicit string16 construction (many places)

[kmadhusu, 2012/10/23 23:44:17]

Done.

+    return std::string();
+
+  if (storage_object_id == string16(kStorageObjectIdA))
+    return kStorageUniqueIdA;
+
+  if (storage_object_id == string16(kStorageObjectIdB))
+    return kStorageUniqueIdB;
+  return std::string();

[Peter Kasting, 2012/10/19 21:31:12]

Nit: Or just:

  return (storage_object_id == kStorageObjectIdB) ?
      string16(kStorageUniqueIdB) : string16();

(several places)

[kmadhusu, 2012/10/23 23:44:17]

Done.

+}
+
+// Returns the storage name of the device specified by |pnp_device_id|.
+// |storage_object_id| specifies the temporary object identifier that
+// uniquely identifies the object on the device.
+string16 GetMtpStorageName(const string16& pnp_device_id,
+                           const string16& storage_object_id) {
+  if (pnp_device_id == string16(kMtpDeviceWithInvalidInfo))
+    return string16();
+
+  if (storage_object_id == string16(kStorageObjectIdA))
+    return string16(kStorageLabelA);
+
+  if (storage_object_id == string16(kStorageObjectIdB))
+    return string16(kStorageLabelB);
+  return string16();
+}
+
+// Returns a list of storage object identifiers of the device given a
+// |pnp_device_id|.
+std::vector<string16> GetMtpStorageObjectId(const string16& pnp_device_id) {

[Peter Kasting, 2012/10/19 21:31:12]

Nit: GetMtpStorageObjectIds?

[kmadhusu, 2012/10/23 23:44:17]

Done.

+  if (pnp_device_id == string16(kMtpDeviceWithInvalidInfo))
+    return std::vector<string16>();
+
+  std::vector<string16> storage_object_ids;
+  storage_object_ids.push_back(string16(kStorageObjectIdA));
+
+  if (pnp_device_id == string16(kMtpDeviceWithMultipleStorage))
+    storage_object_ids.push_back(string16(kStorageObjectIdB));
+  return storage_object_ids;
+}
+
+// Gets the mtp device storage details given a |pnp_device_id| and
+// |storage_object_id|. On success, returns true and fills in
+// |device_location|, |unique_id| and |name|.
+bool GetMtpStorageDetails(

[Peter Kasting, 2012/10/19 21:31:12]

This method can't fail, so return void.

[kmadhusu, 2012/10/23 23:44:17]

Done.

+    const string16& pnp_device_id, const string16& storage_object_id,
+    string16* device_location, std::string* unique_id, string16* name) {
+  if (device_location)
+    *device_location = pnp_device_id;
+
+  if (unique_id)
+    *unique_id = GetMtpStorageUniqueId(pnp_device_id, storage_object_id);
+
+  if (name)
+    *name = GetMtpStorageName(pnp_device_id, storage_object_id);
+  return true;
+}
+
 }  // namespace
 
 namespace chrome {
 
+// Wrapper class for testing PortableDeviceWatcherWin.
+class TestPortableDeviceWatcherWin : public PortableDeviceWatcherWin {
+ public:
+  TestPortableDeviceWatcherWin() {
+  }

[Peter Kasting, 2012/10/19 21:31:12]

Nit: Do not define functions inline in class declarations even for classes
declared in a .cc file.  Separate declarations and definitions.

Go ahead and fix the rest of this file while you're here.

[kmadhusu, 2012/10/23 23:44:17]

Done.

+
+ private:
+  // 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.
+  virtual ~TestPortableDeviceWatcherWin() {
+  }
+
+  // Override PortableDeviceWatcherWin::InitOnBlockingThread.

[Peter Kasting, 2012/10/19 21:31:12]

Nit: Don't comment individual function overrides this way.  Instead do this:

  // PortableDeviceWatcherWin:
  void InitOnBlockingThread() OVERRIDE;
  virtual string16 GetDeviceName(const string16& pnp_device_id) OVERRIDE;
  virtual bool GetStorages(const string16& pnp_device_id,
                           std::vector<DeviceStorageInfo>* storages) OVERRIDE;

[kmadhusu, 2012/10/23 23:44:17]

Done.

+  void InitOnBlockingThread() OVERRIDE {
+  }
+
+  // Override PortableDeviceWatcherWin::GetDeviceName.
+  virtual string16 GetDeviceName(const string16& pnp_device_id) OVERRIDE {
+    if (pnp_device_id == string16(kMtpDeviceWithInvalidInfo))
+      return string16();
+    return string16(kMtpDeviceFriendlyName);
+  }
+
+  // Override PortableDeviceWatcherWin::GetStorages.
+  virtual bool GetStorages(const string16& pnp_device_id,
+                           std::vector<DeviceStorageInfo>* storages) OVERRIDE {
+    if (pnp_device_id == string16(kMtpDeviceWithInvalidInfo))
+      return false;
+
+    std::vector<string16> storage_object_ids =
+        GetMtpStorageObjectId(pnp_device_id);
+    for (size_t index = 0; index < storage_object_ids.size(); ++index) {
+      DeviceStorageInfo storage;
+      storage.unique_id = GetMtpStorageUniqueId(pnp_device_id,
+                                                storage_object_ids[index]);
+      storage.storage_object_id = storage_object_ids[index];
+      storages->push_back(storage);
+    }
+    return true;
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(TestPortableDeviceWatcherWin);
+};
+
 // Wrapper class for testing VolumeMountWatcherWin.
 class TestVolumeMountWatcherWin : public VolumeMountWatcherWin {
  public:
@@ -79,8 +209,8 @@ class TestVolumeMountWatcherWin : public VolumeMountWatcherWin {
   virtual bool GetDeviceInfo(const FilePath& device_path,
     string16* device_location, std::string* unique_id, string16* name,
     bool* removable) OVERRIDE {
-    return GetDeviceDetails(device_path, device_location, unique_id, name,
-                            removable);
+    return GetMassStorageDeviceDetails(device_path, device_location, unique_id,
+                                       name, removable);
   }
 
   // Override VolumeMountWatcherWin::GetAttachedDevices().
@@ -108,8 +238,10 @@ class TestRemovableDeviceNotificationsWindowWin
     : public RemovableDeviceNotificationsWindowWin {
  public:
   explicit TestRemovableDeviceNotificationsWindowWin(
-      TestVolumeMountWatcherWin* volume_mount_watcher)
-      : RemovableDeviceNotificationsWindowWin(volume_mount_watcher),
+      TestVolumeMountWatcherWin* volume_mount_watcher,
+      TestPortableDeviceWatcherWin* portable_device_watcher)
+      : RemovableDeviceNotificationsWindowWin(
+            volume_mount_watcher, portable_device_watcher),
         volume_mount_watcher_(volume_mount_watcher) {
     DCHECK(volume_mount_watcher_);
   }
@@ -146,15 +278,16 @@ class RemovableDeviceNotificationsWindowWinTest : public testing::Test {
   virtual void SetUp() OVERRIDE {
     ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::UI));
     volume_mount_watcher_ = new TestVolumeMountWatcherWin;
+    portable_device_watcher_ = new TestPortableDeviceWatcherWin;
     window_.reset(new TestRemovableDeviceNotificationsWindowWin(
-        volume_mount_watcher_.get()));
+        volume_mount_watcher_.get(), portable_device_watcher_.get()));
     window_->InitWithTestData();
-    message_loop_.RunAllPending();
+    RunAllPending();
     system_monitor_.AddDevicesChangedObserver(&observer_);
   }
 
   virtual void TearDown() {
-    message_loop_.RunAllPending();
+    RunAllPending();
     system_monitor_.RemoveDevicesChangedObserver(&observer_);
   }
 
@@ -162,8 +295,8 @@ class RemovableDeviceNotificationsWindowWinTest : public testing::Test {
     std::string unique_id;
     string16 device_name;
     bool removable;
-    ASSERT_TRUE(GetDeviceDetails(drive, NULL, &unique_id, &device_name,
-                &removable));
+    ASSERT_TRUE(GetMassStorageDeviceDetails(drive, NULL, &unique_id,
+                                            &device_name, &removable));
     if (removable) {
       MediaStorageUtil::Type type =
           MediaStorageUtil::REMOVABLE_MASS_STORAGE_NO_DCIM;
@@ -183,13 +316,22 @@ class RemovableDeviceNotificationsWindowWinTest : public testing::Test {
         AddAttachExpectation(initial_devices[i]);
     }
     window_.reset(new TestRemovableDeviceNotificationsWindowWin(
-        volume_mount_watcher_.get()));
+        volume_mount_watcher_.get(), portable_device_watcher_.get()));
     window_->InitWithTestDataAndAttachedDevices();
+    RunAllPending();
+  }
+
+  // Runs all the pending tasks on UI thread and blocking thread.
+  void RunAllPending() {
+    message_loop_.RunAllPending();
+    content::BrowserThread::GetBlockingPool()->FlushForTesting();
     message_loop_.RunAllPending();
   }
 
-  void DoDevicesAttachedTest(const std::vector<int>& device_indices);
-  void DoDevicesDetachedTest(const std::vector<int>& device_indices);
+  void DoMassStorageDeviceAttachedTest(const std::vector<int>& device_indices);
+  void DoMassStorageDevicesDetachedTest(const std::vector<int>& device_indices);
+  void DoMtpDeviceAttachedTest(const string16& pnp_device_id);
+  void DoMtpDeviceDetachedTest(const string16& pnp_device_id);
 
   MessageLoopForUI message_loop_;
   content::TestBrowserThread ui_thread_;
@@ -199,9 +341,11 @@ class RemovableDeviceNotificationsWindowWinTest : public testing::Test {
   base::MockDevicesChangedObserver observer_;
   scoped_ptr<TestRemovableDeviceNotificationsWindowWin> window_;
   scoped_refptr<TestVolumeMountWatcherWin> volume_mount_watcher_;
+  scoped_refptr<TestPortableDeviceWatcherWin> portable_device_watcher_;
 };
 
-void RemovableDeviceNotificationsWindowWinTest::DoDevicesAttachedTest(
+void RemovableDeviceNotificationsWindowWinTest::
+DoMassStorageDeviceAttachedTest(
     const std::vector<int>& device_indices) {
   DEV_BROADCAST_VOLUME volume_broadcast;
   volume_broadcast.dbcv_size = sizeof(volume_broadcast);
@@ -219,10 +363,11 @@ void RemovableDeviceNotificationsWindowWinTest::DoDevicesAttachedTest(
   }
   window_->InjectDeviceChange(DBT_DEVICEARRIVAL,
                               reinterpret_cast<DWORD>(&volume_broadcast));
-  message_loop_.RunAllPending();
+  RunAllPending();
 }
 
-void RemovableDeviceNotificationsWindowWinTest::DoDevicesDetachedTest(
+void RemovableDeviceNotificationsWindowWinTest::
+DoMassStorageDevicesDetachedTest(
     const std::vector<int>& device_indices) {
   DEV_BROADCAST_VOLUME volume_broadcast;
   volume_broadcast.dbcv_size = sizeof(volume_broadcast);
@@ -237,8 +382,8 @@ void RemovableDeviceNotificationsWindowWinTest::DoDevicesDetachedTest(
       volume_broadcast.dbcv_unitmask |= 0x1 << *it;
       std::string unique_id;
       bool removable;
-      ASSERT_TRUE(GetDeviceDetails(DriveNumberToFilePath(*it), NULL, &unique_id,
-                                   NULL, &removable));
+      ASSERT_TRUE(GetMassStorageDeviceDetails(DriveNumberToFilePath(*it), NULL,
+                                              &unique_id, NULL, &removable));
       if (removable) {
         MediaStorageUtil::Type type =
             MediaStorageUtil::REMOVABLE_MASS_STORAGE_NO_DCIM;
@@ -249,12 +394,94 @@ void RemovableDeviceNotificationsWindowWinTest::DoDevicesDetachedTest(
   }
   window_->InjectDeviceChange(DBT_DEVICEREMOVECOMPLETE,
                               reinterpret_cast<DWORD>(&volume_broadcast));
-  message_loop_.RunAllPending();
+  RunAllPending();
+}
+
+void RemovableDeviceNotificationsWindowWinTest::DoMtpDeviceAttachedTest(
+    const string16& pnp_device_id) {
+  GUID guidDevInterface = GUID_NULL;
+  HRESULT hr = CLSIDFromString(kWPDDevInterfaceGUID, &guidDevInterface);
+  if (FAILED(hr))
+    return;
+
+  size_t device_id_size = pnp_device_id.size() * sizeof(char16);
+  size_t dbcc_name_size = device_id_size + sizeof(char16);

[Peter Kasting, 2012/10/19 21:31:12]

Nit: Inline into next statement (2 places)

[kmadhusu, 2012/10/23 23:44:17]

I would like to leave this as it is. To me, this is easy to read and understand.

[Peter Kasting, 2012/10/24 00:06:32]

Actually, now that I look closer, this statement is wrong.  You're reserving
space for a \0 terminator, but the struct already accounts for this by declaring
dbcc_name as a char[1].  So you shouldn't actually add another char16 here.

+  size_t size = sizeof(DEV_BROADCAST_DEVICEINTERFACE) + dbcc_name_size;
+  scoped_ptr_malloc<DEV_BROADCAST_DEVICEINTERFACE> dev_interface_broadcast(

[Peter Kasting, 2012/10/19 21:31:12]

Ugh, why are you using scoped_ptr_malloc<> and ZeroMemory() and memcpy() and the
like?  Why not just use a standard scoped_ptr<>?

[kmadhusu, 2012/10/23 23:44:17]

By default, DEV_BROADCAST_DEVICEINTERFACE dbcc_name field is a single element
array of type TCHAR. For unit tests, I want to overwrite that data field with a
pnp_device_id (whose length is > 1). So, I allocated sufficient memory using
scoped_ptr_malloc<>, initialized using ZeroMemory() and copied the data using
memcpy(). Since I am allocating more memory than the basic structure size, I
thought it is better to use scoped_ptr_malloc<> rather than doing
scoped_ptr<>(new char16[new_size]);

[Peter Kasting, 2012/10/24 00:06:32]

Yeah, you're fine.  I didn't really realize this struct is basically
variable-size.

+      static_cast<DEV_BROADCAST_DEVICEINTERFACE*>(malloc(size)));
+  DCHECK(dev_interface_broadcast.get());
+  ZeroMemory(dev_interface_broadcast.get(), size);
+  dev_interface_broadcast->dbcc_size = size;
+  dev_interface_broadcast->dbcc_devicetype = DBT_DEVTYP_DEVICEINTERFACE;
+  dev_interface_broadcast->dbcc_classguid = guidDevInterface;
+  memcpy(dev_interface_broadcast->dbcc_name, pnp_device_id.data(),
+         device_id_size);
+  {
+    testing::InSequence sequence;
+    std::vector<string16> storages = GetMtpStorageObjectId(pnp_device_id);
+    for (size_t index = 0; index < storages.size(); ++index) {
+      std::string unique_id;
+      string16 name;
+      string16 location;
+      ASSERT_TRUE(GetMtpStorageDetails(pnp_device_id, storages[index],
+                                       &location, &unique_id, &name));
+      int cardinality = 1;
+      if (name.empty() || unique_id.empty())
+        cardinality = 0;
+      EXPECT_CALL(observer_, OnRemovableStorageAttached(unique_id, name,
+                                                        location))
+          .Times(cardinality);
+    }
+  }
+  window_->InjectDeviceChange(
+      DBT_DEVICEARRIVAL,
+      reinterpret_cast<DWORD>(dev_interface_broadcast.get()));
+  RunAllPending();
+}
+
+void RemovableDeviceNotificationsWindowWinTest::DoMtpDeviceDetachedTest(
+    const string16& pnp_device_id) {
+  GUID guidDevInterface = GUID_NULL;
+  HRESULT hr = CLSIDFromString(kWPDDevInterfaceGUID, &guidDevInterface);
+  if (FAILED(hr))
+    return;
+
+  size_t device_id_size = pnp_device_id.size() * sizeof(char16);
+  size_t dbcc_name_size = device_id_size + sizeof(char16);
+  size_t size = sizeof(DEV_BROADCAST_DEVICEINTERFACE) + dbcc_name_size;
+  scoped_ptr_malloc<DEV_BROADCAST_DEVICEINTERFACE> dev_interface_broadcast(
+      static_cast<DEV_BROADCAST_DEVICEINTERFACE*>(malloc(size)));
+  DCHECK(dev_interface_broadcast.get());
+  ZeroMemory(dev_interface_broadcast.get(), size);
+  dev_interface_broadcast->dbcc_size = size;
+  dev_interface_broadcast->dbcc_devicetype = DBT_DEVTYP_DEVICEINTERFACE;
+  dev_interface_broadcast->dbcc_classguid = guidDevInterface;
+  memcpy(dev_interface_broadcast->dbcc_name, pnp_device_id.data(),
+         device_id_size);
+  {
+    testing::InSequence sequence;
+    std::vector<string16> storages = GetMtpStorageObjectId(pnp_device_id);
+    for (size_t index = 0; index < storages.size(); ++index) {
+      std::string unique_id;
+      string16 name;
+      ASSERT_TRUE(GetMtpStorageDetails(pnp_device_id, storages[index], NULL,
+                                       &unique_id, &name));
+      int cardinality = 1;
+      if (name.empty() || unique_id.empty())
+        cardinality = 0;
+      EXPECT_CALL(observer_, OnRemovableStorageDetached(unique_id))
+          .Times(cardinality);
+    }
+  }
+  window_->InjectDeviceChange(
+      DBT_DEVICEREMOVECOMPLETE,
+      reinterpret_cast<DWORD>(dev_interface_broadcast.get()));
+  RunAllPending();
 }
 
 TEST_F(RemovableDeviceNotificationsWindowWinTest, RandomMessage) {
   window_->InjectDeviceChange(DBT_DEVICEQUERYREMOVE, NULL);
-  message_loop_.RunAllPending();
+  RunAllPending();
 }
 
 TEST_F(RemovableDeviceNotificationsWindowWinTest, DevicesAttached) {
@@ -264,21 +491,21 @@ TEST_F(RemovableDeviceNotificationsWindowWinTest, DevicesAttached) {
   device_indices.push_back(7);
   device_indices.push_back(13);
 
-  DoDevicesAttachedTest(device_indices);
+  DoMassStorageDeviceAttachedTest(device_indices);
 }
 
 TEST_F(RemovableDeviceNotificationsWindowWinTest, DevicesAttachedHighBoundary) {
   std::vector<int> device_indices;
   device_indices.push_back(25);
 
-  DoDevicesAttachedTest(device_indices);
+  DoMassStorageDeviceAttachedTest(device_indices);
 }
 
 TEST_F(RemovableDeviceNotificationsWindowWinTest, DevicesAttachedLowBoundary) {
   std::vector<int> device_indices;
   device_indices.push_back(0);
 
-  DoDevicesAttachedTest(device_indices);
+  DoMassStorageDeviceAttachedTest(device_indices);
 }
 
 TEST_F(RemovableDeviceNotificationsWindowWinTest, DevicesAttachedAdjacentBits) {
@@ -288,7 +515,7 @@ TEST_F(RemovableDeviceNotificationsWindowWinTest, DevicesAttachedAdjacentBits) {
   device_indices.push_back(2);
   device_indices.push_back(3);
 
-  DoDevicesAttachedTest(device_indices);
+  DoMassStorageDeviceAttachedTest(device_indices);
 }
 
 TEST_F(RemovableDeviceNotificationsWindowWinTest, DevicesDetached) {
@@ -300,7 +527,7 @@ TEST_F(RemovableDeviceNotificationsWindowWinTest, DevicesDetached) {
   device_indices.push_back(7);
   device_indices.push_back(13);
 
-  DoDevicesDetachedTest(device_indices);
+  DoMassStorageDevicesDetachedTest(device_indices);
 }
 
 TEST_F(RemovableDeviceNotificationsWindowWinTest, DevicesDetachedHighBoundary) {
@@ -309,7 +536,7 @@ TEST_F(RemovableDeviceNotificationsWindowWinTest, DevicesDetachedHighBoundary) {
   std::vector<int> device_indices;
   device_indices.push_back(25);
 
-  DoDevicesDetachedTest(device_indices);
+  DoMassStorageDevicesDetachedTest(device_indices);
 }
 
 TEST_F(RemovableDeviceNotificationsWindowWinTest, DevicesDetachedLowBoundary) {
@@ -318,7 +545,7 @@ TEST_F(RemovableDeviceNotificationsWindowWinTest, DevicesDetachedLowBoundary) {
   std::vector<int> device_indices;
   device_indices.push_back(0);
 
-  DoDevicesDetachedTest(device_indices);
+  DoMassStorageDevicesDetachedTest(device_indices);
 }
 
 TEST_F(RemovableDeviceNotificationsWindowWinTest, DevicesDetachedAdjacentBits) {
@@ -330,7 +557,7 @@ TEST_F(RemovableDeviceNotificationsWindowWinTest, DevicesDetachedAdjacentBits) {
   device_indices.push_back(2);
   device_indices.push_back(3);
 
-  DoDevicesDetachedTest(device_indices);
+  DoMassStorageDevicesDetachedTest(device_indices);
 }
 
 TEST_F(RemovableDeviceNotificationsWindowWinTest, DeviceInfoFoPath) {
@@ -350,8 +577,8 @@ TEST_F(RemovableDeviceNotificationsWindowWinTest, DeviceInfoFoPath) {
   std::string unique_id;
   string16 device_name;
   bool removable;
-  ASSERT_TRUE(GetDeviceDetails(removable_device, NULL, &unique_id, &device_name,
-                               &removable));
+  ASSERT_TRUE(GetMassStorageDeviceDetails(removable_device, NULL, &unique_id,
+                                          &device_name, &removable));
   EXPECT_TRUE(removable);
   std::string device_id = MediaStorageUtil::MakeDeviceId(
       MediaStorageUtil::REMOVABLE_MASS_STORAGE_NO_DCIM, unique_id);
@@ -363,8 +590,8 @@ TEST_F(RemovableDeviceNotificationsWindowWinTest, DeviceInfoFoPath) {
   FilePath fixed_device(L"N:\\");
   EXPECT_TRUE(window_->GetDeviceInfoForPath(fixed_device, &device_info));
 
-  ASSERT_TRUE(GetDeviceDetails(fixed_device, NULL, &unique_id, &device_name,
-                               &removable));
+  ASSERT_TRUE(GetMassStorageDeviceDetails(fixed_device, NULL, &unique_id,
+                                          &device_name, &removable));
   EXPECT_FALSE(removable);
   device_id = MediaStorageUtil::MakeDeviceId(
       MediaStorageUtil::FIXED_MASS_STORAGE, unique_id);
@@ -373,4 +600,35 @@ TEST_F(RemovableDeviceNotificationsWindowWinTest, DeviceInfoFoPath) {
   EXPECT_EQ(fixed_device.value(), device_info.location);
 }
 
+// Test to verify basic mtp storage attach and detach notifications.
+TEST_F(RemovableDeviceNotificationsWindowWinTest, MtpDeviceBasicAttachDetach) {
+  // Attach a mtp device.
+  DoMtpDeviceAttachedTest(kMtpDeviceWithValidInfo);
+
+  // Detach the attached device.
+  DoMtpDeviceDetachedTest(kMtpDeviceWithValidInfo);
+}
+
+// When a mtp storage device with invalid storage label and id is
+// attached/detached, there should not be any device attach/detach
+// notifications.
+TEST_F(RemovableDeviceNotificationsWindowWinTest, MtpDeviceWithInvalidInfo) {
+  // Attach the mtp storage with invalid storage info.
+  DoMtpDeviceAttachedTest(kMtpDeviceWithInvalidInfo);
+
+  // Detach the attached storage.
+  DoMtpDeviceDetachedTest(kMtpDeviceWithInvalidInfo);
+}
+
+// Attach a device with two data storages. Verify that attach/detach
+// notifications are sent out for each removable storage.
+TEST_F(RemovableDeviceNotificationsWindowWinTest,
+       MtpDeviceWithMultipleStorages) {
+  // Attach the mtp device with multiple storage.
+  DoMtpDeviceAttachedTest(kMtpDeviceWithMultipleStorage);
+
+  // Detach the attached device.
+  DoMtpDeviceDetachedTest(kMtpDeviceWithMultipleStorage);
+}
+
 }  // namespace chrome