[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..d0a14eb8bc672c8b582bd17ed89980c743ce018f 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
@@ -72,18 +72,11 @@ 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) {
+    : profile_(profile),
+      preferences_(NULL) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   DCHECK(profile_);
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_LOADED,
@@ -160,6 +153,15 @@ void GalleryWatchStateTracker::RemoveAllGalleryWatchersForExtension(
   WriteToStorage(extension_id);
 }
 
+void GalleryWatchStateTracker::SetPreferences(
+    chrome::MediaGalleriesPreferences* preferences) {
+  if (preferences_ != NULL) {
+    DCHECK_EQ(preferences_, preferences);
+    return;
+  }
+  preferences_ = preferences;
+}
+
 void GalleryWatchStateTracker::OnGalleryWatchAdded(
     const std::string& extension_id,
     chrome::MediaGalleryPrefId gallery_id) {
@@ -246,6 +248,20 @@ void GalleryWatchStateTracker::ReadFromStorage(const std::string& extension_id,
   if (!value.get() || !value->GetAsList(&list))
     return;
   chrome::MediaGalleryPrefIdSet gallery_ids = WatchedGalleryIdsFromValue(list);
+  if (!gallery_ids.empty()) {
+    g_browser_process->media_file_system_registry()->GetPreferencesAsync(
+        profile_,
+        base::Bind(&GalleryWatchStateTracker::ReadFromStorageWithPreferences,
+                   AsWeakPtr(), extension_id, gallery_ids));
+  }
+}
+
+void GalleryWatchStateTracker::ReadFromStorageWithPreferences(
+    const std::string& extension_id,
+    chrome::MediaGalleryPrefIdSet gallery_ids,
+    chrome::MediaGalleriesPreferences* preferences) {
+  SetPreferences(preferences);
+
   for (chrome::MediaGalleryPrefIdSet::const_iterator id_iter =
            gallery_ids.begin();
        id_iter != gallery_ids.end(); ++id_iter) {
@@ -260,9 +276,9 @@ void GalleryWatchStateTracker::SetupGalleryWatch(
   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));
+  DCHECK(preferences_);
+  base::FilePath gallery_file_path(preferences_->LookUpGalleryPathForExtension(

[vandebo (ex-Chrome), 2013/05/16 18:56:27]

Why are we sure preferences will be set by this point?

[Lei Zhang, 2013/05/16 21:09:12]

I looked at the caller chain and someone must have called SetPreferences()
before getting here. Let me look that up...

[Lei Zhang, 2013/05/16 22:17:52]

Oh, SetupGalleryWatch() has two callers:
1) ReadFromStorageWithPreferences() above which calls SetPreferences().

2) OnGalleryPermissionChanged(), and MediaGalleriesPreferences calls
OnGalleryPermissionChanged(). For this to happen, the extension must exist, and
if the extension exists, then (1) already occurred.

[Greg Billock, 2013/05/16 23:27:55]

One call is from ReadFromStorageWithPreferences above, the other is from
OnGalleryPermissionChanged, which is called from the permissions object itself.
We may need to pass the prefs through that call to make sure it gets set, I'm
not sure yet. Lei, do you know?

[Greg Billock, 2013/05/16 23:27:55]

Is there a race where the prefs can get initialized and call this before the
callback will run SetPreferences? If so, we can pass (this) through that
OnGalleryPermissionChanged call. That's easy enough and makes the point clear.

+      gallery_id, extension, false));
   if (gallery_file_path.empty())
     return;
   MediaGalleriesPrivateEventRouter* router =
@@ -289,9 +305,9 @@ void GalleryWatchStateTracker::RemoveGalleryWatch(
   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));
+  DCHECK(preferences_);
+  base::FilePath gallery_file_path(preferences_->LookUpGalleryPathForExtension(

[vandebo (ex-Chrome), 2013/05/16 18:56:27]

ditto

[Greg Billock, 2013/05/16 23:27:55]

Call situation here is only from OnGalleryPermissionChanged. So we may need to
push the prefs pointer through that call.

+      gallery_id, extension, true));
   if (gallery_file_path.empty())
     return;
   content::BrowserThread::PostTask(