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

chrome/browser/system_monitor/removable_device_notifications_window_win.cc

Index: chrome/browser/system_monitor/removable_device_notifications_window_win.cc
diff --git a/chrome/browser/system_monitor/removable_device_notifications_window_win.cc b/chrome/browser/system_monitor/removable_device_notifications_window_win.cc
index 74250b9ba9b7fcdbd60b32d7ce2615fcc7342c4d..d73613b1a54c3c71b4d93e9af0a5ff5588b015c9 100644
--- a/chrome/browser/system_monitor/removable_device_notifications_window_win.cc
+++ b/chrome/browser/system_monitor/removable_device_notifications_window_win.cc
@@ -8,12 +8,11 @@
 #include <dbt.h>
 #include <fileapi.h>
 
-#include "base/file_path.h"
 #include "base/win/wrapped_window_proc.h"
 #include "chrome/browser/system_monitor/media_storage_util.h"
-
-using base::SystemMonitor;
-using base::win::WrappedWindowProc;
+#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"
 
 namespace chrome {
 
@@ -26,20 +25,65 @@ RemovableDeviceNotificationsWindowWin*
 
 }  // namespace
 
+///////////////////////////////////////////////////////////////////////////
+// RemovableDeviceNotificationsWindowWin::PortableDeviceNotifications    //
+//                                                                       //
+// Manages portable device notification handle for                       //
+// RemovableDeviceNotificationsWindowWin.                                //
+///////////////////////////////////////////////////////////////////////////

[Peter Kasting, 2012/10/26 23:46:23]

Nit: As in other file, I suggest:

...
}  // namespace


// RemovableDeviceNotificationsWindowWin::PortableDeviceNotifications ---------

class RemovableDeviceNotificationsWindowWin::PortableDeviceNotifications {
...

I don't think the descriptive comment you had adds much so I'd omit it.

[kmadhusu, 2012/10/28 22:57:16]

Done.

+class RemovableDeviceNotificationsWindowWin::PortableDeviceNotifications {
+ public:
+  // Register window handle to receive portable device notifications details.
+  explicit PortableDeviceNotifications(HWND hwnd);
+
+  // Unregisters device notifications.
+  ~PortableDeviceNotifications();
+
+ private:
+  HDEVNOTIFY notifications_;
+
+  DISALLOW_IMPLICIT_CONSTRUCTORS(PortableDeviceNotifications);
+};
+
+RemovableDeviceNotificationsWindowWin::PortableDeviceNotifications::
+PortableDeviceNotifications(HWND hwnd) {

[Peter Kasting, 2012/10/26 23:46:23]

Nit: Indent 4

[kmadhusu, 2012/10/28 22:57:16]

Done. Some reviewer said if the function name spans over a line, we don't need
to indent 4 spaces.

+  GUID dev_interface_guid = GUID_NULL;
+  HRESULT hr = CLSIDFromString(kWPDDevInterfaceGUID, &dev_interface_guid);
+  if (FAILED(hr))
+    return;
+  DEV_BROADCAST_DEVICEINTERFACE db = {sizeof(DEV_BROADCAST_DEVICEINTERFACE),

[Peter Kasting, 2012/10/26 23:46:23]

Nit: I suggest:

  DEV_BROADCAST_DEVICEINTERFACE db = {
      sizeof(DEV_BROADCAST_DEVICEINTERFACE),
      DBT_DEVTYP_DEVICEINTERFACE,
      0,
      dev_interface_guid
  };

This makes it easier for a reader to compare this with a declaration of the
struct in question to see what members get what values.

[kmadhusu, 2012/10/28 22:57:16]

Done.

+                                      DBT_DEVTYP_DEVICEINTERFACE, 0,
+                                      dev_interface_guid};
+  notifications_ = RegisterDeviceNotification(hwnd, &db,
+                                              DEVICE_NOTIFY_WINDOW_HANDLE);
+}
+
+RemovableDeviceNotificationsWindowWin::PortableDeviceNotifications::
+~PortableDeviceNotifications() {

[Peter Kasting, 2012/10/26 23:46:23]

Nit: Indent 4

[kmadhusu, 2012/10/28 22:57:16]

Done.

+  UnregisterDeviceNotification(notifications_);
+}
+
+///////////////////////////////////////////////////////////////////////////
+//      RemovableDeviceNotificationsWindowWin implementaion              //
+///////////////////////////////////////////////////////////////////////////
 // static
 RemovableDeviceNotificationsWindowWin*
 RemovableDeviceNotificationsWindowWin::Create() {
-  return new RemovableDeviceNotificationsWindowWin(new VolumeMountWatcherWin());
+  return new RemovableDeviceNotificationsWindowWin(
+      new VolumeMountWatcherWin(), new PortableDeviceWatcherWin());
 }
 
