Redesigned and refactored ScopedMTPDeviceMapEntry, MTPDeviceMapService & MTPDeviceDelegate classes.

chrome/browser/media_gallery/media_file_system_registry.cc

Index: chrome/browser/media_gallery/media_file_system_registry.cc
diff --git a/chrome/browser/media_gallery/media_file_system_registry.cc b/chrome/browser/media_gallery/media_file_system_registry.cc
index 797ee05b011c98df42df0a6d0e5707b9d82e3890..f4361a4a080193fa9632f90d9d9d32c6530d8933 100644
--- a/chrome/browser/media_gallery/media_file_system_registry.cc
+++ b/chrome/browser/media_gallery/media_file_system_registry.cc
@@ -130,18 +130,15 @@ MediaFileSystemInfo::MediaFileSystemInfo(const std::string& fs_name,
 MediaFileSystemInfo::MediaFileSystemInfo() {}
 
 #if defined(SUPPORT_MTP_DEVICE_FILESYSTEM)
-
 ScopedMTPDeviceMapEntry::ScopedMTPDeviceMapEntry(
-    const FilePath::StringType& device_location,
-    const base::Closure& no_references_callback)
-    : device_location_(device_location),
-      delegate_(new MTPDeviceDelegateImpl(device_location)),
-      no_references_callback_(no_references_callback) {
+    const FilePath::StringType& device_location)
+    : device_location_(device_location) {
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       Bind(&MTPDeviceMapService::AddDelegate,
            base::Unretained(MTPDeviceMapService::GetInstance()),
-           device_location_, make_scoped_refptr(delegate_)));
+           device_location_,
+           new MTPDeviceDelegateImpl(device_location)));
 }
 
 ScopedMTPDeviceMapEntry::~ScopedMTPDeviceMapEntry() {
@@ -150,10 +147,8 @@ ScopedMTPDeviceMapEntry::~ScopedMTPDeviceMapEntry() {
       Bind(&MTPDeviceMapService::RemoveDelegate,
            base::Unretained(MTPDeviceMapService::GetInstance()),
            device_location_));
-  no_references_callback_.Run();
 }
-
-#endif  // defined(SUPPORT_MTP_DEVICE_FILESYSTEM)
+#endif
 
 // The main owner of this class is
 // |MediaFileSystemRegistry::extension_hosts_map_|, but a callback may
@@ -223,10 +218,19 @@ class ExtensionGalleriesHost
     pref_id_map_.erase(gallery);
 
 #if defined(SUPPORT_MTP_DEVICE_FILESYSTEM)
-    MediaDeviceEntryReferencesMap::iterator mtp_device_host =
-        media_device_map_references_.find(id);
-    if (mtp_device_host != media_device_map_references_.end())
-      media_device_map_references_.erase(mtp_device_host);
+    for (MTPDeviceGalleryReferencesMap::iterator it =
+             mtp_device_gallery_references_.begin();
+         it != mtp_device_gallery_references_.end(); ++it) {
+      MediaGalleryPrefIdSet::iterator found = it->second.find(id);
+      if (found == it->second.end())
+        continue;
+      it->second.erase(id);
+      if (it->second.empty()) {
+        file_system_context_->RemoveMTPDeviceReferenceForHost(it->first, this);
+        mtp_device_gallery_references_.erase(it);
+      }
+      break;
+    }
 #endif
 
     if (pref_id_map_.empty()) {
@@ -262,9 +266,8 @@ class ExtensionGalleriesHost
   typedef std::map<MediaGalleryPrefId, MediaFileSystemInfo>
       PrefIdFsInfoMap;
 #if defined(SUPPORT_MTP_DEVICE_FILESYSTEM)
-  typedef std::map<MediaGalleryPrefId,
-                   scoped_refptr<ScopedMTPDeviceMapEntry> >
-      MediaDeviceEntryReferencesMap;
+  typedef std::map<const FilePath::StringType, MediaGalleryPrefIdSet>
+      MTPDeviceGalleryReferencesMap;
 #endif
   typedef std::map<const RenderProcessHost*, std::set<const WebContents*> >
       RenderProcessHostRefCount;
@@ -277,7 +280,7 @@ class ExtensionGalleriesHost
     DCHECK(pref_id_map_.empty());
 
 #if defined(SUPPORT_MTP_DEVICE_FILESYSTEM)
-    DCHECK(media_device_map_references_.empty());
+    DCHECK(mtp_device_gallery_references_.empty());
 #endif
   }
 
