Add/update comments and cleanup code around FileSystemPathManager

BUG=none
TEST=existing tests should pass (no functional change)


Review URL: http://codereview.chromium.org/8969015

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

7 files changed, 91 insertions, 79 deletions

diff --git a/webkit/fileapi/file_system_context.h b/webkit/fileapi/file_system_context.h
index de0aab0..a797ceb 100644
--- a/webkit/fileapi/file_system_context.h
+++ b/webkit/fileapi/file_system_context.h
@@ -31,6 +31,7 @@ class SandboxMountPointProvider;
 struct DefaultContextDeleter;
 
 // This class keeps and provides a file system context for FileSystem API.
+// An instance of this class is created and owned by profile.
 class FileSystemContext
     : public base::RefCountedThreadSafe<FileSystemContext,
                                         DefaultContextDeleter> {
diff --git a/webkit/fileapi/file_system_mount_point_provider.h b/webkit/fileapi/file_system_mount_point_provider.h
index 7cf2ce3..36768e3 100644
--- a/webkit/fileapi/file_system_mount_point_provider.h
+++ b/webkit/fileapi/file_system_mount_point_provider.h
@@ -15,8 +15,8 @@
 
 namespace fileapi {
 
-// An interface to provide local filesystem paths.
-
+// An interface to provide mount-point-specific path-related utilities
+// and specialized FileSystemFileUtil instance.
 class FileSystemMountPointProvider {
  public:
   virtual ~FileSystemMountPointProvider() {}
@@ -52,6 +52,7 @@ class FileSystemMountPointProvider {
   // permissions.
   virtual std::vector<FilePath> GetRootDirectories() const = 0;
 
+  // Returns the specialized FileSystemFileUtil for this mount point.
   virtual FileSystemFileUtil* GetFileUtil() = 0;
 };
 
diff --git a/webkit/fileapi/file_system_path_manager.cc b/webkit/fileapi/file_system_path_manager.cc
index 9268106..0c27ad2 100644
--- a/webkit/fileapi/file_system_path_manager.cc
+++ b/webkit/fileapi/file_system_path_manager.cc
@@ -58,45 +58,25 @@ FileSystemPathManager::~FileSystemPathManager() {}
 void FileSystemPathManager::ValidateFileSystemRootAndGetURL(
     const GURL& origin_url, fileapi::FileSystemType type, bool create,
     const GetRootPathCallback& callback) {
-  switch (type) {
-    case kFileSystemTypeTemporary:
-    case kFileSystemTypePersistent:
-      sandbox_provider_->ValidateFileSystemRootAndGetURL(
-          origin_url, type, create, callback);
-      break;
-    case kFileSystemTypeExternal:
-      if (external_provider_.get()) {
-        external_provider_->ValidateFileSystemRootAndGetURL(
-            origin_url, type, create, callback);
-      } else {
-        callback.Run(false, FilePath(), std::string());
-      }
-      break;
-    case kFileSystemTypeUnknown:
-    default:
-      NOTREACHED();
-      callback.Run(false, FilePath(), std::string());
+  FileSystemMountPointProvider* mount_point_provider =
+      GetMountPointProvider(type);
+  if (!mount_point_provider) {
+    callback.Run(false, FilePath(), std::string());
+    return;
   }
+  mount_point_provider->ValidateFileSystemRootAndGetURL(
+      origin_url, type, create, callback);
 }
 
 FilePath FileSystemPathManager::ValidateFileSystemRootAndGetPathOnFileThread(
     const GURL& origin_url, FileSystemType type, const FilePath& virtual_path,
     bool create) {
-  switch (type) {
-  case kFileSystemTypeTemporary:
-  case kFileSystemTypePersistent:
-    return sandbox_provider_->ValidateFileSystemRootAndGetPathOnFileThread(
-        origin_url, type, virtual_path, create);
-  case kFileSystemTypeExternal:
-    return external_provider_.get() ?
-        external_provider_->ValidateFileSystemRootAndGetPathOnFileThread(
-           origin_url, type, virtual_path, create) :
-        FilePath();
-  case kFileSystemTypeUnknown:
-  default:
-    NOTREACHED();
+  FileSystemMountPointProvider* mount_point_provider =
+      GetMountPointProvider(type);
+  if (!mount_point_provider)
     return FilePath();
-  }
+  return mount_point_provider->ValidateFileSystemRootAndGetPathOnFileThread(
+      origin_url, type, virtual_path, create);
 }
 
 bool FileSystemPathManager::IsAllowedScheme(const GURL& url) const {
@@ -122,56 +102,38 @@ std::string FileSystemPathManager::GetFileSystemTypeString(
 // Checks if a given |name| contains any restricted names/chars in it.
 bool FileSystemPathManager::IsRestrictedFileName(
     FileSystemType type, const FilePath& filename) {
-  switch (type) {
-  case kFileSystemTypeTemporary:
-  case kFileSystemTypePersistent:
-    return sandbox_provider_->IsRestrictedFileName(filename);
-  case kFileSystemTypeExternal:
-    return external_provider_.get() ?
-               external_provider_->IsRestrictedFileName(filename) : true;
-  case kFileSystemTypeUnknown:
-  default:
-    NOTREACHED();
+  FileSystemMountPointProvider* mount_point_provider =
+      GetMountPointProvider(type);
+  if (!mount_point_provider)
     return true;
-  }
+  return mount_point_provider->IsRestrictedFileName(filename);
 }
 
 // Checks if an origin has access to a particular filesystem type.
 bool FileSystemPathManager::IsAccessAllowed(
     const GURL& origin, FileSystemType type, const FilePath& virtual_path) {
-  switch (type) {
-    case kFileSystemTypeTemporary:
-    case kFileSystemTypePersistent:
-      if (!sandbox_provider_->IsAccessAllowed(origin, type, virtual_path))
-        return false;
-      break;
-    case kFileSystemTypeExternal:
-      if (!external_provider_.get() ||
-          !external_provider_->IsAccessAllowed(origin, type, virtual_path)) {
-        return false;
-      }
-      break;
-    case kFileSystemTypeUnknown:
-    default:
-      NOTREACHED();
-      return false;
-  }
-  return true;
+  FileSystemMountPointProvider* mount_point_provider =
+      GetMountPointProvider(type);
+  DCHECK(mount_point_provider);
+  return mount_point_provider->IsAccessAllowed(origin, type, virtual_path);
 }
 
 FileSystemFileUtil* FileSystemPathManager::GetFileUtil(
     FileSystemType type) const {
+  FileSystemMountPointProvider* mount_point_provider =
+      GetMountPointProvider(type);
+  DCHECK(mount_point_provider);
+  return mount_point_provider->GetFileUtil();
+}
+
+FileSystemMountPointProvider* FileSystemPathManager::GetMountPointProvider(
+    FileSystemType type) const {
   switch (type) {
     case kFileSystemTypeTemporary:
     case kFileSystemTypePersistent:
-      return sandbox_provider_->GetFileUtil();
+      return sandbox_provider();
     case kFileSystemTypeExternal:
-      if (external_provider_.get()) {
-        return external_provider_->GetFileUtil();
-      } else {
-        NOTREACHED();
-        return NULL;
-      }
+      return external_provider();
     case kFileSystemTypeUnknown:
     default:
       NOTREACHED();
diff --git a/webkit/fileapi/file_system_path_manager.h b/webkit/fileapi/file_system_path_manager.h
index 34aebac..1a7b937 100644
--- a/webkit/fileapi/file_system_path_manager.h
+++ b/webkit/fileapi/file_system_path_manager.h
@@ -22,6 +22,7 @@ namespace fileapi {
 
 class ExternalFileSystemMountPointProvider;
 class FileSystemFileUtil;
+class FileSystemMountPointProvider;
 class SandboxMountPointProvider;
 
 class FileSystemPathManager {
@@ -66,9 +67,9 @@ class FileSystemPathManager {
   // filesystem.
   bool IsAllowedScheme(const GURL& url) const;
 
-  // Returns the string for the given |type|.
+  // Returns the string representation of the given filesystem |type|.
   // Returns an empty string if the |type| is invalid.
-  static std::string GetFileSystemTypeString(fileapi::FileSystemType type);
+  static std::string GetFileSystemTypeString(FileSystemType type);
 
   // Checks if a given |name| contains any restricted names/chars in it.
   bool IsRestrictedFileName(FileSystemType type,
@@ -79,12 +80,19 @@ class FileSystemPathManager {
   bool IsAccessAllowed(const GURL& origin, FileSystemType type,
                        const FilePath& virtual_path);
 
+  // Returns the appropriate FileUtil instance for the given |type|.
+  // This may return NULL if it is given an invalid or unsupported filesystem
+  // type.
   FileSystemFileUtil* GetFileUtil(FileSystemType type) const;
 
+  // Returns a FileSystemMountPointProvider instance for sandboxed filesystem
+  // types (e.g. TEMPORARY or PERSISTENT).
   SandboxMountPointProvider* sandbox_provider() const {
     return sandbox_provider_.get();
   }
 
+  // Returns a FileSystemMountPointProvider instance for external filesystem
+  // type, which is used only by chromeos for now.
   ExternalFileSystemMountPointProvider* external_provider() const {
     return external_provider_.get();
   }
@@ -94,6 +102,12 @@ class FileSystemPathManager {
   }
 
  private:
+  // Returns the mount point provider instance for the given |type|.
+  // This may return NULL if it is given an invalid or unsupported filesystem
+  // type.
+  FileSystemMountPointProvider* GetMountPointProvider(
+      FileSystemType type) const;
+
   const bool is_incognito_;
   const bool allow_file_access_from_files_;
   scoped_ptr<SandboxMountPointProvider> sandbox_provider_;
diff --git a/webkit/fileapi/file_system_types.h b/webkit/fileapi/file_system_types.h
index cfd2cb1..1312f14 100644
--- a/webkit/fileapi/file_system_types.h
+++ b/webkit/fileapi/file_system_types.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -8,9 +8,22 @@
 namespace fileapi {
 
 enum FileSystemType {
+  // 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.
+  // http://www.w3.org/TR/file-system-api/#temporary-vs.-persistent-storage
+  // They are sandboxed filesystems; all the files in the filesystems are
+  // placed under the profile directory with path obfuscation and quota
+  // enforcement.
   kFileSystemTypeTemporary,
   kFileSystemTypePersistent,
+
+  // Indicates non-sandboxed filesystem where files are placed outside the
+  // profile directory (thus called 'external' filesystem).
+  // This filesystem is used only by Chrome OS as of writing.
   kFileSystemTypeExternal,
+
+  // Indicates uninitialized or invalid filesystem type.
   kFileSystemTypeUnknown,
 };
 
diff --git a/webkit/fileapi/file_system_util.h b/webkit/fileapi/file_system_util.h
index 226b85b..25fa947 100644
--- a/webkit/fileapi/file_system_util.h
+++ b/webkit/fileapi/file_system_util.h
@@ -21,18 +21,38 @@ extern const char kPersistentName[];
 extern const char kTemporaryName[];
 extern const char kExternalName[];
 
+// Cracks the given filesystem |url| and populates |origin_url|, |type|
+// and |file_path|.  Returns true if the given |url| is a valid filesystem
+// url and the routine could successfully crack it, returns false otherwise.
 // The file_path this returns will be using '/' as a path separator, no matter
 // what platform you're on.
-bool CrackFileSystemURL(const GURL& url, GURL* origin_url, FileSystemType* type,
+bool CrackFileSystemURL(const GURL& url,
+                        GURL* origin_url,
+                        FileSystemType* type,
                         FilePath* file_path);
 
+// Returns the root URI of the filesystem that can be specified by a pair of
+// |origin_url| and |type|.  The returned URI can be used as a root path
+// of the filesystem (e.g. <returned_URI> + "/relative/path" will compose
+// a path pointing to the entry "/relative/path" in the filesystem).
 GURL GetFileSystemRootURI(const GURL& origin_url, fileapi::FileSystemType type);
 
+// Converts FileSystemType |type| to/from the StorageType |storage_type| that
+// is used for the unified quota system.
+// (Basically this naively maps TEMPORARY storage type to TEMPORARY filesystem
+// type, PERSISTENT storage type to PERSISTENT filesystem type and vice versa.)
 FileSystemType QuotaStorageTypeToFileSystemType(
     quota::StorageType storage_type);
-
 quota::StorageType FileSystemTypeToQuotaStorageType(FileSystemType type);
 
+// Returns the origin identifier string for the given |url| and vice versa.
+// The origin identifier string is a serialized form of a security origin
+// and can be used as a path name as it contains no "/" or other possibly
+// unsafe characters. (See WebKit's SecurityOrigin code for more details.)
+//
+// Example:
+//   "http://www.example.com:80/"'s identifier should look like:
+//   "http_www.example.host_80"
 std::string GetOriginIdentifierFromURL(const GURL& url);
 GURL GetOriginURLFromIdentifier(const std::string& origin_identifier);
 
diff --git a/webkit/fileapi/sandbox_mount_point_provider.h b/webkit/fileapi/sandbox_mount_point_provider.h
index e23bafc..11a4773 100644
--- a/webkit/fileapi/sandbox_mount_point_provider.h
+++ b/webkit/fileapi/sandbox_mount_point_provider.h
@@ -27,10 +27,11 @@ namespace fileapi {
 
 class ObfuscatedFileUtil;
 
-// An interface to construct or crack sandboxed filesystem paths.
-// Currently each sandboxed filesystem path looks like (soon will be changed):
-//   <profile_dir>/FileSystem/<origin_identifier>/<type>/chrome-<unique>/...
-// <type> is either one of "Temporary" or "Persistent".
+// An interface to construct or crack sandboxed filesystem paths for
+// TEMPORARY or PERSISTENT filesystems, which are placed under the user's
+// profile directory in a sandboxed way.
+// This interface also lets one enumerate and remove storage for the origins
+// that use the filesystem.
 class SandboxMountPointProvider
     : public FileSystemMountPointProvider,
       public FileSystemQuotaUtil {