Make more methods of FileSystemPathManager static and portable.

Changes:
- Renamed StorageIdentifier to OriginIdentifier to better reflect its meaning.
- Changed a few methods to take string origin identifier but not origin URL as its parameter, since in some cases it seems to be more convenient just to use the identifier string (which can be easily extracted from the path) than to call WebKit method to convert back the identifier to GURL.

BUG=none
TEST=FileSystemPathManagerTest.*

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

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

6 files changed, 66 insertions, 64 deletions

diff --git a/webkit/fileapi/file_system_context.cc b/webkit/fileapi/file_system_context.cc
index c0546d8..5007fff 100644
--- a/webkit/fileapi/file_system_context.cc
+++ b/webkit/fileapi/file_system_context.cc
@@ -37,10 +37,10 @@ void FileSystemContext::DeleteDataForOriginOnFileThread(
   DCHECK(path_manager_.get());
   DCHECK(file_message_loop_->BelongsToCurrentThread());
 
-  std::string storage_identifier =
-      FileSystemPathManager::GetStorageIdentifierFromURL(origin_url);
+  std::string origin_identifier =
+      FileSystemPathManager::GetOriginIdentifierFromURL(origin_url);
   FilePath path_for_origin = path_manager_->base_path().AppendASCII(
-      storage_identifier);
+      origin_identifier);
 
   file_util::Delete(path_for_origin, true /* recursive */);
 }
diff --git a/webkit/fileapi/file_system_operation.cc b/webkit/fileapi/file_system_operation.cc
index cea953e..5e66453 100644
--- a/webkit/fileapi/file_system_operation.cc
+++ b/webkit/fileapi/file_system_operation.cc
@@ -435,7 +435,7 @@ bool FileSystemOperation::VerifyFileSystemPathForWrite(
     dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_SECURITY);
     return false;
   }
-  if (create && file_system_context_->path_manager()->IsRestrictedFileName(
+  if (create && FileSystemPathManager::IsRestrictedFileName(
           path.BaseName())) {
     dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_SECURITY);
     return false;
diff --git a/webkit/fileapi/file_system_path_manager.cc b/webkit/fileapi/file_system_path_manager.cc
index 3e0cd5e..8aee5d5 100644
--- a/webkit/fileapi/file_system_path_manager.cc
+++ b/webkit/fileapi/file_system_path_manager.cc
@@ -19,7 +19,7 @@
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebSecurityOrigin.h"
 #include "webkit/glue/webkit_glue.h"
 
-// We use some of WebKit types for conversions between storage identifiers
+// We use some of WebKit types for conversions between origin identifiers
 // and origin URLs.
 using WebKit::WebFileSystem;
 using WebKit::WebSecurityOrigin;
@@ -77,14 +77,6 @@ FilePath::StringType CreateUniqueDirectoryName(const GURL& origin_url) {
 
 static const char kExtensionScheme[] = "chrome-extension";
 
-inline std::string GetFileSystemTypeString(fileapi::FileSystemType type) {
-  if (type == fileapi::kFileSystemTypeTemporary)
-    return fileapi::FileSystemPathManager::kTemporaryName;
-  else if (type == fileapi::kFileSystemTypePersistent)
-    return fileapi::FileSystemPathManager::kPersistentName;
-  return std::string();
-}
-
 }  // anonymous namespace
 
 class FileSystemPathManager::GetFileSystemRootPathTask
@@ -206,16 +198,20 @@ void FileSystemPathManager::GetFileSystemRootPath(
     return;
   }
 
+  std::string origin_identifier = GetOriginIdentifierFromURL(origin_url);
   FilePath origin_base_path = GetFileSystemBaseDirectoryForOriginAndType(
-      base_path(), origin_url, type);
+      base_path(), origin_identifier, type);
   if (origin_base_path.empty()) {
     callback->Run(false, FilePath(), std::string());
     return;
   }
 
+  std::string type_string = GetFileSystemTypeString(type);
+  DCHECK(!type_string.empty());
+
   scoped_refptr<GetFileSystemRootPathTask> task(
       new GetFileSystemRootPathTask(file_message_loop_,
-                                    GetFileSystemName(origin_url, type),
+                                    origin_identifier + ":" + type_string,
                                     callback.release()));
   task->Start(origin_url, origin_base_path, create);
 }
