Media Galleries API: Fix MediaGalleriesPreferences finders race.

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/callback.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"
@@ skipping 308 matching lines @@
 
 bool MediaGalleryPrefInfo::IsGalleryAvailable() const {
   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) {
-  AddDefaultGalleriesIfFreshProfile();
-
-  // Look for optional default galleries every time.
-  itunes::ITunesFinder::FindITunesLibrary(
-      base::Bind(&MediaGalleriesPreferences::OnITunesDeviceID,
-                 weak_factory_.GetWeakPtr()));
-
-  // TODO(tommycli): Turn on when Picasa code is ready.
-#if 0
-  picasa::PicasaFinder::FindPicasaDatabaseOnUIThread(
-      base::Bind(&MediaGalleriesPreferences::OnPicasaDeviceID,
-                 weak_factory_.GetWeakPtr()));
-#endif
-
-  InitFromPrefs();
-
-  StorageMonitor::GetInstance()->AddObserver(this);
-}
+      extension_prefs_for_testing_(NULL) {}
 
 MediaGalleriesPreferences::~MediaGalleriesPreferences() {
   if (StorageMonitor::GetInstance())
     StorageMonitor::GetInstance()->RemoveObserver(this);
 }
 
-Profile* MediaGalleriesPreferences::profile() {
-  return profile_;
+void MediaGalleriesPreferences::EnsureInitialized(base::Closure callback) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  if (initialized_) {
+    if (!callback.is_null())
+      callback.Run();
+    return;
+  }
+
+  if (!callback.is_null()) {

[vandebo (ex-Chrome), 2013/09/21 01:20:59]

nit: always push the callback and then you can remove the initializing_
variable.

[tommycli, 2013/09/23 20:39:12]

Done.

+    on_initialize_callbacks_.push_back(callback);
+  }
+
+  if (initializing_)
+    return;
+
+  initializing_ = true;
+
+  AddDefaultGalleriesIfFreshProfile();
+
+  pre_initialization_callbacks_waiting_ = 2;
+
+  // 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()));
+
+  // TODO(tommycli): Turn on when Picasa code is ready.
+#if 0
+  picasa::PicasaFinder::FindPicasaDatabaseOnUIThread(
+      base::Bind(&MediaGalleriesPreferences::OnFinderDeviceID,
+                 weak_factory_.GetWeakPtr()));
+#endif
+}
+
+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();
+  for (size_t i = 0; i < existing_devices.size(); i++) {
+    if (!(StorageInfo::IsMediaDevice(existing_devices[i].device_id()) &&
+          StorageInfo::IsRemovableDevice(existing_devices[i].device_id())))
+      continue;
+    AddGallery(existing_devices[i].device_id(),
+               base::FilePath(),
+               false,
+               existing_devices[i].storage_label(),
+               existing_devices[i].vendor_name(),
+               existing_devices[i].model_name(),
+               existing_devices[i].total_size_in_bytes(),
+               base::Time::Now());
+  }
+
+  InitFromPrefs();
+
+  StorageMonitor::GetInstance()->AddObserver(this);

[vandebo (ex-Chrome), 2013/09/21 01:20:59]

Add the observer before iterating the available storages to preserve existing
behavior and to prevent a race where a device comes in between the two.

[tommycli, 2013/09/23 20:39:12]

Done.

+
+  for (std::vector<base::Closure>::iterator iter =
+           on_initialize_callbacks_.begin();
+       iter != on_initialize_callbacks_.end();
+       ++iter) {
+    iter->Run();
+  }
+  on_initialize_callbacks_.clear();
 }
 
 void MediaGalleriesPreferences::AddDefaultGalleriesIfFreshProfile() {
   // Only add defaults the first time.
   if (APIHasBeenUsed(profile_))
     return;
 
   // Fresh profile case.
   const int kDirectoryKeys[] = {
     chrome::DIR_USER_MUSIC,
@@ skipping 50 matching lines @@
         FOR_EACH_OBSERVER(GalleryChangeObserver,
                           gallery_change_observers_,
                           OnGalleryInfoUpdated(this, pref_id));
       }
       return true;
     }
   }
   return false;
 }
 
