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

chrome/browser/system_monitor/portable_device_watcher_win.cc

Index: chrome/browser/system_monitor/portable_device_watcher_win.cc
diff --git a/chrome/browser/system_monitor/portable_device_watcher_win.cc b/chrome/browser/system_monitor/portable_device_watcher_win.cc
new file mode 100644
index 0000000000000000000000000000000000000000..a3b16b9759cb983e33d11e59d1384befef8e80d6
--- /dev/null
+++ b/chrome/browser/system_monitor/portable_device_watcher_win.cc
@@ -0,0 +1,599 @@
+// 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.
+
+#include "chrome/browser/system_monitor/portable_device_watcher_win.h"
+
+#include <dbt.h>
+#include <portabledevice.h>
+
+#include "base/file_path.h"
+#include "base/i18n/case_conversion.h"
+#include "base/logging.h"
+#include "base/stl_util.h"
+#include "base/stringprintf.h"
+#include "base/threading/sequenced_worker_pool.h"
+#include "base/utf_string_conversions.h"
+#include "base/win/scoped_co_mem.h"
+#include "chrome/browser/system_monitor/media_storage_util.h"
+#include "chrome/browser/system_monitor/removable_device_constants.h"
+#include "chrome/common/chrome_notification_types.h"
+#include "content/public/browser/notification_service.h"
+
+namespace chrome {
+
+using base::Bind;
+using base::SequencedTaskRunner;
+using base::SystemMonitor;
+using base::win::ScopedCoMem;
+using base::win::ScopedComPtr;
+using chrome::MediaStorageUtil;
+using content::BrowserThread;

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

Please avoid using statements unless they save a great deal of space.  We prefer
explicit qualification when possible.

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

Done.

+
+namespace {
+
+// Name of the client application that communicates with the mtp device.
+const char16 kClientName[] = L"Chromium";
+
+// Returns true if |data| represents a class of portable devices.
+bool IsPortableDeviceStructure(LPARAM data) {
+  DEV_BROADCAST_HDR* broadcast_hdr =
+      reinterpret_cast<DEV_BROADCAST_HDR*>(data);
+  if (!broadcast_hdr ||
+      broadcast_hdr->dbch_devicetype != DBT_DEVTYP_DEVICEINTERFACE) {

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

Nit: {} not necessary with one-line conditional bodies (multiple places)

Also, I suggest parens around binary subexpressions (multiple places)

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

If the condition spans over a line, I thought it is better to have parens.
Done.

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

It's not banned, but it's not required either.

+    return false;
+  }
+
+  GUID guidDevInterface = GUID_NULL;
+  if (FAILED(CLSIDFromString(kWPDDevInterfaceGUID, &guidDevInterface)))
+    return false;
+  DEV_BROADCAST_DEVICEINTERFACE* dev_interface =
+      reinterpret_cast<DEV_BROADCAST_DEVICEINTERFACE*>(data);
+  return (IsEqualGUID(dev_interface->dbcc_classguid, guidDevInterface) != 0);
+}
+
+// Returns the portable device plug and play device ID string.
+string16 GetPnpDeviceId(LPARAM data) {
+  DEV_BROADCAST_DEVICEINTERFACE* dev_interface =
+      reinterpret_cast<DEV_BROADCAST_DEVICEINTERFACE*>(data);
+  if (!dev_interface)
+    return string16();
+  return base::i18n::ToLower(dev_interface->dbcc_name);

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

Is ToLower() really appropriate here?  It's locale-specific, so if we actually
need lowering of unicode text or the like it's unlikely to be correct.  Why do
you need to change case at all?

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

When a mtp device is attached, DEV_BROADCAST_DEVICEINTERFACE provides an upper
case plug and play id string (E.g:
L"\\?\USB#VID_FF&PID_000F#32&2&1#{ABCD-1234-FFFE-1112-9172})"). But
IPortableDeviceManager uses lower case pnp device id string. In order to make
the string comparisons simpler, I changed this string to a lower case value.

Modified code to use StringToLowerASCII() function. I think that is the right
way to convert a string16 string.

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

It's correct as long as you're guaranteed that the input is ASCII.  I couldn't
find such a guarantee on MSDN.  Are you sure the input will always be ASCII? 
(Might want a DCHECK(IsStringASCII()) if so, to document this.)

+}
+
+// Returns the friendly name of the device specified by the |pnp_device_id|.
+string16 GetFriendlyName(
+    const string16& pnp_device_id,
+    const IPortableDeviceManager* portable_device_mgr) {
+  DWORD name_len = 0;
+  LPWSTR device_id = const_cast<char16*>(pnp_device_id.c_str());
+  IPortableDeviceManager* device_manager =
+      const_cast<IPortableDeviceManager*>(portable_device_mgr);

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

Don't const-cast.  Take a non-const pointer.  If this means callers can't be
const functions then so be it.

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

Done.

+  HRESULT hr = device_manager->GetDeviceFriendlyName(device_id, NULL,
+                                                     &name_len);
+  if (FAILED(hr))
+    return string16();
+
+  scoped_array<char16> friendly_name(new char16[name_len]);
+  ZeroMemory(friendly_name.get(), name_len);

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

Don't do this... use WriteInto() with a standard string16.  (several places)

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

Done.

+  hr = device_manager->GetDeviceFriendlyName(device_id, friendly_name.get(),
+                                             &name_len);
+  if (FAILED(hr))
+    return string16();
+  return string16(friendly_name.get());

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

Nit: No need for explicit string16 conversions (several places)

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

Done.

+}
+
+// Returns the manufacturer name of the device specified by the
+// |pnp_device_id|.
+string16 GetManufacturerName(
+    const string16& pnp_device_id,
+    const IPortableDeviceManager* portable_device_mgr) {
+  DWORD name_len = 0;
+  LPWSTR device_id = const_cast<char16*>(pnp_device_id.c_str());
+  IPortableDeviceManager* device_manager =
+      const_cast<IPortableDeviceManager*>(portable_device_mgr);
+  HRESULT hr = device_manager->GetDeviceManufacturer(device_id, NULL,
+                                                     &name_len);
+  if (FAILED(hr))
+    return string16();
+
+  scoped_array<char16> manufacturer_name(new char16[name_len]);
+  ZeroMemory(manufacturer_name.get(), name_len);
+  hr = device_manager->GetDeviceManufacturer(device_id,
+                                             manufacturer_name.get(),
+                                             &name_len);
+  if (FAILED(hr))
+    return string16();
+  return string16(manufacturer_name.get());
+}
+
+// Returns the description of the device specified by the |pnp_device_id|.
+string16 GetDeviceDescription(
+    const string16& pnp_device_id,
+    const IPortableDeviceManager* portable_device_mgr) {
+  DWORD desc_len = 0;
+  LPWSTR device_id = const_cast<char16*>(pnp_device_id.c_str());
+  IPortableDeviceManager* device_manager =
+      const_cast<IPortableDeviceManager*>(portable_device_mgr);
+  HRESULT hr = device_manager->GetDeviceDescription(device_id, NULL,
+                                                    &desc_len);
+  if (FAILED(hr))
+    return string16();
+
+  scoped_array<char16> description(new char16[desc_len]);
+  ZeroMemory(description.get(), desc_len);
+  hr = device_manager->GetDeviceDescription(device_id, description.get(),
+                                            &desc_len);
+  if (FAILED(hr))
+    return string16();
+  return string16(description.get());
+}
+
+// On success, returns true and updates |client_info| with a reference to an
+// IPortableDeviceValues interface that holds information about the
+// application that communicates with the device.
+bool GetClientInformation(ScopedComPtr<IPortableDeviceValues>* client_info) {
+  HRESULT hr = client_info->CreateInstance(__uuidof(PortableDeviceValues),
+                                           NULL, CLSCTX_INPROC_SERVER);
+  if (FAILED(hr)) {
+    DPLOG(ERROR) << "Failed to create an instance of IPortableDeviceValues";

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

I've never even heard of DPLOG.  What the heck is it?

If we really need logging we normally use DVLOG(1), but generally logging is
discouraged unless you absolutely know you have to have it since it's hard to
get log data back from user machines.  (many places)

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

DPLOG appends the last system error (GetLastError()) to the log message in
string form. As you pointed, it's hard to get log data back from the users.
Therefore, I removed most of the messages and retained only the important ones.

+    return false;
+  }
+
+  hr = (*client_info)->SetStringValue(WPD_CLIENT_NAME, kClientName);
+  if (FAILED(hr))
+    DPLOG(ERROR) << "Failed to set client name";
+
+  hr = (*client_info)->SetUnsignedIntegerValue(WPD_CLIENT_MAJOR_VERSION, 0);
+  if (FAILED(hr))
+    DPLOG(ERROR) << "Failed to set client major version";
+
+  hr = (*client_info)->SetUnsignedIntegerValue(WPD_CLIENT_MINOR_VERSION, 0);
+  if (FAILED(hr))
+    DPLOG(ERROR) << "Failed to set client minor version";
+
+  hr = (*client_info)->SetUnsignedIntegerValue(WPD_CLIENT_REVISION, 0);
+  if (FAILED(hr))
+    DPLOG(ERROR) << "Failed to set client revision";
+
+  hr = (*client_info)->SetUnsignedIntegerValue(
+      WPD_CLIENT_SECURITY_QUALITY_OF_SERVICE, SECURITY_IMPERSONATION);
+  if (FAILED(hr))
+    DPLOG(ERROR) << "Failed to set security quality of service";
+
+  hr = (*client_info)->SetUnsignedIntegerValue(WPD_CLIENT_DESIRED_ACCESS,
+                                               GENERIC_READ);
+  if (FAILED(hr))
+    DPLOG(ERROR) << "Failed to set client desired access value";
+  return true;

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

It seems strange that we check for a bunch of failures above but then continue
anyway and return true.  Is this correct?  Can these calls ever actually fail?

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

These calls can fail. It is not mandatory to set all these client information.
We are just attempting to set all these client details. We can always open the
device for communication without providing any of these client details.

These failure log messages are not important. So, I am removing these log
messages.

+}
+
+// Opens the device for communication. |pnp_device_id| specifies the plug and
+// play device ID string. On success, returns true and updates |device| with a
+// reference to the portable device interface.
+bool Setup(const string16& pnp_device_id,
+           ScopedComPtr<IPortableDevice>* device) {
+  ScopedComPtr<IPortableDeviceValues> client_info;
+  if (!GetClientInformation(&client_info))
+    return false;
+
+  HRESULT hr = device->CreateInstance(__uuidof(PortableDevice), NULL,
+                                      CLSCTX_INPROC_SERVER);
+  if (FAILED(hr)) {
+    DPLOG(ERROR) << "Failed to create an instance of IPortableDevice";
+    return false;
+  }
+
+  LPWSTR device_id = const_cast<char16*>(pnp_device_id.c_str());

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

Again, don't const_cast.  In this case MSDN claims Open() takes an LPCWSTR so
you shouldn't need to supply a non-const pointer anyway.

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

Done.

+  hr = (*device)->Open(device_id, client_info.get());
+  if (FAILED(hr)) {
+    if (hr == E_ACCESSDENIED)
+      DPLOG(ERROR) << "Access denied to open the device";
+    return false;
+  }
+  return true;
+}
+
+// Returns true if the object specified by the |object_id| is the device root
+// object.
+bool IsDeviceObjectId(const string16& object_id) {
+  return object_id == WPD_DEVICE_OBJECT_ID;
+}
+
+// Returns the unique id property key of the object specified by the
+// |object_id|.
+REFPROPERTYKEY GetUniqueIdPropertyKey(const string16& object_id) {
+  if (IsDeviceObjectId(object_id))

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

Nit: Or just

  return IsDeviceObjectId(object_id) ?
      WPD_DEVICE_SERIAL_NUMBER : WPD_OBJECT_PERSISTENT_UNIQUE_ID;

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

Done.

+    return WPD_DEVICE_SERIAL_NUMBER;
+  return WPD_OBJECT_PERSISTENT_UNIQUE_ID;
+}
+
+// On success, returns true and populates |properties_to_read| with the
+// property key of the object specified by the |object_id|.
+bool PopulatePropertyKeyCollection(
+    const string16& object_id,
+    ScopedComPtr<IPortableDeviceKeyCollection>* properties_to_read) {
+  HRESULT hr = properties_to_read->CreateInstance(
+      __uuidof(PortableDeviceKeyCollection), NULL, CLSCTX_INPROC_SERVER);
+  if (FAILED(hr)) {
+    DPLOG(ERROR) << "Failed to create IPortableDeviceKeyCollection instance";
+    return false;
+  }
+  REFPROPERTYKEY key = GetUniqueIdPropertyKey(object_id);
+  hr = (*properties_to_read)->Add(key);
+  return SUCCEEDED(hr);
+}
+
+// Wrapper function to get content property string value.
+bool GetStringPropertyValue(
+    const ScopedComPtr<IPortableDeviceValues>& properties_values,

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

Nit: Passing ScopedComPtr by const ref is weird.  I suggest passing a const
IPortableDeviceValues& instead unless you actually need some functionality of
the ScopedComPtr class.  (several places)

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

When I pass "const IPortableDeviceValues&", I need to do a const_cast before
calling IPortableDeviceValues functions. Therefore, modified function to take
"IPortableDeviceValues*" directly.

+    REFPROPERTYKEY key, string16* value) {
+  ScopedCoMem<char16> buffer;
+  HRESULT hr = properties_values->GetStringValue(key, &buffer);
+  if (FAILED(hr))
+    return false;
+
+  if (value)
+    *value = string16(buffer);
+  return true;
+}
+
+// Constructs a unique identifier for the object specified by the |object_id|.
+// On success, returns true and fills in |unique_id|.
+bool GetObjectUniqueId(const ScopedComPtr<IPortableDevice>& device,
+                       const string16& object_id,
+                       string16* unique_id) {
+  ScopedComPtr<IPortableDeviceContent> content;
+  HRESULT hr = device->Content(content.Receive());
+  if (FAILED(hr)) {
+    DPLOG(ERROR) << "Failed to get IPortableDeviceContent interface";
+    return false;
+  }
+
+  ScopedComPtr<IPortableDeviceProperties> properties;
+  hr = content->Properties(properties.Receive());
+  if (FAILED(hr)) {
+    DPLOG(ERROR) << "Failed to get IPortableDeviceProperties interface";
+    return false;
+  }
+
+  ScopedComPtr<IPortableDeviceKeyCollection> properties_to_read;
+  if (!PopulatePropertyKeyCollection(object_id, &properties_to_read))
+    return false;
+
+  ScopedComPtr<IPortableDeviceValues> properties_values;
+  hr = properties->GetValues(const_cast<char16*>(object_id.c_str()),
+                             properties_to_read.get(),
+                             properties_values.Receive());
+  if (FAILED(hr)) {
+    DPLOG(ERROR) << "Failed to get property values";
+    return false;
+  }
+
+  REFPROPERTYKEY key = GetUniqueIdPropertyKey(object_id);
+  return GetStringPropertyValue(properties_values, key, unique_id);
+}
+
+// Constructs the device storage unique identifier using |device_serial_num| and
+// |storage_id|. On success, returns true and fills in |device_storage_id|.
+bool GetDeviceStorageUniqueId(const string16& device_serial_num,
+                              const string16& storage_id,
+                              std::string* device_storage_id) {
+  if (device_serial_num.empty() && storage_id.empty())
+    return false;
+
+  // |unique_id| format: StorageSerial:|storage_id|:|device_serial_num|
+  string16 unique_id = base::StringPrintf(L"%ls%ls%ls%ls",

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

Nit: I find it clearer to just do:

  string16 unique_id(chrome::kMtpDeviceStorageIdPrefix + storage_id +
      chrome::kNonSpaceDelim + device_serial_num);

It's slightly less efficient but who cares.

You can even inline this into the next statement.

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

I would like to leave it as it is. I have added the format of |unique_id| to
understand this code.

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

Are you sure %ls is correct on non-Windows, then?  I thought %ls was "wchar_t*",
which a string16 isn't (on non-Windows).

The benefit of just appending strings together is that not only is the code
shorter but you're safed against incorrect type specifiers.

+                                          chrome::kMtpDeviceStorageIdPrefix,
+                                          storage_id.c_str(),
+                                          chrome::kNonSpaceDelim,
+                                          device_serial_num.c_str());
+
+  // |device_storage_id| format: mtp:|unique_id|
+  *device_storage_id = MediaStorageUtil::MakeDeviceId(
+       MediaStorageUtil::MTP_OR_PTP, UTF16ToUTF8(unique_id));
+  return true;
+}
+
+// Gets a list of removable storage object identifiers present in |device|.
+// On success, returns true and fills in |storage_ids|.
+bool GetRemovableStorageObjectIds(const ScopedComPtr<IPortableDevice>& device,
+                                  std::vector<string16>* storage_ids) {
+  ScopedComPtr<IPortableDeviceCapabilities> capabilities;
+  HRESULT hr = device->Capabilities(capabilities.Receive());
+  if (FAILED(hr)) {
+    DPLOG(ERROR) << "Failed to get IPortableDeviceCapabilities interface";
+    return false;
+  }
+
+  ScopedComPtr<IPortableDevicePropVariantCollection> storage_obj_ids;
+  hr = capabilities->GetFunctionalObjects(WPD_FUNCTIONAL_CATEGORY_STORAGE,
+                                          storage_obj_ids.Receive());
+  if (FAILED(hr)) {
+    DPLOG(ERROR) << "Failed to get IPortableDevicePropVariantCollection";
+    return false;
+  }
+
+  DWORD num_storage_obj_ids = 0;
+  hr = storage_obj_ids->GetCount(&num_storage_obj_ids);
+  if (FAILED(hr))
+    return false;
+
+  for (DWORD index = 0; index < num_storage_obj_ids; ++index) {
+    PROPVARIANT object_id = {0};
+    PropVariantInit(&object_id);
+    hr = storage_obj_ids->GetAt(index, &object_id);
+    if (SUCCEEDED(hr) && object_id.pwszVal != NULL &&
+        object_id.vt == VT_LPWSTR) {
+      storage_ids->push_back(object_id.pwszVal);
+    }
+    PropVariantClear(&object_id);
+  }
+  return true;
+}
+
+}  // namespace
+
+PortableDeviceWatcherWin::PortableDeviceWatcherWin()
+    : on_shutdown_event_(true, false) {
+  registrar_.Add(this, chrome::NOTIFICATION_APP_TERMINATING,
+                 content::NotificationService::AllSources());
+}
+
+void PortableDeviceWatcherWin::Init() {
+  base::SequencedWorkerPool* pool = BrowserThread::GetBlockingPool();
+  base::SequencedWorkerPool::SequenceToken media_sequence_token =
+      pool->GetNamedSequenceToken("media-task-runner");
+  media_task_runner_ = pool->GetSequencedTaskRunner(media_sequence_token);
+  DCHECK(media_task_runner_);
+
+  media_task_runner_->PostTask(
+      FROM_HERE, Bind(&PortableDeviceWatcherWin::InitOnBlockingThread, this));
+}
+
+bool PortableDeviceWatcherWin::GetDeviceInfo(const FilePath& device_path,
+                                             string16* location,
+                                             std::string* unique_id,
+                                             string16* name,
+                                             bool* removable) {
+  NOTIMPLEMENTED();
+  return false;
+}
+
+void PortableDeviceWatcherWin::OnWindowMessage(UINT event_type, LPARAM data) {

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

I can't find anywhere that actually calls this or supplies its address to
anyone?

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

It is called from RemovableDeviceNotificationsWindowWin::OnDeviceChange()
function. (Ref:
http://codereview.chromium.org/11088012/diff/15004/chrome/browser/system_moni...)

+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  switch (event_type) {
+    case DBT_DEVICEARRIVAL: {
+      if (IsPortableDeviceStructure(data)) {
+        DCHECK(media_task_runner_.get());
+        media_task_runner_->PostTask(
+            FROM_HERE,
+            Bind(&PortableDeviceWatcherWin::
+                      HandleDeviceAttachEventOnBlockingThread,
+                 this, GetPnpDeviceId(data)));
+      }
+      break;
+    }
+    case DBT_DEVICEREMOVECOMPLETE: {
+      if (IsPortableDeviceStructure(data))
+        HandleDeviceDetachEventOnUIThread(GetPnpDeviceId(data));
+      break;
+    }
+  }
+}
+
+PortableDeviceWatcherWin::~PortableDeviceWatcherWin() {
+  registrar_.RemoveAll();

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

Nit: Not necessary, this happens automatically

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

Removed.

+}
+
+void PortableDeviceWatcherWin::InitOnBlockingThread() {
+  DCHECK(media_task_runner_->RunsTasksOnCurrentThread());
+  if (on_shutdown_event_.IsSignaled()) {
+    // Process is in shut down mode.
+    return;
+  }
+  DCHECK(!portable_device_mgr_.get());
+  HRESULT hr = portable_device_mgr_.CreateInstance(
+      __uuidof(PortableDeviceManager), NULL, CLSCTX_INPROC_SERVER);
+  if (FAILED(hr)) {
+    // Either there is no portable device support (it must be a XP with
+    // Windows Media Player Version < 10) or the thread does not have COM
+    // initialized.
+    if (hr == CO_E_NOTINITIALIZED)

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

Nit: DCHECK_NE(CO_E_NOTINITIALIZED, hr);

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

Done.

+      NOTREACHED();
+    return;
+  }
+
+  EnumerateStoragesOnBlockingThread();
+}
+
+string16 PortableDeviceWatcherWin::GetDeviceName(
+    const string16& pnp_device_id) {
+  DCHECK(media_task_runner_->RunsTasksOnCurrentThread());
+  if (on_shutdown_event_.IsSignaled()) {
+    // Process is in shut down mode.
+    return string16();
+  }
+
+  DCHECK(portable_device_mgr_.get());
+  string16 name = GetFriendlyName(pnp_device_id, portable_device_mgr_.get());

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

Nit: If you change these functions to return a bool, you can condense this to
just:

  string16 name;
  return GetFriendlyName(pnp_device_id, portable_device_mgr_.get()) ||
      GetDeviceDescription(pnp_device_id, portable_device_mgr_.get()) ||
      GetManufacturerName(pnp_device_id, portable_device_mgr_.get());

The fact that all the functions do the same mucking with getting a device ID and
an IPortableDeviceManager* initially suggests that you should factor that code
out to have it happen here, and then pass those values in directly.

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

Condensed the code as suggested. 

+  if (!name.empty())
+    return name;
+
+  name = GetDeviceDescription(pnp_device_id, portable_device_mgr_.get());
+  if (!name.empty())
+    return name;
+
+  return GetManufacturerName(pnp_device_id, portable_device_mgr_.get());
+}
+
+bool PortableDeviceWatcherWin::GetStorages(
+    const string16& pnp_device_id,
+    std::vector<DeviceStorageInfo>* storages) {
+  DCHECK(media_task_runner_->RunsTasksOnCurrentThread());
+  if (on_shutdown_event_.IsSignaled()) {
+    // Process is in shut down mode.
+    return false;
+  }
+
+  ScopedComPtr<IPortableDevice> device;
+  if (!Setup(pnp_device_id, &device))
+    return false;
+
+  std::vector<string16> storage_obj_ids;
+  if (!GetRemovableStorageObjectIds(device, &storage_obj_ids))
+    return false;
+
+  // Get the device serial number (E.g.: 4889033500677371).
+  string16 device_serial_num;
+  if (!GetObjectUniqueId(device, WPD_DEVICE_OBJECT_ID, &device_serial_num))
+    return false;
+
+  for (size_t index = 0; index < storage_obj_ids.size(); ++index) {
+    // Get the storage object persistent id (E.g.: SID-{10001,D,31080448}).
+    string16 storage_unique_id;
+    if (!GetObjectUniqueId(device, storage_obj_ids[index],
+                           &storage_unique_id)) {
+      continue;
+    }
+    std::string device_storage_id;
+    if (GetDeviceStorageUniqueId(device_serial_num, storage_unique_id,
+                                 &device_storage_id)) {
+      DeviceStorageInfo storage_info;
+      storage_info.storage_object_id = storage_obj_ids[index];
+      storage_info.unique_id = device_storage_id;
+      storages->push_back(storage_info);
+    }
+  }
+  return true;
+}
+
+void PortableDeviceWatcherWin::EnumerateStoragesOnBlockingThread() {
+  DCHECK(media_task_runner_->RunsTasksOnCurrentThread());
+  if (on_shutdown_event_.IsSignaled()) {
+    // Process is in shut down mode.
+    return;
+  }
+
+  DCHECK(portable_device_mgr_.get());
+
+  // Get the total number of devices found on the system.
+  DWORD pnp_device_count = 0;
+  HRESULT hr = portable_device_mgr_->GetDevices(NULL, &pnp_device_count);
+  if (FAILED(hr))
+    return;
+
+  scoped_array<LPWSTR> pnp_device_ids(new LPWSTR[pnp_device_count]);
+  ZeroMemory(pnp_device_ids.get(), pnp_device_count);
+  hr = portable_device_mgr_->GetDevices(
+      reinterpret_cast<LPWSTR*>(pnp_device_ids.get()), &pnp_device_count);
+  if (FAILED(hr))
+    return;
+
+  for (DWORD index = 0; index < pnp_device_count; ++index) {

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

Nit: {} unnecessary

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

Done.

+    GetDeviceInfoOnBlockingThread(pnp_device_ids[index]);
+  }
+}
+
+void PortableDeviceWatcherWin::HandleDeviceAttachEventOnBlockingThread(
+    const string16& pnp_device_id) {
+  DCHECK(media_task_runner_->RunsTasksOnCurrentThread());
+
+  if (on_shutdown_event_.IsSignaled()) {
+    // Process is in shut down mode.
+    return;
+  }
+
+  // |portable_device_mgr_| is uninitialized during unit tests.
+  if (portable_device_mgr_.get())
+    portable_device_mgr_->RefreshDeviceList();
+  GetDeviceInfoOnBlockingThread(pnp_device_id);
+}
+
+void PortableDeviceWatcherWin::HandleDeviceDetachEventOnUIThread(
+    const string16& pnp_device_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  MtpDeviceMap::iterator device_entry = device_map_.find(pnp_device_id);
+  if (device_entry == device_map_.end())
+    return;
+
+  SystemMonitor* system_monitor = SystemMonitor::Get();
+  DCHECK(system_monitor);
+
+  StorageInfoList storages = device_entry->second;
+  for (size_t index = 0; index < storages.size(); ++index) {
+    std::string storage_id = storages[index].unique_id;
+    MtpStorageMap::iterator storage_entry = storage_map_.find(storage_id);
+    if (storage_entry == storage_map_.end()) {
+      NOTREACHED();
+      continue;

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

Don't handle assertion failure.

This block should just be a DCHECK_NE(storage_map_.end(), storage_entry);

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

DCHECK_NE doesn't seem to work with iterators. Modified code to use
DCHECK(storage_entry != storage_map_.end())

+    }
+    system_monitor->ProcessRemovableStorageDetached(
+        storage_entry->second.device_id);
+    storage_map_.erase(storage_entry);
+  }
+  device_map_.erase(device_entry);
+}
+
+void PortableDeviceWatcherWin::GetDeviceInfoOnBlockingThread(
+    const string16& pnp_device_id) {
+  DCHECK(media_task_runner_->RunsTasksOnCurrentThread());
+  DCHECK(!pnp_device_id.empty());
+  string16 device_name = GetDeviceName(pnp_device_id);
+  StorageInfoList storages;
+  if (!GetStorages(pnp_device_id, &storages))
+    return;
+
+  if (on_shutdown_event_.IsSignaled()) {

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

Nit: Shouldn't this be checked before we make any other calls, to save wasted
work?

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

Done.

+    // Process is in shut down mode.
+    return;
+  }
+
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      Bind(&PortableDeviceWatcherWin::AddMtpDeviceOnUIThread,
+           this, storages, device_name, pnp_device_id));
+}
+
+void PortableDeviceWatcherWin::AddMtpDeviceOnUIThread(
+    const StorageInfoList& storages,
+    const string16& name,
+    const string16& location) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(!ContainsKey(device_map_, location));
+  SystemMonitor* system_monitor = SystemMonitor::Get();
+  DCHECK(system_monitor);
+  for (size_t index = 0; index < storages.size(); ++index) {
+    const std::string& storage_id = storages[index].unique_id;
+    DCHECK(!ContainsKey(storage_map_, storage_id));
+
+    // Keep track of storage id and storage name to see how often we receive
+    // empty values.
+    MediaStorageUtil::RecordDeviceInfoHistogram(false, storage_id, name);
+    if (storage_id.empty() || name.empty())
+      return;
+
+    // Device can have several data storages. Therefore, add the partition
+    // details to the storage name.
+    // E.g.: "Nexus 7 (SID-{10001,D,31080448})"
+    string16 storage_name = base::StringPrintf(
+        L"%ls (%ls)", name.c_str(), storages[index].storage_object_id.c_str());
+
+    SystemMonitor::RemovableStorageInfo storage_info(storage_id, storage_name,
+                                                     location);
+    storage_map_[storage_id] = storage_info;
+    system_monitor->ProcessRemovableStorageAttached(storage_id, storage_name,
+                                                    location);
+  }
+  device_map_[location] = storages;
+}
+
+void PortableDeviceWatcherWin::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  DCHECK_EQ(chrome::NOTIFICATION_APP_TERMINATING, type);
+  on_shutdown_event_.Signal();
+}
+
+}  // namespace chrome