[Media Galleries] Lazily initialize the storage monitor.

chrome/browser/extensions/api/media_galleries_private/gallery_watch_state_tracker.cc

Index: chrome/browser/extensions/api/media_galleries_private/gallery_watch_state_tracker.cc
diff --git a/chrome/browser/extensions/api/media_galleries_private/gallery_watch_state_tracker.cc b/chrome/browser/extensions/api/media_galleries_private/gallery_watch_state_tracker.cc
index e580d9cc2d3153a0de7d5e1697fe589d186e380e..a889ffc142b1ef4823252881e8660ad9e7c06946 100644
--- a/chrome/browser/extensions/api/media_galleries_private/gallery_watch_state_tracker.cc
+++ b/chrome/browser/extensions/api/media_galleries_private/gallery_watch_state_tracker.cc
@@ -22,6 +22,7 @@
 #include "chrome/browser/media_galleries/media_file_system_registry.h"
 #include "chrome/browser/media_galleries/media_galleries_preferences.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/storage_monitor/storage_monitor.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/extensions/extension.h"
 #include "content/public/browser/browser_thread.h"
@@ -72,20 +73,18 @@ const Extension* GetExtensionById(Profile* profile,
   return service->GetExtensionById(extension_id, false);
 }
 
-// Returns the initialized media galleries preferences for the specified
-// |profile|.
-chrome::MediaGalleriesPreferences* GetMediaGalleryPreferences(
-    Profile* profile) {
-  return g_browser_process->media_file_system_registry()->GetPreferences(
-      profile);
-}
-
 }  // namespace
 
 GalleryWatchStateTracker::GalleryWatchStateTracker(Profile* profile)
     : profile_(profile) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   DCHECK(profile_);
+  chrome::StorageMonitor::GetInstance()->Initialize(
+      base::Bind(&GalleryWatchStateTracker::OnStorageMonitorInitialized,
+                 AsWeakPtr()));
+}
+
+void GalleryWatchStateTracker::OnStorageMonitorInitialized() {
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_LOADED,

[vandebo (ex-Chrome), 2013/05/22 16:27:52]

Why do we need to wait for storage monitor to be initialized before doing these
registrations?  Is it correct for GalleryWatchStateTracker to exist in a half
initialized state?

[Greg Billock, 2013/05/22 18:42:52]

Observe() calls ReadFromStorage(), which requires initialization.

I wondered about the half-initialized state, and investigated that. The only
other calls into the class carry the initialized permissions, so we're OK.

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

I dug into this a bit and I think it can be untangled.  Registration for the
events that Observe() receives is done in the GalleryWatchStateTracker
constructor. But the tracker doesn't care about the events until someone calls
the API, so we could delay creating the state tracker until it's needed.  In
fact, it seems that's what's done with MediaGalleriesPrivateEventRouter.
MediaGalleriesPrivateApi (which, by the way is a profile keyed service) owns
both MediaGalleriesPrivateEventRouter and GalleryWatchStateTracker.  If we just
move GalleryWatchStateTracker to be owned by MediaGalleriesPrivateEventRouter, I
think we can be assured that the StorageMonitor is initialized before any
notification arrives.

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

Shall I do that here? That's part of the refactor we talked about. I could split
it out separately, as well...

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

Seems like that too would benefit from its own CL.

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

It was in the other one

                  content::Source<Profile>(profile_));
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_UNLOADED,
@@ -116,17 +115,18 @@ GalleryWatchStateTracker* GalleryWatchStateTracker::GetForProfile(
 void GalleryWatchStateTracker::OnGalleryPermissionChanged(
     const std::string& extension_id,
     chrome::MediaGalleryPrefId gallery_id,
-    bool has_permission) {
+    bool has_permission,
+    chrome::MediaGalleriesPreferences* preferences) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   // Granted gallery permission.
   if (has_permission && HasGalleryWatchInfo(extension_id, gallery_id, false)) {
-    SetupGalleryWatch(extension_id, gallery_id);
+    SetupGalleryWatch(extension_id, gallery_id, preferences);
     return;
   }
 
   // Revoked gallery permission.
   if (!has_permission && HasGalleryWatchInfo(extension_id, gallery_id, true))
-    RemoveGalleryWatch(extension_id, gallery_id);
+    RemoveGalleryWatch(extension_id, gallery_id, preferences);
 }
 
 chrome::MediaGalleryPrefIdSet