-void MediaGalleriesPreferences::OnITunesDeviceID(const std::string& device_id) {
-  if (device_id.empty())
-    return;
-  if (!UpdateDeviceIDForSingletonType(device_id)) {
-    AddGalleryInternal(device_id, ASCIIToUTF16(kITunesGalleryName),
+void MediaGalleriesPreferences::OnFinderDeviceID(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);
   }
-}
 
-void MediaGalleriesPreferences::OnPicasaDeviceID(const std::string& device_id) {
-  DCHECK(!device_id.empty());
-  if (!UpdateDeviceIDForSingletonType(device_id)) {
-    AddGalleryInternal(device_id, ASCIIToUTF16(kPicasaGalleryName),
-                       base::FilePath(), false /*not user added*/,
-                       string16(), string16(), string16(), 0,
-                       base::Time(), false, 2);
-  }
+  if (!IsInitialized())
+    OnInitializationCallbackReturned();
 }
 
 void MediaGalleriesPreferences::InitFromPrefs() {
   known_galleries_.clear();
   device_map_.clear();
 
   PrefService* prefs = profile_->GetPrefs();
   const ListValue* list = prefs->GetList(
       prefs::kMediaGalleriesRememberedGalleries);
   if (list) {
     for (ListValue::const_iterator it = list->begin();
          it != list->end(); ++it) {
       const DictionaryValue* dict = NULL;
       if (!(*it)->GetAsDictionary(&dict))
         continue;
 
       MediaGalleryPrefInfo gallery_info;
       if (!PopulateGalleryPrefInfoFromDictionary(*dict, &gallery_info))
         continue;
 
       known_galleries_[gallery_info.pref_id] = gallery_info;
       device_map_[gallery_info.device_id].insert(gallery_info.pref_id);
     }
   }
 }
 
 void MediaGalleriesPreferences::AddGalleryChangeObserver(
     GalleryChangeObserver* observer) {
+  DCHECK(IsInitialized());
   gallery_change_observers_.AddObserver(observer);
 }
 
 void MediaGalleriesPreferences::RemoveGalleryChangeObserver(
     GalleryChangeObserver* observer) {
+  DCHECK(IsInitialized());
   gallery_change_observers_.RemoveObserver(observer);
 }
 
 void MediaGalleriesPreferences::OnRemovableStorageAttached(
     const StorageInfo& info) {
+  DCHECK(IsInitialized());
   if (!StorageInfo::IsMediaDevice(info.device_id()))
     return;
 
   AddGallery(info.device_id(), base::FilePath(),
              false /*not user added*/,
              info.storage_label(),
              info.vendor_name(),
              info.model_name(),
              info.total_size_in_bytes(),
              base::Time::Now());
 }
 
 bool MediaGalleriesPreferences::LookUpGalleryByPath(
     const base::FilePath& path,
     MediaGalleryPrefInfo* gallery_info) const {
+  DCHECK(IsInitialized());
   StorageInfo info;
   base::FilePath relative_path;
   if (!MediaStorageUtil::GetDeviceInfoFromPath(path, &info, &relative_path)) {
     if (gallery_info)
       *gallery_info = MediaGalleryPrefInfo();
     return false;
   }
 
   relative_path = relative_path.NormalizePathSeparators();
   MediaGalleryPrefIdSet galleries_on_device =
@@ skipping 38 matching lines @@
   DeviceIdPrefIdsMap::const_iterator found = device_map_.find(device_id);
   if (found == device_map_.end())
     return MediaGalleryPrefIdSet();
   return found->second;
 }
 
 base::FilePath MediaGalleriesPreferences::LookUpGalleryPathForExtension(
     MediaGalleryPrefId gallery_id,
     const extensions::Extension* extension,
     bool include_unpermitted_galleries) {
+  DCHECK(IsInitialized());
   DCHECK(extension);
   if (!include_unpermitted_galleries &&
       !ContainsKey(GalleriesForExtension(*extension), gallery_id))
     return base::FilePath();
 
   MediaGalleriesPrefInfoMap::const_iterator it =
       known_galleries_.find(gallery_id);
   if (it == known_galleries_.end())
     return base::FilePath();
   return MediaStorageUtil::FindDevicePathById(it->second.device_id);
 }
 
 MediaGalleryPrefId MediaGalleriesPreferences::AddGallery(
     const std::string& device_id,
     const base::FilePath& relative_path, bool user_added,
     const string16& volume_label, const string16& vendor_name,
     const string16& model_name, uint64 total_size_in_bytes,
     base::Time last_attach_time) {
+  DCHECK(IsInitialized());
   return AddGalleryInternal(device_id, string16(), relative_path, user_added,
                             volume_label, vendor_name, model_name,
                             total_size_in_bytes, last_attach_time, true, 2);
 }
 
 MediaGalleryPrefId MediaGalleriesPreferences::AddGalleryInternal(
     const std::string& device_id, const string16& display_name,
     const base::FilePath& relative_path, bool user_added,
     const string16& volume_label, const string16& vendor_name,
     const string16& model_name, uint64 total_size_in_bytes,
@@ skipping 112 matching lines @@
   InitFromPrefs();
   FOR_EACH_OBSERVER(GalleryChangeObserver,
                     gallery_change_observers_,
                     OnGalleryAdded(this, gallery_info.pref_id));
 
   return gallery_info.pref_id;
 }
 
 MediaGalleryPrefId MediaGalleriesPreferences::AddGalleryByPath(
     const base::FilePath& path) {
+  DCHECK(IsInitialized());
   MediaGalleryPrefInfo gallery_info;
   if (LookUpGalleryByPath(path, &gallery_info) &&
       gallery_info.type != MediaGalleryPrefInfo::kBlackListed) {
     return gallery_info.pref_id;
   }
   return AddGalleryInternal(gallery_info.device_id,
                             gallery_info.display_name,
                             gallery_info.path,
                             true /*user added*/,
                             gallery_info.volume_label,
                             gallery_info.vendor_name,
                             gallery_info.model_name,
                             gallery_info.total_size_in_bytes,
                             gallery_info.last_attach_time,
                             gallery_info.volume_metadata_valid,
                             gallery_info.prefs_version);
 }
 
 void MediaGalleriesPreferences::ForgetGalleryById(MediaGalleryPrefId pref_id) {
+  DCHECK(IsInitialized());
   PrefService* prefs = profile_->GetPrefs();
   scoped_ptr<ListPrefUpdate> update(new ListPrefUpdate(
       prefs, prefs::kMediaGalleriesRememberedGalleries));
   ListValue* list = update->Get();
 
   if (!ContainsKey(known_galleries_, pref_id))
     return;
 
   for (ListValue::iterator iter = list->begin(); iter != list->end(); ++iter) {
     DictionaryValue* dict;
@@ skipping 15 matching lines @@
       FOR_EACH_OBSERVER(GalleryChangeObserver,
                         gallery_change_observers_,
                         OnGalleryRemoved(this, pref_id));
       return;
     }
   }
 }
 
 MediaGalleryPrefIdSet MediaGalleriesPreferences::GalleriesForExtension(
     const extensions::Extension& extension) const {
+  DCHECK(IsInitialized());
   MediaGalleryPrefIdSet result;
 
   if (HasAutoDetectedGalleryPermission(extension)) {
     for (MediaGalleriesPrefInfoMap::const_iterator it =
              known_galleries_.begin(); it != known_galleries_.end(); ++it) {
       if (it->second.type == MediaGalleryPrefInfo::kAutoDetected)
         result.insert(it->second.pref_id);
     }
   }
 
@@ skipping 14 matching lines @@
       }
     }
   }
   return result;
 }
 
 void MediaGalleriesPreferences::SetGalleryPermissionForExtension(
     const extensions::Extension& extension,
     MediaGalleryPrefId pref_id,
     bool has_permission) {
+  DCHECK(IsInitialized());
   // The gallery may not exist anymore if the user opened a second config
   // surface concurrently and removed it. Drop the permission update if so.
   MediaGalleriesPrefInfoMap::const_iterator gallery_info =
       known_galleries_.find(pref_id);
   if (gallery_info == known_galleries_.end())
     return;
 
   bool default_permission = false;
   if (gallery_info->second.type == MediaGalleryPrefInfo::kAutoDetected)
     default_permission = HasAutoDetectedGalleryPermission(extension);
   // When the permission matches the default, we don't need to remember it.
   if (has_permission == default_permission) {
     if (!UnsetGalleryPermissionInPrefs(extension.id(), pref_id))
       // If permission wasn't set, assume nothing has changed.
       return;
   } else {
     if (!SetGalleryPermissionInPrefs(extension.id(), pref_id, has_permission))
       return;
   }
   if (has_permission)
     FOR_EACH_OBSERVER(GalleryChangeObserver,
                       gallery_change_observers_,
                       OnPermissionAdded(this, extension.id(), pref_id));
   else
     FOR_EACH_OBSERVER(GalleryChangeObserver,
                       gallery_change_observers_,
                       OnPermissionRemoved(this, extension.id(), pref_id));
 }
 
