Media Galleries API: Use Scheduler in MediaGalleriesPreferences initialization.

chrome/browser/media_galleries/media_galleries_preferences.cc

Index: chrome/browser/media_galleries/media_galleries_preferences.cc
diff --git a/chrome/browser/media_galleries/media_galleries_preferences.cc b/chrome/browser/media_galleries/media_galleries_preferences.cc
index 67fbf220b755212df900a9a85534485cc37e8561..8e5c9b4bb05b6481a5c6753e88fdbedd8f991323 100644
--- a/chrome/browser/media_galleries/media_galleries_preferences.cc
+++ b/chrome/browser/media_galleries/media_galleries_preferences.cc
@@ -22,6 +22,7 @@
 #include "chrome/browser/prefs/scoped_user_pref_update.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/storage_monitor/media_storage_util.h"
+#include "chrome/browser/storage_monitor/schedule.h"
 #include "chrome/browser/storage_monitor/storage_monitor.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/extensions/extension.h"
@@ -334,7 +335,6 @@ MediaGalleriesPreferences::MediaGalleriesPreferences(Profile* profile)
     : weak_factory_(this),
       initializing_(false),
       initialized_(false),
-      pre_initialization_callbacks_waiting_(0),
       profile_(profile),
       extension_prefs_for_testing_(NULL) {}
 
@@ -363,37 +363,34 @@ void MediaGalleriesPreferences::EnsureInitialized(base::Closure callback) {
 
   AddDefaultGalleriesIfFreshProfile();
 
-  pre_initialization_callbacks_waiting_ = 2;
+  chrome::ScheduleBuilder b("MediaGalleriesPreferencesInit", FROM_HERE);
 
   // Ensure StorageMonitor is initialized.
-  StorageMonitor::GetInstance()->EnsureInitialized(
-      base::Bind(&MediaGalleriesPreferences::OnInitializationCallbackReturned,
-                 weak_factory_.GetWeakPtr()));
-
-  // Look for optional default galleries every time.
-  itunes::ITunesFinder::FindITunesLibrary(
-      base::Bind(&MediaGalleriesPreferences::OnFinderDeviceID,
-                 weak_factory_.GetWeakPtr()));
-
+  b.RunAsync(base::Bind(&StorageMonitor::EnsureInitialized,
+                        base::Unretained(StorageMonitor::GetInstance()),
+                        b.ContinueClosure()));
+  b.RunAsync(base::Bind(&itunes::ITunesFinder::FindITunesLibrary,
+                        base::Bind(&MediaGalleriesPreferences::OnFinderDeviceID,
+                                   weak_factory_.GetWeakPtr(),
+                                   b.ContinueClosure())));
   // TODO(tommycli): Turn on when Picasa code is ready.
 #if 0
-  picasa::PicasaFinder::FindPicasaDatabaseOnUIThread(
-      base::Bind(&MediaGalleriesPreferences::OnFinderDeviceID,
-                 weak_factory_.GetWeakPtr()));
+  b.RunAsync(base::Bind(&picasa::PicasaFinder::FindPicasaDatabaseOnUIThread,
+                        base::Bind(&MediaGalleriesPreferences::OnFinderDeviceID,
+                                   weak_factory_.GetWeakPtr(),
+                                   b.ContinueClosure())));
 #endif
+
+  b.NextStage();
+  b.Run(base::Bind(&MediaGalleriesPreferences::FinishInitialization,
+                   weak_factory_.GetWeakPtr()));
+  b.Build()->Start();

[Greg Billock, 2013/09/19 22:34:08]

This looks nice. Agreed it makes it easier to follow.

It'd benefit from a helper method like

RunAsync(const base::Callback<base::Closure>), which would enable you to not
need any ContinueClosure call.

[tommycli, 2013/09/19 22:37:05]

Yeah! That'd be pimp. I'm also a bit annoyed that i have to write
RunAsync(base::Bind( everywhere. It's probably a huge difficulty to remove the
"base::Bind(" string due to the variable number of arguments huh?

 }
 
 bool MediaGalleriesPreferences::IsInitialized() const { return initialized_; }
 
 Profile* MediaGalleriesPreferences::profile() { return profile_; }
 
-void MediaGalleriesPreferences::OnInitializationCallbackReturned() {
-  DCHECK(!IsInitialized());
-  DCHECK(pre_initialization_callbacks_waiting_ > 0);
-  if (--pre_initialization_callbacks_waiting_ == 0)
-    FinishInitialization();
-}
-
 void MediaGalleriesPreferences::FinishInitialization() {
   DCHECK(!IsInitialized());
 
@@ -499,7 +496,8 @@ bool MediaGalleriesPreferences::UpdateDeviceIDForSingletonType(
   return false;
 }
 
-void MediaGalleriesPreferences::OnFinderDeviceID(const std::string& device_id) {
+void MediaGalleriesPreferences::OnFinderDeviceID(base::Closure callback,
+                                                 const std::string& device_id) {
   if (!device_id.empty() && !UpdateDeviceIDForSingletonType(device_id)) {
     std::string gallery_name;
     if (StorageInfo::IsITunesDevice(device_id))
@@ -515,8 +513,8 @@ void MediaGalleriesPreferences::OnFinderDeviceID(const std::string& device_id) {
                        base::Time(), false, 2);
   }
 
-  if (!IsInitialized())
-    OnInitializationCallbackReturned();
+  if (!callback.is_null())

[Greg Billock, 2013/09/19 22:34:08]

Although we may not use it, this is a nice testability enhancement -- you can
instrument the method out to this callback instead of having to subclass, use a
mock, or something.

[tommycli, 2013/09/19 22:37:05]

Yeah I could see exploiting that in a test. Making sure it gets called for
instance.

+    callback.Run();
 }
 
 void MediaGalleriesPreferences::InitFromPrefs() {