Media Galleries API: Use Scheduler in MediaGalleriesPreferences initialization.

chrome/browser/media_galleries/media_galleries_preferences.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/media_galleries/media_galleries_preferences.h"
 
 #include "base/i18n/time_formatting.h"
 #include "base/path_service.h"
 #include "base/prefs/pref_service.h"
 #include "base/stl_util.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/api/media_galleries_private/media_galleries_private_api.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/media_galleries/fileapi/itunes_finder.h"
 #include "chrome/browser/media_galleries/fileapi/picasa_finder.h"
 #include "chrome/browser/media_galleries/media_file_system_registry.h"
 #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"
 #include "chrome/common/extensions/permissions/api_permission.h"
 #include "chrome/common/extensions/permissions/media_galleries_permission.h"
 #include "chrome/common/extensions/permissions/permissions_data.h"
 #include "chrome/common/pref_names.h"
 #include "components/user_prefs/pref_registry_syncable.h"
 #include "grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
@@ skipping 292 matching lines @@
   return !StorageInfo::IsRemovableDevice(device_id) ||
          MediaStorageUtil::IsRemovableStorageAttached(device_id);
 }
 
 MediaGalleriesPreferences::GalleryChangeObserver::~GalleryChangeObserver() {}
 
 MediaGalleriesPreferences::MediaGalleriesPreferences(Profile* profile)
     : weak_factory_(this),
       initializing_(false),
       initialized_(false),
-      pre_initialization_callbacks_waiting_(0),
       profile_(profile),
       extension_prefs_for_testing_(NULL) {}
 
 MediaGalleriesPreferences::~MediaGalleriesPreferences() {
   if (StorageMonitor::GetInstance())
     StorageMonitor::GetInstance()->RemoveObserver(this);
 }
 
 void MediaGalleriesPreferences::EnsureInitialized(base::Closure callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (initialized_) {
     if (!callback.is_null())
       callback.Run();
     return;
   }
 
   if (!callback.is_null()) {
     on_initialize_callbacks_.push_back(callback);
   }
 
   if (initializing_)
     return;
 
   initializing_ = true;
 
   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());
 
   initialized_ = true;
 
   StorageMonitor* monitor = StorageMonitor::GetInstance();
   DCHECK(monitor->IsInitialized());
 
   std::vector<StorageInfo> existing_devices =
       monitor->GetAllAvailableStorages();
@@ skipping 85 matching lines @@
         FOR_EACH_OBSERVER(GalleryChangeObserver,
                           gallery_change_observers_,
                           OnGalleryInfoUpdated(this, pref_id));
       }
       return true;
     }
   }
   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))
       gallery_name = kITunesGalleryName;
     else if (StorageInfo::IsPicasaDevice(device_id))
       gallery_name = kPicasaGalleryName;
     else
       NOTREACHED();
 
     AddGalleryInternal(device_id, ASCIIToUTF16(gallery_name),
                        base::FilePath(), false /*not user added*/,
                        string16(), string16(), string16(), 0,
                        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() {
   known_galleries_.clear();
   device_map_.clear();
 
   PrefService* prefs = profile_->GetPrefs();
   const ListValue* list = prefs->GetList(
       prefs::kMediaGalleriesRememberedGalleries);
   if (list) {
@@ skipping 518 matching lines @@
   if (extension_prefs_for_testing_)
     return extension_prefs_for_testing_;
   return extensions::ExtensionPrefs::Get(profile_);
 }
 
 void MediaGalleriesPreferences::SetExtensionPrefsForTesting(
     extensions::ExtensionPrefs* extension_prefs) {
   DCHECK(IsInitialized());
   extension_prefs_for_testing_ = extension_prefs;
 }