Revert 81319 - More filesystem cleanup: convert URL-encoded-as-FilePath to actual URL, where possible without WebKit API changes. The WebKit changes will happen in another CL. This is a resubmit of http://codereview.chromium.org/6767010/ [third try lucky,fourth try persistent?], which bounced most recently due to bugs manifestingonly on XP. There are a few changes here that weren't there [infile_system_operation_write_unittest.cc and file_system_operation.cc], butthey're pretty trivial build/test fixes.BUG=none TEST=noneReview URL: http://codereview.chromium.org/6821065

TBR=ericu@google.com
Review URL: http://codereview.chromium.org/6836002

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

1 files changed, 26 insertions, 61 deletions

diff --git a/webkit/fileapi/file_system_operation.cc b/webkit/fileapi/file_system_operation.cc
index 0e258c1..4ddfd0c 100644
--- a/webkit/fileapi/file_system_operation.cc
+++ b/webkit/fileapi/file_system_operation.cc
@@ -5,8 +5,6 @@
 #include "webkit/fileapi/file_system_operation.h"
 
 #include "base/time.h"
-#include "base/utf_string_conversions.h"
-#include "net/base/escape.h"
 #include "net/url_request/url_request_context.h"
 #include "webkit/fileapi/file_system_callback_dispatcher.h"
 #include "webkit/fileapi/file_system_context.h"
@@ -65,7 +63,7 @@ void FileSystemOperation::OpenFileSystem(
       callback_factory_.NewCallback(&FileSystemOperation::DidGetRootPath));
 }
 
-void FileSystemOperation::CreateFile(const GURL& path,
+void FileSystemOperation::CreateFile(const FilePath& path,
                                      bool exclusive) {
 #ifndef NDEBUG
   DCHECK(kOperationNone == pending_operation_);
@@ -88,7 +86,7 @@ void FileSystemOperation::CreateFile(const GURL& path,
                     : &FileSystemOperation::DidEnsureFileExistsNonExclusive));
 }
 
-void FileSystemOperation::CreateDirectory(const GURL& path,
+void FileSystemOperation::CreateDirectory(const FilePath& path,
                                           bool exclusive,
                                           bool recursive) {
 #ifndef NDEBUG
@@ -112,8 +110,8 @@ void FileSystemOperation::CreateDirectory(const GURL& path,
           &FileSystemOperation::DidFinishFileOperation));
 }
 
-void FileSystemOperation::Copy(const GURL& src_path,
-                               const GURL& dest_path) {
+void FileSystemOperation::Copy(const FilePath& src_path,
+                               const FilePath& dest_path) {
 #ifndef NDEBUG
   DCHECK(kOperationNone == pending_operation_);
   pending_operation_ = kOperationCopy;
@@ -151,8 +149,8 @@ void FileSystemOperation::Copy(const GURL& src_path,
         &FileSystemOperation::DidFinishFileOperation));
 }
 
