Tiny cleanup in FileSystemOperation

BUG=none
TEST=existing tests should pass

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

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

2 files changed, 28 insertions, 65 deletions

diff --git a/webkit/fileapi/file_system_operation.cc b/webkit/fileapi/file_system_operation.cc
index 5db7268..e4c9593 100644
--- a/webkit/fileapi/file_system_operation.cc
+++ b/webkit/fileapi/file_system_operation.cc
@@ -743,42 +743,8 @@ void FileSystemOperation::OnFileOpenedForWrite(
 bool FileSystemOperation::VerifyFileSystemPathForRead(
     const GURL& path, GURL* origin_url, FileSystemType* type,
     FilePath* virtual_path, FileSystemFileUtil** file_util) {
-
-  // If we have no context, we just allow any operations, for testing.
-  // TODO(ericu): Revisit this hack for security.
-  if (!file_system_context()) {
-#ifdef OS_WIN
-    // On Windows, the path will look like /C:/foo/bar; we need to remove the
-    // leading slash to make it valid.  But if it's empty, we shouldn't do
-    // anything.
-    std::string temp = net::UnescapeURLComponent(path.path(),
-        UnescapeRule::SPACES | UnescapeRule::URL_SPECIAL_CHARS);
-    if (temp.size())
-      temp = temp.substr(1);
-    *virtual_path = FilePath(UTF8ToWide(temp)).NormalizeWindowsPathSeparators();
-#else
-    *virtual_path = FilePath(path.path());
-#endif
-    *type = operation_context_.src_type();
-    *origin_url = operation_context_.src_origin_url();
-    *file_util = NULL;
-    return true;
-  }
-
-  // We may want do more checks, but for now it just checks if the given
-  // URL is valid.
-  if (!CrackFileSystemURL(path, origin_url, type, virtual_path)) {
-    dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_INVALID_URL);
+  if (!VerifyFileSystemPath(path, origin_url, type, virtual_path, file_util))
     return false;
-  }
-  if (!file_system_context()->path_manager()->IsAccessAllowed(
-      *origin_url, *type, *virtual_path)) {
-    dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_SECURITY);
-    return false;
-  }
-  DCHECK(file_util);
-  *file_util = file_system_context()->path_manager()->GetFileUtil(*type);
-  DCHECK(*file_util);
 
   // We notify this read access whether the read access succeeds or not.
   // This must be ok since this is used to let the QM's eviction logic know
@@ -798,37 +764,9 @@ bool FileSystemOperation::VerifyFileSystemPathForRead(
 bool FileSystemOperation::VerifyFileSystemPathForWrite(
     const GURL& path, bool create, GURL* origin_url, FileSystemType* type,
     FilePath* virtual_path, FileSystemFileUtil** file_util) {
-
-  // If we have no context, we just allow any operations, for testing.
-  // TODO(ericu): Revisit this hack for security.
-  if (!file_system_context()) {
-#ifdef OS_WIN
-    // On Windows, the path will look like /C:/foo/bar; we need to remove the
-    // leading slash to make it valid.  But if it's empty, we shouldn't do
-    // anything.
-    std::string temp = net::UnescapeURLComponent(path.path(),
-        UnescapeRule::SPACES | UnescapeRule::URL_SPECIAL_CHARS);
-    if (temp.size())
-      temp = temp.substr(1);
-    *virtual_path = FilePath(UTF8ToWide(temp)).NormalizeWindowsPathSeparators();
-#else
-    *virtual_path = FilePath(path.path());
-#endif
-    *type = operation_context_.dest_type();
-    *origin_url = operation_context_.dest_origin_url();
-    *file_util = NULL;
-    return true;
-  }
-
-  if (!CrackFileSystemURL(path, origin_url, type, virtual_path)) {
-    dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_INVALID_URL);
+  if (!VerifyFileSystemPath(path, origin_url, type, virtual_path, file_util))
     return false;
-  }
-  if (!file_system_context()->path_manager()->IsAccessAllowed(
-      *origin_url, *type, *virtual_path)) {
-    dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_SECURITY);
-    return false;
-  }
+
   // Any write access is disallowed on the root path.
   if (virtual_path->value().length() == 0 ||
       virtual_path->DirName().value() == virtual_path->value()) {
@@ -840,6 +778,24 @@ bool FileSystemOperation::VerifyFileSystemPathForWrite(
     dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_SECURITY);
     return false;
   }
+
+  return true;
+}
+
+bool FileSystemOperation::VerifyFileSystemPath(
+    const GURL& path, GURL* origin_url, FileSystemType* type,
+    FilePath* virtual_path, FileSystemFileUtil** file_util) {
+  DCHECK(file_system_context());
+
+  if (!CrackFileSystemURL(path, origin_url, type, virtual_path)) {
+    dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_INVALID_URL);
+    return false;
+  }
+  if (!file_system_context()->path_manager()->IsAccessAllowed(
+      *origin_url, *type, *virtual_path)) {
+    dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_SECURITY);
+    return false;
+  }
   DCHECK(file_util);
   *file_util = file_system_context()->path_manager()->GetFileUtil(*type);
   DCHECK(*file_util);
diff --git a/webkit/fileapi/file_system_operation.h b/webkit/fileapi/file_system_operation.h
index fdf26fc..a85f3f1 100644
--- a/webkit/fileapi/file_system_operation.h
+++ b/webkit/fileapi/file_system_operation.h
@@ -218,6 +218,13 @@ class FileSystemOperation {
                                     FilePath* virtual_path,
                                     FileSystemFileUtil** file_util);
 
+  // Common internal routine for VerifyFileSystemPathFor{Read,Write}.
+  bool VerifyFileSystemPath(const GURL& path,
+                            GURL* root_url,
+                            FileSystemType* type,
+                            FilePath* virtual_path,
+                            FileSystemFileUtil** file_util);
+
   // Setup*Context*() functions will call the appropriate VerifyFileSystem
   // function and store the results to operation_context_ and
   // *_virtual_path_.