OpenFileSystem should accept not mount_type but lower filesystem type

chrome/browser/chromeos/fileapi/file_system_backend.cc

 // Copyright 2013 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/chromeos/fileapi/file_system_backend.h"
 
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/path_service.h"
@@ skipping 91 matching lines @@
 }
 
 void FileSystemBackend::Initialize(fileapi::FileSystemContext* context) {
 }
 
 void FileSystemBackend::OpenFileSystem(
     const GURL& origin_url,
     fileapi::FileSystemType type,
     fileapi::OpenFileSystemMode mode,
     const OpenFileSystemCallback& callback) {
-  DCHECK(fileapi::IsolatedContext::IsIsolatedType(type));
+  DCHECK(CanHandleType(type));
+  fileapi::FileSystemType mount_type = fileapi::kFileSystemTypeExternal;
   // Nothing to validate for external filesystem.
-  callback.Run(GetFileSystemRootURI(origin_url, type),
-               GetFileSystemName(origin_url, type),
+  callback.Run(GetFileSystemRootURI(origin_url, mount_type),
+               GetFileSystemName(origin_url, mount_type),

[kinuko, 2013/09/23 23:57:34]

Hmm... this looks to be a very unfortunate change, I think at least we need a
TODO comment here.

Here's why: if cros backend's going to migrate to multiple filesystems this
should return a root URL that includes mount-name path for the given fs type
(like we do for syncfs).  It doesn't look correct to always assume kExternal
type for all filesystem types either.

Some more thoughts:
- OpenFileSystem for non-sandboxed filesystems should be probably just
deprecated as it has no points.  Existing callsites (there're three, [1]) should
be revised either to use ResolveURL or to directly use GetFileSystemRootURI /
GetExternalFileSystemRootURIString
- When crbug.com/284963 is done ResolveURI should start to return the correct
root URI which includes the mount name for isolated/external types (and the part
must be able to be generalized so that we don't need to call per-backend
OpenFileSystem for non-sandboxed filesystems just to get the rootURI & name)

[1]
https://code.google.com/p/chromium/codesearch#search/&q=%5C-%3EOpenFileSystem...

[satorux1, 2013/09/24 04:12:51]

Agreed. TODO comment is a crbug.com URL would be nice here.

[nhiroki, 2013/09/24 06:14:28]

Filed (http://crbug.com/297412) and added a comment.

                base::PLATFORM_FILE_OK);
 }
 
 fileapi::FileSystemQuotaUtil* FileSystemBackend::GetQuotaUtil() {
   // No quota support.
   return NULL;
 }
 
 bool FileSystemBackend::IsAccessAllowed(
     const fileapi::FileSystemURL& url) const {
@@ skipping 166 matching lines @@
 }
 
 bool FileSystemBackend::GetVirtualPath(
     const base::FilePath& filesystem_path,
     base::FilePath* virtual_path) {
   return mount_points_->GetVirtualPath(filesystem_path, virtual_path) ||
          system_mount_points_->GetVirtualPath(filesystem_path, virtual_path);
 }
 
 }  // namespace chromeos