diff options
24 files changed, 552 insertions, 410 deletions
diff --git a/webkit/fileapi/cross_file_util_helper.cc b/webkit/fileapi/cross_file_util_helper.cc new file mode 100644 index 0000000..1227d19 --- /dev/null +++ b/webkit/fileapi/cross_file_util_helper.cc @@ -0,0 +1,183 @@ +// Copyright (c) 2012 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. + +#include "webkit/fileapi/cross_file_util_helper.h" + +#include "webkit/fileapi/file_system_file_util.h" +#include "webkit/fileapi/file_system_operation_context.h" +#include "webkit/fileapi/file_system_path.h" + +using base::PlatformFileError; + +namespace fileapi { + +CrossFileUtilHelper::CrossFileUtilHelper( + FileSystemOperationContext* context, + FileSystemFileUtil* src_util, + FileSystemFileUtil* dest_util, + const FileSystemPath& src_path, + const FileSystemPath& dest_path, + Operation operation) + : context_(context), + src_util_(src_util), + dest_util_(dest_util), + src_root_path_(src_path), + dest_root_path_(dest_path), + operation_(operation) {} + +CrossFileUtilHelper::~CrossFileUtilHelper() {} + +base::PlatformFileError CrossFileUtilHelper::DoWork() { + base::PlatformFileError error = + PerformErrorCheckAndPreparationForMoveAndCopy(); + if (error != base::PLATFORM_FILE_OK) + return error; + if (src_util_->DirectoryExists(context_, src_root_path_)) { + return CopyOrMoveDirectory(src_root_path_, dest_root_path_); + } + return CopyOrMoveFile(src_root_path_, dest_root_path_); +} + +PlatformFileError +CrossFileUtilHelper::PerformErrorCheckAndPreparationForMoveAndCopy() { + // Exits earlier if the source path does not exist. + if (!src_util_->PathExists(context_, src_root_path_)) + return base::PLATFORM_FILE_ERROR_NOT_FOUND; + + bool same_file_system = + (src_root_path_.origin() == dest_root_path_.origin()) && + (src_root_path_.type() == dest_root_path_.type()); + + // The parent of the |dest_root_path_| does not exist. + if (!ParentExists(dest_root_path_, dest_util_)) + return base::PLATFORM_FILE_ERROR_NOT_FOUND; + + // It is an error to try to copy/move an entry into its child. + if (same_file_system && src_root_path_.IsParent(dest_root_path_)) + return base::PLATFORM_FILE_ERROR_INVALID_OPERATION; + + // Now it is ok to return if the |dest_root_path_| does not exist. + if (!dest_util_->PathExists(context_, dest_root_path_)) + return base::PLATFORM_FILE_OK; + + // |src_root_path_| exists and is a directory. + // |dest_root_path_| exists and is a file. + bool src_is_directory = src_util_->DirectoryExists(context_, src_root_path_); + bool dest_is_directory = + dest_util_->DirectoryExists(context_, dest_root_path_); + + // Either one of |src_root_path_| or |dest_root_path_| is directory, + // while the other is not. + if (src_is_directory != dest_is_directory) + return base::PLATFORM_FILE_ERROR_INVALID_OPERATION; + + // It is an error to copy/move an entry into the same path. + if (same_file_system && (src_root_path_.internal_path() == + dest_root_path_.internal_path())) + return base::PLATFORM_FILE_ERROR_EXISTS; + + if (dest_is_directory) { + // It is an error to copy/move an entry to a non-empty directory. + // Otherwise the copy/move attempt must overwrite the destination, but + // the file_util's Copy or Move method doesn't perform overwrite + // on all platforms, so we delete the destination directory here. + if (base::PLATFORM_FILE_OK != + dest_util_->Delete(context_, dest_root_path_, false /* recursive */)) { + if (!dest_util_->IsDirectoryEmpty(context_, dest_root_path_)) + return base::PLATFORM_FILE_ERROR_NOT_EMPTY; + return base::PLATFORM_FILE_ERROR_FAILED; + } + } + return base::PLATFORM_FILE_OK; +} + +bool CrossFileUtilHelper::ParentExists( + const FileSystemPath& path, FileSystemFileUtil* file_util) { + // If path is in the root, path.DirName() will be ".", + // since we use paths with no leading '/'. + FilePath parent = path.internal_path().DirName(); + if (parent == FilePath(FILE_PATH_LITERAL("."))) + return true; + return file_util->DirectoryExists( + context_, path.WithInternalPath(parent)); +} + +PlatformFileError CrossFileUtilHelper::CopyOrMoveDirectory( + const FileSystemPath& src_path, + const FileSystemPath& dest_path) { + // At this point we must have gone through + // PerformErrorCheckAndPreparationForMoveAndCopy so this must be true. + DCHECK(!((src_path.origin() == dest_path.origin()) && + (src_path.type() == dest_path.type())) || + !src_path.IsParent(dest_path)); + + PlatformFileError error = dest_util_->CreateDirectory( + context_, dest_path, false, false); + if (error != base::PLATFORM_FILE_OK) + return error; + + scoped_ptr<FileSystemFileUtil::AbstractFileEnumerator> file_enum( + src_util_->CreateFileEnumerator(context_, src_path)); + FilePath src_file_path_each; + while (!(src_file_path_each = file_enum->Next()).empty()) { + FilePath dest_file_path_each(dest_path.internal_path()); + src_path.internal_path().AppendRelativePath( + src_file_path_each, &dest_file_path_each); + + if (file_enum->IsDirectory()) { + PlatformFileError error = dest_util_->CreateDirectory( + context_, + dest_path.WithInternalPath(dest_file_path_each), + true /* exclusive */, false /* recursive */); + if (error != base::PLATFORM_FILE_OK) + return error; + } else { + PlatformFileError error = CopyOrMoveFile( + src_path.WithInternalPath(src_file_path_each), + dest_path.WithInternalPath(dest_file_path_each)); + if (error != base::PLATFORM_FILE_OK) + return error; + } + } + + if (operation_ == OPERATION_MOVE) { + PlatformFileError error = src_util_->Delete(context_, src_path, true); + if (error != base::PLATFORM_FILE_OK) + return error; + } + + return base::PLATFORM_FILE_OK; +} + +PlatformFileError CrossFileUtilHelper::CopyOrMoveFile( + const FileSystemPath& src_path, + const FileSystemPath& dest_path) { + if ((src_path.origin() == dest_path.origin()) && + (src_path.type() == dest_path.type())) { + DCHECK(src_util_ == dest_util_); + // Source and destination are in the same FileSystemFileUtil; now we can + // safely call FileSystemFileUtil method on src_util_ (== dest_util_). + return src_util_->CopyOrMoveFile(context_, src_path, dest_path, + operation_ == OPERATION_COPY); + } + + // Resolve the src_path's underlying file path. + base::PlatformFileInfo file_info; + FilePath platform_file_path; + PlatformFileError error = src_util_->GetFileInfo( + context_, src_path, &file_info, &platform_file_path); + if (error != base::PLATFORM_FILE_OK) + return error; + + // Call CopyInForeignFile() on the dest_util_ with the resolved source path + // to perform limited cross-FileSystemFileUtil copy/move. + error = dest_util_->CopyInForeignFile( + context_, src_path.WithInternalPath(platform_file_path), dest_path); + + if (operation_ == OPERATION_COPY || error != base::PLATFORM_FILE_OK) + return error; + return src_util_->DeleteFile(context_, src_path); +} + +} // namespace fileapi diff --git a/webkit/fileapi/cross_file_util_helper.h b/webkit/fileapi/cross_file_util_helper.h new file mode 100644 index 0000000..3466928 --- /dev/null +++ b/webkit/fileapi/cross_file_util_helper.h @@ -0,0 +1,71 @@ +// Copyright (c) 2012 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. + +#ifndef WEBKIT_FILEAPI_CROSS_FILE_UTIL_HELPER_H_ +#define WEBKIT_FILEAPI_CROSS_FILE_UTIL_HELPER_H_ + +#include "base/callback.h" +#include "base/platform_file.h" + +namespace fileapi { + +class FileSystemFileUtil; +class FileSystemOperationContext; +class FileSystemPath; + +// A helper class for cross-FileUtil Copy/Move operations. +class CrossFileUtilHelper { + public: + enum Operation { + OPERATION_COPY, + OPERATION_MOVE + }; + + CrossFileUtilHelper(FileSystemOperationContext* context, + FileSystemFileUtil* src_util, + FileSystemFileUtil* dest_util, + const FileSystemPath& src_path, + const FileSystemPath& dest_path, + Operation operation); + ~CrossFileUtilHelper(); + + base::PlatformFileError DoWork(); + + private: + // Performs common pre-operation check and preparation. + // This may delete the destination directory if it's empty. + base::PlatformFileError PerformErrorCheckAndPreparationForMoveAndCopy(); + + // This assumes that the root exists. + bool ParentExists(const FileSystemPath& path, FileSystemFileUtil* file_util); + + // Performs recursive copy or move by calling CopyOrMoveFile for individual + // files. Operations for recursive traversal are encapsulated in this method. + // It assumes src_path and dest_path have passed + // PerformErrorCheckAndPreparationForMoveAndCopy(). + base::PlatformFileError CopyOrMoveDirectory( + const FileSystemPath& src_path, + const FileSystemPath& dest_path); + + // Determines whether a simple same-filesystem move or copy can be done. If + // so, it delegates to CopyOrMoveFile. Otherwise it looks up the true + // platform path of the source file, delegates to CopyInForeignFile, and [for + // move] calls DeleteFile on the source file. + base::PlatformFileError CopyOrMoveFile( + const FileSystemPath& src_path, + const FileSystemPath& dest_path); + + FileSystemOperationContext* context_; + FileSystemFileUtil* src_util_; // Not owned. + FileSystemFileUtil* dest_util_; // Not owned. + const FileSystemPath& src_root_path_; + const FileSystemPath& dest_root_path_; + Operation operation_; + + DISALLOW_COPY_AND_ASSIGN(CrossFileUtilHelper); +}; + +} // namespace fileapi + +#endif // WEBKIT_FILEAPI_CROSS_FILE_UTIL_HELPER_H_ diff --git a/webkit/fileapi/file_system_dir_url_request_job_unittest.cc b/webkit/fileapi/file_system_dir_url_request_job_unittest.cc index f0459ba..c0f3bdf 100644 --- a/webkit/fileapi/file_system_dir_url_request_job_unittest.cc +++ b/webkit/fileapi/file_system_dir_url_request_job_unittest.cc @@ -114,8 +114,7 @@ class FileSystemDirURLRequestJobTest : public testing::Test { FileSystemPath CreatePath(const FilePath& file_path) { return FileSystemPath(GURL("http://remote"), fileapi::kFileSystemTypeTemporary, - file_path, - file_util()); + file_path); } FileSystemOperationContext* NewOperationContext() { diff --git a/webkit/fileapi/file_system_file_util.cc b/webkit/fileapi/file_system_file_util.cc index 9156075..484a3c5 100644 --- a/webkit/fileapi/file_system_file_util.cc +++ b/webkit/fileapi/file_system_file_util.cc @@ -12,22 +12,6 @@ namespace fileapi { -namespace { - -// This assumes that the root exists. -bool ParentExists(FileSystemOperationContext* context, - const FileSystemPath& path) { - // If path is in the root, path.DirName() will be ".", - // since we use paths with no leading '/'. - FilePath parent = path.internal_path().DirName(); - if (parent == FilePath(FILE_PATH_LITERAL("."))) - return true; - return path.file_util()->DirectoryExists( - context, path.WithInternalPath(parent)); -} - -} // namespace - FileSystemFileUtil::FileSystemFileUtil() { } @@ -38,43 +22,6 @@ FileSystemFileUtil::FileSystemFileUtil(FileSystemFileUtil* underlying_file_util) FileSystemFileUtil::~FileSystemFileUtil() { } -PlatformFileError FileSystemFileUtil::Copy( - FileSystemOperationContext* context, - const FileSystemPath& src_path, - const FileSystemPath& dest_path) { - PlatformFileError error_code; - error_code = - PerformCommonCheckAndPreparationForMoveAndCopy( - context, src_path, dest_path); - if (error_code != base::PLATFORM_FILE_OK) - return error_code; - - if (DirectoryExists(context, src_path)) - return CopyOrMoveDirectory(context, src_path, dest_path, - true /* copy */); - return CopyOrMoveFileHelper(context, src_path, dest_path, - true /* copy */); -} - -PlatformFileError FileSystemFileUtil::Move( - FileSystemOperationContext* context, - const FileSystemPath& src_path, - const FileSystemPath& dest_path) { - PlatformFileError error_code; - error_code = - PerformCommonCheckAndPreparationForMoveAndCopy( - context, src_path, dest_path); - if (error_code != base::PLATFORM_FILE_OK) - return error_code; - - // TODO(dmikurube): ReplaceFile if in the same domain and filesystem type. - if (DirectoryExists(context, src_path)) - return CopyOrMoveDirectory(context, src_path, dest_path, - false /* copy */); - return CopyOrMoveFileHelper(context, src_path, dest_path, - false /* copy */); -} - PlatformFileError FileSystemFileUtil::Delete( FileSystemOperationContext* context, const FileSystemPath& path, @@ -299,151 +246,6 @@ PlatformFileError FileSystemFileUtil::DeleteSingleDirectory( return base::PLATFORM_FILE_ERROR_FAILED; } -PlatformFileError -FileSystemFileUtil::PerformCommonCheckAndPreparationForMoveAndCopy( - FileSystemOperationContext* context, - const FileSystemPath& src_path, - const FileSystemPath& dest_path) { - // Exits earlier if the source path does not exist. - if (!PathExists(context, src_path)) - return base::PLATFORM_FILE_ERROR_NOT_FOUND; - - // It is an error to copy/move an entry into the same path. - if (src_path == dest_path) - return base::PLATFORM_FILE_ERROR_EXISTS; - - bool same_file_system = - (src_path.origin() == dest_path.origin()) && - (src_path.type() == dest_path.type()); - FileSystemFileUtil* dest_util = dest_path.file_util(); - DCHECK(dest_util); - - // The parent of the |dest_path| does not exist. - if (!ParentExists(context, dest_path)) - return base::PLATFORM_FILE_ERROR_NOT_FOUND; - - // It is an error to try to copy/move an entry into its child. - if (same_file_system && src_path.IsParent(dest_path)) - return base::PLATFORM_FILE_ERROR_INVALID_OPERATION; - - // Now it is ok to return if the |dest_path| does not exist. - if (!dest_util->PathExists(context, dest_path)) - return base::PLATFORM_FILE_OK; - - // |src_path| exists and is a directory. - // |dest_path| exists and is a file. - bool src_is_directory = DirectoryExists(context, src_path); - bool dest_is_directory = - dest_util->DirectoryExists(context, dest_path); - if (src_is_directory && !dest_is_directory) - return base::PLATFORM_FILE_ERROR_INVALID_OPERATION; - - // |src_path| exists and is a file. - // |dest_path| exists and is a directory. - if (!src_is_directory && dest_is_directory) - return base::PLATFORM_FILE_ERROR_INVALID_OPERATION; - - if (dest_is_directory) { - // It is an error to copy/move an entry to a non-empty directory. - // Otherwise the copy/move attempt must overwrite the destination, but - // the file_util's Copy or Move method doesn't perform overwrite - // on all platforms, so we delete the destination directory here. - // TODO(kinuko): may be better to change the file_util::{Copy,Move}. - if (base::PLATFORM_FILE_OK != - dest_util->Delete(context, dest_path, false /* recursive */)) { - if (!dest_util->IsDirectoryEmpty(context, dest_path)) - return base::PLATFORM_FILE_ERROR_NOT_EMPTY; - return base::PLATFORM_FILE_ERROR_FAILED; - } - } - return base::PLATFORM_FILE_OK; -} - -PlatformFileError FileSystemFileUtil::CopyOrMoveDirectory( - FileSystemOperationContext* context, - const FileSystemPath& src_path, - const FileSystemPath& dest_path, - bool copy) { - FileSystemFileUtil* dest_util = dest_path.file_util(); - - // Re-check PerformCommonCheckAndPreparationForMoveAndCopy() by DCHECK. - DCHECK(DirectoryExists(context, src_path)); - DCHECK(ParentExists(context, dest_path)); - DCHECK(!dest_util->PathExists(context, dest_path)); - if ((src_path.origin() == dest_path.origin()) && - (src_path.type() == dest_path.type())) - DCHECK(!src_path.IsParent(dest_path)); - - if (!dest_util->DirectoryExists(context, dest_path)) { - PlatformFileError error = dest_util->CreateDirectory( - context, dest_path, false, false); - if (error != base::PLATFORM_FILE_OK) - return error; - } - - scoped_ptr<AbstractFileEnumerator> file_enum( - CreateFileEnumerator(context, src_path)); - FilePath src_file_path_each; - while (!(src_file_path_each = file_enum->Next()).empty()) { - FilePath dest_file_path_each(dest_path.internal_path()); - src_path.internal_path().AppendRelativePath( - src_file_path_each, &dest_file_path_each); - - if (file_enum->IsDirectory()) { - PlatformFileError error = dest_util->CreateDirectory( - context, - dest_path.WithInternalPath(dest_file_path_each), - false, false); - if (error != base::PLATFORM_FILE_OK) - return error; - } else { - PlatformFileError error = CopyOrMoveFileHelper( - context, - src_path.WithInternalPath(src_file_path_each), - dest_path.WithInternalPath(dest_file_path_each), - copy); - if (error != base::PLATFORM_FILE_OK) - return error; - } - } - - if (!copy) { - PlatformFileError error = Delete(context, src_path, true); - if (error != base::PLATFORM_FILE_OK) - return error; - } - - return base::PLATFORM_FILE_OK; -} - -PlatformFileError FileSystemFileUtil::CopyOrMoveFileHelper( - FileSystemOperationContext* context, - const FileSystemPath& src_path, - const FileSystemPath& dest_path, - bool copy) { - // CopyOrMoveFile here is the virtual overridden member function. - if ((src_path.origin() == dest_path.origin()) && - (src_path.type() == dest_path.type())) { - DCHECK(src_path.file_util() == dest_path.file_util()); - return CopyOrMoveFile(context, src_path, dest_path, copy); - } - - base::PlatformFileInfo file_info; - FilePath underlying_file_path; - PlatformFileError error_code; - error_code = src_path.file_util()->GetFileInfo( - context, src_path, &file_info, &underlying_file_path); - if (error_code != base::PLATFORM_FILE_OK) - return error_code; - - DCHECK(dest_path.file_util()); - error_code = dest_path.file_util()->CopyInForeignFile( - context, src_path.WithInternalPath(underlying_file_path), dest_path); - if (copy || error_code != base::PLATFORM_FILE_OK) - return error_code; - return src_path.file_util()->DeleteFile(context, src_path); -} - PlatformFileError FileSystemFileUtil::DeleteDirectoryRecursive( FileSystemOperationContext* context, const FileSystemPath& path) { diff --git a/webkit/fileapi/file_system_file_util.h b/webkit/fileapi/file_system_file_util.h index 08b2891..0306886 100644 --- a/webkit/fileapi/file_system_file_util.h +++ b/webkit/fileapi/file_system_file_util.h @@ -58,42 +58,6 @@ class FileSystemFileUtil { virtual ~FileSystemFileUtil(); - // Copies or moves a single file. - // Copies a file or a directory from |src_path| to |dest_path|. - // - // Error cases: - // If destination's parent doesn't exist. - // If source dir exists but destination path is an existing file. - // If source file exists but destination path is an existing directory. - // If source is a parent of destination. - // If source doesn't exist. - // - // This method calls one of the following methods depending on whether the - // target is a directory or not. - // - (virtual) CopyOrMoveFile or - // - (non-virtual) CopyOrMoveDirectory. - // - // TODO(kinuko,kinaba): Move this cross-FileUtil code out of this interface. - PlatformFileError Copy( - FileSystemOperationContext* context, - const FileSystemPath& src_path, - const FileSystemPath& dest_path); - - // Moves a file or a directory from src_path to dest_path. - // - // Error cases are similar to Copy method's error cases. - // - // This method calls one of the following methods depending on whether the - // target is a directory or not. - // - (virtual) CopyOrMoveFile or - // - (non-virtual) CopyOrMoveDirectory. - // - // TODO(kinuko,kinaba): Move this cross-FileUtil code out of this interface. - PlatformFileError Move( - FileSystemOperationContext* context, - const FileSystemPath& src_path, - const FileSystemPath& dest_path); - // Deletes a file or a directory. // It is an error to delete a non-empty directory with recursive=false. // @@ -243,34 +207,6 @@ class FileSystemFileUtil { FileSystemFileUtil(); explicit FileSystemFileUtil(FileSystemFileUtil* underlying_file_util); - // This also removes the destination directory if it's non-empty and all - // other checks are passed (so that the copy/move correctly overwrites the - // destination). - PlatformFileError PerformCommonCheckAndPreparationForMoveAndCopy( - FileSystemOperationContext* context, - const FileSystemPath& src_path, - const FileSystemPath& dest_path); - - // Performs recursive copy or move by calling CopyOrMoveFile for individual - // files. Operations for recursive traversal are encapsulated in this method. - // It assumes src_path and dest_path have passed - // PerformCommonCheckAndPreparationForMoveAndCopy(). - PlatformFileError CopyOrMoveDirectory( - FileSystemOperationContext* context, - const FileSystemPath& src_path, - const FileSystemPath& dest_path, - bool copy); - - // Determines whether a simple same-filesystem move or copy can be done. If - // so, it delegates to CopyOrMoveFile. Otherwise it looks up the true - // platform path of the source file, delegates to CopyInForeignFile, and [for - // move] calls DeleteFile on the source file. - PlatformFileError CopyOrMoveFileHelper( - FileSystemOperationContext* context, - const FileSystemPath& src_path, - const FileSystemPath& dest_path, - bool copy); - // Deletes a directory and all entries under the directory. // // This method is called from Delete. It internally calls two following diff --git a/webkit/fileapi/file_system_file_util_proxy.cc b/webkit/fileapi/file_system_file_util_proxy.cc index 380e42b..daa78a4 100644 --- a/webkit/fileapi/file_system_file_util_proxy.cc +++ b/webkit/fileapi/file_system_file_util_proxy.cc @@ -5,7 +5,13 @@ #include "webkit/fileapi/file_system_file_util_proxy.h" #include "base/bind.h" +#include "base/memory/scoped_ptr.h" #include "base/message_loop_proxy.h" +#include "webkit/fileapi/cross_file_util_helper.h" +#include "webkit/fileapi/file_system_file_util.h" +#include "webkit/fileapi/file_system_operation_context.h" + +namespace fileapi { namespace { @@ -17,6 +23,28 @@ using base::Unretained; typedef fileapi::FileSystemFileUtilProxy Proxy; +class CopyOrMoveHelper { + public: + CopyOrMoveHelper(CrossFileUtilHelper* helper) + : helper_(helper), + error_(base::PLATFORM_FILE_OK) {} + ~CopyOrMoveHelper() {} + + void RunWork() { + error_ = helper_->DoWork(); + } + + void Reply(const Proxy::StatusCallback& callback) { + if (!callback.is_null()) + callback.Run(error_); + } + + private: + scoped_ptr<CrossFileUtilHelper> helper_; + base::PlatformFileError error_; + DISALLOW_COPY_AND_ASSIGN(CopyOrMoveHelper); +}; + class EnsureFileExistsHelper { public: EnsureFileExistsHelper() : error_(base::PLATFORM_FILE_OK), created_(false) {} @@ -80,7 +108,43 @@ class ReadDirectoryHelper { } // namespace -namespace fileapi { +// static +bool FileSystemFileUtilProxy::Copy( + scoped_refptr<MessageLoopProxy> message_loop_proxy, + FileSystemOperationContext* context, + FileSystemFileUtil* src_util, + FileSystemFileUtil* dest_util, + const FileSystemPath& src_path, + const FileSystemPath& dest_path, + const StatusCallback& callback) { + CopyOrMoveHelper* helper = new CopyOrMoveHelper( + new CrossFileUtilHelper( + context, src_util, dest_util, src_path, dest_path, + CrossFileUtilHelper::OPERATION_COPY)); + return message_loop_proxy->PostTaskAndReply( + FROM_HERE, + Bind(&CopyOrMoveHelper::RunWork, Unretained(helper)), + Bind(&CopyOrMoveHelper::Reply, Owned(helper), callback)); +} + +// static +bool FileSystemFileUtilProxy::Move( + scoped_refptr<MessageLoopProxy> message_loop_proxy, + FileSystemOperationContext* context, + FileSystemFileUtil* src_util, + FileSystemFileUtil* dest_util, + const FileSystemPath& src_path, + const FileSystemPath& dest_path, + const StatusCallback& callback) { + CopyOrMoveHelper* helper = new CopyOrMoveHelper( + new CrossFileUtilHelper( + context, src_util, dest_util, src_path, dest_path, + CrossFileUtilHelper::OPERATION_MOVE)); + return message_loop_proxy->PostTaskAndReply( + FROM_HERE, + Bind(&CopyOrMoveHelper::RunWork, Unretained(helper)), + Bind(&CopyOrMoveHelper::Reply, Owned(helper), callback)); +} // static bool FileSystemFileUtilProxy::RelayEnsureFileExists( diff --git a/webkit/fileapi/file_system_file_util_proxy.h b/webkit/fileapi/file_system_file_util_proxy.h index 514c461..e2d2f25 100644 --- a/webkit/fileapi/file_system_file_util_proxy.h +++ b/webkit/fileapi/file_system_file_util_proxy.h @@ -25,20 +25,23 @@ using base::PlatformFile; using base::PlatformFileError; using base::PlatformFileInfo; -// This class provides relay methods for supporting asynchronous access to -// FileSystem API operations. (Most of necessary relay methods are provided -// by base::FileUtilProxy, but there are a few operations that are not -// covered or are slightly different from the version of base::FileUtilProxy. +class FileSystemFileUtil; +class FileSystemOperationContext; +class FileSystemPath; + +// This class provides asynchronous access to FileSystemFileUtil methods for +// FileSystem API operations. This also implements cross-FileUtil copy/move +// operations on top of FileSystemFileUtil methods. class FileSystemFileUtilProxy { public: typedef base::FileUtilProxy::Entry Entry; - - typedef base::Callback<void(PlatformFileError, - bool /* created */ + typedef base::Callback<void(PlatformFileError status)> StatusCallback; + typedef base::Callback<void(PlatformFileError status, + bool created )> EnsureFileExistsCallback; - typedef base::Callback<void(PlatformFileError, - const PlatformFileInfo&, - const FilePath& /* platform_path */ + typedef base::Callback<void(PlatformFileError status, + const PlatformFileInfo& info, + const FilePath& platform_path )> GetFileInfoCallback; typedef base::Callback<void(PlatformFileError, const std::vector<Entry>&, @@ -51,6 +54,42 @@ class FileSystemFileUtilProxy { typedef base::Callback<PlatformFileError(std::vector<Entry>* )> ReadDirectoryTask; + // Copies a file or a directory from |src_path| to |dest_path| by calling + // FileSystemFileUtil's following methods on the given |message_loop_proxy|. + // - CopyOrMoveFile() for same-filesystem operations + // - CopyInForeignFile() for (limited) cross-filesystem operations + // + // Error cases: + // If destination's parent doesn't exist. + // If source dir exists but destination path is an existing file. + // If source file exists but destination path is an existing directory. + // If source is a parent of destination. + // If source doesn't exist. + // If source and dest are the same path in the same filesystem. + static bool Copy( + scoped_refptr<MessageLoopProxy> message_loop_proxy, + FileSystemOperationContext* context, + FileSystemFileUtil* src_util, + FileSystemFileUtil* dest_util, + const FileSystemPath& src_path, + const FileSystemPath& dest_path, + const StatusCallback& callback); + + // Moves a file or a directory from |src_path| to |dest_path| by calling + // FileSystemFileUtil's following methods on the given |message_loop_proxy|. + // - CopyOrMoveFile() for same-filesystem operations + // - CopyInForeignFile() for (limited) cross-filesystem operations + // + // This method returns an error on the same error cases with Copy. + static bool Move( + scoped_refptr<MessageLoopProxy> message_loop_proxy, + FileSystemOperationContext* context, + FileSystemFileUtil* src_util, + FileSystemFileUtil* dest_util, + const FileSystemPath& src_path, + const FileSystemPath& dest_path, + const StatusCallback& callback); + // Calls EnsureFileExistsTask |task| on the given |message_loop_proxy|. static bool RelayEnsureFileExists( scoped_refptr<MessageLoopProxy> message_loop_proxy, diff --git a/webkit/fileapi/file_system_file_util_unittest.cc b/webkit/fileapi/file_system_file_util_unittest.cc index 13071d4..40cd803 100644 --- a/webkit/fileapi/file_system_file_util_unittest.cc +++ b/webkit/fileapi/file_system_file_util_unittest.cc @@ -8,6 +8,7 @@ #include "base/platform_file.h" #include "base/scoped_temp_dir.h" #include "testing/gtest/include/gtest/gtest.h" +#include "webkit/fileapi/cross_file_util_helper.h" #include "webkit/fileapi/file_system_context.h" #include "webkit/fileapi/file_system_operation_context.h" #include "webkit/fileapi/file_system_test_helper.h" @@ -89,12 +90,15 @@ class FileSystemFileUtilTest : public testing::Test { src_helper.NewOperationContext()); copy_context->set_allowed_bytes_growth(1024 * 1024); // OFSFU path quota. - if (copy) - ASSERT_EQ(base::PLATFORM_FILE_OK, - file_util->Copy(copy_context.get(), src_root, dest_root)); - else - ASSERT_EQ(base::PLATFORM_FILE_OK, - file_util->Move(copy_context.get(), src_root, dest_root)); + CrossFileUtilHelper cross_util_helper( + copy_context.get(), + src_helper.file_util(), + dest_helper.file_util(), + src_root, + dest_root, + copy ? CrossFileUtilHelper::OPERATION_COPY + : CrossFileUtilHelper::OPERATION_MOVE); + ASSERT_EQ(base::PLATFORM_FILE_OK, cross_util_helper.DoWork()); // Validate that the destination paths are correct. for (size_t i = 0; i < test::kRegularTestCaseSize; ++i) { diff --git a/webkit/fileapi/file_system_operation.cc b/webkit/fileapi/file_system_operation.cc index 3dbe7d8..cc5676d 100644 --- a/webkit/fileapi/file_system_operation.cc +++ b/webkit/fileapi/file_system_operation.cc @@ -63,7 +63,7 @@ FileSystemOperation::~FileSystemOperation() { base::FileUtilProxy::RelayClose( proxy_, base::Bind(&FileSystemFileUtil::Close, - base::Unretained(src_path_.file_util()), + base::Unretained(src_util_), base::Owned(c)), file_writer_delegate_->file(), base::FileUtilProxy::StatusCallback()); @@ -76,7 +76,7 @@ void FileSystemOperation::CreateFile(const GURL& path_url, DCHECK(SetPendingOperationType(kOperationCreateFile)); base::PlatformFileError result = SetUpFileSystemPath( - path_url, &src_path_, PATH_FOR_CREATE); + path_url, &src_path_, &src_util_, PATH_FOR_CREATE); if (result != base::PLATFORM_FILE_OK) { callback.Run(result); delete this; @@ -103,7 +103,7 @@ void FileSystemOperation::DelayedCreateFileForQuota( FileSystemFileUtilProxy::RelayEnsureFileExists( proxy_, base::Bind(&FileSystemFileUtil::EnsureFileExists, - base::Unretained(src_path_.file_util()), + base::Unretained(src_util_), &operation_context_, src_path_), base::Bind( @@ -119,7 +119,7 @@ void FileSystemOperation::CreateDirectory(const GURL& path_url, DCHECK(SetPendingOperationType(kOperationCreateDirectory)); base::PlatformFileError result = SetUpFileSystemPath( - path_url, &src_path_, PATH_FOR_CREATE); + path_url, &src_path_, &src_util_, PATH_FOR_CREATE); if (result != base::PLATFORM_FILE_OK) { callback.Run(result); delete this; @@ -145,7 +145,7 @@ void FileSystemOperation::DelayedCreateDirectoryForQuota( base::FileUtilProxy::RelayFileTask( proxy_, FROM_HERE, base::Bind(&FileSystemFileUtil::CreateDirectory, - base::Unretained(src_path_.file_util()), + base::Unretained(src_util_), &operation_context_, src_path_, exclusive, recursive), base::Bind(&FileSystemOperation::DidFinishFileOperation, @@ -158,9 +158,10 @@ void FileSystemOperation::Copy(const GURL& src_path_url, DCHECK(SetPendingOperationType(kOperationCopy)); base::PlatformFileError result = SetUpFileSystemPath( - src_path_url, &src_path_, PATH_FOR_READ); + src_path_url, &src_path_, &src_util_, PATH_FOR_READ); if (result == base::PLATFORM_FILE_OK) - result = SetUpFileSystemPath(dest_path_url, &dest_path_, PATH_FOR_CREATE); + result = SetUpFileSystemPath( + dest_path_url, &dest_path_, &dest_util_, PATH_FOR_CREATE); if (result != base::PLATFORM_FILE_OK) { callback.Run(result); delete this; @@ -183,13 +184,10 @@ void FileSystemOperation::DelayedCopyForQuota(const StatusCallback& callback, dest_path_.origin(), dest_path_.type())); - base::FileUtilProxy::RelayFileTask( - proxy_, FROM_HERE, - base::Bind(&FileSystemFileUtil::Copy, - base::Unretained(dest_path_.file_util()), - &operation_context_, - src_path_, - dest_path_), + FileSystemFileUtilProxy::Copy( + proxy_, &operation_context_, + src_util_, dest_util_, + src_path_, dest_path_, base::Bind(&FileSystemOperation::DidFinishFileOperation, base::Owned(this), callback)); } @@ -200,9 +198,10 @@ void FileSystemOperation::Move(const GURL& src_path_url, DCHECK(SetPendingOperationType(kOperationMove)); base::PlatformFileError result = SetUpFileSystemPath( - src_path_url, &src_path_, PATH_FOR_WRITE); + src_path_url, &src_path_, &src_util_, PATH_FOR_WRITE); if (result == base::PLATFORM_FILE_OK) - result = SetUpFileSystemPath(dest_path_url, &dest_path_, PATH_FOR_CREATE); + result = SetUpFileSystemPath( + dest_path_url, &dest_path_, &dest_util_, PATH_FOR_CREATE); if (result != base::PLATFORM_FILE_OK) { callback.Run(result); delete this; @@ -225,13 +224,10 @@ void FileSystemOperation::DelayedMoveForQuota(const StatusCallback& callback, dest_path_.origin(), dest_path_.type())); - base::FileUtilProxy::RelayFileTask( - proxy_, FROM_HERE, - base::Bind(&FileSystemFileUtil::Move, - base::Unretained(dest_path_.file_util()), - &operation_context_, - src_path_, - dest_path_), + FileSystemFileUtilProxy::Move( + proxy_, &operation_context_, + src_util_, dest_util_, + src_path_, dest_path_, base::Bind(&FileSystemOperation::DidFinishFileOperation, base::Owned(this), callback)); } @@ -241,7 +237,7 @@ void FileSystemOperation::DirectoryExists(const GURL& path_url, DCHECK(SetPendingOperationType(kOperationDirectoryExists)); base::PlatformFileError result = SetUpFileSystemPath( - path_url, &src_path_, PATH_FOR_READ); + path_url, &src_path_, &src_util_, PATH_FOR_READ); if (result != base::PLATFORM_FILE_OK) { callback.Run(result); delete this; @@ -251,7 +247,7 @@ void FileSystemOperation::DirectoryExists(const GURL& path_url, FileSystemFileUtilProxy::RelayGetFileInfo( proxy_, base::Bind(&FileSystemFileUtil::GetFileInfo, - base::Unretained(src_path_.file_util()), + base::Unretained(src_util_), &operation_context_, src_path_), base::Bind(&FileSystemOperation::DidDirectoryExists, @@ -263,7 +259,7 @@ void FileSystemOperation::FileExists(const GURL& path_url, DCHECK(SetPendingOperationType(kOperationFileExists)); base::PlatformFileError result = SetUpFileSystemPath( - path_url, &src_path_, PATH_FOR_READ); + path_url, &src_path_, &src_util_, PATH_FOR_READ); if (result != base::PLATFORM_FILE_OK) { callback.Run(result); delete this; @@ -273,7 +269,7 @@ void FileSystemOperation::FileExists(const GURL& path_url, FileSystemFileUtilProxy::RelayGetFileInfo( proxy_, base::Bind(&FileSystemFileUtil::GetFileInfo, - base::Unretained(src_path_.file_util()), + base::Unretained(src_util_), &operation_context_, src_path_), base::Bind(&FileSystemOperation::DidFileExists, base::Owned(this), callback)); @@ -284,7 +280,7 @@ void FileSystemOperation::GetMetadata(const GURL& path_url, DCHECK(SetPendingOperationType(kOperationGetMetadata)); base::PlatformFileError result = SetUpFileSystemPath( - path_url, &src_path_, PATH_FOR_READ); + path_url, &src_path_, &src_util_, PATH_FOR_READ); if (result != base::PLATFORM_FILE_OK) { callback.Run(result, base::PlatformFileInfo(), FilePath()); delete this; @@ -294,7 +290,7 @@ void FileSystemOperation::GetMetadata(const GURL& path_url, FileSystemFileUtilProxy::RelayGetFileInfo( proxy_, base::Bind(&FileSystemFileUtil::GetFileInfo, - base::Unretained(src_path_.file_util()), + base::Unretained(src_util_), &operation_context_, src_path_), base::Bind(&FileSystemOperation::DidGetMetadata, base::Owned(this), callback)); @@ -305,7 +301,7 @@ void FileSystemOperation::ReadDirectory(const GURL& path_url, DCHECK(SetPendingOperationType(kOperationReadDirectory)); base::PlatformFileError result = SetUpFileSystemPath( - path_url, &src_path_, PATH_FOR_READ); + path_url, &src_path_, &src_util_, PATH_FOR_READ); if (result != base::PLATFORM_FILE_OK) { callback.Run(result, std::vector<base::FileUtilProxy::Entry>(), false); delete this; @@ -315,7 +311,7 @@ void FileSystemOperation::ReadDirectory(const GURL& path_url, FileSystemFileUtilProxy::RelayReadDirectory( proxy_, base::Bind(&FileSystemFileUtil::ReadDirectory, - base::Unretained(src_path_.file_util()), + base::Unretained(src_util_), &operation_context_, src_path_), base::Bind(&FileSystemOperation::DidReadDirectory, base::Owned(this), callback)); @@ -326,7 +322,7 @@ void FileSystemOperation::Remove(const GURL& path_url, bool recursive, DCHECK(SetPendingOperationType(kOperationRemove)); base::PlatformFileError result = SetUpFileSystemPath( - path_url, &src_path_, PATH_FOR_WRITE); + path_url, &src_path_, &src_util_, PATH_FOR_WRITE); if (result != base::PLATFORM_FILE_OK) { callback.Run(result); delete this; @@ -336,7 +332,7 @@ void FileSystemOperation::Remove(const GURL& path_url, bool recursive, base::FileUtilProxy::RelayFileTask( proxy_, FROM_HERE, base::Bind(&FileSystemFileUtil::Delete, - base::Unretained(src_path_.file_util()), + base::Unretained(src_util_), &operation_context_, src_path_, recursive), base::Bind(&FileSystemOperation::DidFinishFileOperation, @@ -352,7 +348,7 @@ void FileSystemOperation::Write( DCHECK(SetPendingOperationType(kOperationWrite)); base::PlatformFileError result = SetUpFileSystemPath( - path_url, &src_path_, PATH_FOR_WRITE); + path_url, &src_path_, &src_util_, PATH_FOR_WRITE); if (result != base::PLATFORM_FILE_OK) { callback.Run(result, 0, false); delete this; @@ -388,12 +384,12 @@ void FileSystemOperation::DelayedWriteForQuota(quota::QuotaStatusCode status, base::FileUtilProxy::RelayCreateOrOpen( proxy_, base::Bind(&FileSystemFileUtil::CreateOrOpen, - base::Unretained(src_path_.file_util()), + base::Unretained(src_util_), &operation_context_, src_path_, file_flags), base::Bind(&FileSystemFileUtil::Close, - base::Unretained(src_path_.file_util()), + base::Unretained(src_util_), &operation_context_), base::Bind(&FileSystemOperation::OnFileOpenedForWrite, base::Unretained(this))); @@ -404,7 +400,7 @@ void FileSystemOperation::Truncate(const GURL& path_url, int64 length, DCHECK(SetPendingOperationType(kOperationTruncate)); base::PlatformFileError result = SetUpFileSystemPath( - path_url, &src_path_, PATH_FOR_WRITE); + path_url, &src_path_, &src_util_, PATH_FOR_WRITE); if (result != base::PLATFORM_FILE_OK) { callback.Run(result); delete this; @@ -429,7 +425,7 @@ void FileSystemOperation::DelayedTruncateForQuota( base::FileUtilProxy::RelayFileTask( proxy_, FROM_HERE, base::Bind(&FileSystemFileUtil::Truncate, - base::Unretained(src_path_.file_util()), + base::Unretained(src_util_), &operation_context_, src_path_, length), base::Bind(&FileSystemOperation::DidFinishFileOperation, base::Owned(this), callback)); @@ -442,7 +438,7 @@ void FileSystemOperation::TouchFile(const GURL& path_url, DCHECK(SetPendingOperationType(kOperationTouchFile)); base::PlatformFileError result = SetUpFileSystemPath( - path_url, &src_path_, PATH_FOR_WRITE); + path_url, &src_path_, &src_util_, PATH_FOR_WRITE); if (result != base::PLATFORM_FILE_OK) { callback.Run(result); delete this; @@ -452,7 +448,7 @@ void FileSystemOperation::TouchFile(const GURL& path_url, base::FileUtilProxy::RelayFileTask( proxy_, FROM_HERE, base::Bind(&FileSystemFileUtil::Touch, - base::Unretained(src_path_.file_util()), + base::Unretained(src_util_), &operation_context_, src_path_, last_access_time, last_modified_time), @@ -483,14 +479,14 @@ void FileSystemOperation::OpenFile(const GURL& path_url, base::PLATFORM_FILE_DELETE_ON_CLOSE | base::PLATFORM_FILE_WRITE_ATTRIBUTES)) { base::PlatformFileError result = SetUpFileSystemPath( - path_url, &src_path_, PATH_FOR_CREATE); + path_url, &src_path_, &src_util_, PATH_FOR_CREATE); if (result != base::PLATFORM_FILE_OK) { callback.Run(result, base::PlatformFile(), base::ProcessHandle()); return; } } else { base::PlatformFileError result = SetUpFileSystemPath( - path_url, &src_path_, PATH_FOR_READ); + path_url, &src_path_, &src_util_, PATH_FOR_READ); if (result != base::PLATFORM_FILE_OK) { callback.Run(result, base::PlatformFile(), base::ProcessHandle()); return; @@ -515,11 +511,11 @@ void FileSystemOperation::DelayedOpenFileForQuota( base::FileUtilProxy::RelayCreateOrOpen( proxy_, base::Bind(&FileSystemFileUtil::CreateOrOpen, - base::Unretained(src_path_.file_util()), + base::Unretained(src_util_), &operation_context_, src_path_, file_flags), base::Bind(&FileSystemFileUtil::Close, - base::Unretained(src_path_.file_util()), + base::Unretained(src_util_), &operation_context_), base::Bind(&FileSystemOperation::DidOpenFile, base::Owned(this), callback)); @@ -567,13 +563,13 @@ void FileSystemOperation::SyncGetPlatformPath(const GURL& path_url, DCHECK(SetPendingOperationType(kOperationGetLocalPath)); base::PlatformFileError result = SetUpFileSystemPath( - path_url, &src_path_, PATH_FOR_READ); + path_url, &src_path_, &src_util_, PATH_FOR_READ); if (result != base::PLATFORM_FILE_OK) { delete this; return; } - src_path_.file_util()->GetLocalFilePath( + src_util_->GetLocalFilePath( &operation_context_, src_path_, platform_path); delete this; @@ -584,6 +580,8 @@ FileSystemOperation::FileSystemOperation( FileSystemContext* file_system_context) : proxy_(proxy), operation_context_(file_system_context), + src_util_(NULL), + dest_util_(NULL), peer_handle_(base::kNullProcessHandle), pending_operation_(kOperationNone) { } @@ -720,6 +718,7 @@ void FileSystemOperation::OnFileOpenedForWrite( base::PlatformFileError FileSystemOperation::SetUpFileSystemPath( const GURL& path_url, FileSystemPath* file_system_path, + FileSystemFileUtil** file_util, SetUpPathMode mode) { DCHECK(file_system_path); GURL origin_url; @@ -732,15 +731,15 @@ base::PlatformFileError FileSystemOperation::SetUpFileSystemPath( origin_url, type, cracked_path)) return base::PLATFORM_FILE_ERROR_SECURITY; - FileSystemFileUtil* file_util = file_system_context()->GetFileUtil(type); - if (!file_util) + DCHECK(file_util); + if (!*file_util) + *file_util = file_system_context()->GetFileUtil(type); + if (!*file_util) return base::PLATFORM_FILE_ERROR_SECURITY; file_system_path->set_origin(origin_url); file_system_path->set_type(type); file_system_path->set_internal_path(cracked_path); - if (!file_system_path->file_util()) - file_system_path->set_file_util(file_util); if (mode == PATH_FOR_READ) { // We notify this read access whether the read access succeeds or not. @@ -751,8 +750,7 @@ base::PlatformFileError FileSystemOperation::SetUpFileSystemPath( if (quota_util) { quota_util->NotifyOriginWasAccessedOnIOThread( file_system_context()->quota_manager_proxy(), - file_system_path->origin(), - file_system_path->type()); + file_system_path->origin(), file_system_path->type()); } return base::PLATFORM_FILE_OK; } diff --git a/webkit/fileapi/file_system_operation.h b/webkit/fileapi/file_system_operation.h index b70d3dfa..9ca9dab 100644 --- a/webkit/fileapi/file_system_operation.h +++ b/webkit/fileapi/file_system_operation.h @@ -123,8 +123,8 @@ class FileSystemOperation : public FileSystemOperationInterface { // file_util on their own should call this before performing the actual // operation. If it is given it will not be overwritten by the class. void set_override_file_util(FileSystemFileUtil* file_util) { - src_path_.set_file_util(file_util); - dest_path_.set_file_util(file_util); + src_util_ = file_util; + dest_util_ = file_util; } void GetUsageAndQuotaThenCallback( @@ -209,11 +209,12 @@ class FileSystemOperation : public FileSystemOperationInterface { PATH_FOR_CREATE, }; - // Checks the validity of a given |path_url| and and sets up the - // |file_system_path| for |mode|. + // Checks the validity of a given |path_url| and and populates + // |path| and |file_util| for |mode|. base::PlatformFileError SetUpFileSystemPath( const GURL& path_url, FileSystemPath* file_system_path, + FileSystemFileUtil** file_util, SetUpPathMode mode); // Used only for internal assertions. @@ -226,6 +227,8 @@ class FileSystemOperation : public FileSystemOperationInterface { FileSystemOperationContext operation_context_; FileSystemPath src_path_; FileSystemPath dest_path_; + FileSystemFileUtil* src_util_; // Not owned. + FileSystemFileUtil* dest_util_; // Not owned. scoped_ptr<ScopedQuotaUtilHelper> quota_util_helper_; diff --git a/webkit/fileapi/file_system_path.cc b/webkit/fileapi/file_system_path.cc index ed2d89d..8dff348 100644 --- a/webkit/fileapi/file_system_path.cc +++ b/webkit/fileapi/file_system_path.cc @@ -9,18 +9,15 @@ namespace fileapi { FileSystemPath::FileSystemPath() - : type_(kFileSystemTypeUnknown), - file_util_(NULL) {} + : type_(kFileSystemTypeUnknown) {} FileSystemPath::FileSystemPath( const GURL& origin, FileSystemType type, - const FilePath& internal_path, - FileSystemFileUtil* file_util) + const FilePath& internal_path) : origin_(origin), type_(type), - internal_path_(internal_path), - file_util_(file_util) {} + internal_path_(internal_path) {} FileSystemPath::~FileSystemPath() {} diff --git a/webkit/fileapi/file_system_path.h b/webkit/fileapi/file_system_path.h index 7a9e9fe..9568874 100644 --- a/webkit/fileapi/file_system_path.h +++ b/webkit/fileapi/file_system_path.h @@ -26,8 +26,7 @@ class FileSystemPath { FileSystemPath(); FileSystemPath(const GURL& origin, FileSystemType type, - const FilePath& internal_path, - FileSystemFileUtil* file_util); + const FilePath& internal_path); ~FileSystemPath(); // Gets and sets the origin URL of this filesystem. @@ -47,11 +46,6 @@ class FileSystemPath { internal_path_ = internal_path; } - // Gets and sets the FileSystemFileUtil with which the file pointed by this - // FileSystemPath is handled. - FileSystemFileUtil* file_util() const { return file_util_; } - void set_file_util(FileSystemFileUtil* util) { file_util_ = util; } - // Returns a new FileSystemPath with the given internal_path. // This doesn't change the calling instance's data. FileSystemPath WithInternalPath(const FilePath& internal_path) const; @@ -82,11 +76,6 @@ class FileSystemPath { GURL origin_; FileSystemType type_; FilePath internal_path_; - - // Not owned. - // TODO(kinuko): This needs to be cleaned up; this value doesn't always - // reflect the current FileUtil in underlying filesystems. - FileSystemFileUtil* file_util_; }; } // namespace fileapi diff --git a/webkit/fileapi/file_system_quota_client_unittest.cc b/webkit/fileapi/file_system_quota_client_unittest.cc index 837c479..8d487cb 100644 --- a/webkit/fileapi/file_system_quota_client_unittest.cc +++ b/webkit/fileapi/file_system_quota_client_unittest.cc @@ -132,7 +132,7 @@ class FileSystemQuotaClientTest : public testing::Test { FileSystemType type = QuotaStorageTypeToFileSystemType(storage_type); FileSystemFileUtil* file_util = file_system_context_->GetFileUtil(type); - FileSystemPath path(GURL(origin_url), type, file_path, file_util); + FileSystemPath path(GURL(origin_url), type, file_path); scoped_ptr<FileSystemOperationContext> context( CreateFileSystemOperationContext()); @@ -154,7 +154,7 @@ class FileSystemQuotaClientTest : public testing::Test { sandbox_provider()->GetFileUtil(); FileSystemType type = QuotaStorageTypeToFileSystemType(storage_type); - FileSystemPath path(GURL(origin_url), type, file_path, file_util); + FileSystemPath path(GURL(origin_url), type, file_path); scoped_ptr<FileSystemOperationContext> context( CreateFileSystemOperationContext()); diff --git a/webkit/fileapi/file_system_test_helper.cc b/webkit/fileapi/file_system_test_helper.cc index 96f50ea..9f3705d 100644 --- a/webkit/fileapi/file_system_test_helper.cc +++ b/webkit/fileapi/file_system_test_helper.cc @@ -127,7 +127,31 @@ FilePath FileSystemTestOriginHelper::GetUsageCachePath() const { FileSystemPath FileSystemTestOriginHelper::CreatePath( const FilePath& path) const { - return FileSystemPath(origin_, type_, path, file_util_); + return FileSystemPath(origin_, type_, path); +} + +base::PlatformFileError FileSystemTestOriginHelper::SameFileUtilCopy( + FileSystemOperationContext* context, + const FileSystemPath& src, + const FileSystemPath& dest) const { + CrossFileUtilHelper cross_util_helper( + context, + file_util(), file_util(), + src, dest, + CrossFileUtilHelper::OPERATION_COPY); + return cross_util_helper.DoWork(); +} + +base::PlatformFileError FileSystemTestOriginHelper::SameFileUtilMove( + FileSystemOperationContext* context, + const FileSystemPath& src, + const FileSystemPath& dest) const { + CrossFileUtilHelper cross_util_helper( + context, + file_util(), file_util(), + src, dest, + CrossFileUtilHelper::OPERATION_MOVE); + return cross_util_helper.DoWork(); } int64 FileSystemTestOriginHelper::GetCachedOriginUsage() const { diff --git a/webkit/fileapi/file_system_test_helper.h b/webkit/fileapi/file_system_test_helper.h index af654ce..132226b 100644 --- a/webkit/fileapi/file_system_test_helper.h +++ b/webkit/fileapi/file_system_test_helper.h @@ -12,6 +12,7 @@ #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" #include "googleurl/src/gurl.h" +#include "webkit/fileapi/cross_file_util_helper.h" #include "webkit/fileapi/file_system_path.h" #include "webkit/fileapi/file_system_types.h" #include "webkit/fileapi/file_system_util.h" @@ -63,6 +64,16 @@ class FileSystemTestOriginHelper { return CreatePath(FilePath::FromUTF8Unsafe(utf8)); } + // Helper methods for same-FileUtil copy/move. + base::PlatformFileError SameFileUtilCopy( + FileSystemOperationContext* context, + const FileSystemPath& src, + const FileSystemPath& dest) const; + base::PlatformFileError SameFileUtilMove( + FileSystemOperationContext* context, + const FileSystemPath& src, + const FileSystemPath& dest) const; + int64 GetCachedOriginUsage() const; bool RevokeUsageCache() const; @@ -81,7 +92,7 @@ class FileSystemTestOriginHelper { quota::StorageType storage_type() const { return FileSystemTypeToQuotaStorageType(type_); } - FileSystemFileUtil* file_util() { return file_util_; } + FileSystemFileUtil* file_util() const { return file_util_; } private: scoped_refptr<FileSystemContext> file_system_context_; diff --git a/webkit/fileapi/file_system_url_request_job_unittest.cc b/webkit/fileapi/file_system_url_request_job_unittest.cc index 5664289..16986e2 100644 --- a/webkit/fileapi/file_system_url_request_job_unittest.cc +++ b/webkit/fileapi/file_system_url_request_job_unittest.cc @@ -143,8 +143,7 @@ class FileSystemURLRequestJobTest : public testing::Test { sandbox_provider()->GetFileUtil(); FileSystemPath path(GURL("http://remote"), kFileSystemTypeTemporary, - FilePath().AppendASCII(dir_name), - file_util); + FilePath().AppendASCII(dir_name)); FileSystemOperationContext context(file_system_context_); context.set_allowed_bytes_growth(1024); @@ -162,8 +161,7 @@ class FileSystemURLRequestJobTest : public testing::Test { sandbox_provider()->GetFileUtil(); FileSystemPath path(GURL("http://remote"), kFileSystemTypeTemporary, - FilePath().AppendASCII(file_name), - file_util); + FilePath().AppendASCII(file_name)); FileSystemOperationContext context(file_system_context_); context.set_allowed_bytes_growth(1024); diff --git a/webkit/fileapi/local_file_util_unittest.cc b/webkit/fileapi/local_file_util_unittest.cc index 4851dc9..2f3493c5 100644 --- a/webkit/fileapi/local_file_util_unittest.cc +++ b/webkit/fileapi/local_file_util_unittest.cc @@ -90,6 +90,10 @@ class LocalFileUtilTest : public testing::Test { Path(file_name), created); } + const FileSystemTestOriginHelper& test_helper() const { + return test_helper_; + } + private: scoped_ptr<LocalFileUtil> local_file_util_; ScopedTempDir data_dir_; @@ -150,8 +154,8 @@ TEST_F(LocalFileUtilTest, CopyFile) { bool created; ASSERT_EQ(base::PLATFORM_FILE_OK, EnsureFileExists(from_file, &created)); ASSERT_TRUE(created); - scoped_ptr<FileSystemOperationContext> context; + scoped_ptr<FileSystemOperationContext> context; context.reset(NewContext()); ASSERT_EQ(base::PLATFORM_FILE_OK, FileUtil()->Truncate(context.get(), Path(from_file), 1020)); @@ -161,11 +165,13 @@ TEST_F(LocalFileUtilTest, CopyFile) { context.reset(NewContext()); ASSERT_EQ(base::PLATFORM_FILE_OK, - FileUtil()->Copy(context.get(), Path(from_file), Path(to_file1))); + test_helper().SameFileUtilCopy(context.get(), + Path(from_file), Path(to_file1))); context.reset(NewContext()); ASSERT_EQ(base::PLATFORM_FILE_OK, - FileUtil()->Copy(context.get(), Path(from_file), Path(to_file2))); + test_helper().SameFileUtilCopy(context.get(), + Path(from_file), Path(to_file2))); EXPECT_TRUE(FileExists(from_file)); EXPECT_EQ(1020, GetSize(from_file)); @@ -200,7 +206,8 @@ TEST_F(LocalFileUtilTest, CopyDirectory) { context.reset(NewContext()); ASSERT_EQ(base::PLATFORM_FILE_OK, - FileUtil()->Copy(context.get(), Path(from_dir), Path(to_dir))); + test_helper().SameFileUtilCopy(context.get(), + Path(from_dir), Path(to_dir))); EXPECT_TRUE(DirectoryExists(from_dir)); EXPECT_TRUE(FileExists(from_file)); @@ -227,7 +234,8 @@ TEST_F(LocalFileUtilTest, MoveFile) { context.reset(NewContext()); ASSERT_EQ(base::PLATFORM_FILE_OK, - FileUtil()->Move(context.get(), Path(from_file), Path(to_file))); + test_helper().SameFileUtilMove(context.get(), + Path(from_file), Path(to_file))); EXPECT_FALSE(FileExists(from_file)); EXPECT_TRUE(FileExists(to_file)); @@ -259,7 +267,8 @@ TEST_F(LocalFileUtilTest, MoveDirectory) { context.reset(NewContext()); ASSERT_EQ(base::PLATFORM_FILE_OK, - FileUtil()->Move(context.get(), Path(from_dir), Path(to_dir))); + test_helper().SameFileUtilMove(context.get(), + Path(from_dir), Path(to_dir))); EXPECT_FALSE(DirectoryExists(from_dir)); EXPECT_TRUE(DirectoryExists(to_dir)); diff --git a/webkit/fileapi/obfuscated_file_util.cc b/webkit/fileapi/obfuscated_file_util.cc index 06090d8..221db1b 100644 --- a/webkit/fileapi/obfuscated_file_util.cc +++ b/webkit/fileapi/obfuscated_file_util.cc @@ -1096,8 +1096,7 @@ PlatformFileError ObfuscatedFileUtil::CreateFile( dest_file_path = dest_file_path.AppendASCII( StringPrintf("%02" PRIu64, directory_number)); - FileSystemPath dest_path(dest_origin, dest_type, dest_file_path, - underlying_file_util()); + FileSystemPath dest_path(dest_origin, dest_type, dest_file_path); PlatformFileError error; error = underlying_file_util()->CreateDirectory( @@ -1197,9 +1196,8 @@ FileSystemPath ObfuscatedFileUtil::DataPathToLocalPath( const GURL& origin, FileSystemType type, const FilePath& data_path) { FilePath root = GetDirectoryForOriginAndType(origin, type, false); if (root.empty()) - return FileSystemPath(origin, type, root, underlying_file_util()); - return FileSystemPath(origin, type, root.Append(data_path), - underlying_file_util()); + return FileSystemPath(origin, type, root); + return FileSystemPath(origin, type, root.Append(data_path)); } FilePath ObfuscatedFileUtil::LocalPathToDataPath( diff --git a/webkit/fileapi/obfuscated_file_util_unittest.cc b/webkit/fileapi/obfuscated_file_util_unittest.cc index 8eb2344..1a36478 100644 --- a/webkit/fileapi/obfuscated_file_util_unittest.cc +++ b/webkit/fileapi/obfuscated_file_util_unittest.cc @@ -517,6 +517,8 @@ class ObfuscatedFileUtilTest : public testing::Test { EXPECT_NE(base::Time(), GetModifiedTime(dest_dir_path)); } + const FileSystemTestOriginHelper& test_helper() const { return test_helper_; } + private: ScopedTempDir data_dir_; scoped_refptr<ObfuscatedFileUtil> obfuscated_file_util_; @@ -1171,7 +1173,7 @@ TEST_F(ObfuscatedFileUtilTest, TestEnumerator) { EXPECT_FALSE(ofu()->DirectoryExists(context.get(), dest_path)); context.reset(NewContext(NULL)); ASSERT_EQ(base::PLATFORM_FILE_OK, - ofu()->Copy(context.get(), src_path, dest_path)); + test_helper().SameFileUtilCopy(context.get(), src_path, dest_path)); ValidateTestDirectory(dest_path, files, directories); context.reset(NewContext(NULL)); diff --git a/webkit/fileapi/quota_file_util.cc b/webkit/fileapi/quota_file_util.cc index ed973b3e..c73c67f 100644 --- a/webkit/fileapi/quota_file_util.cc +++ b/webkit/fileapi/quota_file_util.cc @@ -118,6 +118,8 @@ base::PlatformFileError QuotaFileUtil::CopyOrMoveFile( // It assumes copy/move operations are always in the same fs currently. // TODO(dmikurube): Do quota check if moving between different fs. + // TODO(kinuko,kinaba): FileSystemFileUtil shouldn't support cross-filesystem + // operations from the beginning. if (copy) { int64 allowed_bytes_growth = fs_context->allowed_bytes_growth(); // The third argument (growth) is not used for now. diff --git a/webkit/fileapi/quota_file_util_unittest.cc b/webkit/fileapi/quota_file_util_unittest.cc index da29ba1..675b25b 100644 --- a/webkit/fileapi/quota_file_util_unittest.cc +++ b/webkit/fileapi/quota_file_util_unittest.cc @@ -184,9 +184,10 @@ TEST_F(QuotaFileUtilTest, CopyFile) { context.reset(NewContext()); context->set_allowed_bytes_growth(1020); ASSERT_EQ(base::PLATFORM_FILE_OK, - quota_file_util()->Copy(context.get(), - Path(from_file), - Path(to_file1))); + quota_test_helper().SameFileUtilCopy( + context.get(), + Path(from_file), + Path(to_file1))); ASSERT_EQ(2041, quota_test_helper().GetCachedOriginUsage()); ASSERT_EQ(quota_test_helper().ComputeCurrentOriginUsage(), quota_test_helper().GetCachedOriginUsage()); @@ -194,9 +195,10 @@ TEST_F(QuotaFileUtilTest, CopyFile) { context.reset(NewContext()); context->set_allowed_bytes_growth(1019); ASSERT_EQ(base::PLATFORM_FILE_ERROR_NO_SPACE, - quota_file_util()->Copy(context.get(), - Path(from_file), - Path(to_file2))); + quota_test_helper().SameFileUtilCopy( + context.get(), + Path(from_file), + Path(to_file2))); ASSERT_EQ(2041, quota_test_helper().GetCachedOriginUsage()); ASSERT_EQ(quota_test_helper().ComputeCurrentOriginUsage(), quota_test_helper().GetCachedOriginUsage()); @@ -204,9 +206,10 @@ TEST_F(QuotaFileUtilTest, CopyFile) { context.reset(NewContext()); context->set_allowed_bytes_growth(1019); ASSERT_EQ(base::PLATFORM_FILE_OK, - quota_file_util()->Copy(context.get(), - Path(from_file), - Path(obstacle_file))); + quota_test_helper().SameFileUtilCopy( + context.get(), + Path(from_file), + Path(obstacle_file))); ASSERT_EQ(3060, quota_test_helper().GetCachedOriginUsage()); ASSERT_EQ(quota_test_helper().ComputeCurrentOriginUsage(), quota_test_helper().GetCachedOriginUsage()); @@ -224,9 +227,10 @@ TEST_F(QuotaFileUtilTest, CopyFile) { context.reset(NewContext()); context->set_allowed_bytes_growth(-2); // quota exceeded ASSERT_EQ(base::PLATFORM_FILE_OK, - quota_file_util()->Copy(context.get(), - Path(from_file), - Path(obstacle_file))); + quota_test_helper().SameFileUtilCopy( + context.get(), + Path(from_file), + Path(obstacle_file))); ASSERT_EQ(3058, quota_test_helper().GetCachedOriginUsage()); ASSERT_EQ(quota_test_helper().ComputeCurrentOriginUsage(), quota_test_helper().GetCachedOriginUsage()); @@ -274,9 +278,10 @@ TEST_F(QuotaFileUtilTest, CopyDirectory) { context.reset(NewContext()); context->set_allowed_bytes_growth(1020); ASSERT_EQ(base::PLATFORM_FILE_OK, - quota_file_util()->Copy(context.get(), - Path(from_dir), - Path(to_dir1))); + quota_test_helper().SameFileUtilCopy( + context.get(), + Path(from_dir), + Path(to_dir1))); ASSERT_EQ(2040, quota_test_helper().GetCachedOriginUsage()); ASSERT_EQ(quota_test_helper().ComputeCurrentOriginUsage(), quota_test_helper().GetCachedOriginUsage()); @@ -284,9 +289,10 @@ TEST_F(QuotaFileUtilTest, CopyDirectory) { context.reset(NewContext()); context->set_allowed_bytes_growth(1019); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NO_SPACE, - quota_file_util()->Copy(context.get(), - Path(from_dir), - Path(to_dir2))); + quota_test_helper().SameFileUtilCopy( + context.get(), + Path(from_dir), + Path(to_dir2))); ASSERT_TRUE(2540 == quota_test_helper().GetCachedOriginUsage() || 2560 == quota_test_helper().GetCachedOriginUsage()); ASSERT_EQ(quota_test_helper().ComputeCurrentOriginUsage(), @@ -315,9 +321,10 @@ TEST_F(QuotaFileUtilTest, MoveFile) { context.reset(NewContext()); context->set_allowed_bytes_growth(0); ASSERT_EQ(base::PLATFORM_FILE_OK, - quota_file_util()->Move(context.get(), - Path(from_file), - Path(to_file))); + quota_test_helper().SameFileUtilMove( + context.get(), + Path(from_file), + Path(to_file))); ASSERT_EQ(1020, quota_test_helper().GetCachedOriginUsage()); ASSERT_EQ(quota_test_helper().ComputeCurrentOriginUsage(), quota_test_helper().GetCachedOriginUsage()); @@ -350,9 +357,10 @@ TEST_F(QuotaFileUtilTest, MoveFile) { context.reset(NewContext()); context->set_allowed_bytes_growth(0); ASSERT_EQ(base::PLATFORM_FILE_OK, - quota_file_util()->Move(context.get(), - Path(from_file), - Path(obstacle_file))); + quota_test_helper().SameFileUtilMove( + context.get(), + Path(from_file), + Path(obstacle_file))); ASSERT_EQ(2040, quota_test_helper().GetCachedOriginUsage()); ASSERT_EQ(quota_test_helper().ComputeCurrentOriginUsage(), quota_test_helper().GetCachedOriginUsage()); @@ -373,9 +381,10 @@ TEST_F(QuotaFileUtilTest, MoveFile) { context.reset(NewContext()); context->set_allowed_bytes_growth(-1020 - 1); // quota exceeded, after ASSERT_EQ(base::PLATFORM_FILE_OK, - quota_file_util()->Move(context.get(), - Path(from_file), - Path(obstacle_file))); + quota_test_helper().SameFileUtilMove( + context.get(), + Path(from_file), + Path(obstacle_file))); ASSERT_EQ(1030, quota_test_helper().GetCachedOriginUsage()); ASSERT_EQ(quota_test_helper().ComputeCurrentOriginUsage(), quota_test_helper().GetCachedOriginUsage()); @@ -410,9 +419,10 @@ TEST_F(QuotaFileUtilTest, MoveDirectory) { context.reset(NewContext()); context->set_allowed_bytes_growth(1020); ASSERT_EQ(base::PLATFORM_FILE_OK, - quota_file_util()->Move(context.get(), - Path(from_dir), - Path(to_dir1))); + quota_test_helper().SameFileUtilMove( + context.get(), + Path(from_dir), + Path(to_dir1))); ASSERT_EQ(1020, quota_test_helper().GetCachedOriginUsage()); ASSERT_EQ(quota_test_helper().ComputeCurrentOriginUsage(), quota_test_helper().GetCachedOriginUsage()); @@ -438,9 +448,10 @@ TEST_F(QuotaFileUtilTest, MoveDirectory) { context.reset(NewContext()); context->set_allowed_bytes_growth(1019); EXPECT_EQ(base::PLATFORM_FILE_OK, - quota_file_util()->Move(context.get(), - Path(from_dir), - Path(to_dir2))); + quota_test_helper().SameFileUtilMove( + context.get(), + Path(from_dir), + Path(to_dir2))); ASSERT_EQ(2040, quota_test_helper().GetCachedOriginUsage()); ASSERT_EQ(quota_test_helper().ComputeCurrentOriginUsage(), quota_test_helper().GetCachedOriginUsage()); diff --git a/webkit/fileapi/sandbox_mount_point_provider.cc b/webkit/fileapi/sandbox_mount_point_provider.cc index a659615..3bdd417 100644 --- a/webkit/fileapi/sandbox_mount_point_provider.cc +++ b/webkit/fileapi/sandbox_mount_point_provider.cc @@ -521,7 +521,7 @@ int64 SandboxMountPointProvider::GetOriginUsageOnFileThread( FileSystemUsageCache::Delete(usage_file_path); FileSystemOperationContext context(NULL); - FileSystemPath path(origin_url, type, FilePath(), sandbox_file_util_); + FileSystemPath path(origin_url, type, FilePath()); scoped_ptr<FileSystemFileUtil::AbstractFileEnumerator> enumerator( sandbox_file_util_->CreateFileEnumerator(&context, path)); diff --git a/webkit/fileapi/sandbox_mount_point_provider_unittest.cc b/webkit/fileapi/sandbox_mount_point_provider_unittest.cc index 7dd6355..5433722 100644 --- a/webkit/fileapi/sandbox_mount_point_provider_unittest.cc +++ b/webkit/fileapi/sandbox_mount_point_provider_unittest.cc @@ -213,7 +213,7 @@ class SandboxMountPointProviderMigrationTest : public testing::Test { FilePath seed_file_path = FilePath().AppendASCII( URLAndTypeToSeedString(origin_url, type)); - FileSystemPath root(origin_url, type, FilePath(), file_util()); + FileSystemPath root(origin_url, type, FilePath()); FileSystemPath seed = root.Append(seed_file_path); context.reset(NewContext()); diff --git a/webkit/fileapi/webkit_fileapi.gypi b/webkit/fileapi/webkit_fileapi.gypi index 02781d4..8b4688b 100644 --- a/webkit/fileapi/webkit_fileapi.gypi +++ b/webkit/fileapi/webkit_fileapi.gypi @@ -15,6 +15,8 @@ '<(DEPTH)/webkit/support/webkit_support.gyp:quota', ], 'sources': [ + 'cross_file_util_helper.cc', + 'cross_file_util_helper.h', 'file_system_callback_dispatcher.cc', 'file_system_callback_dispatcher.h', 'file_system_context.cc', |