@@ -284,8 +280,16 @@ bool FileSystemPathManager::CrackFileSystemPath(
   return true;
 }
 
-bool FileSystemPathManager::IsRestrictedFileName(
-    const FilePath& filename) const {
+bool FileSystemPathManager::IsAllowedScheme(const GURL& url) const {
+  // Basically we only accept http or https. We allow file:// URLs
+  // only if --allow-file-access-from-files flag is given.
+  return url.SchemeIs("http") || url.SchemeIs("https") ||
+         url.SchemeIs(kExtensionScheme) ||
+         (url.SchemeIsFile() && allow_file_access_from_files_);
+}
+
+// static
+bool FileSystemPathManager::IsRestrictedFileName(const FilePath& filename) {
   if (filename.value().size() == 0)
     return false;
 
@@ -314,23 +318,18 @@ bool FileSystemPathManager::IsRestrictedFileName(
   return false;
 }
 
-bool FileSystemPathManager::IsAllowedScheme(const GURL& url) const {
-  // Basically we only accept http or https. We allow file:// URLs
-  // only if --allow-file-access-from-files flag is given.
-  return url.SchemeIs("http") || url.SchemeIs("https") ||
-         url.SchemeIs(kExtensionScheme) ||
-         (url.SchemeIsFile() && allow_file_access_from_files_);
-}
-
 // static
-std::string FileSystemPathManager::GetFileSystemName(
-    const GURL& origin_url, fileapi::FileSystemType type) {
-  return GetStorageIdentifierFromURL(origin_url)
-      .append(":").append(GetFileSystemTypeString(type));
+std::string FileSystemPathManager::GetFileSystemTypeString(
+    fileapi::FileSystemType type) {
+  if (type == fileapi::kFileSystemTypeTemporary)
+    return fileapi::FileSystemPathManager::kTemporaryName;
+  else if (type == fileapi::kFileSystemTypePersistent)
+    return fileapi::FileSystemPathManager::kPersistentName;
+  return std::string();
 }
 
 // static
-std::string FileSystemPathManager::GetStorageIdentifierFromURL(
+std::string FileSystemPathManager::GetOriginIdentifierFromURL(
     const GURL& url) {
   WebKit::WebSecurityOrigin web_security_origin =
       WebKit::WebSecurityOrigin::createFromString(UTF8ToUTF16(url.spec()));
@@ -339,16 +338,16 @@ std::string FileSystemPathManager::GetStorageIdentifierFromURL(
 
 // static
 FilePath FileSystemPathManager::GetFileSystemBaseDirectoryForOriginAndType(
-    const FilePath& base_path, const GURL& origin_url,
+    const FilePath& base_path, const std::string& origin_identifier,
     fileapi::FileSystemType type) {
-  if (!origin_url.is_valid())
+  if (origin_identifier.empty())
     return FilePath();
   std::string type_string = GetFileSystemTypeString(type);
   if (type_string.empty()) {
     LOG(WARNING) << "Unknown filesystem type is requested:" << type;
     return FilePath();
   }
-  return base_path.AppendASCII(GetStorageIdentifierFromURL(origin_url))
+  return base_path.AppendASCII(origin_identifier)
                   .AppendASCII(type_string);
 }
 
diff --git a/webkit/fileapi/file_system_path_manager.h b/webkit/fileapi/file_system_path_manager.h
index f7a3803..be9ceff 100644
--- a/webkit/fileapi/file_system_path_manager.h
+++ b/webkit/fileapi/file_system_path_manager.h
@@ -23,7 +23,7 @@ namespace fileapi {
 //
 //   <profile_dir>/FileSystem/<origin_identifier>/<type>/chrome-<unique>/...
 //
-// where <type> is either one of "Temporary" or "Persistent".
+// <type> is either one of "Temporary" or "Persistent".
 class FileSystemPathManager {
  public:
   FileSystemPathManager(scoped_refptr<base::MessageLoopProxy> file_message_loop,
@@ -51,19 +51,14 @@ class FileSystemPathManager {
                              GetRootPathCallback* callback);
 
   // Cracks the given |path|, retrieves the information embedded in the path
-  // and populates |origin_url| and |type|.  Also it populates |virtual_path|
-  // that is a sandboxed path in the file system, i.e. the relative path to
-  // the file system's root path for the given origin and type.
-  // It returns false if the path does not conform to the expected
-  // filesystem path format.
+  // and populates |origin_url|, |type| and |virtual_path|.  The |virtual_path|
+  // is a sandboxed path in the file system, i.e. the relative path to the
+  // filesystem root for the given domain and type.
   bool CrackFileSystemPath(const FilePath& path,
                            GURL* origin_url,
                            FileSystemType* type,
                            FilePath* virtual_path) const;
 
-  // Checks if a given |name| contains any restricted names/chars in it.
-  bool IsRestrictedFileName(const FilePath& filename) const;
-
   // Returns true if the given |url|'s scheme is allowed to access
   // filesystem.
   bool IsAllowedScheme(const GURL& url) const;
@@ -78,15 +73,19 @@ class FileSystemPathManager {
     return base_path_;
   }
 
-  // Returns the filesystem name string for the given |origin_url| and |type|.
-  static std::string GetFileSystemName(const GURL& url,
-                                       fileapi::FileSystemType type);
+  // Checks if a given |name| contains any restricted names/chars in it.
+  static bool IsRestrictedFileName(const FilePath& filename);
+
+  // Returns the string for the given |type|.
+  // Returns an empty string if the |type| is invalid.
+  static std::string GetFileSystemTypeString(fileapi::FileSystemType type);
 
-  // Returns the storage identifier string for the given |url|.
-  static std::string GetStorageIdentifierFromURL(const GURL& url);
+  // Returns the origin identifier string, which is used as a part of the
+  // sandboxed path component, for the given |url|.
+  static std::string GetOriginIdentifierFromURL(const GURL& url);
 
   // Gets a base directory path of the sandboxed filesystem that is
-  // specified by |origin_url| and |type|.
+  // specified by |origin_identifier| and |type|.
   // |base_path| must be pointing the FileSystem's data directory
   // under the profile directory, i.e. <profile_dir>/kFileSystemDirectory.
   // Returns an empty path if any of the given parameters are invalid.
@@ -94,7 +93,7 @@ class FileSystemPathManager {
   // it is not an actural root path for the filesystem.
   static FilePath GetFileSystemBaseDirectoryForOriginAndType(
       const FilePath& base_path,
-      const GURL& origin_url,
+      const std::string& origin_identifier,
       fileapi::FileSystemType type);
 
  private:
diff --git a/webkit/fileapi/file_system_path_manager_unittest.cc b/webkit/fileapi/file_system_path_manager_unittest.cc
index 2201215c..e0c1d81 100644
--- a/webkit/fileapi/file_system_path_manager_unittest.cc
+++ b/webkit/fileapi/file_system_path_manager_unittest.cc
@@ -415,12 +415,11 @@ TEST_F(FileSystemPathManagerTest, CheckValidPath) {
 }
 
 TEST_F(FileSystemPathManagerTest, IsRestrictedName) {
-  scoped_ptr<FileSystemPathManager> manager(NewPathManager(false, false));
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kIsRestrictedNameTestCases); ++i) {
     SCOPED_TRACE(testing::Message() << "IsRestrictedName #" << i << " "
                  << kIsRestrictedNameTestCases[i].name);
     FilePath name(kIsRestrictedNameTestCases[i].name);
     EXPECT_EQ(kIsRestrictedNameTestCases[i].expected_dangerous,
-              manager->IsRestrictedFileName(name));
+              FileSystemPathManager::IsRestrictedFileName(name));
   }
 }
diff --git a/webkit/fileapi/file_system_usage_tracker.cc b/webkit/fileapi/file_system_usage_tracker.cc
index 2b16a1f..f58e4aa 100644
--- a/webkit/fileapi/file_system_usage_tracker.cc
+++ b/webkit/fileapi/file_system_usage_tracker.cc
@@ -23,13 +23,13 @@ class FileSystemUsageTracker::GetUsageTask
   GetUsageTask(
       FileSystemUsageTracker* tracker,
       scoped_refptr<base::MessageLoopProxy> file_message_loop,
-      std::string fs_name,
+      std::string fs_identifier,
       const FilePath& origin_base_path)
       : tracker_(tracker),
         file_message_loop_(file_message_loop),
         original_message_loop_(
             base::MessageLoopProxy::CreateForCurrentThread()),
-        fs_name_(fs_name),
+        fs_identifier_(fs_identifier),
         fs_usage_(0),
         origin_base_path_(origin_base_path) {
   }
@@ -62,14 +62,14 @@ class FileSystemUsageTracker::GetUsageTask
     DCHECK(original_message_loop_->BelongsToCurrentThread());
     if (tracker_) {
       tracker_->UnregisterUsageTask(this);
-      tracker_->DidGetOriginUsage(fs_name_, fs_usage_);
+      tracker_->DidGetOriginUsage(fs_identifier_, fs_usage_);
     }
   }
 
   FileSystemUsageTracker* tracker_;
   scoped_refptr<base::MessageLoopProxy> file_message_loop_;
   scoped_refptr<base::MessageLoopProxy> original_message_loop_;
-  std::string fs_name_;
+  std::string fs_identifier_;
   int64 fs_usage_;
   FilePath origin_base_path_;
 };
@@ -104,13 +104,17 @@ void FileSystemUsageTracker::GetOriginUsage(
     return;
   }
 
-  std::string fs_name = FileSystemPathManager::GetFileSystemName(
-      origin_url, type);
-  if (pending_usage_callbacks_.find(fs_name) !=
+  std::string origin_identifier =
+      FileSystemPathManager::GetOriginIdentifierFromURL(origin_url);
+  std::string type_string =
+      FileSystemPathManager::GetFileSystemTypeString(type);
+  std::string fs_identifier = origin_identifier + ":" + type_string;
+
+  if (pending_usage_callbacks_.find(fs_identifier) !=
       pending_usage_callbacks_.end()) {
     // Another get usage task is running.  Add the callback to
     // the pending queue and return.
-    pending_usage_callbacks_[fs_name].push_back(callback.release());
+    pending_usage_callbacks_[fs_identifier].push_back(callback.release());
     return;
   }
 
@@ -118,16 +122,17 @@ void FileSystemUsageTracker::GetOriginUsage(
   // without unique part).
   FilePath origin_base_path =
       FileSystemPathManager::GetFileSystemBaseDirectoryForOriginAndType(
-          base_path_, origin_url, type);
+          base_path_, origin_identifier, type);
   if (origin_base_path.empty()) {
     // The directory does not exist.
     callback->Run(0);
     return;
   }
 
-  pending_usage_callbacks_[fs_name].push_back(callback.release());
+  pending_usage_callbacks_[fs_identifier].push_back(callback.release());
   scoped_refptr<GetUsageTask> task(
-      new GetUsageTask(this, file_message_loop_, fs_name, origin_base_path));
+      new GetUsageTask(this, file_message_loop_, fs_identifier,
+                       origin_base_path));
   task->Start();
 }
 
@@ -141,9 +146,9 @@ void FileSystemUsageTracker::UnregisterUsageTask(GetUsageTask* task) {
 }
 
 void FileSystemUsageTracker::DidGetOriginUsage(
-    const std::string& fs_name, int64 usage) {
+    const std::string& fs_identifier, int64 usage) {
   PendingUsageCallbackMap::iterator cb_list_iter =
-      pending_usage_callbacks_.find(fs_name);
+      pending_usage_callbacks_.find(fs_identifier);
   DCHECK(cb_list_iter != pending_usage_callbacks_.end());
   PendingCallbackList cb_list = cb_list_iter->second;
   for (PendingCallbackList::iterator cb_iter = cb_list.begin();