Redesigned and refactored ScopedMTPDeviceMapEntry, MTPDeviceMapService & MTPDeviceDelegate classes.

chrome/browser/media_gallery/media_file_system_registry.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.
 
 // MediaFileSystemRegistry registers pictures directories and media devices as
 // File API filesystems and keeps track of the path to filesystem ID mappings.
 
 #ifndef CHROME_BROWSER_MEDIA_GALLERY_MEDIA_FILE_SYSTEM_REGISTRY_H_
 #define CHROME_BROWSER_MEDIA_GALLERY_MEDIA_FILE_SYSTEM_REGISTRY_H_
 
 #include <map>
+#include <set>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "base/basictypes.h"
 #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"
 
 #if defined(SUPPORT_MTP_DEVICE_FILESYSTEM)
 #include "chrome/browser/media_gallery/mtp_device_delegate_impl.h"
 #endif
 
 class Profile;
 
@@ skipping 19 matching lines @@
                       const FilePath& fs_path,
                       const std::string& filesystem_id);
   MediaFileSystemInfo();
 
   std::string name;  // JSON string, must not contain slashes.
   FilePath path;
   std::string fsid;
 };
 
 #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);
+
+  // 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();
 
  private:
-  // Friend declaration for ref counted implementation.
-  friend class base::RefCounted<ScopedMTPDeviceMapEntry>;
-
-  // Private because this class is ref-counted.
-  ~ScopedMTPDeviceMapEntry();
-
-  // Store the MTP or PTP device location.
+  // 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
 
 class MediaFileSystemContext {
  public:
   virtual ~MediaFileSystemContext() {}
 
   // Register a media file system (filtered to media files) for |path| and
   // return the new file system id.
   virtual std::string RegisterFileSystemForMassStorage(
       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|.
   virtual void RevokeFileSystem(const std::string& fsid) = 0;
 };
 
 typedef base::Callback<void(const std::vector<MediaFileSystemInfo>&)>
     MediaFileSystemsCallback;
 
 class MediaFileSystemRegistry
@@ skipping 32 matching lines @@
 
   // Map an extension to the ExtensionGalleriesHost.
   typedef std::map<std::string /*extension_id*/,
                    scoped_refptr<ExtensionGalleriesHost> > ExtensionHostMap;
   // Map a profile and extension to the ExtensionGalleriesHost.
   typedef std::map<Profile*, ExtensionHostMap> ExtensionGalleriesHostMap;
   // Map a profile to a PrefChangeRegistrar.
   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().
   MediaFileSystemRegistry();
   virtual ~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,
                                      const std::string& extension_id);
 
   // Only accessed on the UI thread. This map owns all the
   // ExtensionGalleriesHost objects created.
   ExtensionGalleriesHostMap extension_hosts_map_;
 
   PrefChangeRegistrarMap pref_change_registrar_map_;
 
 #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_;
 
   DISALLOW_COPY_AND_ASSIGN(MediaFileSystemRegistry);
 };
 
 }  // namespace chrome
 
 #endif  // CHROME_BROWSER_MEDIA_GALLERY_MEDIA_FILE_SYSTEM_REGISTRY_H_