@@ -346,11 +349,9 @@ class ExtensionGalleriesHost
             device_id, path);
       } else {
 #if defined(SUPPORT_MTP_DEVICE_FILESYSTEM)
-        scoped_refptr<ScopedMTPDeviceMapEntry> mtp_device_host;
         fsid = file_system_context_->RegisterFileSystemForMTPDevice(
-            device_id, path, &mtp_device_host);
-        DCHECK(mtp_device_host.get());
-        media_device_map_references_[pref_id] = mtp_device_host;
+            device_id, path, this);
+        mtp_device_gallery_references_[path.value()].insert(pref_id);
 #else
         NOTIMPLEMENTED();
         continue;
@@ -426,11 +427,14 @@ class ExtensionGalleriesHost
     pref_id_map_.clear();
 
 #if defined(SUPPORT_MTP_DEVICE_FILESYSTEM)
-    media_device_map_references_.clear();
+    for (MTPDeviceGalleryReferencesMap::iterator it =
+             mtp_device_gallery_references_.begin();
+         it != mtp_device_gallery_references_.end(); ++it) {
+      file_system_context_->RemoveMTPDeviceReferenceForHost(it->first, this);
+    }
+    mtp_device_gallery_references_.clear();
 #endif
-
     registrar_.RemoveAll();
-
     no_references_callback_.Run();
   }
 
@@ -445,9 +449,9 @@ class ExtensionGalleriesHost
   PrefIdFsInfoMap pref_id_map_;
 
 #if defined(SUPPORT_MTP_DEVICE_FILESYSTEM)
-  // A map from the gallery preferences id to the corresponding media device
-  // host object.
-  MediaDeviceEntryReferencesMap media_device_map_references_;
+  // A map from the MTP device id to a set of media galleries that references to
+  // that device.
+  MTPDeviceGalleryReferencesMap mtp_device_gallery_references_;
 #endif
 
   // The set of render processes and web contents that may have references to
@@ -540,7 +544,6 @@ void MediaFileSystemRegistry::OnRemovableStorageAttached(
     const std::string& id, const string16& name,
     const FilePath::StringType& location) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
   if (!MediaStorageUtil::IsMediaDevice(id))
     return;
 
