Replace mountType by volumeType on fileBrowserPrivate APIs.

This is API clean up. The mountType is being replaced by VolumeType. This CL
refactors private APIs about the flag.
Note that this is preparation to return Drive as a Volume.

BUG=279276
TEST=Ran unit_tests, browser_tests --gtest_filter="*FileSystemExtensionApiTest*:*FileManagerBrowserTest*:*FileBrowserPrivateApiTest*" and tested manually.
R=benwells@chromium.org, kinaba@chromium.org, satorux@chromium.org, yoshiki@chromium.org

Review URL: https://codereview.chromium.org/23658009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@221978 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/chromeos/extensions/file_manager/event_router.cc

@@ -16,6 +16,7 @@
 #include "chrome/browser/chromeos/drive/drive_integration_service.h"
 #include "chrome/browser/chromeos/drive/file_system_interface.h"
 #include "chrome/browser/chromeos/drive/file_system_util.h"
+#include "chrome/browser/chromeos/extensions/file_manager/private_api_util.h"
 #include "chrome/browser/chromeos/file_manager/app_id.h"
 #include "chrome/browser/chromeos/file_manager/desktop_notifications.h"
 #include "chrome/browser/chromeos/file_manager/fileapi_util.h"
@@ -209,6 +210,23 @@ bool IsGooglePhotosInstalled(Profile *profile) {
   return false;
 }
 
+// This method is temporarily introduced to make a change step by step.
+// After mounting logic is moved to VolumeManager, this should be removed.
+std::string MountTypeToString(chromeos::MountType type) {
+  switch (type) {
+    case chromeos::MOUNT_TYPE_INVALID:
+      return "";
+    case chromeos::MOUNT_TYPE_DEVICE:
+      return util::VolumeTypeToStringEnum(VOLUME_TYPE_REMOVABLE_DISK_PARTITION);
+    case chromeos::MOUNT_TYPE_ARCHIVE:
+      return util::VolumeTypeToStringEnum(VOLUME_TYPE_MOUNTED_ARCHIVE_FILE);
+    case chromeos::MOUNT_TYPE_GOOGLE_DRIVE:
+      return util::VolumeTypeToStringEnum(VOLUME_TYPE_GOOGLE_DRIVE);
+  }
+  NOTREACHED();
+  return "";
+}
+
 }  // namespace
 
 // Pass dummy value to JobInfo's constructor for make it default constructible.
@@ -690,8 +708,7 @@ void EventRouter::DispatchMountEvent(
       event == DiskMountManager::MOUNTING ? "mount" : "unmount");
   mount_info_value->SetString("status", MountErrorToString(error_code));
   mount_info_value->SetString(
-      "mountType",
-      DiskMountManager::MountTypeToString(mount_info.mount_type));
+      "volumeType", MountTypeToString(mount_info.mount_type));
 
   // Add sourcePath to the event.
   mount_info_value->SetString("sourcePath", mount_info.source_path);

chrome/browser/chromeos/extensions/file_manager/private_api_mount.cc

