Redesigned and refactored ScopedMTPDeviceMapEntry, MTPDeviceMapService & MTPDeviceDelegate classes.

chrome/browser/media_gallery/media_file_system_registry.h

Index: chrome/browser/media_gallery/media_file_system_registry.h
diff --git a/chrome/browser/media_gallery/media_file_system_registry.h b/chrome/browser/media_gallery/media_file_system_registry.h
index fe1236571a2e357f8d16b5c1b7a30fb1c3b4dbbd..fc8e7e0425051cae59f9b478f0ebdb99c7ad3219 100644
--- a/chrome/browser/media_gallery/media_file_system_registry.h
+++ b/chrome/browser/media_gallery/media_file_system_registry.h
@@ -9,6 +9,7 @@
 #define CHROME_BROWSER_MEDIA_GALLERY_MEDIA_FILE_SYSTEM_REGISTRY_H_
 
 #include <map>
+#include <set>
 #include <string>
 #include <utility>
 #include <vector>
@@ -17,7 +18,6 @@
 #include "base/lazy_instance.h"
 #include "base/file_path.h"
 #include "base/memory/ref_counted.h"
-#include "base/memory/weak_ptr.h"
 #include "base/prefs/public/pref_change_registrar.h"
 #include "base/system_monitor/system_monitor.h"
 #include "webkit/fileapi/media/mtp_device_file_system_config.h"
@@ -57,38 +57,22 @@ struct MediaFileSystemInfo {
 };
 
 #if defined(SUPPORT_MTP_DEVICE_FILESYSTEM)
-// Class to manage MTPDeviceDelegateImpl object for the attached MTP device.
-// Refcounted to reuse the same MTP device delegate entry across extensions.
-// This class supports WeakPtr (extends SupportsWeakPtr) to expose itself as
-// a weak pointer to MediaFileSystemRegistry.
-class ScopedMTPDeviceMapEntry
-    : public base::RefCounted<ScopedMTPDeviceMapEntry>,
-      public base::SupportsWeakPtr<ScopedMTPDeviceMapEntry> {
+// Class to manage the lifetime of MTPDeviceDelegateImpl object for the attached
+// media transfer protocol(MTP) device. This object is constructed for each MTP

[Lei Zhang, 2012/11/21 02:32:48]

nit: space after protocol.

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

Done.

+// device.
+class ScopedMTPDeviceMapEntry {
  public:
-  // |no_references_callback| is called when the last ScopedMTPDeviceMapEntry
-  // reference goes away.
-  ScopedMTPDeviceMapEntry(const FilePath::StringType& device_location,
-                          const base::Closure& no_references_callback);
+  explicit ScopedMTPDeviceMapEntry(const FilePath::StringType& device_location);
 
- private:
-  // Friend declaration for ref counted implementation.
-  friend class base::RefCounted<ScopedMTPDeviceMapEntry>;
-
-  // Private because this class is ref-counted.
+  // Destructed when the last user of this MTP device is destroyed or when the
+  // MTP device is detached from the system or when the browser is in shutdown
+  // mode or when the last extension revokes the MTP device gallery permissions.
   ~ScopedMTPDeviceMapEntry();
 
-  // Store the MTP or PTP device location.
+ private:
+  // The MTP or PTP device location.
   const FilePath::StringType device_location_;
 
-  // Store a raw pointer of MTPDeviceDelegateImpl object.
-  // MTPDeviceDelegateImpl is ref-counted and owned by MTPDeviceMapService.
-  // This class tells MTPDeviceMapService to dispose of it when the last
-  // reference to |this| goes away.
-  MTPDeviceDelegateImpl* delegate_;
-
-  // A callback to call when the last reference of this object goes away.
-  base::Closure no_references_callback_;
-
   DISALLOW_COPY_AND_ASSIGN(ScopedMTPDeviceMapEntry);
 };
 #endif
@@ -103,12 +87,16 @@ class MediaFileSystemContext {
       const std::string& device_id, const FilePath& path) = 0;
 
 #if defined(SUPPORT_MTP_DEVICE_FILESYSTEM)
-  // Registers and returns the file system id for the MTP or PTP device
-  // specified by |device_id| and |path|. Updates |entry| with the corresponding
-  // ScopedMTPDeviceMapEntry object.
+  // For the given |galleries_host|, this function returns the file system id
+  // of the registered MTP or PTP device specified by |device_id| and |path|.
   virtual std::string RegisterFileSystemForMTPDevice(
       const std::string& device_id, const FilePath& path,
-      scoped_refptr<ScopedMTPDeviceMapEntry>* entry) = 0;
+      const ExtensionGalleriesHost* galleries_host) = 0;
+
+  // Removes the MTP or PTP device reference for the given |galleries_host|.
+  virtual void RemoveMTPDeviceReferenceForHost(
+      const FilePath::StringType& device_location,
+      const ExtensionGalleriesHost* galleries_host) = 0;
 #endif
 
   // Revoke the passed |fsid|.
