Media Gallery: Register isolated file system on a per RenderProcessHost basis....

Media Gallery: Register isolated file system on a per RenderProcessHost basis. Otherwise when one RenderProcessHost closed, it revokes the file system for all RenderProcessHosts.

BUG=none
TEST=none

Review URL: https://chromiumcodereview.appspot.com/10537092

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@141629 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/extensions/api/media_gallery/media_gallery_api.cc

@@ -28,11 +28,13 @@ using content::ChildProcessSecurityPolicy;
 GetMediaFileSystemsFunction::~GetMediaFileSystemsFunction() {}
 
 bool GetMediaFileSystemsFunction::RunImpl() {
+  const content::RenderProcessHost* rph = render_view_host()->GetProcess();
   chrome::MediaFileSystemRegistry* media_fs_registry =
       MediaFileSystemRegistry::GetInstance();
   const std::vector<MediaFileSystemRegistry::MediaFSIDAndPath> filesystems =
-      media_fs_registry->GetMediaFileSystems();
+      media_fs_registry->GetMediaFileSystems(rph);
 
+  const int child_id = rph->GetID();
   base::ListValue* list = new base::ListValue();
   for (size_t i = 0; i < filesystems.size(); i++) {
     // TODO(thestig) Check permissions to file systems when that capability
@@ -56,7 +58,6 @@ bool GetMediaFileSystemsFunction::RunImpl() {
         "dirname", Value::CreateStringValue(basepath_utf8));
     list->Append(dict_value);
 
-    const int child_id = render_view_host()->GetProcess()->GetID();
     content::ChildProcessSecurityPolicy* policy =
         ChildProcessSecurityPolicy::GetInstance();
     if (!policy->CanReadFile(child_id, path)) {

chrome/browser/media_gallery/media_file_system_registry.cc

@@ -9,9 +9,11 @@
 #include <set>
 
 #include "base/path_service.h"
-#include "base/sys_string_conversions.h"
 #include "chrome/common/chrome_paths.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/notification_source.h"
+#include "content/public/browser/notification_types.h"
+#include "content/public/browser/render_process_host.h"
 #include "webkit/fileapi/isolated_context.h"
 
 namespace chrome {
@@ -20,19 +22,41 @@ static base::LazyInstance<MediaFileSystemRegistry>::Leaky
     g_media_file_system_registry = LAZY_INSTANCE_INITIALIZER;
 
 using content::BrowserThread;
+using content::RenderProcessHost;
 using fileapi::IsolatedContext;
 
+/******************
+ * Public methods
+ ******************/
+
 // static
 MediaFileSystemRegistry* MediaFileSystemRegistry::GetInstance() {
   return g_media_file_system_registry.Pointer();
 }
 
 std::vector<MediaFileSystemRegistry::MediaFSIDAndPath>
-MediaFileSystemRegistry::GetMediaFileSystems() const {
+MediaFileSystemRegistry::GetMediaFileSystems(
+    const content::RenderProcessHost* rph) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
   std::vector<MediaFSIDAndPath> results;
-  for (MediaPathToFSIDMap::const_iterator it = media_fs_map_.begin();
-       it != media_fs_map_.end();
+  std::pair<ChildIdToMediaFSMap::iterator, bool> ret =
+      media_fs_map_.insert(std::make_pair(rph, MediaPathToFSIDMap()));
+  ChildIdToMediaFSMap::iterator& child_it = ret.first;
+  if (ret.second) {
+    // Never seen a GetMediaFileSystems call from this RPH. Initialize its
+    // file system mappings.
+    RegisterForRPHGoneNotifications(rph);
+    FilePath pictures_path;
+    if (PathService::Get(chrome::DIR_USER_PICTURES, &pictures_path)) {
+      std::string fsid = RegisterPathAsFileSystem(pictures_path);
+      child_it->second.insert(std::make_pair(pictures_path, fsid));
+    }
+  }
+
+  MediaPathToFSIDMap& child_map = child_it->second;
+  for (MediaPathToFSIDMap::const_iterator it = child_map.begin();
+       it != child_map.end();
        ++it) {
     const FilePath path = it->first;
     const std::string fsid = it->second;
@@ -41,17 +65,50 @@ MediaFileSystemRegistry::GetMediaFileSystems() const {
   return results;
 }
 
+void MediaFileSystemRegistry::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  DCHECK(type == content::NOTIFICATION_RENDERER_PROCESS_CLOSED ||
+         type == content::NOTIFICATION_RENDERER_PROCESS_TERMINATED);
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  const RenderProcessHost* rph =
+      content::Source<content::RenderProcessHost>(source).ptr();
+  ChildIdToMediaFSMap::iterator child_it = media_fs_map_.find(rph);
+  CHECK(child_it != media_fs_map_.end());
+  // No need to revoke the isolated file systems. The RPH will do that.
+  media_fs_map_.erase(child_it);
+  UnregisterForRPHGoneNotifications(rph);
+}
+
+/******************
+ * Private methods
+ ******************/
+
 MediaFileSystemRegistry::MediaFileSystemRegistry() {
-  FilePath pictures_path;
-  if (PathService::Get(chrome::DIR_USER_PICTURES, &pictures_path)) {
-    RegisterPathAsFileSystem(pictures_path);
-  }
 }
 
 MediaFileSystemRegistry::~MediaFileSystemRegistry() {
 }
 
-void MediaFileSystemRegistry::RegisterPathAsFileSystem(const FilePath& path) {
+void MediaFileSystemRegistry::RegisterForRPHGoneNotifications(
+    const content::RenderProcessHost* rph) {
+  registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_TERMINATED,
+                 content::Source<RenderProcessHost>(rph));
+  registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_CLOSED,
+                 content::Source<RenderProcessHost>(rph));
+}
+
+void MediaFileSystemRegistry::UnregisterForRPHGoneNotifications(
+    const content::RenderProcessHost* rph) {
+  registrar_.Remove(this, content::NOTIFICATION_RENDERER_PROCESS_TERMINATED,
+                    content::Source<RenderProcessHost>(rph));
+  registrar_.Remove(this, content::NOTIFICATION_RENDERER_PROCESS_CLOSED,
+                    content::Source<RenderProcessHost>(rph));
+}
+
+std::string MediaFileSystemRegistry::RegisterPathAsFileSystem(
+    const FilePath& path) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // Sanity checks for |path|.
@@ -67,7 +124,7 @@ void MediaFileSystemRegistry::RegisterPathAsFileSystem(const FilePath& path) {
   const std::string fsid =
       IsolatedContext::GetInstance()->RegisterIsolatedFileSystem(fileset);
   CHECK(!fsid.empty());
-  media_fs_map_.insert(std::make_pair(path, fsid));
+  return fsid;
 }
 
 }  // namespace chrome

chrome/browser/media_gallery/media_file_system_registry.h

@@ -17,16 +17,22 @@
 
 #include "base/basictypes.h"
 #include "base/lazy_instance.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
 
 class FilePath;
 
+namespace content {
+class RenderProcessHost;
+}
+
 namespace fileapi {
 class IsolatedContext;
 }
 
 namespace chrome {
 
-class MediaFileSystemRegistry {
+class MediaFileSystemRegistry : public content::NotificationObserver {
  public:
   // (Filesystem ID, path)
   typedef std::pair<std::string, FilePath> MediaFSIDAndPath;
@@ -34,9 +40,15 @@ class MediaFileSystemRegistry {
   // The instance is lazily created per browser process.
   static MediaFileSystemRegistry* GetInstance();
 
-  // Returns the list of media filesystem IDs and paths.
+  // Returns the list of media filesystem IDs and paths for a given RPH.
   // Called on the UI thread.
-  std::vector<MediaFSIDAndPath> GetMediaFileSystems() const;
+  std::vector<MediaFSIDAndPath> GetMediaFileSystems(
+      const content::RenderProcessHost* rph);
+
+  // content::NotificationObserver implementation.
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE;
 
  private:
   friend struct base::DefaultLazyInstanceTraits<MediaFileSystemRegistry>;
@@ -44,15 +56,27 @@ class MediaFileSystemRegistry {
   // Mapping of media directories to filesystem IDs.
   typedef std::map<FilePath, std::string> MediaPathToFSIDMap;
 
+  // Mapping of RPH to MediaPathToFSIDMaps.
+  typedef std::map<const content::RenderProcessHost*,
+                   MediaPathToFSIDMap> ChildIdToMediaFSMap;
+
   // Obtain an instance of this class via GetInstance().
   MediaFileSystemRegistry();
   virtual ~MediaFileSystemRegistry();
 
-  // Registers a path as a media file system.
-  void RegisterPathAsFileSystem(const FilePath& path);
+  // Helper functions to register / unregister listening for renderer process
+  // closed / terminiated notifications.
+  void RegisterForRPHGoneNotifications(const content::RenderProcessHost* rph);
+  void UnregisterForRPHGoneNotifications(const content::RenderProcessHost* rph);
+
+  // Registers a path as a media file system and return the filesystem id.
+  std::string RegisterPathAsFileSystem(const FilePath& path);
 
   // Only accessed on the UI thread.
-  MediaPathToFSIDMap media_fs_map_;
+  ChildIdToMediaFSMap media_fs_map_;
+
+  // Is only used on the UI thread.
+  content::NotificationRegistrar registrar_;
 
   DISALLOW_COPY_AND_ASSIGN(MediaFileSystemRegistry);
 };