@@ -25,34 +25,16 @@ using content::BrowserThread;
 namespace extensions {
 namespace {
 
-// Returns string representaion of VolumeType.
-std::string VolumeTypeToString(file_manager::VolumeType type) {
-  switch (type) {
-    case file_manager::VOLUME_TYPE_GOOGLE_DRIVE:
-      return "drive";
-    case file_manager::VOLUME_TYPE_DOWNLOADS_DIRECTORY:
-      // Return empty string here for backword compatibility.
-      // TODO(hidehiko): Fix this to return "downloads".
-      return "";
-    case file_manager::VOLUME_TYPE_REMOVABLE_DISK_PARTITION:
-      return "device";
-    case file_manager::VOLUME_TYPE_MOUNTED_ARCHIVE_FILE:
-      return "file";
-  }
-
-  NOTREACHED();
-  return "";
-}
-
 // Returns the Value of the |volume_info|.
 base::Value* CreateValueFromVolumeInfo(
     const file_manager::VolumeInfo& volume_info,
     Profile* profile,
     const std::string& extension_id) {
   base::DictionaryValue* result = new base::DictionaryValue;
-  std::string mount_type = VolumeTypeToString(volume_info.type);
-  if (!mount_type.empty())
-    result->SetString("mountType", mount_type);
+  std::string volume_type =
+      file_manager::util::VolumeTypeToStringEnum(volume_info.type);
+  if (!volume_type.empty())
+    result->SetString("volumeType", volume_type);
 
   if (!volume_info.source_path.empty())
     result->SetString("sourcePath", volume_info.source_path.value());
@@ -86,83 +68,64 @@ bool FileBrowserPrivateAddMountFunction::RunImpl() {
   }
 
   std::string file_url;
-  if (!args_->GetString(0, &file_url)) {
+  if (!args_->GetString(0, &file_url))
     return false;
-  }
 
-  std::string mount_type_str;
-  if (!args_->GetString(1, &mount_type_str)) {
+  std::string mount_type;
+  if (!args_->GetString(1, &mount_type))
     return false;
-  }
 
   drive::util::Log(logging::LOG_INFO,
                    "%s[%d] called. (source: '%s', type:'%s')",
                    name().c_str(),
                    request_id(),
                    file_url.empty() ? "(none)" : file_url.c_str(),
-                   mount_type_str.c_str());
+                   mount_type.c_str());
   set_log_on_completion(true);
 
-  // Set default return source path to the empty string.
-  SetResult(new base::StringValue(""));
-
-  chromeos::MountType mount_type =
-      DiskMountManager::MountTypeFromString(mount_type_str);
-  switch (mount_type) {
-    case chromeos::MOUNT_TYPE_INVALID: {
-      error_ = "Invalid mount type";
-      SendResponse(false);
-      break;
-    }
-    case chromeos::MOUNT_TYPE_GOOGLE_DRIVE: {
-      // Dispatch fake 'mounted' event because JS code depends on it.
-      // TODO(hashimoto): Remove this redanduncy.
-      file_manager::FileBrowserPrivateAPI::Get(profile_)->event_router()->
-          OnFileSystemMounted();
-
-      // Pass back the drive mount point path as source path.
-      const std::string& drive_path =
-          drive::util::GetDriveMountPointPathAsString();
-      SetResult(new base::StringValue(drive_path));
-      SendResponse(true);
-      break;
-    }
-    default: {
-      const base::FilePath path = file_manager::util::GetLocalPathFromURL(
-          render_view_host(), profile(), GURL(file_url));
-
-      if (path.empty()) {
-        SendResponse(false);
-        break;
-      }
-
-      const base::FilePath::StringType display_name = path.BaseName().value();
-
-      // Check if the source path is under Drive cache directory.
-      if (drive::util::IsUnderDriveMountPoint(path)) {
-        drive::FileSystemInterface* file_system =
-            drive::util::GetFileSystemByProfile(profile());
-        if (!file_system) {
-          SendResponse(false);
-          break;
-        }
-        file_system->MarkCacheFileAsMounted(
-            drive::util::ExtractDrivePath(path),
-            base::Bind(&FileBrowserPrivateAddMountFunction::OnMountedStateSet,
-                       this, mount_type_str, display_name));
-      } else {
-        OnMountedStateSet(mount_type_str, display_name,
-                          drive::FILE_ERROR_OK, path);
-      }
-      break;
+  if (mount_type == "drive") {
+    // Dispatch fake 'mounted' event because JS code depends on it.
+    // TODO(hashimoto): Remove this redanduncy.
+    file_manager::FileBrowserPrivateAPI::Get(profile_)->event_router()->
+        OnFileSystemMounted();
+
+    // Pass back the drive mount point path as source path.
+    const std::string& drive_path =
+        drive::util::GetDriveMountPointPathAsString();
+    SetResult(new base::StringValue(drive_path));
+    SendResponse(true);
+  } else if (mount_type == "archive") {
+    const base::FilePath path = file_manager::util::GetLocalPathFromURL(
+        render_view_host(), profile(), GURL(file_url));
+
+    if (path.empty())
+      return false;
+
+    const base::FilePath::StringType display_name = path.BaseName().value();
+
+    // Check if the source path is under Drive cache directory.
+    if (drive::util::IsUnderDriveMountPoint(path)) {
+      drive::FileSystemInterface* file_system =
+          drive::util::GetFileSystemByProfile(profile());
+      if (!file_system)
+        return false;
+
+      file_system->MarkCacheFileAsMounted(
+          drive::util::ExtractDrivePath(path),
+          base::Bind(&FileBrowserPrivateAddMountFunction::OnMountedStateSet,
+                     this, display_name));
+    } else {
+      OnMountedStateSet(display_name, drive::FILE_ERROR_OK, path);
     }
+  } else {
+    error_ = "Invalid mount type";
+    return false;
   }
 
   return true;
 }
 
 void FileBrowserPrivateAddMountFunction::OnMountedStateSet(
-    const std::string& mount_type,
     const base::FilePath::StringType& file_name,
     drive::FileError error,
     const base::FilePath& file_path) {
@@ -180,7 +143,7 @@ void FileBrowserPrivateAddMountFunction::OnMountedStateSet(
   // MountPath() takes a std::string.
   disk_mount_manager->MountPath(
       file_path.AsUTF8Unsafe(), base::FilePath(file_name).Extension(),
-      file_name, DiskMountManager::MountTypeFromString(mount_type));
+      file_name, chromeos::MOUNT_TYPE_ARCHIVE);
 }
 
 FileBrowserPrivateRemoveMountFunction::FileBrowserPrivateRemoveMountFunction() {

chrome/browser/chromeos/extensions/file_manager/private_api_mount.h

@@ -37,8 +37,7 @@ class FileBrowserPrivateAddMountFunction : public LoggedAsyncExtensionFunction {
 
  private:
   // A callback method to handle the result of MarkCacheAsMounted.
-  void OnMountedStateSet(const std::string& mount_type,
-                         const base::FilePath::StringType& file_name,
+  void OnMountedStateSet(const base::FilePath::StringType& file_name,
                          drive::FileError error,
                          const base::FilePath& file_path);
 };

chrome/browser/chromeos/extensions/file_manager/private_api_util.cc

@@ -118,6 +118,23 @@ void ContinueGetSelectedFileInfo(Profile* profile,
 
 }  // namespace
 
+// Returns string representaion of VolumeType.
+std::string VolumeTypeToStringEnum(VolumeType type) {
+  switch (type) {
+    case VOLUME_TYPE_GOOGLE_DRIVE:
+      return "drive";
+    case VOLUME_TYPE_DOWNLOADS_DIRECTORY:
+      return "downloads";
+    case VOLUME_TYPE_REMOVABLE_DISK_PARTITION:
+      return "removable";
+    case VOLUME_TYPE_MOUNTED_ARCHIVE_FILE:
+      return "archive";
+  }
+
+  NOTREACHED();
+  return "";
+}
+
 int32 GetTabId(ExtensionFunctionDispatcher* dispatcher) {
   if (!dispatcher) {
     LOG(WARNING) << "No dispatcher";

chrome/browser/chromeos/extensions/file_manager/private_api_util.h

@@ -8,6 +8,7 @@
 #define CHROME_BROWSER_CHROMEOS_EXTENSIONS_FILE_MANAGER_PRIVATE_API_UTIL_H_
 
 #include "base/callback_forward.h"
+#include "chrome/browser/chromeos/file_manager/volume_manager.h"
 #include "chrome/browser/google_apis/gdata_wapi_parser.h"
 #include "url/gurl.h"
 
@@ -25,6 +26,9 @@ struct SelectedFileInfo;
 namespace file_manager {
 namespace util {
 
+// Converts VolumeType to a string enum (volumeType) used in the private API.
+std::string VolumeTypeToStringEnum(VolumeType type);
+
 // Returns the ID of the tab associated with the dispatcher. Returns 0 on
 // error.
 int32 GetTabId(ExtensionFunctionDispatcher* dispatcher);

chrome/browser/resources/file_manager/js/volume_manager.js

@@ -514,7 +514,7 @@ VolumeManager.prototype.initMountPoints_ = function() {
       var mountPoint = mountPointList[i];
 
       // TODO(hidehiko): It should be ok that the drive is mounted already.
-      if (mountPoint.mountType == 'drive')
+      if (mountPoint.volumeType == 'drive')
         console.error('Drive is not expected initially mounted');
 
       var error = mountPoint.mountCondition ?
@@ -562,7 +562,7 @@ VolumeManager.prototype.onMountCompleted_ = function(event) {
   if (event.eventType == 'mount') {
     if (event.mountPath) {
       var requestKey = this.makeRequestKey_(
-          'mount', event.mountType, event.sourcePath);
+          'mount', event.volumeType, event.sourcePath);
       var error = event.status == 'success' ? '' : event.status;
       volumeManagerUtil.createVolumeInfo(
           event.mountPath, error, function(volume) {
@@ -599,7 +599,7 @@ VolumeManager.prototype.onMountCompleted_ = function(event) {
     }
   }
 
-  if (event.mountType == 'drive') {
+  if (event.volumeType == 'drive') {
     if (event.status == 'success') {
       if (event.eventType == 'mount') {
         // If the mount is not requested, the mount status will not be changed
@@ -657,16 +657,16 @@ VolumeManager.prototype.waitDriveLoaded_ = function(mountPath, callback) {
 /**
  * Creates string to match mount events with requests.
  * @param {string} requestType 'mount' | 'unmount'.
- * @param {string} mountType 'device' | 'file' | 'network' | 'drive'.
+ * @param {string} volumeType 'drive' | 'downloads' | 'removable' | 'archive'.
  * @param {string} mountOrSourcePath Source path provided by API after
  *     resolving mount request or mountPath for unmount request.
  * @return {string} Key for |this.requests_|.
  * @private
  */
 VolumeManager.prototype.makeRequestKey_ = function(requestType,
-                                                   mountType,
+                                                   volumeType,
                                                    mountOrSourcePath) {
-  return requestType + ':' + mountType + ':' + mountOrSourcePath;
+  return requestType + ':' + volumeType + ':' + mountOrSourcePath;
 };
 
 
@@ -702,7 +702,7 @@ VolumeManager.prototype.mountDrive = function(successCallback, errorCallback) {
  */
 VolumeManager.prototype.mountArchive = function(fileUrl, successCallback,
                                                 errorCallback) {
-  this.mount_(fileUrl, 'file', successCallback, errorCallback);
+  this.mount_(fileUrl, 'archive', successCallback, errorCallback);
 };
 
 /**
@@ -743,25 +743,25 @@ VolumeManager.prototype.getVolumeInfo = function(mountPath) {
 
 /**
  * @param {string} url URL for for |fileBrowserPrivate.addMount|.
- * @param {'drive'|'file'} mountType Mount type for
+ * @param {'drive'|'archive'} volumeType Volume type for
  *     |fileBrowserPrivate.addMount|.
  * @param {function(string)} successCallback Success callback.
  * @param {function(VolumeManager.Error)} errorCallback Error callback.
  * @private
  */
-VolumeManager.prototype.mount_ = function(url, mountType,
+VolumeManager.prototype.mount_ = function(url, volumeType,
                                           successCallback, errorCallback) {
   if (this.deferredQueue_) {
     this.deferredQueue_.push(this.mount_.bind(this,
-        url, mountType, successCallback, errorCallback));
+        url, volumeType, successCallback, errorCallback));
     return;
   }
 
-  chrome.fileBrowserPrivate.addMount(url, mountType, {},
+  chrome.fileBrowserPrivate.addMount(url, volumeType, {},
                                      function(sourcePath) {
-    console.info('Mount request: url=' + url + '; mountType=' + mountType +
+    console.info('Mount request: url=' + url + '; volumeType=' + volumeType +
                  '; sourceUrl=' + sourcePath);
-    var requestKey = this.makeRequestKey_('mount', mountType, sourcePath);
+    var requestKey = this.makeRequestKey_('mount', volumeType, sourcePath);
     this.startRequest_(requestKey, successCallback, errorCallback);
   }.bind(this));
 };

chrome/common/extensions/api/file_browser_private.json

@@ -128,11 +128,11 @@
             "optional": true,
             "description": "The path to the mounted device, archive file or network resource."
           },
-          "mountType": {
+          "volumeType": {
             "type": "string",
             "optional": true,
-            "enum": ["device", "file", "network"],
-            "description": "Type of the mount."
+            "enum": ["drive", "downloads", "removable", "archive"],
+            "description": "Type of the mounted volume."
           },
           "mountCondition": {
             "type": "string",
@@ -272,10 +272,10 @@
             "optional": true,
             "description": "Path that sourcePath was mounted to."
           },
-          "mountType": {
+          "volumeType": {
             "type": "string",
-            "enum": ["device", "file", "network", "drive"],
-            "description": "Type of the mount."
+            "enum": ["drive", "downloads", "removable", "archive"],
+            "description": "Type of the mounted volume."
           }
         }
       },
@@ -731,11 +731,13 @@
             "type": "string",
             "description": "Mount point source. For compressed files it is relative file path within external file system"
           },
+	  // TODO(hidehiko): After the refactoring, we won't need to request
+	  // mounting the Drive. Then remove this param.
           {
-            "name": "mountType",
+            "name": "volumeType",
             "type": "string",
-            "enum": ["device", "file", "network", "drive"],
-            "description": "Mount point type. 'file' for compressed files"
+            "enum": ["drive", "archive"],
+            "description": "Type of the mounted volume"
           },
           {
             "name": "options",

chrome/test/data/extensions/api_test/filebrowser_mount/test.js

@@ -59,30 +59,31 @@ var expectedDownloadsVolume = {
 var expectedMountPoints = [
   {
     mountPath: 'Downloads',
+    volumeType: 'downloads',
     mountCondition: ''
   },
   {
     sourcePath: 'archive_path',
     mountPath: 'archive/archive_mount_path',
-    mountType: 'file',
+    volumeType: 'archive',
     mountCondition: ''
   },
   {
     sourcePath: 'device_path1',
     mountPath: 'removable/mount_path1',
-    mountType: 'device',
+    volumeType: 'removable',
     mountCondition: ''
   },
   {
     sourcePath: 'device_path2',
     mountPath: 'removable/mount_path2',
-    mountType: 'device',
+    volumeType: 'removable',
     mountCondition: ''
   },
   {
     sourcePath: 'device_path3',
     mountPath: 'removable/mount_path3',
-    mountType: 'device',
+    volumeType: 'removable',
     mountCondition: ''
   }
 ];

chromeos/dbus/cros_disks_client.h

@@ -33,8 +33,9 @@ enum MountType {
   MOUNT_TYPE_INVALID,
   MOUNT_TYPE_DEVICE,
   MOUNT_TYPE_ARCHIVE,
+  // TODO(hidehiko): Drive is not managed by DiskMountManager nor
+  // CrosDisksClient. Remove this after fileBrowserPrivate API is cleaned.
   MOUNT_TYPE_GOOGLE_DRIVE,
-  MOUNT_TYPE_NETWORK_STORAGE,
 };
 
 // Type of device.

chromeos/disks/disk_mount_manager.cc

@@ -669,25 +669,6 @@ bool DiskMountManager::AddMountPointForTest(const MountPointInfo& mount_point) {
   return false;
 }
 
-// static
-std::string DiskMountManager::MountTypeToString(MountType type) {
-  switch (type) {
-    case MOUNT_TYPE_DEVICE:
-      return "device";
-    case MOUNT_TYPE_ARCHIVE:
-      return "file";
-    case MOUNT_TYPE_NETWORK_STORAGE:
-      return "network";
-    case MOUNT_TYPE_GOOGLE_DRIVE:
-      return "drive";
-    case MOUNT_TYPE_INVALID:
-      return "invalid";
-    default:
-      NOTREACHED();
-  }
-  return "";
-}
-
 // static
 std::string DiskMountManager::MountConditionToString(MountCondition condition) {
   switch (condition) {
@@ -703,20 +684,6 @@ std::string DiskMountManager::MountConditionToString(MountCondition condition) {
   return "";
 }
 
-// static
-MountType DiskMountManager::MountTypeFromString(const std::string& type_str) {
-  if (type_str == "device")
-    return MOUNT_TYPE_DEVICE;
-  else if (type_str == "network")
-    return MOUNT_TYPE_NETWORK_STORAGE;
-  else if (type_str == "file")
-    return MOUNT_TYPE_ARCHIVE;
-  else if (type_str == "drive")
-    return MOUNT_TYPE_GOOGLE_DRIVE;
-  else
-    return MOUNT_TYPE_INVALID;
-}
-
 // static
 std::string DiskMountManager::DeviceTypeToString(DeviceType type) {
   switch (type) {

chromeos/disks/disk_mount_manager.h

@@ -272,12 +272,6 @@ class CHROMEOS_EXPORT DiskMountManager {
   virtual bool AddDiskForTest(Disk* disk);
   virtual bool AddMountPointForTest(const MountPointInfo& mount_point);
 
-  // Returns corresponding string to |type| like "device" or "file".
-  static std::string MountTypeToString(MountType type);
-
-  // The inverse function of MountTypeToString.
-  static MountType MountTypeFromString(const std::string& type_str);
-
   // Returns corresponding string to |type| like "unknown_filesystem".
   static std::string MountConditionToString(MountCondition type);