@@ -161,11 +149,19 @@ class MediaFileSystemRegistry
   typedef std::map<Profile*, PrefChangeRegistrar*> PrefChangeRegistrarMap;
 
 #if defined(SUPPORT_MTP_DEVICE_FILESYSTEM)
-  // Map a MTP or PTP device location to the weak pointer of
-  // ScopedMTPDeviceMapEntry.
-  typedef std::map<const FilePath::StringType,
-                   base::WeakPtr<ScopedMTPDeviceMapEntry> >
+  // Map a MTP or PTP device location to the ScopedMTPDeviceMapEntry.
+  // This map owns ScopedMTPDeviceMapEntry.
+  typedef std::map<const FilePath::StringType, ScopedMTPDeviceMapEntry*>
       MTPDeviceDelegateMap;
+
+  // Set of extension galleries host.
+  typedef std::set<const ExtensionGalleriesHost*> ExtensionGalleriesHostSet;
+
+  // Map a device location to a set of extension galleries host.
+  // This map keeps track of extension galleries that currently registered
+  // the MTP device gallery as a file system.
+  typedef std::map<const FilePath::StringType, ExtensionGalleriesHostSet>

[Lei Zhang, 2012/11/21 02:32:48]

Isn't this just used as another way of ref-counting a ScopedMTPDeviceMapEntry?

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

Yes.

[Lei Zhang, 2012/11/21 06:46:39]

So why can't ScopedMTPDeviceMapEntry just be ref-counted?

[kmadhusu, 2012/11/22 00:49:07]

As we discussed, reverted ScopedMTPDeviceMapEntry to be a RefCounted class.

+      MTPDeviceReferencesMap;
 #endif
 
   // Obtain an instance of this class via GetInstance().
@@ -175,14 +171,17 @@ class MediaFileSystemRegistry
   void OnMediaGalleriesRememberedGalleriesChanged(PrefServiceBase* service);
 
 #if defined(SUPPORT_MTP_DEVICE_FILESYSTEM)
-  // Returns ScopedMTPDeviceMapEntry object for the given |device_location|.
-  ScopedMTPDeviceMapEntry* GetOrCreateScopedMTPDeviceMapEntry(
-      const FilePath::StringType& device_location);
+  // Adds the |galleries_host| reference for the MTP device specified by the
+  // |device_location|.
+  void AddGalleriesHostReferenceForMTPDevice(
+      const FilePath::StringType& device_location,
+      const ExtensionGalleriesHost* galleries_host);
 
-  // Removes the ScopedMTPDeviceMapEntry associated with the given
+  // Removes the |galleries_host| reference for the MTP device specified by the
   // |device_location|.
-  void RemoveScopedMTPDeviceMapEntry(
-      const FilePath::StringType& device_location);
+  void RemoveGalleriesHostReferenceForMTPDevice(
+      const FilePath::StringType& device_location,
+      const ExtensionGalleriesHost* galleries_host);
 #endif
 
   void OnExtensionGalleriesHostEmpty(Profile* profile,
@@ -196,7 +195,8 @@ class MediaFileSystemRegistry
 
 #if defined(SUPPORT_MTP_DEVICE_FILESYSTEM)
   // Only accessed on the UI thread.
-  MTPDeviceDelegateMap mtp_delegate_map_;
+  MTPDeviceDelegateMap mtp_device_delegate_map_;
+  MTPDeviceReferencesMap mtp_device_references_map_;
 #endif
 
   scoped_ptr<MediaFileSystemContext> file_system_context_;