Media Galleries: Always call MediaFileSystemRegistry::GetMediaFileSystemsForExtension() before show…

chrome/browser/extensions/api/media_galleries/media_galleries_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.
 
 // Implements the Chrome Extensions Media Galleries API.
 
 #include "chrome/browser/extensions/api/media_galleries/media_galleries_api.h"
 
 #include <set>
 #include <string>
@@ skipping 20 matching lines @@
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 
 #if defined(OS_WIN)
 #include "base/sys_string_conversions.h"
 #endif
 
 using chrome::MediaFileSystemInfo;
 using chrome::MediaFileSystemRegistry;
+using chrome::MediaFileSystemsCallback;
 using content::ChildProcessSecurityPolicy;
 using content::WebContents;
 
 namespace MediaGalleries = extensions::api::media_galleries;
 namespace GetMediaFileSystems = MediaGalleries::GetMediaFileSystems;
 
 namespace extensions {
 
 namespace {
 
@@ skipping 29 matching lines @@
       GetMediaFileSystems::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
   MediaGalleries::GetMediaFileSystemsInteractivity interactive =
       MediaGalleries::GET_MEDIA_FILE_SYSTEMS_INTERACTIVITY_NO;
   if (params->details.get() && params->details->interactive != MediaGalleries::
          GET_MEDIA_FILE_SYSTEMS_INTERACTIVITY_NONE) {
     interactive = params->details->interactive;
   }
 
   switch (interactive) {
-    case MediaGalleries::GET_MEDIA_FILE_SYSTEMS_INTERACTIVITY_YES:
-      ShowDialog();
+    case MediaGalleries::GET_MEDIA_FILE_SYSTEMS_INTERACTIVITY_YES: {
+      // To keep the media galleries permissions dialogs in sync with pref

[vandebo (ex-Chrome), 2013/01/30 23:23:12]

nit nit: the cause and effect seem backward in this first sentence... an attempt
to tighten it up...

The MediaFileSystemRegistry only updates preferences for extensions that it
knows are in use. Since this may be the first call to chrome.getMediaFileSystems
for this extension, GetMediaFileSystemsForExtension() is called here solely so
that MediaFileSystemRegistry will send preference changes.

[Lei Zhang, 2013/01/30 23:29:42]

Sounds good.

+      // changes, the MediaFileSystemRegistry has to update the preferences
+      // for all the extensions it knows about. If this is the first time an
+      // extension called chrome.getMediaFileSystems, the
+      // MediaFileSystemRegistry would not know about the extension yet. To let
+      // MediaFileSystemRegistry know about the extension, call
+      // GetMediaFileSystemsForExtension() here even though the results
+      // returned in the callback are unused.
+      GetMediaFileSystemsForExtension(base::Bind(
+          &MediaGalleriesGetMediaFileSystemsFunction::AlwaysShowDialog, this));
       return true;
+    }
     case MediaGalleries::GET_MEDIA_FILE_SYSTEMS_INTERACTIVITY_IF_NEEDED: {
-      MediaFileSystemRegistry* registry =
-          g_browser_process->media_file_system_registry();
-      registry->GetMediaFileSystemsForExtension(
-          render_view_host(), GetExtension(), base::Bind(
-              &MediaGalleriesGetMediaFileSystemsFunction::
-                  ShowDialogIfNoGalleries,
-              this));
+      GetMediaFileSystemsForExtension(base::Bind(
+          &MediaGalleriesGetMediaFileSystemsFunction::ShowDialogIfNoGalleries,
+          this));
       return true;
     }
     case MediaGalleries::GET_MEDIA_FILE_SYSTEMS_INTERACTIVITY_NO:
       GetAndReturnGalleries();
       return true;
     case MediaGalleries::GET_MEDIA_FILE_SYSTEMS_INTERACTIVITY_NONE:
       NOTREACHED();
   }
   error_ = kInvalidInteractive;
   return false;
 }
 
+void MediaGalleriesGetMediaFileSystemsFunction::AlwaysShowDialog(
+    const std::vector<MediaFileSystemInfo>& /*filesystems*/) {
+  ShowDialog();
+}
+
 void MediaGalleriesGetMediaFileSystemsFunction::ShowDialogIfNoGalleries(
     const std::vector<MediaFileSystemInfo>& filesystems) {
   if (filesystems.empty())
     ShowDialog();
   else
     ReturnGalleries(filesystems);
 }
 
 void MediaGalleriesGetMediaFileSystemsFunction::GetAndReturnGalleries() {
-  MediaFileSystemRegistry* registry =
-      g_browser_process->media_file_system_registry();
-  registry->GetMediaFileSystemsForExtension(
-      render_view_host(), GetExtension(), base::Bind(
-          &MediaGalleriesGetMediaFileSystemsFunction::ReturnGalleries, this));
+  GetMediaFileSystemsForExtension(base::Bind(
+      &MediaGalleriesGetMediaFileSystemsFunction::ReturnGalleries, this));
 }
 
 void MediaGalleriesGetMediaFileSystemsFunction::ReturnGalleries(
     const std::vector<MediaFileSystemInfo>& filesystems) {
   content::RenderViewHost* rvh = render_view_host();
   if (!rvh) {
     SendResponse(false);
     return;
   }
   MediaGalleriesPermission::CheckParam read_param(
@@ skipping 77 matching lines @@
       return;
     }
   }
 
   // Controller will delete itself.
   base::Closure cb = base::Bind(
       &MediaGalleriesGetMediaFileSystemsFunction::GetAndReturnGalleries, this);
   new chrome::MediaGalleriesDialogController(contents, *GetExtension(), cb);
 }
 
+void MediaGalleriesGetMediaFileSystemsFunction::GetMediaFileSystemsForExtension(
+    const chrome::MediaFileSystemsCallback& cb) {
+  MediaFileSystemRegistry* registry =
+      g_browser_process->media_file_system_registry();
+  registry->GetMediaFileSystemsForExtension(
+      render_view_host(), GetExtension(), cb);
+}
+
 // MediaGalleriesAssembleMediaFileFunction -------------------------------------
 
 MediaGalleriesAssembleMediaFileFunction::
     ~MediaGalleriesAssembleMediaFileFunction() {}
 
 bool MediaGalleriesAssembleMediaFileFunction::RunImpl() {
   if (!ApiIsAccessible(&error_))
     return false;
 
   // TODO(vandebo) Update the metadata and return the new file.
   SetResult(base::Value::CreateNullValue());
   return true;
 }
 
 }  // namespace extensions