-void FileSystemOperation::Move(const GURL& src_path,
-                               const GURL& dest_path) {
+void FileSystemOperation::Move(const FilePath& src_path,
+                               const FilePath& dest_path) {
 #ifndef NDEBUG
   DCHECK(kOperationNone == pending_operation_);
   pending_operation_ = kOperationMove;
@@ -164,8 +162,6 @@ void FileSystemOperation::Move(const GURL& src_path,
   FileSystemType src_type;
   FileSystemType dest_type;
 
-  //TODO(ericu): Move alters the source path as well, so we should be checking
-  //both for write!
   if (!VerifyFileSystemPathForRead(src_path, &src_origin_url, &src_type,
         &virtual_path_0) ||
       !VerifyFileSystemPathForWrite(dest_path, true /* create */,
@@ -190,7 +186,7 @@ void FileSystemOperation::Move(const GURL& src_path,
         &FileSystemOperation::DidFinishFileOperation));
 }
 
-void FileSystemOperation::DirectoryExists(const GURL& path) {
+void FileSystemOperation::DirectoryExists(const FilePath& path) {
 #ifndef NDEBUG
   DCHECK(kOperationNone == pending_operation_);
   pending_operation_ = kOperationDirectoryExists;
@@ -211,7 +207,7 @@ void FileSystemOperation::DirectoryExists(const GURL& path) {
           &FileSystemOperation::DidDirectoryExists));
 }
 
-void FileSystemOperation::FileExists(const GURL& path) {
+void FileSystemOperation::FileExists(const FilePath& path) {
 #ifndef NDEBUG
   DCHECK(kOperationNone == pending_operation_);
   pending_operation_ = kOperationFileExists;
@@ -232,7 +228,7 @@ void FileSystemOperation::FileExists(const GURL& path) {
           &FileSystemOperation::DidFileExists));
 }
 
-void FileSystemOperation::GetMetadata(const GURL& path) {
+void FileSystemOperation::GetMetadata(const FilePath& path) {
 #ifndef NDEBUG
   DCHECK(kOperationNone == pending_operation_);
   pending_operation_ = kOperationGetMetadata;
@@ -253,7 +249,7 @@ void FileSystemOperation::GetMetadata(const GURL& path) {
           &FileSystemOperation::DidGetMetadata));
 }
 
-void FileSystemOperation::ReadDirectory(const GURL& path) {
+void FileSystemOperation::ReadDirectory(const FilePath& path) {
 #ifndef NDEBUG
   DCHECK(kOperationNone == pending_operation_);
   pending_operation_ = kOperationReadDirectory;
@@ -274,7 +270,7 @@ void FileSystemOperation::ReadDirectory(const GURL& path) {
           &FileSystemOperation::DidReadDirectory));
 }
 
-void FileSystemOperation::Remove(const GURL& path, bool recursive) {
+void FileSystemOperation::Remove(const FilePath& path, bool recursive) {
 #ifndef NDEBUG
   DCHECK(kOperationNone == pending_operation_);
   pending_operation_ = kOperationRemove;
@@ -298,7 +294,7 @@ void FileSystemOperation::Remove(const GURL& path, bool recursive) {
 
 void FileSystemOperation::Write(
     scoped_refptr<net::URLRequestContext> url_request_context,
-    const GURL& path,
+    const FilePath& path,
     const GURL& blob_url,
     int64 offset) {
 #ifndef NDEBUG
@@ -330,7 +326,7 @@ void FileSystemOperation::Write(
           &FileSystemOperation::OnFileOpenedForWrite));
 }
 
-void FileSystemOperation::Truncate(const GURL& path, int64 length) {
+void FileSystemOperation::Truncate(const FilePath& path, int64 length) {
 #ifndef NDEBUG
   DCHECK(kOperationNone == pending_operation_);
   pending_operation_ = kOperationTruncate;
@@ -351,7 +347,7 @@ void FileSystemOperation::Truncate(const GURL& path, int64 length) {
           &FileSystemOperation::DidFinishFileOperation));
 }
 
-void FileSystemOperation::TouchFile(const GURL& path,
+void FileSystemOperation::TouchFile(const FilePath& path,
                                     const base::Time& last_access_time,
                                     const base::Time& last_modified_time) {
 #ifndef NDEBUG
@@ -412,13 +408,14 @@ void FileSystemOperation::DidGetRootPath(
     bool success,
     const FilePath& path, const std::string& name) {
   DCHECK(success || path.empty());
-  GURL result;
+  FilePath result;
   // We ignore the path, and return a URL instead.  The point was just to verify
   // that we could create/find the path.
   if (success) {
-    result = GetFileSystemRootURI(
+    GURL root_url = GetFileSystemRootURI(
         file_system_operation_context_.src_origin_url(),
         file_system_operation_context_.src_type());
+    result = FilePath().AppendASCII(root_url.spec());
   }
   dispatcher_->DidOpenFileSystem(name, result);
   delete this;
@@ -540,37 +537,21 @@ void FileSystemOperation::OnFileOpenedForWrite(
 }
 
 bool FileSystemOperation::VerifyFileSystemPathForRead(
-    const GURL& path, GURL* origin_url, FileSystemType* type,
+    const FilePath& path, GURL* origin_url, FileSystemType* type,
     FilePath* virtual_path) {
 
   // 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 = 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
+    *virtual_path = path;
     *type = file_system_operation_context_.src_type();
-    *origin_url = file_system_operation_context_.src_origin_url();
     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_SECURITY);
-    return false;
-  }
-  if (!file_system_context()->path_manager()->IsAllowedFileSystemType(
-      *origin_url, *type)) {
+  // |path| is under the valid FileSystem root path for this host context.
+  if (!file_system_context()->path_manager()->CrackFileSystemPath(
+          path, origin_url, type, virtual_path)) {
     dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_SECURITY);
     return false;
   }
@@ -579,35 +560,19 @@ bool FileSystemOperation::VerifyFileSystemPathForRead(
 }
 
 bool FileSystemOperation::VerifyFileSystemPathForWrite(
-    const GURL& path, bool create, GURL* origin_url, FileSystemType* type,
+    const FilePath& path, bool create, GURL* origin_url, FileSystemType* type,
     FilePath* virtual_path) {
 
   // 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 = 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
+    *virtual_path = path;
     *type = file_system_operation_context_.dest_type();
-    *origin_url = file_system_operation_context_.dest_origin_url();
     return true;
   }
 
-  if (!CrackFileSystemURL(path, origin_url, type, virtual_path)) {
-    dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_SECURITY);
-    return false;
-  }
-  if (!file_system_context()->path_manager()->IsAllowedFileSystemType(
-      *origin_url, *type)) {
+  if (!file_system_context()->path_manager()->CrackFileSystemPath(
+          path, origin_url, type, virtual_path)) {
     dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_SECURITY);
     return false;
   }