kFileSystemTypeIsolated should be only used in the URL exposed to renderer

Also added verbose description in file_system_types.h.

BUG=none
TEST=none

Review URL: https://chromiumcodereview.appspot.com/10879002

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

10 files changed, 51 insertions, 50 deletions

diff --git a/webkit/chromeos/fileapi/cros_mount_point_provider.cc b/webkit/chromeos/fileapi/cros_mount_point_provider.cc
index ff5b401..79642c4 100644
--- a/webkit/chromeos/fileapi/cros_mount_point_provider.cc
+++ b/webkit/chromeos/fileapi/cros_mount_point_provider.cc
@@ -65,8 +65,8 @@ void CrosMountPointProvider::ValidateFileSystemRoot(
     fileapi::FileSystemType type,
     bool create,
     const ValidateFileSystemCallback& callback) {
+  DCHECK(fileapi::IsolatedContext::IsIsolatedType(type));
   // Nothing to validate for external filesystem.
-  DCHECK(type == fileapi::kFileSystemTypeExternal);
   callback.Run(base::PLATFORM_FILE_OK);
 }
 
@@ -75,7 +75,7 @@ FilePath CrosMountPointProvider::GetFileSystemRootPathOnFileThread(
     fileapi::FileSystemType type,
     const FilePath& virtual_path,
     bool create) {
-  DCHECK(type == fileapi::kFileSystemTypeExternal);
+  DCHECK(fileapi::IsolatedContext::IsIsolatedType(type));
   fileapi::FileSystemURL url(origin_url, type, virtual_path);
   if (!url.is_valid())
     return FilePath();
@@ -89,12 +89,13 @@ FilePath CrosMountPointProvider::GetFileSystemRootPathOnFileThread(
 
 bool CrosMountPointProvider::IsAccessAllowed(
     const fileapi::FileSystemURL& url) {
-  // TODO(kinuko): this should call CanHandleURL() once
-  // http://crbug.com/142267 is fixed.
-  if (url.type() != fileapi::kFileSystemTypeNativeLocal &&
-      url.type() != fileapi::kFileSystemTypeDrive)
+  if (!CanHandleURL(url))
     return false;
 
+  // No extra check is needed for isolated file systems.
+  if (url.mount_type() == fileapi::kFileSystemTypeIsolated)
+    return true;
+
   // Permit access to mount points from internal WebUI.
   const GURL& origin_url = url.origin();
   if (origin_url.SchemeIs(kChromeUIScheme))
diff --git a/webkit/fileapi/file_system_types.h b/webkit/fileapi/file_system_types.h
index 936c81f..72d3ac4 100644
--- a/webkit/fileapi/file_system_types.h
+++ b/webkit/fileapi/file_system_types.h
@@ -13,6 +13,10 @@ enum FileSystemType {
   // Indicates uninitialized or invalid filesystem type.
   kFileSystemTypeUnknown = -1,
 
+  // ------------------------------------------------------------------------
+  // Public FileSystem types, that are embedded in filesystem: URL and exposed
+  // to WebKit/renderer. Both Chrome and WebKit know how to handle these types.
+
   // Following two types are for TEMPORARY or PERSISTENT filesystems that
   // can be used by webapps via standard app-facing API
   // as defined in File API: Directories and System.
@@ -31,15 +35,21 @@ enum FileSystemType {
   // This filesystem is used only by Chrome OS as of writing.
   kFileSystemTypeExternal = WebKit::WebFileSystem::TypeExternal,
 
+  // ------------------------------------------------------------------------
+  // Private FileSystem types, that should not appear in filesystem: URL as
+  // WebKit has no idea how to handle those types.
+  //
+  // One can register (mount) a new file system with a private file system type
+  // using IsolatedContext.  Files in such file systems can be accessed via
+  // either Isolated or External public file system types (depending on
+  // how the file system is registered).
+  // See the comments for IsolatedContext and/or FileSystemURL for more details.
+
   // Should be used only for testing.
   kFileSystemTypeTest = 100,
 
-  // Following file system types are internal and they are not exposed to
-  // WebKit, but are accessible via IsolatedContext.
-
-  // Indicates a transient, isolated file system for a native local path.
-  // TODO(kinuko): Rename all kFileSystemTypeIsolated used as internal type
-  // with this one.
+  // Indicates a local filesystem where we can access files using native
+  // local path.
   kFileSystemTypeNativeLocal,
 
   // Indicates a transient, isolated file system for dragged files (which could
@@ -58,18 +68,6 @@ enum FileSystemType {
   kFileSystemTypeDrive,
 };
 
-enum FileSystemMountType {
-  kFileSystemMountTypeUnknown = -1,
-
-  // For kFileSystemTypeIsolated file systems. URLs for this type of
-  // file system is cracked via IsolatedContext.
-  kFileSystemMountTypeIsolated = kFileSystemTypeIsolated,
-
-  // For kFileSystemTypeIsolated file systems. URLs for this type of
-  // file system is cracked via IsolatedContext.
-  kFileSystemMountTypeExternal = kFileSystemTypeExternal,
-};
-
 }  // namespace fileapi
 
 #endif  // WEBKIT_FILEAPI_FILE_SYSTEM_TYPES_H_
diff --git a/webkit/fileapi/file_system_url.cc b/webkit/fileapi/file_system_url.cc
index f5247a0..e7193e3 100644
--- a/webkit/fileapi/file_system_url.cc
+++ b/webkit/fileapi/file_system_url.cc
@@ -12,12 +12,10 @@ namespace fileapi {
 
 FileSystemURL::FileSystemURL()
     : type_(kFileSystemTypeUnknown),
-      mount_type_(kFileSystemMountTypeUnknown),
       is_valid_(false) {}
 
 FileSystemURL::FileSystemURL(const GURL& url)
-    : type_(kFileSystemTypeUnknown),
-      mount_type_(kFileSystemMountTypeUnknown) {
+    : type_(kFileSystemTypeUnknown) {
   is_valid_ = CrackFileSystemURL(url, &origin_, &type_, &virtual_path_);
   MayCrackIsolatedPath();
 }
@@ -57,8 +55,8 @@ bool FileSystemURL::operator==(const FileSystemURL& that) const {
 
 void FileSystemURL::MayCrackIsolatedPath() {
   path_ = virtual_path_;
+  mount_type_ = type_;
   if (is_valid_ && IsolatedContext::IsIsolatedType(type_)) {
-    mount_type_ = static_cast<FileSystemMountType>(type_);
     // If the type is isolated, crack the path further to get the 'real'
     // filesystem type and path.
     is_valid_ = IsolatedContext::GetInstance()->CrackIsolatedPath(
diff --git a/webkit/fileapi/file_system_url.h b/webkit/fileapi/file_system_url.h
index 238965f..809cd9a 100644
--- a/webkit/fileapi/file_system_url.h
+++ b/webkit/fileapi/file_system_url.h
@@ -23,7 +23,7 @@ namespace fileapi {
 //
 // Example: For a URL 'filesystem:http://foo.com/temporary/foo/bar':
 //   origin() returns 'http://foo.com',
-//   type() returns kFileSystemTypeTemporary,
+//   type() and mount_type() return kFileSystemTypeTemporary,
 //   path() and virtual_path() return 'foo/bar', and
 //   filesystem_id() returns an empty string.
 //
@@ -40,7 +40,7 @@ namespace fileapi {
 //   path() returns '/media/removable/foo/bar',
 //   virtual_path() returns 'mount_name/foo/bar',
 //   filesystem_id() returns 'mount_name', and
-//   mount_type() returns kFileSystemMountTypeExternal.
+//   mount_type() returns kFileSystemTypeExternal.
 //
 class FILEAPI_EXPORT FileSystemURL {
  public:
@@ -73,8 +73,9 @@ class FILEAPI_EXPORT FileSystemURL {
   // See the class comment for details.
   const std::string& filesystem_id() const { return filesystem_id_; }
 
-  // Returns the mount type of this URL for isolated/external file system URLs.
-  FileSystemMountType mount_type() const { return mount_type_; }
+  // Returns the public file system type of this URL.
+  // See the class comment for details.
+  FileSystemType mount_type() const { return mount_type_; }
 
   std::string spec() const;
 
@@ -94,7 +95,7 @@ class FILEAPI_EXPORT FileSystemURL {
   // For isolated filesystem.
   std::string filesystem_id_;
   FilePath virtual_path_;
-  FileSystemMountType mount_type_;
+  FileSystemType mount_type_;
 
   bool is_valid_;
 };
diff --git a/webkit/fileapi/file_system_util.h b/webkit/fileapi/file_system_util.h
index af48b80..d6d45cc 100644
--- a/webkit/fileapi/file_system_util.h
+++ b/webkit/fileapi/file_system_util.h
@@ -66,6 +66,8 @@ class FILEAPI_EXPORT VirtualPath {
 //
 // For Isolated filesystem this returns the 'common' root part, e.g.
 // returns URL without the filesystem ID.
+//
+// |type| needs to be public type as the returned URI is given to the renderer.
 FILEAPI_EXPORT GURL GetFileSystemRootURI(const GURL& origin_url,
                                          FileSystemType type);
 
@@ -75,6 +77,8 @@ FILEAPI_EXPORT GURL GetFileSystemRootURI(const GURL& origin_url,
 // but can be read as the .name field of the returned FileSystem object
 // as a user-friendly name in the javascript layer).
 //
+// |type| needs to be public type as the returned name is given to the renderer.
+//
 // Example:
 //   The name for a TEMPORARY filesystem of "http://www.example.com:80/"
 //   should look like: "http_www.example.host_80:temporary"
diff --git a/webkit/fileapi/isolated_context.cc b/webkit/fileapi/isolated_context.cc
index a625693..b42a304 100644
--- a/webkit/fileapi/isolated_context.cc
+++ b/webkit/fileapi/isolated_context.cc
@@ -107,7 +107,7 @@ bool IsolatedContext::FileInfoSet::AddPathWithName(
 
 class IsolatedContext::Instance {
  public:
-  typedef FileSystemMountType MountType;
+  typedef FileSystemType MountType;
 
   // For a single-path isolated file system, which could be registered by
   // IsolatedContext::RegisterFileSystemForPath().
@@ -157,7 +157,7 @@ class IsolatedContext::Instance {
 
 IsolatedContext::Instance::Instance(FileSystemType type,
                                     const FileInfo& file_info)
-    : mount_type_(kFileSystemMountTypeIsolated),
+    : mount_type_(kFileSystemTypeIsolated),
       type_(type),
       file_info_(file_info),
       ref_counts_(0) {
@@ -166,7 +166,7 @@ IsolatedContext::Instance::Instance(FileSystemType type,
 
 IsolatedContext::Instance::Instance(FileSystemType type,
                                     const std::set<FileInfo>& files)
-    : mount_type_(kFileSystemMountTypeIsolated),
+    : mount_type_(kFileSystemTypeIsolated),
       type_(type),
       files_(files),
       ref_counts_(0) {
@@ -175,7 +175,7 @@ IsolatedContext::Instance::Instance(FileSystemType type,
 
 IsolatedContext::Instance::Instance(FileSystemType type,
                                     const FilePath& path)
-    : mount_type_(kFileSystemMountTypeExternal),
+    : mount_type_(kFileSystemTypeExternal),
       type_(type),
       file_info_(FileInfo("", path)),
       ref_counts_(0) {
@@ -265,7 +265,7 @@ IsolatedContext::GetExternalMountPoints() const {
   for (IDToInstance::const_iterator iter = instance_map_.begin();
        iter != instance_map_.end();
        ++iter) {
-    if (iter->second->mount_type() == kFileSystemMountTypeExternal)
+    if (iter->second->mount_type() == kFileSystemTypeExternal)
       files.push_back(FileInfo(iter->first, iter->second->file_info().path));
   }
   return files;
@@ -312,7 +312,7 @@ void IsolatedContext::RemoveReference(const std::string& filesystem_id) {
   DCHECK(instance->ref_counts() > 0);
   instance->RemoveRef();
   if (instance->ref_counts() == 0 &&
-      instance->mount_type() != kFileSystemMountTypeExternal) {
+      instance->mount_type() != kFileSystemTypeExternal) {
     bool deleted = UnregisterFileSystem(filesystem_id);
     DCHECK(deleted);
   }
@@ -351,7 +351,7 @@ bool IsolatedContext::CrackIsolatedPath(const FilePath& virtual_path,
     if (type)
       *type = instance->type();
     switch (instance->mount_type()) {
-      case kFileSystemMountTypeIsolated: {
+      case kFileSystemTypeIsolated: {
         if (component_iter == components.end()) {
           // The virtual root case.
           path->clear();
@@ -363,10 +363,10 @@ bool IsolatedContext::CrackIsolatedPath(const FilePath& virtual_path,
           return false;
         break;
       }
-      case kFileSystemMountTypeExternal:
+      case kFileSystemTypeExternal:
         cracked_path = instance->file_info().path;
         break;
-      case kFileSystemMountTypeUnknown:
+      default:
         NOTREACHED();
         break;
     }
diff --git a/webkit/fileapi/isolated_context_unittest.cc b/webkit/fileapi/isolated_context_unittest.cc
index a2e637d..a66ff75 100644
--- a/webkit/fileapi/isolated_context_unittest.cc
+++ b/webkit/fileapi/isolated_context_unittest.cc
@@ -114,7 +114,7 @@ TEST_F(IsolatedContextTest, RegisterAndRevokeTest) {
   isolated_context()->RemoveReference(id_);
 
   std::string id2 = isolated_context()->RegisterFileSystemForPath(
-      kFileSystemTypeIsolated, FilePath(DRIVE FPL("/foo")), NULL);
+      kFileSystemTypeNativeLocal, FilePath(DRIVE FPL("/foo")), NULL);
 
   // Make sure the GetDraggedFileInfo returns false for both ones.
   ASSERT_FALSE(isolated_context()->GetDraggedFileInfo(id2, &toplevels));
@@ -126,11 +126,11 @@ TEST_F(IsolatedContextTest, RegisterAndRevokeTest) {
 
   // Try registering three more file systems for the same path as id2.
   std::string id3 = isolated_context()->RegisterFileSystemForPath(
-      kFileSystemTypeIsolated, path, NULL);
+      kFileSystemTypeNativeLocal, path, NULL);
   std::string id4 = isolated_context()->RegisterFileSystemForPath(
-      kFileSystemTypeIsolated, path, NULL);
+      kFileSystemTypeNativeLocal, path, NULL);
   std::string id5 = isolated_context()->RegisterFileSystemForPath(
-      kFileSystemTypeIsolated, path, NULL);
+      kFileSystemTypeNativeLocal, path, NULL);
 
   // Remove file system for id4.
   isolated_context()->AddReference(id4);
diff --git a/webkit/fileapi/isolated_mount_point_provider.cc b/webkit/fileapi/isolated_mount_point_provider.cc
index 609b0cd..47d6311 100644
--- a/webkit/fileapi/isolated_mount_point_provider.cc
+++ b/webkit/fileapi/isolated_mount_point_provider.cc
@@ -101,7 +101,6 @@ bool IsolatedMountPointProvider::IsRestrictedFileName(
 FileSystemFileUtil* IsolatedMountPointProvider::GetFileUtil(
     FileSystemType type) {
   switch (type) {
-    case kFileSystemTypeIsolated:
     case kFileSystemTypeNativeLocal:
       return isolated_file_util_.get();
     case kFileSystemTypeDragged:
diff --git a/webkit/tools/test_shell/simple_file_system.cc b/webkit/tools/test_shell/simple_file_system.cc
index aae827b0..441396d 100644
--- a/webkit/tools/test_shell/simple_file_system.cc
+++ b/webkit/tools/test_shell/simple_file_system.cc
@@ -264,8 +264,8 @@ void SimpleFileSystem::CleanupOnIOThread() {
 
 bool SimpleFileSystem::HasFilePermission(
     const fileapi::FileSystemURL& url, FilePermission permission) {
-  // Disallow writing on isolated file system, otherwise return ok.
-  return (url.type() != fileapi::kFileSystemTypeIsolated ||
+  // Disallow writing on dragged file system, otherwise return ok.
+  return (url.type() != fileapi::kFileSystemTypeDragged ||
           permission == FILE_PERMISSION_READ);
 }
 
diff --git a/webkit/tools/test_shell/simple_file_writer.cc b/webkit/tools/test_shell/simple_file_writer.cc
index 2a38493..3e6f4ae 100644
--- a/webkit/tools/test_shell/simple_file_writer.cc
+++ b/webkit/tools/test_shell/simple_file_writer.cc
@@ -98,7 +98,7 @@ class SimpleFileWriter::IOThreadProxy
 
   // Returns true if it is not writable.
   bool FailIfNotWritable(const FileSystemURL& url) {
-    if (url.type() == fileapi::kFileSystemTypeIsolated) {
+    if (url.type() == fileapi::kFileSystemTypeDragged) {
       // Write is not allowed in isolate file system in SimpleFileWriter.
       DidFailOnMainThread(base::PLATFORM_FILE_ERROR_SECURITY);
       return true;