Change base::SystemMonitor's media device functions to take a type and string16 instead of a FilePa…

base/system_monitor/system_monitor.h

 // 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.
 
 #ifndef BASE_SYSTEM_MONITOR_SYSTEM_MONITOR_H_
 #define BASE_SYSTEM_MONITOR_SYSTEM_MONITOR_H_
 
+#include <map>
 #include <string>
 #include <vector>
 
 #include "base/base_export.h"
 #include "base/basictypes.h"
+#include "base/string16.h"
 #include "base/tuple.h"
 #include "build/build_config.h"
 
 // Windows HiRes timers drain the battery faster so we need to know the battery
 // status.  This isn't true for other platforms.
 #if defined(OS_WIN)
 #define ENABLE_BATTERY_MONITORING 1
 #else
 #undef ENABLE_BATTERY_MONITORING
 #endif  // !OS_WIN
 
 #include "base/observer_list_threadsafe.h"
 #if defined(ENABLE_BATTERY_MONITORING)
 #include "base/timer.h"
 #endif  // defined(ENABLE_BATTERY_MONITORING)
 
 #if defined(OS_MACOSX) && !defined(OS_IOS)
 #include <IOKit/pwr_mgt/IOPMLib.h>
 #include <IOKit/IOMessage.h>
 #endif  // OS_MACOSX && !OS_IOS
 
 #if defined(OS_IOS)
 #include <objc/runtime.h>
 #endif  // OS_IOS
 
-class FilePath;
-
 namespace base {
 
 // Class for monitoring various system-related subsystems
 // such as power management, network status, etc.
 // TODO(mbelshe):  Add support beyond just power management.
 class BASE_EXPORT SystemMonitor {
  public:
   // Normalized list of power events.
   enum PowerEvent {
     POWER_STATE_EVENT,  // The Power status of the system has changed.
     SUSPEND_EVENT,      // The system is being suspended.
     RESUME_EVENT        // The system is being resumed.
   };
 
+  enum MediaDeviceType {
+    TYPE_PATH,
+    TYPE_MTP,
+  };
+
   typedef unsigned int DeviceIdType;

[vandebo (ex-Chrome), 2012/07/19 19:05:20]

With unique_id being a string, I think you can get rid of DevieIdType.

[Lei Zhang, 2012/07/19 23:07:25]

Done.

-  // (Media device id, Media device name, Media device path)
-  typedef Tuple3<DeviceIdType, std::string, FilePath> MediaDeviceInfo;
+  // (Media device id, Media device name, Media device type, Media device data)
+  typedef Tuple4<DeviceIdType, std::string, MediaDeviceType, string16>

[vandebo (ex-Chrome), 2012/07/19 19:05:20]

Why a Tuple4 and not just a struct?

[Lei Zhang, 2012/07/19 21:17:10]

Because you were ok with a Tuple3 in the previous review? ;) A struct will be
more readable. I'll make the change on the next upload.

+      MediaDeviceInfo;
 
   // Create SystemMonitor. Only one SystemMonitor instance per application
   // is allowed.
   SystemMonitor();
   ~SystemMonitor();
 
   // Get the application-wide SystemMonitor (if not present, returns NULL).
   static SystemMonitor* Get();
 
 #if defined(OS_MACOSX)
@@ skipping 41 matching lines @@
   };
 
   class BASE_EXPORT DevicesChangedObserver {
    public:
     // Notification that the devices connected to the system have changed.
     // This is only implemented on Windows currently.
     virtual void OnDevicesChanged() {}
 
     // When a media device is attached or detached, one of these two events
     // is triggered.
-    // TODO(vandebo) Pass an appropriate device identifier or way to interact
-    // with the devices instead of FilePath.
     virtual void OnMediaDeviceAttached(const DeviceIdType& id,
                                        const std::string& name,
-                                       const FilePath& path) {}
+                                       MediaDeviceType type,
+                                       const string16& data) {}
 
     virtual void OnMediaDeviceDetached(const DeviceIdType& id) {}
 
    protected:
     virtual ~DevicesChangedObserver() {}
   };
 
   // Add a new observer.
   // Can be called from any thread.
   // Must not be called from within a notification callback.
@@ skipping 13 matching lines @@
   void ProcessWmPowerBroadcastMessage(int event_id);
 #endif
 
   // Cross-platform handling of a power event.
   void ProcessPowerMessage(PowerEvent event_id);
 
   // Cross-platform handling of a device change event.
   void ProcessDevicesChanged();
   void ProcessMediaDeviceAttached(const DeviceIdType& id,
                                   const std::string& name,
-                                  const FilePath& path);
+                                  MediaDeviceType type,
+                                  const string16& data);
+#if defined(OS_POSIX)
+  // Provided for convenience.
+  void ProcessMediaDeviceAttached(const DeviceIdType& id,
+                                  const std::string& name,
+                                  MediaDeviceType type,
+                                  const std::string& data);
+#endif
   void ProcessMediaDeviceDetached(const DeviceIdType& id);
 
   // Returns information for attached media devices.
   std::vector<MediaDeviceInfo> GetAttachedMediaDevices() const;

[vandebo (ex-Chrome), 2012/07/19 19:05:20]

This being down here implies that it's intended not for consumers of
SystemMonitor but for providers.  Should we move above all the Process* methods?

[Lei Zhang, 2012/07/19 23:07:25]

Done.

 
  private:
   typedef std::map<base::SystemMonitor::DeviceIdType,
                    MediaDeviceInfo> MediaDeviceMap;
 #if defined(OS_MACOSX)
   void PlatformInit();
   void PlatformDestroy();
 #endif
 
   // Platform-specific method to check whether the system is currently
   // running on battery power.  Returns true if running on batteries,
   // false otherwise.
   bool IsBatteryPower();
 
   // Checks the battery status and notifies observers if the battery
   // status has changed.
   void BatteryCheck();
 
   // Functions to trigger notifications.
   void NotifyDevicesChanged();
   void NotifyMediaDeviceAttached(const DeviceIdType& id,
                                  const std::string& name,
-                                 const FilePath& path);
+                                 MediaDeviceType type,
+                                 const string16& data);
   void NotifyMediaDeviceDetached(const DeviceIdType& id);
   void NotifyPowerStateChange();
   void NotifySuspend();
   void NotifyResume();
 
   scoped_refptr<ObserverListThreadSafe<PowerObserver> > power_observer_list_;
   scoped_refptr<ObserverListThreadSafe<DevicesChangedObserver> >
       devices_changed_observer_list_;
   bool battery_in_use_;
   bool suspended_;
 
 #if defined(ENABLE_BATTERY_MONITORING)
   base::OneShotTimer<SystemMonitor> delayed_battery_check_;
 #endif
 
 #if defined(OS_IOS)
   // Holds pointers to system event notification observers.
   std::vector<id> notification_observers_;
 #endif
 
   MediaDeviceMap media_device_map_;
 
   DISALLOW_COPY_AND_ASSIGN(SystemMonitor);
 };
 
 }  // namespace base
 
 #endif  // BASE_SYSTEM_MONITOR_SYSTEM_MONITOR_H_