[Media Galleries] Lazily initialize the storage monitor.

chrome/browser/media_galleries/media_file_system_registry.cc

Index: chrome/browser/media_galleries/media_file_system_registry.cc
diff --git a/chrome/browser/media_galleries/media_file_system_registry.cc b/chrome/browser/media_galleries/media_file_system_registry.cc
index 200b4f31a3647d81cc823b1e590cdf6b3c7c6414..685db6f7a3603051a1b22a150c95bf7b774329ef 100644
--- a/chrome/browser/media_galleries/media_file_system_registry.cc
+++ b/chrome/browser/media_galleries/media_file_system_registry.cc
@@ -37,6 +37,7 @@
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
+#include "content/public/browser/render_view_host_observer.h"
 #include "content/public/browser/web_contents.h"
 #include "webkit/fileapi/file_system_types.h"
 #include "webkit/fileapi/isolated_context.h"
@@ -409,18 +410,68 @@ class ExtensionGalleriesHost
   DISALLOW_COPY_AND_ASSIGN(ExtensionGalleriesHost);
 };
 
+// This self-deleting transaction object contains the state needed to
+// asynchronously initialize the storage monitor if needed when an
+// extension calles |GetMediaFileSystemsForExtension|. Specifically,
+// it tracks if the RenderViewHost with which it was initialized was
+// destroyed, and if so, it never calls the callback.
+// When it receives the OnStorageMonitorInitialized callback, it deletes itself.
+class GetPreferencesTransaction : public content::RenderViewHostObserver {

[vandebo (ex-Chrome), 2013/05/22 21:40:34]

You probably don't need this.  But if you do the StorageMonitor init at the API
level, it'll be clear that you don't need it.

[Greg Billock, 2013/05/23 00:55:59]

We need it since the RVH can disappear while we're initializing the storage
monitor. GetMediaFileSystemsForExtension is the entry point of the API, so
that's our trigger to initialize. Doing that async means the RVH could be
closed.

[vandebo (ex-Chrome), 2013/05/23 15:04:17]

We talked about the extension system probably having a rvh listener, did you
check for that?

[Greg Billock, 2013/05/30 22:17:47]

This is moved to the API now. You'll see a couple classes needed weak ptrs,
which I've added.

+ public:
+  GetPreferencesTransaction(
+      content::RenderViewHost* rvh,
+      base::Closure callback)
+    : content::RenderViewHostObserver(rvh),
+      callback_(callback) {}
+  virtual ~GetPreferencesTransaction() {}
+
+  virtual void RenderViewHostDestroyed(content::RenderViewHost* rvh) OVERRIDE {
+    callback_.Reset();
+  }
+
+  void OnStorageMonitorInitialized() {
+    if (!callback_.is_null())
+      callback_.Run();
+
+    delete this;
+  }
+
+ private:
+  base::Closure callback_;
+};
+
 /******************
  * Public methods
  ******************/
 
 void MediaFileSystemRegistry::GetMediaFileSystemsForExtension(
-    const content::RenderViewHost* rvh,
+    content::RenderViewHost* rvh,
     const extensions::Extension* extension,
     const MediaFileSystemsCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  Profile* profile =
-      Profile::FromBrowserContext(rvh->GetProcess()->GetBrowserContext());
+  // This object is self-deleting when it receives the callback
+  // from GetPreferencesAsync.
+  GetPreferencesTransaction* transaction = new GetPreferencesTransaction(
+      rvh,
+      base::Bind(
+          &MediaFileSystemRegistry::GetMediaFileSystemsPostInit,
+          weak_ptr_factory_.GetWeakPtr(),
+          rvh, extension, callback));
+  StorageMonitor* monitor = StorageMonitor::GetInstance();
+  if (monitor) {
+    monitor->Initialize(base::Bind(
+        &GetPreferencesTransaction::OnStorageMonitorInitialized,
+        base::Unretained(transaction)));
+  }
+}
+
+void MediaFileSystemRegistry::GetMediaFileSystemsPostInit(
+    const content::RenderViewHost* rvh,
+    const extensions::Extension* extension,
+    const MediaFileSystemsCallback& callback) {
+  Profile* profile = Profile::FromBrowserContext(
+      rvh->GetProcess()->GetBrowserContext());
   MediaGalleriesPreferences* preferences = GetPreferences(profile);
   MediaGalleryPrefIdSet galleries =
       preferences->GalleriesForExtension(*extension);
@@ -467,11 +518,12 @@ MediaGalleriesPreferences* MediaFileSystemRegistry::GetPreferences(
   // once per profile.
   extension_hosts_map_[profile] = ExtensionHostMap();
 
-  // TODO(gbillock): Move this stanza to MediaGalleriesPreferences init code.
   // StorageMonitor may be NULL in unit tests.
+  // TODO(gbillock): Make sure all tests have a storage monitor and remove this.
   StorageMonitor* monitor = StorageMonitor::GetInstance();
   if (!monitor)
     return preferences;
+  DCHECK(monitor->IsInitialized());
   std::vector<StorageInfo> existing_devices = monitor->GetAttachedStorage();
   for (size_t i = 0; i < existing_devices.size(); i++) {
     if (!StorageInfo::IsMediaDevice(existing_devices[i].device_id))
@@ -616,7 +668,8 @@ class MediaFileSystemRegistry::MediaFileSystemContextImpl
 };
 
 MediaFileSystemRegistry::MediaFileSystemRegistry()
-    : file_system_context_(new MediaFileSystemContextImpl(this)) {
+    : file_system_context_(new MediaFileSystemContextImpl(this)),
+      weak_ptr_factory_(this) {
   // StorageMonitor may be NULL in unit tests.
   StorageMonitor* monitor = StorageMonitor::GetInstance();
   if (monitor)