Redesigned and refactored ScopedMTPDeviceMapEntry, MTPDeviceMapService & MTPDeviceDelegate classes.

webkit/fileapi/media/mtp_device_map_service.h

Index: webkit/fileapi/media/mtp_device_map_service.h
diff --git a/webkit/fileapi/media/mtp_device_map_service.h b/webkit/fileapi/media/mtp_device_map_service.h
index 3002097ad7963f4361317e850148ba163c46d64f..87f00cbc4fd5fbab6a810e478f20efeee6ca135c 100644
--- a/webkit/fileapi/media/mtp_device_map_service.h
+++ b/webkit/fileapi/media/mtp_device_map_service.h
@@ -8,8 +8,8 @@
 #include <map>
 
 #include "base/file_path.h"
-#include "base/memory/ref_counted.h"
 #include "base/memory/singleton.h"
+#include "base/memory/weak_ptr.h"
 #include "base/threading/thread_checker.h"
 #include "webkit/storage/webkit_storage_export.h"
 
@@ -17,43 +17,43 @@ namespace fileapi {
 
 class MTPDeviceDelegate;
 
-// Helper class to manage media device delegates which can communicate with mtp
-// devices to complete media file system operations.
+// This class provides media transfer protocol(MTP) device delegate to

[Lei Zhang, 2012/11/21 01:33:30]

nit: space after protocol

[kmadhusu, 2012/11/21 04:09:53]

Done.

+// complete media file system operations. ScopedMTPDeviceMapEntry class

[Lei Zhang, 2012/11/21 01:33:30]

I think it's fine to mention related classes, but the comments to the methods
don't belong here. If someone changes ScopedMTPDeviceMapEntry, how will they
know they have to also update a comment in another file?

Comments should explain how the code is used, not exactly who gets to call it
from where.

[kmadhusu, 2012/11/21 04:09:53]

Done.

+// manages the device map entries. This class operates on the IO thread.
 class WEBKIT_STORAGE_EXPORT MTPDeviceMapService {
  public:
   static MTPDeviceMapService* GetInstance();
 
-  // Adds the media device delegate for the given |device_location|. Called on
-  // IO thread.
+  // Called from ScopedMTPDeviceMapEntry constructor to add the delegate for the
+  // MTP device specified by |device_location|. Called on the IO thread.
   void AddDelegate(const FilePath::StringType& device_location,
-                   scoped_refptr<MTPDeviceDelegate> delegate);
+                   MTPDeviceDelegate* delegate);
 
-  // Removes the media device delegate for the given |device_location| if
-  // exists. Called on IO thread.
+  // Called from ScopedMTPDeviceMapEntry destructor to remove the delegate for
+  // the MTP device specified by |device_location|. Called on the IO thread.
   void RemoveDelegate(const FilePath::StringType& device_location);
 
   // Gets the media device delegate associated with |filesystem_id|.
   // Return NULL if the |filesystem_id| is no longer valid (e.g. because the
-  // corresponding device is detached etc). Called on IO thread.
-  MTPDeviceDelegate* GetMTPDeviceDelegate(const std::string& filesystem_id);
+  // corresponding device is detached etc). Called by
+  // IsolatedMountPointProvider::CreateFileSystemOperation() on IO thread.
+ MTPDeviceDelegate* GetMTPDeviceDelegate(const std::string& filesystem_id);
 
  private:
   friend struct DefaultSingletonTraits<MTPDeviceMapService>;
 
-  typedef scoped_refptr<MTPDeviceDelegate> MTPDeviceDelegateObj;
-
-  // Mapping of device_location and MTPDeviceDelegate object.
-  typedef std::map<FilePath::StringType, MTPDeviceDelegateObj> DelegateMap;
+  // Mapping of device_location and MTPDeviceDelegate* object. It is safe to
+  // store and access the raw pointer. This class operates on the IO thread.
+  typedef std::map<FilePath::StringType, MTPDeviceDelegate*> DelegateMap;
 
   // Get access to this class using GetInstance() method.
   MTPDeviceMapService();
   ~MTPDeviceMapService();
 
-  // Stores a map of attached mtp device delegates.
+  // Map of attached mtp device delegates.
   DelegateMap delegate_map_;
 
-  // Stores a |thread_checker_| object to verify all methods of this class are
-  // called on same thread.
+  // Object to verify all methods of this class are called on the same thread.
   base::ThreadChecker thread_checker_;
 
   DISALLOW_COPY_AND_ASSIGN(MTPDeviceMapService);