Media Galleries API Extension functions: Refactor Preference initialization block.

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 48 matching lines @@
 const char kDisallowedByPolicy[] =
     "Media Galleries API is disallowed by policy: ";
 
 const char kDeviceIdKey[] = "deviceId";
 const char kGalleryIdKey[] = "galleryId";
 const char kIsAvailableKey[] = "isAvailable";
 const char kIsMediaDeviceKey[] = "isMediaDevice";
 const char kIsRemovableKey[] = "isRemovable";
 const char kNameKey[] = "name";
 
+MediaFileSystemRegistry* media_file_system_registry() {
+  return g_browser_process->media_file_system_registry();
+}
+
 // Checks whether the MediaGalleries API is currently accessible (it may be
-// disallowed even if an extension has the requisite permission).
-bool ApiIsAccessible(std::string* error) {
+// disallowed even if an extension has the requisite permission). Then
+// initializes the MediaGalleriesPreferences
+bool SetupCall(Profile* profile, std::string* error, base::Closure callback) {

[vandebo (ex-Chrome), 2014/01/14 17:03:39]

nit: Initialize or Setup

[tommycli, 2014/01/14 17:42:41]

Done.

   if (!ChromeSelectFilePolicy::FileSelectDialogsAllowed()) {
     *error = std::string(kDisallowedByPolicy) +
         prefs::kAllowFileSelectionDialogs;
     return false;
   }
 
+  MediaGalleriesPreferences* preferences =
+      media_file_system_registry()->GetPreferences(profile);
+  preferences->EnsureInitialized(callback);
   return true;
 }
 
-MediaFileSystemRegistry* media_file_system_registry() {
-  return g_browser_process->media_file_system_registry();
-}
-
 WebContents* GetWebContents(content::RenderViewHost* rvh,
                             Profile* profile,
                             const std::string& app_id) {
   WebContents* contents = WebContents::FromRenderViewHost(rvh);
   WebContentsModalDialogManager* web_contents_modal_dialog_manager =
       WebContentsModalDialogManager::FromWebContents(contents);
   if (!web_contents_modal_dialog_manager) {
     // If there is no WebContentsModalDialogManager, then this contents is
     // probably the background page for an app. Try to find a shell window to
     // host the dialog.
@@ skipping 126 matching lines @@
 
   DISALLOW_COPY_AND_ASSIGN(SelectDirectoryDialog);
 };
 
 }  // namespace
 
 MediaGalleriesGetMediaFileSystemsFunction::
     ~MediaGalleriesGetMediaFileSystemsFunction() {}
 
 bool MediaGalleriesGetMediaFileSystemsFunction::RunImpl() {
-  if (!ApiIsAccessible(&error_))
-    return false;
+  return SetupCall(GetProfile(), &error_, base::Bind(
+      &MediaGalleriesGetMediaFileSystemsFunction::OnPreferencesInit, this));
+}
 
+void MediaGalleriesGetMediaFileSystemsFunction::OnPreferencesInit() {
   media_galleries::UsageCount(media_galleries::GET_MEDIA_FILE_SYSTEMS);
   scoped_ptr<GetMediaFileSystems::Params> params(
       GetMediaFileSystems::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
   MediaGalleries::GetMediaFileSystemsInteractivity interactive =

[vandebo (ex-Chrome), 2014/01/14 17:03:39]

Parameter validation should happen before RunImpl returns. 

[tommycli, 2014/01/14 17:42:41]

Done. By the way, since the parameter checks now occur before the
ApiIsAccessible check, if we have both an inaccessible API AND invalid
parameters, the error returned will be different with this change.

If that's not acceptable, I'm not sure we can do much cleanup here...

[vandebo (ex-Chrome), 2014/01/14 17:52:25]

That seems ok since none of the parameters checks on dependent on instance
specific data.

       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;
   }
 
-  MediaGalleriesPreferences* preferences =
-      media_file_system_registry()->GetPreferences(GetProfile());
-  preferences->EnsureInitialized(base::Bind(
-      &MediaGalleriesGetMediaFileSystemsFunction::OnPreferencesInit,
-      this,
-      interactive));
-  return true;
-}
-
-void MediaGalleriesGetMediaFileSystemsFunction::OnPreferencesInit(
-    MediaGalleries::GetMediaFileSystemsInteractivity interactive) {
   switch (interactive) {
     case MediaGalleries::GET_MEDIA_FILE_SYSTEMS_INTERACTIVITY_YES: {
       // 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, call
       // GetMediaFileSystemsForExtension() here solely so that
       // MediaFileSystemRegistry will send preference changes.
       GetMediaFileSystemsForExtension(base::Bind(
           &MediaGalleriesGetMediaFileSystemsFunction::AlwaysShowDialog, this));
       return;
@@ skipping 66 matching lines @@
   MediaFileSystemRegistry* registry = media_file_system_registry();
   DCHECK(registry->GetPreferences(GetProfile())->IsInitialized());
   registry->GetMediaFileSystemsForExtension(
       render_view_host(), GetExtension(), cb);
 }
 
 MediaGalleriesGetAllMediaFileSystemMetadataFunction::
     ~MediaGalleriesGetAllMediaFileSystemMetadataFunction() {}
 
 bool MediaGalleriesGetAllMediaFileSystemMetadataFunction::RunImpl() {
-  if (!ApiIsAccessible(&error_))
-    return false;
-
-  media_galleries::UsageCount(
-      media_galleries::GET_ALL_MEDIA_FILE_SYSTEM_METADATA);
-  MediaGalleriesPreferences* preferences =
-      media_file_system_registry()->GetPreferences(GetProfile());
-  preferences->EnsureInitialized(base::Bind(
+  return SetupCall(GetProfile(), &error_, base::Bind(
       &MediaGalleriesGetAllMediaFileSystemMetadataFunction::OnPreferencesInit,
       this));
-  return true;
 }
 
 void MediaGalleriesGetAllMediaFileSystemMetadataFunction::OnPreferencesInit() {
+  media_galleries::UsageCount(
+      media_galleries::GET_ALL_MEDIA_FILE_SYSTEM_METADATA);
+
   MediaFileSystemRegistry* registry = media_file_system_registry();
   MediaGalleriesPreferences* prefs = registry->GetPreferences(GetProfile());
   DCHECK(prefs->IsInitialized());
   MediaGalleryPrefIdSet permitted_gallery_ids =
       prefs->GalleriesForExtension(*GetExtension());
 
   MediaStorageUtil::DeviceIdSet* device_ids = new MediaStorageUtil::DeviceIdSet;
   const MediaGalleriesPrefInfoMap& galleries = prefs->known_galleries();
   for (MediaGalleryPrefIdSet::const_iterator it = permitted_gallery_ids.begin();
        it != permitted_gallery_ids.end(); ++it) {
@@ skipping 34 matching lines @@
   }
 
   SetResult(list);
   SendResponse(true);
 }
 
 MediaGalleriesAddUserSelectedFolderFunction::
     ~MediaGalleriesAddUserSelectedFolderFunction() {}
 
 bool MediaGalleriesAddUserSelectedFolderFunction::RunImpl() {
-  if (!ApiIsAccessible(&error_))
-    return false;
-
-  media_galleries::UsageCount(media_galleries::ADD_USER_SELECTED_FOLDER);
-  MediaGalleriesPreferences* preferences =
-      media_file_system_registry()->GetPreferences(GetProfile());
-  preferences->EnsureInitialized(base::Bind(
-      &MediaGalleriesAddUserSelectedFolderFunction::OnPreferencesInit,
-      this));
-  return true;
+  return SetupCall(GetProfile(), &error_, base::Bind(
+      &MediaGalleriesAddUserSelectedFolderFunction::OnPreferencesInit, this));
 }
 
 void MediaGalleriesAddUserSelectedFolderFunction::OnPreferencesInit() {
+  media_galleries::UsageCount(media_galleries::ADD_USER_SELECTED_FOLDER);
+
   if (!user_gesture()) {
     OnDirectorySelected(base::FilePath());
     return;
   }
 
   Profile* profile = GetProfile();
   const std::string& app_id = GetExtension()->id();
   WebContents* contents = GetWebContents(render_view_host(), profile, app_id);
   base::FilePath last_used_path =
       extensions::file_system_api::GetLastChooseEntryDirectory(
@@ skipping 68 matching lines @@
   }
   MediaFileSystemRegistry* registry = media_file_system_registry();
   DCHECK(registry->GetPreferences(GetProfile())->IsInitialized());
   registry->GetMediaFileSystemsForExtension(
       render_view_host(), GetExtension(), cb);
 }
 
 MediaGalleriesGetMetadataFunction::~MediaGalleriesGetMetadataFunction() {}
 
 bool MediaGalleriesGetMetadataFunction::RunImpl() {
-  if (!ApiIsAccessible(&error_))
-    return false;
+  return SetupCall(GetProfile(), &error_, base::Bind(
+      &MediaGalleriesGetMetadataFunction::OnPreferencesInit, this));
+}
+
+void MediaGalleriesGetMetadataFunction::OnPreferencesInit() {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
   std::string blob_uuid;
   EXTENSION_FUNCTION_VALIDATE(args_->GetString(0, &blob_uuid));
 
   const base::Value* options_value = NULL;
-  if (!args_->Get(1, &options_value))
-    return false;
+  if (!args_->Get(1, &options_value)) {

[vandebo (ex-Chrome), 2014/01/14 17:03:39]

And here

[tommycli, 2014/01/14 17:42:41]

Done.

+    SendResponse(false);
+    return;
+  }
   scoped_ptr<MediaGalleries::MediaMetadataOptions> options =
       MediaGalleries::MediaMetadataOptions::FromValue(*options_value);
-  if (!options)
-    return false;
+  if (!options) {
+    SendResponse(false);
+    return;
+  }
 
-  MediaGalleriesPreferences* preferences =
-      media_file_system_registry()->GetPreferences(GetProfile());
   bool mime_type_only = options->metadata_type ==
       MediaGalleries::GET_METADATA_TYPE_MIMETYPEONLY;
-  preferences->EnsureInitialized(base::Bind(
-      &MediaGalleriesGetMetadataFunction::OnPreferencesInit,
-      this, mime_type_only, blob_uuid));
-
-  return true;
-}
-
-void MediaGalleriesGetMetadataFunction::OnPreferencesInit(
-    bool mime_type_only,
-    const std::string& blob_uuid) {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
   // BlobReader is self-deleting.
   BlobReader* reader = new BlobReader(
       GetProfile(),
       blob_uuid,
       base::Bind(&MediaGalleriesGetMetadataFunction::SniffMimeType, this,
                  mime_type_only));
   reader->SetByteRange(0, net::kMaxBytesToSniff);
   reader->Start();
 }
@@ skipping 10 matching lines @@
   if (mime_type_sniffed)
     metadata.mime_type = mime_type;
 
   // TODO(tommycli): Kick off SafeMediaMetadataParser if |mime_type_only| false.
 
   SetResult(metadata.ToValue().release());
   SendResponse(true);
 }
 
 }  // namespace extensions