Media Galleries API: Fix MediaGalleriesPreferences finders race.

chrome/browser/extensions/api/media_galleries_private/media_galleries_private_api.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/extensions/api/media_galleries_private/media_galleries_private_api.h"
 
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/files/file_path.h"
 #include "base/lazy_instance.h"
@@ skipping 45 matching lines @@
 // by the |gallery_id|. Returns true and set |gallery_file_path| and
 // |gallery_pref_id| if the |gallery_id| is valid and returns false otherwise.
 bool GetGalleryFilePathAndId(const std::string& gallery_id,
                              Profile* profile,
                              const Extension* extension,
                              base::FilePath* gallery_file_path,
                              MediaGalleryPrefId* gallery_pref_id) {
   MediaGalleryPrefId pref_id;
   if (!base::StringToUint64(gallery_id, &pref_id))
     return false;
-  MediaFileSystemRegistry* registry =
-      g_browser_process->media_file_system_registry();
-   base::FilePath file_path(
-      registry->GetPreferences(profile)->LookUpGalleryPathForExtension(
-          pref_id, extension, false));
+  MediaGalleriesPreferences* preferences =
+      g_browser_process->media_file_system_registry()->GetPreferences(profile);
+  DCHECK(preferences->IsInitialized());
+  base::FilePath file_path(
+      preferences->LookUpGalleryPathForExtension(pref_id, extension, false));
   if (file_path.empty())
     return false;
   *gallery_pref_id = pref_id;
   *gallery_file_path = file_path;
   return true;
 }
 
 }  // namespace
 
 
@@ skipping 34 matching lines @@
 }
 
 // static
 MediaGalleriesPrivateAPI* MediaGalleriesPrivateAPI::Get(Profile* profile) {
   return
       ProfileKeyedAPIFactory<MediaGalleriesPrivateAPI>::GetForProfile(profile);
 }
 
 void MediaGalleriesPrivateAPI::OnListenerAdded(
     const EventListenerInfo& details) {
-  // Start the StorageMonitor if it is not already initialized. After that,
-  // try to initialize the event router for the listener.
+  // Make sure MediaGalleriesPreferences and StorageMonitor are initialized.

[Lei Zhang, 2013/09/21 00:12:35]

Whether StorageMonitor is initialized or not is an impl detail at this point?

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

Done.

+  // After that, try to initialize the event router for the listener.
   // This method is called synchronously with the message handler for the
   // JS invocation.
 
-  StorageMonitor::GetInstance()->EnsureInitialized(base::Bind(
-      &MediaGalleriesPrivateAPI::MaybeInitializeEventRouterAndTracker,
-      weak_ptr_factory_.GetWeakPtr()));
+  g_browser_process->media_file_system_registry()
+      ->GetPreferences(profile_)

[Lei Zhang, 2013/09/21 00:12:35]

I hate it when things get so long that you have to do this, but if you must,
know that files with foo->\nbar outnumbers files with foo\n->bar 10:1.

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

Done. Either way is fine with me. I just did git cl format.

+      ->EnsureInitialized(base::Bind(
+            &MediaGalleriesPrivateAPI::MaybeInitializeEventRouterAndTracker,
+            weak_ptr_factory_.GetWeakPtr()));
 }
 
 MediaGalleriesPrivateEventRouter* MediaGalleriesPrivateAPI::GetEventRouter() {
   MaybeInitializeEventRouterAndTracker();
   return media_galleries_private_event_router_.get();
 }
 
 GalleryWatchStateTracker*
 MediaGalleriesPrivateAPI::GetGalleryWatchStateTracker() {
   MaybeInitializeEventRouterAndTracker();
@@ skipping 107 matching lines @@
       RemoveGalleryWatch::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
 
   StorageMonitor::GetInstance()->EnsureInitialized(base::Bind(
       &MediaGalleriesPrivateRemoveGalleryWatchFunction::OnStorageMonitorInit,
       this,
       params->gallery_id));
   return true;
 }
 
 void MediaGalleriesPrivateRemoveGalleryWatchFunction::OnStorageMonitorInit(

[Lei Zhang, 2013/09/21 00:12:35]

Doesn't this also need to change?

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

Done. Yes it does. Thanks.

     const std::string& pref_id) {
   DCHECK(StorageMonitor::GetInstance()->IsInitialized());
 #if defined(OS_WIN)
   base::FilePath gallery_file_path;
   MediaGalleryPrefId gallery_pref_id = 0;
   if (!GetGalleryFilePathAndId(pref_id, profile_, GetExtension(),
                                &gallery_file_path, &gallery_pref_id)) {
     error_ = kInvalidGalleryIDError;
     SendResponse(false);
     return;
@@ skipping 55 matching lines @@
 
 MediaGalleriesPrivateRemoveAllGalleryWatchFunction::
 ~MediaGalleriesPrivateRemoveAllGalleryWatchFunction() {
 }
 
 bool MediaGalleriesPrivateRemoveAllGalleryWatchFunction::RunImpl() {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   if (!render_view_host() || !render_view_host()->GetProcess())
     return false;
 
-  StorageMonitor::GetInstance()->EnsureInitialized(base::Bind(
-      &MediaGalleriesPrivateRemoveAllGalleryWatchFunction::OnStorageMonitorInit,
-      this));
+  g_browser_process->media_file_system_registry()
+      ->GetPreferences(profile_)
+      ->EnsureInitialized(
+            base::Bind(&MediaGalleriesPrivateRemoveAllGalleryWatchFunction::
+                            OnPreferencesInit,
+                       this));
   return true;
 }
 
-void
-MediaGalleriesPrivateRemoveAllGalleryWatchFunction::OnStorageMonitorInit() {
+void MediaGalleriesPrivateRemoveAllGalleryWatchFunction::OnPreferencesInit() {
 #if defined(OS_WIN)
   DCHECK(StorageMonitor::GetInstance()->IsInitialized());
-  MediaFileSystemRegistry* registry =
-      g_browser_process->media_file_system_registry();
-  MediaGalleriesPreferences* preferences = registry->GetPreferences(profile_);
+  MediaGalleriesPreferences* preferences =
+      g_browser_process->media_file_system_registry()->GetPreferences(profile_);
+  DCHECK(preferences->IsInitialized());
   GalleryWatchStateTracker* state_tracker =
       MediaGalleriesPrivateAPI::Get(profile_)->GetGalleryWatchStateTracker();
   state_tracker->RemoveAllGalleryWatchersForExtension(
       extension_id(), preferences);
 #endif
   SendResponse(true);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 //              MediaGalleriesPrivateGetHandlersFunction                     //
@@ skipping 39 matching lines @@
     }
   }
 
   SetResult(result_list);
   SendResponse(true);
 
   return true;
 }
 
 }  // namespace extensions