@@ -560,7 +563,6 @@ size_t MediaFileSystemRegistry::GetExtensionHostCountForTests() const {
 void MediaFileSystemRegistry::OnRemovableStorageDetached(
     const std::string& id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
   // Since revoking a gallery in the ExtensionGalleriesHost may cause it
   // to be removed from the map and therefore invalidate any iterator pointing
   // to it, this code first copies all the invalid gallery ids and the
@@ -637,7 +639,7 @@ class MediaFileSystemRegistry::MediaFileSystemContextImpl
 #if defined(SUPPORT_MTP_DEVICE_FILESYSTEM)
   virtual std::string RegisterFileSystemForMTPDevice(
       const std::string& device_id, const FilePath& path,
-      scoped_refptr<ScopedMTPDeviceMapEntry>* entry) {
+      const ExtensionGalleriesHost* galleries_host) {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
     DCHECK(!MediaStorageUtil::IsMassStorageDevice(device_id));
 
@@ -649,10 +651,17 @@ class MediaFileSystemRegistry::MediaFileSystemContextImpl
         IsolatedContext::GetInstance()->RegisterFileSystemForPath(
             fileapi::kFileSystemTypeDeviceMedia, path, &fs_name);
     CHECK(!fsid.empty());
-    DCHECK(entry);
-    *entry = registry_->GetOrCreateScopedMTPDeviceMapEntry(path.value());
+    registry_->AddGalleriesHostReferenceForMTPDevice(path.value(),
+                                                     galleries_host);
     return fsid;
   }
+
+  virtual void RemoveMTPDeviceReferenceForHost(
+      const FilePath::StringType& device_location,
+      const ExtensionGalleriesHost* galleries_host) {
+    registry_->RemoveGalleriesHostReferenceForMTPDevice(device_location,
+                                                        galleries_host);
+  }
 #endif
 
   virtual void RevokeFileSystem(const std::string& fsid) {
@@ -678,6 +687,8 @@ MediaFileSystemRegistry::~MediaFileSystemRegistry() {
   SystemMonitor* system_monitor = SystemMonitor::Get();
   if (system_monitor)
     system_monitor->RemoveDevicesChangedObserver(this);
+  DCHECK(mtp_device_delegate_map_.empty());
+  DCHECK(mtp_device_references_map_.empty());
 }
 
 void MediaFileSystemRegistry::OnMediaGalleriesRememberedGalleriesChanged(
@@ -717,27 +728,31 @@ void MediaFileSystemRegistry::OnMediaGalleriesRememberedGalleriesChanged(
 }
 
 #if defined(SUPPORT_MTP_DEVICE_FILESYSTEM)
-ScopedMTPDeviceMapEntry*
-MediaFileSystemRegistry::GetOrCreateScopedMTPDeviceMapEntry(
-    const FilePath::StringType& device_location) {
+void MediaFileSystemRegistry::AddGalleriesHostReferenceForMTPDevice(
+    const FilePath::StringType& device_location,
+    const ExtensionGalleriesHost* galleries_host) {
   MTPDeviceDelegateMap::iterator delegate_it =
-      mtp_delegate_map_.find(device_location);
-  if (delegate_it != mtp_delegate_map_.end() && delegate_it->second.get())
-    return delegate_it->second;
-  ScopedMTPDeviceMapEntry* mtp_device_host = new ScopedMTPDeviceMapEntry(
-      device_location, base::Bind(
-          &MediaFileSystemRegistry::RemoveScopedMTPDeviceMapEntry,
-          base::Unretained(this), device_location));
-  mtp_delegate_map_[device_location] = mtp_device_host->AsWeakPtr();
-  return mtp_device_host;
+      mtp_device_delegate_map_.find(device_location);
+  if (delegate_it == mtp_device_delegate_map_.end()) {
+    mtp_device_delegate_map_[device_location] =
+        new ScopedMTPDeviceMapEntry(device_location);
+  }
+  mtp_device_references_map_[device_location].insert(galleries_host);
 }
 
-void MediaFileSystemRegistry::RemoveScopedMTPDeviceMapEntry(
-    const FilePath::StringType& device_location) {
-  MTPDeviceDelegateMap::iterator delegate_it =
-      mtp_delegate_map_.find(device_location);
-  DCHECK(delegate_it != mtp_delegate_map_.end());
-  mtp_delegate_map_.erase(delegate_it);
+void MediaFileSystemRegistry::RemoveGalleriesHostReferenceForMTPDevice(
+    const FilePath::StringType& device_location,
+    const ExtensionGalleriesHost* galleries_host) {
+  MTPDeviceReferencesMap::iterator hosts_it =
+      mtp_device_references_map_.find(device_location);
+  DCHECK(hosts_it != mtp_device_references_map_.end());
+  DCHECK(hosts_it->second.find(galleries_host) != hosts_it->second.end());
+  hosts_it->second.erase(galleries_host);
+  if (hosts_it->second.empty()) {
+    delete mtp_device_delegate_map_[device_location];
+    mtp_device_delegate_map_.erase(device_location);
+    mtp_device_references_map_.erase(hosts_it);
+  }
 }
 #endif