@@ -147,7 +147,8 @@ GalleryWatchStateTracker::GetAllWatchedGalleryIDsForExtension(
 }
 
 void GalleryWatchStateTracker::RemoveAllGalleryWatchersForExtension(
-    const std::string& extension_id) {
+    const std::string& extension_id,
+    chrome::MediaGalleriesPreferences* preferences) {
   WatchedExtensionsMap::iterator extension_id_iter =
       watched_extensions_map_.find(extension_id);
   if (extension_id_iter == watched_extensions_map_.end())
@@ -155,7 +156,7 @@ void GalleryWatchStateTracker::RemoveAllGalleryWatchersForExtension(
   const WatchedGalleriesMap& galleries = extension_id_iter->second;
   for (WatchedGalleriesMap::const_iterator gallery_id_iter = galleries.begin();
        gallery_id_iter != galleries.end(); ++gallery_id_iter)
-    RemoveGalleryWatch(extension_id, gallery_id_iter->second);
+    RemoveGalleryWatch(extension_id, gallery_id_iter->second, preferences);
   watched_extensions_map_.erase(extension_id_iter);
   WriteToStorage(extension_id);
 }
@@ -239,30 +240,36 @@ void GalleryWatchStateTracker::WriteToStorage(const std::string& extension_id) {
       WatchedGalleryIdsToValue(gallery_ids).PassAs<base::Value>());
 }
 
-void GalleryWatchStateTracker::ReadFromStorage(const std::string& extension_id,
-                                               scoped_ptr<base::Value> value) {
+void GalleryWatchStateTracker::ReadFromStorage(
+    const std::string& extension_id,
+    scoped_ptr<base::Value> value) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  chrome::MediaGalleriesPreferences* preferences =

[vandebo (ex-Chrome), 2013/05/22 16:27:52]

DCHECK that StorageMonitor is initialized?

[Greg Billock, 2013/05/22 18:42:52]

This is done in GetPreferences().

+      g_browser_process->media_file_system_registry()->GetPreferences(profile_);
   base::ListValue* list = NULL;
   if (!value.get() || !value->GetAsList(&list))
     return;
   chrome::MediaGalleryPrefIdSet gallery_ids = WatchedGalleryIdsFromValue(list);
+  if (gallery_ids.empty())
+    return;
+
   for (chrome::MediaGalleryPrefIdSet::const_iterator id_iter =
            gallery_ids.begin();
        id_iter != gallery_ids.end(); ++id_iter) {
     watched_extensions_map_[extension_id][*id_iter] = false;
-    SetupGalleryWatch(extension_id, *id_iter);
+    SetupGalleryWatch(extension_id, *id_iter, preferences);
   }
 }
 
 void GalleryWatchStateTracker::SetupGalleryWatch(
     const std::string& extension_id,
-    chrome::MediaGalleryPrefId gallery_id) {
+    chrome::MediaGalleryPrefId gallery_id,
+    chrome::MediaGalleriesPreferences* preferences) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   const Extension* extension = GetExtensionById(profile_, extension_id);
   DCHECK(extension);
-  base::FilePath gallery_file_path(
-      GetMediaGalleryPreferences(profile_)->LookUpGalleryPathForExtension(
-          gallery_id, extension, false));
+  base::FilePath gallery_file_path(preferences->LookUpGalleryPathForExtension(
+      gallery_id, extension, false));
   if (gallery_file_path.empty())
     return;
   MediaGalleriesPrivateEventRouter* router =
@@ -285,13 +292,13 @@ void GalleryWatchStateTracker::SetupGalleryWatch(
 
 void GalleryWatchStateTracker::RemoveGalleryWatch(
     const std::string& extension_id,
-    chrome::MediaGalleryPrefId gallery_id) {
+    chrome::MediaGalleryPrefId gallery_id,
+    chrome::MediaGalleriesPreferences* preferences) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   const Extension* extension = GetExtensionById(profile_, extension_id);
   DCHECK(extension);
-  base::FilePath gallery_file_path(
-      GetMediaGalleryPreferences(profile_)->LookUpGalleryPathForExtension(
-          gallery_id, extension, true));
+  base::FilePath gallery_file_path(preferences->LookUpGalleryPathForExtension(
+      gallery_id, extension, true));
   if (gallery_file_path.empty())
     return;
   content::BrowserThread::PostTask(