[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..3f5a5eb1f22958bab1398bf8db0e134bf3da5a5f 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
@@ -10,25 +10,54 @@
 #include <vector>
 
 #include "base/file_util.h"
+#include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
 #include "base/scoped_temp_dir.h"
 #include "base/system_monitor/system_monitor.h"
 #include "base/test/mock_devices_changed_observer.h"
+#include "base/utf_string_conversions.h"
 #include "chrome/browser/system_monitor/media_storage_util.h"
+#include "chrome/browser/system_monitor/portable_device_watcher_win.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"
 #include "testing/gtest/include/gtest/gtest.h"
 
+namespace chrome {
+
+typedef std::vector<FilePath> AttachedDevices;
+
 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 kMtpDeviceWithMultipleStorageObjects[] =
+    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) {
   if (device_path.value().length() != 3 || device_path.value()[0] < L'A' ||
       device_path.value()[0] > L'Z') {
     return false;
@@ -53,8 +82,8 @@ FilePath DriveNumberToFilePath(int drive_number) {
   return FilePath(path);
 }
 
-std::vector<FilePath> GetTestAttachedDevices() {
-  std::vector<FilePath> result;
+AttachedDevices GetTestAttachedDevices() {
+  AttachedDevices result;
   result.push_back(DriveNumberToFilePath(0));
   result.push_back(DriveNumberToFilePath(1));
   result.push_back(DriveNumberToFilePath(2));
@@ -65,30 +94,122 @@ 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 (storage_object_id == kStorageObjectIdA)
+    return kStorageUniqueIdA;
+  return (storage_object_id == kStorageObjectIdB) ?
+         kStorageUniqueIdB : std::string();

[Peter Kasting, 2012/10/29 21:57:19]

Nit: Indent 4 (2 places)

[kmadhusu, 2012/10/30 01:29:24]

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 == kMtpDeviceWithInvalidInfo)
+    return string16();
+
+  if (storage_object_id == kStorageObjectIdA)
+    return kStorageLabelA;
+  return (storage_object_id == kStorageObjectIdB) ?
+         kStorageLabelB : string16();
+}
+
+// Returns a list of storage object identifiers of the device given a
+// |pnp_device_id|.
+PortableDeviceWatcherWin::StorageObjectIDs GetMtpStorageObjectIds(
+    const string16& pnp_device_id) {
+  PortableDeviceWatcherWin::StorageObjectIDs storage_object_ids;
+  storage_object_ids.push_back(kStorageObjectIdA);
+  if (pnp_device_id == kMtpDeviceWithMultipleStorageObjects)
+    storage_object_ids.push_back(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|.
+void GetMtpStorageDetails(const string16& pnp_device_id,
+                          const string16& storage_object_id,
+                          string16* device_location,
+                          std::string* unique_id,
+                          string16* name) {
+  std::string storage_unique_id = GetMtpStorageUniqueId(pnp_device_id,
+                                                        storage_object_id);
+
+  if (device_location)
+    *device_location = UTF8ToUTF16("\\\\" + storage_unique_id);
+
+  if (unique_id)
+    *unique_id = storage_unique_id;
+
+  if (name)
+    *name = GetMtpStorageName(pnp_device_id, storage_object_id);
+}
+
+// Returns a list of device storage details for the given device specified by
+// |pnp_device_id|.
+PortableDeviceWatcherWin::StorageObjects GetDeviceStorageObjects(
+    const string16& pnp_device_id) {
+  PortableDeviceWatcherWin::StorageObjects storage_objects;
+  PortableDeviceWatcherWin::StorageObjectIDs storage_object_ids =
+      GetMtpStorageObjectIds(pnp_device_id);
+  for (PortableDeviceWatcherWin::StorageObjectIDs::const_iterator it =
+      storage_object_ids.begin();

[Peter Kasting, 2012/10/29 21:57:19]

Nit: Indent 1 more (for loop indenting is weird)

[kmadhusu, 2012/10/30 01:29:24]

Fixed. Moved the iterator declaration outside the loop.

PortableDeviceWatcherWin::StorageObjectIDs::const_iterator it =
storage_object_ids.begin();
for (; it != storage_object_ids.end(); ++it) {
}

[Peter Kasting, 2012/10/30 23:17:52]

No, do not do that.  Never declare loop variables outside the loop unless they
will be needed outside the loop.

[kmadhusu, 2012/10/30 23:49:50]

Fixed.

+       it != storage_object_ids.end(); ++it) {
+    storage_objects.push_back(PortableDeviceWatcherWin::DeviceStorageObject(
+        *it, GetMtpStorageUniqueId(pnp_device_id, *it)));
+  }
+  return storage_objects;
+}
+
 }  // namespace
 
-namespace chrome {
+// TestPortableDeviceWatcherWin -----------------------------------------------

[Peter Kasting, 2012/10/29 21:57:19]

Nit: Consider a blank line above these dividers (4 places)

[kmadhusu, 2012/10/30 01:29:24]

Done.

 
-// Wrapper class for testing VolumeMountWatcherWin.
-class TestVolumeMountWatcherWin : public VolumeMountWatcherWin {
+class TestPortableDeviceWatcherWin : public PortableDeviceWatcherWin {
  public:
-  TestVolumeMountWatcherWin() : pre_attach_devices_(false) {
-  }
+  TestPortableDeviceWatcherWin();
+  virtual ~TestPortableDeviceWatcherWin();
 
-  // Override VolumeMountWatcherWin::GetDeviceInfo().
-  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);
-  }
+ private:
+  // Override PortableDeviceWatcherWin:

[Peter Kasting, 2012/10/29 21:57:19]

Nit: Remove "override" (2 places)

[kmadhusu, 2012/10/30 01:29:24]

Done.

+  virtual void EnumerateAttachedDevices() OVERRIDE;
+  virtual void HandleDeviceAttachEvent(const string16& pnp_device_id) OVERRIDE;
 
-  // Override VolumeMountWatcherWin::GetAttachedDevices().
-  virtual std::vector<FilePath> GetAttachedDevices() OVERRIDE{
-    if (pre_attach_devices_)
-      return GetTestAttachedDevices();
-    return std::vector<FilePath>();
-  }
+  DISALLOW_COPY_AND_ASSIGN(TestPortableDeviceWatcherWin);
+};
+
+TestPortableDeviceWatcherWin::TestPortableDeviceWatcherWin() {
+}
+
+TestPortableDeviceWatcherWin::~TestPortableDeviceWatcherWin() {
+}
+
+void TestPortableDeviceWatcherWin::EnumerateAttachedDevices() {
+}
+
+void TestPortableDeviceWatcherWin::HandleDeviceAttachEvent(
+    const string16& pnp_device_id) {
+  DeviceDetails device_details = {
+      (pnp_device_id != kMtpDeviceWithInvalidInfo) ? kMtpDeviceFriendlyName :

[Peter Kasting, 2012/10/29 21:57:19]

Nit: Break after ?, not : (2 places)

[kmadhusu, 2012/10/30 01:29:24]

Done.

+          string16(),
+      pnp_device_id,
+      GetDeviceStorageObjects(pnp_device_id)
+  };
+  bool result(true);

[Peter Kasting, 2012/10/29 21:57:19]

Nit: Don't use constructor-style initialization for POD types

[kmadhusu, 2012/10/30 01:29:24]

Done.

+  OnDidHandleDeviceAttachEvent(&device_details, &result);
+}
+
+// TestVolumeMountWatcherWin --------------------------------------------------
+
+class TestVolumeMountWatcherWin : public VolumeMountWatcherWin {
+ public:
+  TestVolumeMountWatcherWin();
 
   void set_pre_attach_devices(bool pre_attach_devices) {
     pre_attach_devices_ = pre_attach_devices;
@@ -96,100 +217,111 @@ class TestVolumeMountWatcherWin : public VolumeMountWatcherWin {
 
  private:
   // Private, this class is ref-counted.
-  virtual ~TestVolumeMountWatcherWin() {
-  }
+  virtual ~TestVolumeMountWatcherWin();
+
+  // Override VolumeMountWatcherWin:
+  virtual bool GetDeviceInfo(const FilePath& device_path,
+                             string16* device_location,
+                             std::string* unique_id,
+                             string16* name,
+                             bool* removable) OVERRIDE;
+  virtual AttachedDevices GetAttachedDevices() OVERRIDE;
 
   // Set to true to pre-attach test devices.
   bool pre_attach_devices_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestVolumeMountWatcherWin);
 };
 
-// Wrapper class for testing RemovableDeviceNotificationsWindowWin.
+TestVolumeMountWatcherWin::TestVolumeMountWatcherWin()
+    : pre_attach_devices_(false) {
+}
+
+TestVolumeMountWatcherWin::~TestVolumeMountWatcherWin() {
+}
+
+bool TestVolumeMountWatcherWin::GetDeviceInfo(const FilePath& device_path,
+                                              string16* device_location,
+                                              std::string* unique_id,
+                                              string16* name,
+                                              bool* removable) {
+  return GetMassStorageDeviceDetails(device_path, device_location, unique_id,
+                                     name, removable);
+}
+
+AttachedDevices TestVolumeMountWatcherWin::GetAttachedDevices() {
+  return pre_attach_devices_ ? GetTestAttachedDevices() :
+                               std::vector<FilePath>();

[Peter Kasting, 2012/10/29 21:57:19]

Nit: AttachedDevices()

[kmadhusu, 2012/10/30 01:29:24]

Done.

+}
+
+// TestRemovableDeviceNotificationsWindowWin ------------

[Peter Kasting, 2012/10/29 21:57:19]

Nit: More dashes

[kmadhusu, 2012/10/30 01:29:24]

Done.

+
 class TestRemovableDeviceNotificationsWindowWin
     : public RemovableDeviceNotificationsWindowWin {
  public:
-  explicit TestRemovableDeviceNotificationsWindowWin(
-      TestVolumeMountWatcherWin* volume_mount_watcher)
-      : RemovableDeviceNotificationsWindowWin(volume_mount_watcher),
-        volume_mount_watcher_(volume_mount_watcher) {
-    DCHECK(volume_mount_watcher_);
-  }
+  TestRemovableDeviceNotificationsWindowWin(
+      TestVolumeMountWatcherWin* volume_mount_watcher,
+      TestPortableDeviceWatcherWin* portable_device_watcher);
 
-  virtual ~TestRemovableDeviceNotificationsWindowWin() {
-  }
+  virtual ~TestRemovableDeviceNotificationsWindowWin();
 
-  void InitWithTestData() {
-    volume_mount_watcher_->set_pre_attach_devices(false);
-    Init();
-  }
-
-  void InitWithTestDataAndAttachedDevices() {
-    volume_mount_watcher_->set_pre_attach_devices(true);
-    Init();
-  }
-
-  void InjectDeviceChange(UINT event_type, DWORD data) {
-    OnDeviceChange(event_type, data);
-  }
+  void InitWithTestData(bool pre_attach_devices);
+  void InjectDeviceChange(UINT event_type, DWORD data);
 
  private:
   scoped_refptr<TestVolumeMountWatcherWin> volume_mount_watcher_;
 };
 
+TestRemovableDeviceNotificationsWindowWin::
+    TestRemovableDeviceNotificationsWindowWin(
+    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_);
+}
+
+TestRemovableDeviceNotificationsWindowWin::
+    ~TestRemovableDeviceNotificationsWindowWin() {
+}
+
+void TestRemovableDeviceNotificationsWindowWin::InitWithTestData(
+    bool pre_attach_devices) {
+  volume_mount_watcher_->set_pre_attach_devices(pre_attach_devices);
+  Init();
+}
+
+void TestRemovableDeviceNotificationsWindowWin::InjectDeviceChange(
+    UINT event_type, DWORD data) {

[Peter Kasting, 2012/10/29 21:57:19]

Nit: One arg per line

[kmadhusu, 2012/10/30 01:29:24]

Done.

+  OnDeviceChange(event_type, data);
+}
+
+// RemovableDeviceNotificationsWindowWinTest ----------------------------------
+
 class RemovableDeviceNotificationsWindowWinTest : public testing::Test {
  public:
-  RemovableDeviceNotificationsWindowWinTest()
-      : ui_thread_(BrowserThread::UI, &message_loop_),
-        file_thread_(BrowserThread::FILE, &message_loop_) { }
-  virtual ~RemovableDeviceNotificationsWindowWinTest() { }
+  RemovableDeviceNotificationsWindowWinTest();
+  virtual ~RemovableDeviceNotificationsWindowWinTest();
 
  protected:
-  virtual void SetUp() OVERRIDE {
-    ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::UI));
-    volume_mount_watcher_ = new TestVolumeMountWatcherWin;
-    window_.reset(new TestRemovableDeviceNotificationsWindowWin(
-        volume_mount_watcher_.get()));
-    window_->InitWithTestData();
-    message_loop_.RunAllPending();
-    system_monitor_.AddDevicesChangedObserver(&observer_);
-  }
+  // testing::Test:
+  virtual void SetUp() OVERRIDE;
+  virtual void TearDown() OVERRIDE;
 
-  virtual void TearDown() {
-    message_loop_.RunAllPending();
-    system_monitor_.RemoveDevicesChangedObserver(&observer_);
-  }
+  void AddMassStorageDeviceAttachExpectation(FilePath drive);
+  void PreAttachDevices();
 
-  void AddAttachExpectation(FilePath drive) {
-    std::string unique_id;
-    string16 device_name;
-    bool removable;
-    ASSERT_TRUE(GetDeviceDetails(drive, NULL, &unique_id, &device_name,
-                &removable));
-    if (removable) {
-      MediaStorageUtil::Type type =
-          MediaStorageUtil::REMOVABLE_MASS_STORAGE_NO_DCIM;
-      std::string device_id = MediaStorageUtil::MakeDeviceId(type, unique_id);
-      EXPECT_CALL(observer_, OnRemovableStorageAttached(device_id, device_name,
-                                                        drive.value()))
-          .Times(1);
-    }
-  }
+  // Runs all the pending tasks on UI thread, FILE thread and blocking thread.
+  void RunAllPending();
 
-  void PreAttachDevices() {
-    window_.reset();
-    {
-      testing::InSequence sequence;
-      std::vector<FilePath> initial_devices = GetTestAttachedDevices();
-      for (size_t i = 0; i < initial_devices.size(); i++)
-        AddAttachExpectation(initial_devices[i]);
-    }
-    window_.reset(new TestRemovableDeviceNotificationsWindowWin(
-        volume_mount_watcher_.get()));
-    window_->InitWithTestDataAndAttachedDevices();
-    message_loop_.RunAllPending();
-  }
+  void DoMassStorageDeviceAttachedTest(const std::vector<int>& device_indices);

[Peter Kasting, 2012/10/29 21:57:19]

Nit: Consider a vector<int> typedef

[kmadhusu, 2012/10/30 01:29:24]

Done.

typedef std::vector<int> DeviceIndices;

+  void DoMassStorageDevicesDetachedTest(const std::vector<int>& device_indices);
 
-  void DoDevicesAttachedTest(const std::vector<int>& device_indices);
-  void DoDevicesDetachedTest(const std::vector<int>& device_indices);
+  // Helper function to test media transfer protocol device attach and detach
+  // events. Set |attached| to true, to test device attach event else set to
+  // false.

[Peter Kasting, 2012/10/29 21:57:19]

Nit: How about:

Injects a device attach or detach change (depeneding on the value of |attached|)
and tests that the appropriate handler is called.

[kmadhusu, 2012/10/30 01:29:24]

Done.

+  void DoMtpDeviceTest(const string16& pnp_device_id, bool attached);

[Peter Kasting, 2012/10/29 21:57:19]

Nit: Consider renaming |attached| to |test_attach|

[kmadhusu, 2012/10/30 01:29:24]

Done.

 
   MessageLoopForUI message_loop_;
   content::TestBrowserThread ui_thread_;
@@ -201,8 +333,70 @@ class RemovableDeviceNotificationsWindowWinTest : public testing::Test {
   scoped_refptr<TestVolumeMountWatcherWin> volume_mount_watcher_;
 };
 
-void RemovableDeviceNotificationsWindowWinTest::DoDevicesAttachedTest(
-    const std::vector<int>& device_indices) {
+RemovableDeviceNotificationsWindowWinTest::
+    RemovableDeviceNotificationsWindowWinTest()
+   : ui_thread_(BrowserThread::UI, &message_loop_),
+     file_thread_(BrowserThread::FILE, &message_loop_) {
+}
+
+RemovableDeviceNotificationsWindowWinTest::
+    ~RemovableDeviceNotificationsWindowWinTest() {
+}
+
+void RemovableDeviceNotificationsWindowWinTest::SetUp() {
+  ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  volume_mount_watcher_ = new TestVolumeMountWatcherWin;
+  window_.reset(new TestRemovableDeviceNotificationsWindowWin(
+      volume_mount_watcher_.get(), new TestPortableDeviceWatcherWin));
+  window_->InitWithTestData(false);
+  RunAllPending();
+  system_monitor_.AddDevicesChangedObserver(&observer_);
+}
+
+void RemovableDeviceNotificationsWindowWinTest::TearDown() {
+  RunAllPending();
+  system_monitor_.RemoveDevicesChangedObserver(&observer_);
+}
+
+void RemovableDeviceNotificationsWindowWinTest::
+    AddMassStorageDeviceAttachExpectation(FilePath drive) {
+  std::string unique_id;
+  string16 device_name;
+  bool removable;
+  ASSERT_TRUE(GetMassStorageDeviceDetails(drive, NULL, &unique_id,
+                                          &device_name, &removable));
+  if (removable) {
+    MediaStorageUtil::Type type =
+        MediaStorageUtil::REMOVABLE_MASS_STORAGE_NO_DCIM;
+    std::string device_id = MediaStorageUtil::MakeDeviceId(type, unique_id);
+    EXPECT_CALL(observer_, OnRemovableStorageAttached(device_id, device_name,
+                                                      drive.value()))
+        .Times(1);
+  }
+}
+
+void RemovableDeviceNotificationsWindowWinTest::PreAttachDevices() {
+  window_.reset();
+  {
+    testing::InSequence sequence;
+    AttachedDevices initial_devices = GetTestAttachedDevices();
+    for (AttachedDevices::const_iterator it = initial_devices.begin();
+         it != initial_devices.end(); ++it) {
+      AddMassStorageDeviceAttachExpectation(*it);
+    }
+  }
+  window_.reset(new TestRemovableDeviceNotificationsWindowWin(
+      volume_mount_watcher_.get(), new TestPortableDeviceWatcherWin));
+  window_->InitWithTestData(true);
+  RunAllPending();
+}
+
+void RemovableDeviceNotificationsWindowWinTest::RunAllPending() {
+  message_loop_.RunAllPending();
+}
+
+void RemovableDeviceNotificationsWindowWinTest::
+    DoMassStorageDeviceAttachedTest(const std::vector<int>& device_indices) {
   DEV_BROADCAST_VOLUME volume_broadcast;
   volume_broadcast.dbcv_size = sizeof(volume_broadcast);
   volume_broadcast.dbcv_devicetype = DBT_DEVTYP_VOLUME;
@@ -214,16 +408,16 @@ void RemovableDeviceNotificationsWindowWinTest::DoDevicesAttachedTest(
          it != device_indices.end();
          ++it) {
       volume_broadcast.dbcv_unitmask |= 0x1 << *it;
-      AddAttachExpectation(DriveNumberToFilePath(*it));
+      AddMassStorageDeviceAttachExpectation(DriveNumberToFilePath(*it));
     }
   }
   window_->InjectDeviceChange(DBT_DEVICEARRIVAL,
                               reinterpret_cast<DWORD>(&volume_broadcast));
-  message_loop_.RunAllPending();
+  RunAllPending();
 }
 
-void RemovableDeviceNotificationsWindowWinTest::DoDevicesDetachedTest(
-    const std::vector<int>& device_indices) {
+void RemovableDeviceNotificationsWindowWinTest::
+    DoMassStorageDevicesDetachedTest(const std::vector<int>& device_indices) {
   DEV_BROADCAST_VOLUME volume_broadcast;
   volume_broadcast.dbcv_size = sizeof(volume_broadcast);
   volume_broadcast.dbcv_devicetype = DBT_DEVTYP_VOLUME;
@@ -237,8 +431,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 +443,57 @@ void RemovableDeviceNotificationsWindowWinTest::DoDevicesDetachedTest(
   }
   window_->InjectDeviceChange(DBT_DEVICEREMOVECOMPLETE,
                               reinterpret_cast<DWORD>(&volume_broadcast));
-  message_loop_.RunAllPending();
+  RunAllPending();
+}
+
+void RemovableDeviceNotificationsWindowWinTest::DoMtpDeviceTest(
+    const string16& pnp_device_id, bool attached) {
+  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 size = sizeof(DEV_BROADCAST_DEVICEINTERFACE) + device_id_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;
+    PortableDeviceWatcherWin::StorageObjectIDs storage_object_ids =
+        GetMtpStorageObjectIds(pnp_device_id);
+    for (PortableDeviceWatcherWin::StorageObjectIDs::const_iterator it =
+        storage_object_ids.begin();
+         it != storage_object_ids.end(); ++it) {
+      std::string unique_id;
+      string16 name;
+      string16 location;
+      GetMtpStorageDetails(pnp_device_id, *it, &location, &unique_id, &name);
+      if (attached) {
+        EXPECT_CALL(observer_, OnRemovableStorageAttached(unique_id, name,
+                                                          location))
+            .Times((name.empty() || unique_id.empty()) ? 0 : 1);
+      } else {
+        EXPECT_CALL(observer_, OnRemovableStorageDetached(unique_id))
+          .Times((name.empty() || unique_id.empty()) ? 0 : 1);
+      }
+    }
+  }
+  window_->InjectDeviceChange(
+      attached ? DBT_DEVICEARRIVAL : 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 +503,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 +527,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 +539,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 +548,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 +557,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 +569,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 +589,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 +602,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 +612,26 @@ 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) {
+  DoMtpDeviceTest(kMtpDeviceWithValidInfo, true);
+  DoMtpDeviceTest(kMtpDeviceWithValidInfo, false);
+}
+
+// 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) {
+  DoMtpDeviceTest(kMtpDeviceWithInvalidInfo, true);
+  DoMtpDeviceTest(kMtpDeviceWithInvalidInfo, false);
+}
+
+// Attach a device with two data partitions. Verify that attach/detach
+// notifications are sent out for each removable storage.
+TEST_F(RemovableDeviceNotificationsWindowWinTest,
+       MtpDeviceWithMultipleStorageObjects) {
+  DoMtpDeviceTest(kMtpDeviceWithMultipleStorageObjects, true);
+  DoMtpDeviceTest(kMtpDeviceWithMultipleStorageObjects, false);
+}
+
 }  // namespace chrome