 RemovableDeviceNotificationsWindowWin::
 RemovableDeviceNotificationsWindowWin(
-    VolumeMountWatcherWin* volume_mount_watcher)
+    VolumeMountWatcherWin* volume_mount_watcher,
+    PortableDeviceWatcherWin* portable_device_watcher)
     : window_class_(0),
       instance_(NULL),
       window_(NULL),
-      volume_mount_watcher_(volume_mount_watcher) {
+      volume_mount_watcher_(volume_mount_watcher),
+      portable_device_watcher_(portable_device_watcher) {
   DCHECK(volume_mount_watcher_);
+  DCHECK(portable_device_watcher_);
   DCHECK(!g_removable_device_notifications_window_win);
   g_removable_device_notifications_window_win = this;
 }
@@ -65,11 +109,13 @@ RemovableDeviceNotificationsWindowWin::GetInstance() {
 
 void RemovableDeviceNotificationsWindowWin::Init() {
   volume_mount_watcher_->Init();
+  portable_device_watcher_->Init();
 
   WNDCLASSEX window_class;
   base::win::InitializeWindowClass(
       kWindowClassName,
-      &WrappedWindowProc<RemovableDeviceNotificationsWindowWin::WndProcThunk>,
+      &base::win::WrappedWindowProc<
+          RemovableDeviceNotificationsWindowWin::WndProcThunk>,
       0, 0, 0, NULL, NULL, NULL, NULL, NULL,
       &window_class);
   instance_ = window_class.hInstance;
@@ -79,6 +125,8 @@ void RemovableDeviceNotificationsWindowWin::Init() {
   window_ = CreateWindow(MAKEINTATOM(window_class_), 0, 0, 0, 0, 0, 0, 0, 0,
                          instance_, 0);
   SetWindowLongPtr(window_, GWLP_USERDATA, reinterpret_cast<LONG_PTR>(this));
+  portable_device_notifications_.reset(
+      new PortableDeviceNotifications(window_));
 }
 
 bool RemovableDeviceNotificationsWindowWin::GetDeviceInfoForPath(
@@ -94,11 +142,11 @@ bool RemovableDeviceNotificationsWindowWin::GetDeviceInfoForPath(
   // To compute the device id, the device type is needed.  For removable
   // devices, that requires knowing if there's a DCIM directory, which would
   // require bouncing over to the file thread.  Instead, just iterate the
-  // devices in SystemMonitor.
+  // devices in base::SystemMonitor.
   std::string device_id;
   if (removable) {
-    std::vector<SystemMonitor::RemovableStorageInfo> attached_devices =
-        SystemMonitor::Get()->GetAttachedRemovableStorage();
+    std::vector<base::SystemMonitor::RemovableStorageInfo> attached_devices =
+        base::SystemMonitor::Get()->GetAttachedRemovableStorage();
     bool found = false;
     for (size_t i = 0; i < attached_devices.size(); i++) {
       MediaStorageUtil::Type type;
@@ -153,6 +201,9 @@ LRESULT CALLBACK RemovableDeviceNotificationsWindowWin::WndProc(
 bool RemovableDeviceNotificationsWindowWin::GetDeviceInfo(
     const FilePath& device_path, string16* device_location,
     std::string* unique_id, string16* name, bool* removable) {
+  // TODO(kmadhusu) Implement PortableDeviceWatcherWin::GetDeviceInfo()
+  // function when we have the functionality to add a sub directory of
+  // portable device as a media gallery.
   return volume_mount_watcher_->GetDeviceInfo(device_path, device_location,
                                               unique_id, name, removable);
 }
@@ -160,6 +211,7 @@ bool RemovableDeviceNotificationsWindowWin::GetDeviceInfo(
 void RemovableDeviceNotificationsWindowWin::OnDeviceChange(UINT event_type,
                                                            LPARAM data) {
   volume_mount_watcher_->OnWindowMessage(event_type, data);
+  portable_device_watcher_->OnWindowMessage(event_type, data);
 }
 
 }  // namespace chrome