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();
-      return true;
-    case MediaGalleries::GET_MEDIA_FILE_SYSTEMS_INTERACTIVITY_IF_NEEDED: {
+    case MediaGalleries::GET_MEDIA_FILE_SYSTEMS_INTERACTIVITY_YES: {
+      MediaFileSystemsCallback cb = base::Bind(
+          &MediaGalleriesGetMediaFileSystemsFunction::AlwaysShowDialog, this);
       MediaFileSystemRegistry* registry =
           g_browser_process->media_file_system_registry();
       registry->GetMediaFileSystemsForExtension(

[vandebo (ex-Chrome), 2013/01/30 17:47:30]

Since this isn't strictly required, add a comment about why we do it anyway.

[Lei Zhang, 2013/01/30 21:00:47]

Done.

-          render_view_host(), GetExtension(), base::Bind(
-              &MediaGalleriesGetMediaFileSystemsFunction::
-                  ShowDialogIfNoGalleries,
-              this));
+          render_view_host(), GetExtension(), cb);
+      return true;
+    }
+    case MediaGalleries::GET_MEDIA_FILE_SYSTEMS_INTERACTIVITY_IF_NEEDED: {
+      MediaFileSystemsCallback cb = base::Bind(
+          &MediaGalleriesGetMediaFileSystemsFunction::ShowDialogIfNoGalleries,

[vandebo (ex-Chrome), 2013/01/30 17:47:30]

Can you rearrange this code to eliminate the code duplication?

[Lei Zhang, 2013/01/30 21:00:47]

Done.

+          this);
+      MediaFileSystemRegistry* registry =
+          g_browser_process->media_file_system_registry();
+      registry->GetMediaFileSystemsForExtension(
+          render_view_host(), GetExtension(), cb);
       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();

[vandebo (ex-Chrome), 2013/01/30 17:47:30]

Or maybe the comment should go here.

+}
+
 void MediaGalleriesGetMediaFileSystemsFunction::ShowDialogIfNoGalleries(
     const std::vector<MediaFileSystemInfo>& filesystems) {
   if (filesystems.empty())
     ShowDialog();
   else
     ReturnGalleries(filesystems);
 }
 
 void MediaGalleriesGetMediaFileSystemsFunction::GetAndReturnGalleries() {
   MediaFileSystemRegistry* registry =
@@ skipping 106 matching lines @@
 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