+const MediaGalleriesPrefInfoMap& MediaGalleriesPreferences::known_galleries()
+    const {
+  DCHECK(IsInitialized());
+  return known_galleries_;
+}
+
 void MediaGalleriesPreferences::Shutdown() {
   weak_factory_.InvalidateWeakPtrs();
   profile_ = NULL;
 }
 
 // static
 bool MediaGalleriesPreferences::APIHasBeenUsed(Profile* profile) {
   MediaGalleryPrefId current_id =
       profile->GetPrefs()->GetUint64(prefs::kMediaGalleriesUniqueId);
   return current_id != kInvalidMediaGalleryPrefId + 1;
 }
 
 // static
 void MediaGalleriesPreferences::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* registry) {
   registry->RegisterListPref(prefs::kMediaGalleriesRememberedGalleries,
                              user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
   registry->RegisterUint64Pref(
       prefs::kMediaGalleriesUniqueId,
       kInvalidMediaGalleryPrefId + 1,
       user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
 }
 
 bool MediaGalleriesPreferences::SetGalleryPermissionInPrefs(
     const std::string& extension_id,
     MediaGalleryPrefId gallery_id,
     bool has_access) {
-  ExtensionPrefs::ScopedListUpdate update(GetExtensionPrefs(),
-                                          extension_id,
-                                          kMediaGalleriesPermissions);
+  DCHECK(IsInitialized());
+  ExtensionPrefs::ScopedListUpdate update(

[vandebo (ex-Chrome), 2013/09/21 01:20:59]

Style guide seems to prefer this the way it was.
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Function_Decla...

[tommycli, 2013/09/23 20:39:12]

Done.

+      GetExtensionPrefs(), extension_id, kMediaGalleriesPermissions);
   ListValue* permissions = update.Get();
   if (!permissions) {
     permissions = update.Create();
   } else {
     // If the gallery is already in the list, update the permission...
     for (ListValue::iterator iter = permissions->begin();
          iter != permissions->end(); ++iter) {
       DictionaryValue* dict = NULL;
       if (!(*iter)->GetAsDictionary(&dict))
         continue;
@@ skipping 14 matching lines @@
   DictionaryValue* dict = new DictionaryValue;
   dict->SetString(kMediaGalleryIdKey, base::Uint64ToString(gallery_id));
   dict->SetBoolean(kMediaGalleryHasPermissionKey, has_access);
   permissions->Append(dict);
   return true;
 }
 
 bool MediaGalleriesPreferences::UnsetGalleryPermissionInPrefs(
     const std::string& extension_id,
     MediaGalleryPrefId gallery_id) {
-  ExtensionPrefs::ScopedListUpdate update(GetExtensionPrefs(),
-                                          extension_id,
-                                          kMediaGalleriesPermissions);
+  DCHECK(IsInitialized());
+  ExtensionPrefs::ScopedListUpdate update(
+      GetExtensionPrefs(), extension_id, kMediaGalleriesPermissions);
   ListValue* permissions = update.Get();
   if (!permissions)
     return false;
 
   for (ListValue::iterator iter = permissions->begin();
        iter != permissions->end(); ++iter) {
     const DictionaryValue* dict = NULL;
     if (!(*iter)->GetAsDictionary(&dict))
       continue;
     MediaGalleryPermission perm;
     if (!GetMediaGalleryPermissionFromDictionary(dict, &perm))
       continue;
     if (perm.pref_id == gallery_id) {
       permissions->Erase(iter, NULL);
       return true;
     }
   }
   return false;
 }
 
 std::vector<MediaGalleryPermission>
 MediaGalleriesPreferences::GetGalleryPermissionsFromPrefs(
     const std::string& extension_id) const {
+  DCHECK(IsInitialized());
   std::vector<MediaGalleryPermission> result;
   const ListValue* permissions;
   if (!GetExtensionPrefs()->ReadPrefAsList(extension_id,
                                            kMediaGalleriesPermissions,
                                            &permissions)) {
     return result;
   }
 
   for (ListValue::const_iterator iter = permissions->begin();
        iter != permissions->end(); ++iter) {
     DictionaryValue* dict = NULL;
     if (!(*iter)->GetAsDictionary(&dict))
       continue;
     MediaGalleryPermission perm;
     if (!GetMediaGalleryPermissionFromDictionary(dict, &perm))
       continue;
     result.push_back(perm);
   }
 
   return result;
 }
 
 void MediaGalleriesPreferences::RemoveGalleryPermissionsFromPrefs(
     MediaGalleryPrefId gallery_id) {
+  DCHECK(IsInitialized());
   ExtensionPrefs* prefs = GetExtensionPrefs();
   const DictionaryValue* extensions =
       prefs->pref_service()->GetDictionary(prefs::kExtensionsPref);
   if (!extensions)
     return;
 
   for (DictionaryValue::Iterator iter(*extensions); !iter.IsAtEnd();
        iter.Advance()) {
     if (!extensions::Extension::IdIsValid(iter.key())) {
       NOTREACHED();
       continue;
     }
     UnsetGalleryPermissionInPrefs(iter.key(), gallery_id);
   }
 }
 
 ExtensionPrefs* MediaGalleriesPreferences::GetExtensionPrefs() const {
+  DCHECK(IsInitialized());
   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;
 }