diff options
author | kinuko@chromium.org <kinuko@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-10-05 06:23:59 +0000 |
---|---|---|
committer | kinuko@chromium.org <kinuko@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-10-05 06:23:59 +0000 |
commit | 406059253f11ef6b1521a633d1d5640805587a20 (patch) | |
tree | 13917db4d051ee7cdd3fb6c48adec60b24997c09 | |
parent | e42c4d260d85a91ef08bfd84ab0dc0423ff3c99f (diff) | |
download | chromium_src-406059253f11ef6b1521a633d1d5640805587a20.zip chromium_src-406059253f11ef6b1521a633d1d5640805587a20.tar.gz chromium_src-406059253f11ef6b1521a633d1d5640805587a20.tar.bz2 |
Revert 61480 - Support removeRecursively and new copy/move behaviors added to the spec recently.
http://lists.w3.org/Archives/Public/public-webapps/2010JulSep/1101.html
> For a move/copy of a file on top of existing file, or a directory on
> top of an existing empty directory, you get an automatic overwrite.
> A move/copy of a file on top of an existing directory, or of a
> directory on top of an existing file, will always fail.
> A move/copy of a file or directory on top of an existing non-empty
> directory will always fail.
BUG=32277
TEST=FileSystemOperationTest.*
Review URL: http://codereview.chromium.org/3567012
TBR=kinuko@chromium.org
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@61482 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | base/file_util_proxy.cc | 125 | ||||
-rw-r--r-- | base/file_util_proxy.h | 5 | ||||
-rw-r--r-- | base/platform_file.h | 4 | ||||
-rw-r--r-- | chrome/browser/file_system/file_system_dispatcher_host.cc | 5 | ||||
-rw-r--r-- | chrome/browser/file_system/file_system_dispatcher_host.h | 3 | ||||
-rw-r--r-- | chrome/common/file_system/file_system_dispatcher.cc | 4 | ||||
-rw-r--r-- | chrome/common/file_system/file_system_dispatcher.h | 1 | ||||
-rw-r--r-- | chrome/common/file_system/webfilesystem_impl.cc | 11 | ||||
-rw-r--r-- | chrome/common/file_system/webfilesystem_impl.h | 4 | ||||
-rw-r--r-- | chrome/common/render_messages_internal.h | 5 | ||||
-rw-r--r-- | chrome/renderer/pepper_plugin_delegate_impl.cc | 2 | ||||
-rw-r--r-- | webkit/blob/deletable_file_reference.cc | 2 | ||||
-rw-r--r-- | webkit/fileapi/file_system_operation.cc | 7 | ||||
-rw-r--r-- | webkit/fileapi/file_system_operation.h | 5 | ||||
-rw-r--r-- | webkit/fileapi/file_system_operation_unittest.cc | 238 | ||||
-rw-r--r-- | webkit/glue/webkit_glue.cc | 2 | ||||
-rw-r--r-- | webkit/tools/test_shell/simple_file_system.cc | 4 | ||||
-rw-r--r-- | webkit/tools/test_shell/simple_file_system.h | 7 |
18 files changed, 117 insertions, 317 deletions
diff --git a/base/file_util_proxy.cc b/base/file_util_proxy.cc index b3b18f8..d71f374 100644 --- a/base/file_util_proxy.cc +++ b/base/file_util_proxy.cc @@ -10,63 +10,6 @@ // that all of the base:: prefixes would be unnecessary. namespace { -namespace { - -// Performs common checks for move and copy. -// 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). -static base::PlatformFileError PerformCommonCheckAndPreparationForMoveAndCopy( - const FilePath& src_file_path, - const FilePath& dest_file_path) { - // Exits earlier if the source path does not exist. - if (!file_util::PathExists(src_file_path)) - return base::PLATFORM_FILE_ERROR_NOT_FOUND; - - // The parent of the |dest_file_path| does not exist. - if (!file_util::DirectoryExists(dest_file_path.DirName())) - return base::PLATFORM_FILE_ERROR_NOT_FOUND; - - // It is an error to try to copy/move an entry into its child. - if (src_file_path.IsParent(dest_file_path)) - return base::PLATFORM_FILE_ERROR_INVALID_OPERATION; - - // Now it is ok to return if the |dest_file_path| does not exist. - if (!file_util::PathExists(dest_file_path)) - return base::PLATFORM_FILE_OK; - - // |src_file_path| exists and is a directory. - // |dest_file_path| exists and is a file. - bool src_is_directory = file_util::DirectoryExists(src_file_path); - bool dest_is_directory = file_util::DirectoryExists(dest_file_path); - if (src_is_directory && !dest_is_directory) - return base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY; - - // |src_file_path| exists and is a file. - // |dest_file_path| exists and is a directory. - if (!src_is_directory && dest_is_directory) - return base::PLATFORM_FILE_ERROR_NOT_A_FILE; - - // It is an error to copy/move an entry into the same path. - if (src_file_path.value() == dest_file_path.value()) - 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. - // TODO(kinuko): may be better to change the file_util::{Copy,Move}. - if (!file_util::Delete(dest_file_path, false /* recursive */)) { - if (!file_util::IsDirectoryEmpty(dest_file_path)) - return base::PLATFORM_FILE_ERROR_NOT_EMPTY; - return base::PLATFORM_FILE_ERROR_FAILED; - } - } - return base::PLATFORM_FILE_OK; -} - -} // anonymous namespace - class MessageLoopRelay : public base::RefCountedThreadSafe<MessageLoopRelay> { public: @@ -262,7 +205,7 @@ class RelayDelete : public RelayWithStatusCallback { } if (!file_util::Delete(file_path_, recursive_)) { if (!recursive_ && !file_util::IsDirectoryEmpty(file_path_)) { - set_error_code(base::PLATFORM_FILE_ERROR_NOT_EMPTY); + set_error_code(base::PLATFORM_FILE_ERROR_INVALID_OPERATION); return; } set_error_code(base::PLATFORM_FILE_ERROR_FAILED); @@ -286,13 +229,36 @@ class RelayCopy : public RelayWithStatusCallback { protected: virtual void RunWork() { - set_error_code(PerformCommonCheckAndPreparationForMoveAndCopy( - src_file_path_, dest_file_path_)); - if (error_code() != base::PLATFORM_FILE_OK) + bool dest_path_exists = file_util::PathExists(dest_file_path_); + if (!dest_path_exists && + !file_util::DirectoryExists(dest_file_path_.DirName())) { + set_error_code(base::PLATFORM_FILE_ERROR_NOT_FOUND); return; + } + // |src_file_path| exists and is a directory. + // |dest_file_path| exists and is a file. + if (file_util::DirectoryExists(src_file_path_) && + dest_path_exists && !file_util::DirectoryExists(dest_file_path_)) { + set_error_code(base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY); + return; + } + if (file_util::ContainsPath(src_file_path_, dest_file_path_)) { + set_error_code(base::PLATFORM_FILE_ERROR_FAILED); + return; + } if (!file_util::CopyDirectory(src_file_path_, dest_file_path_, - true /* recursive */)) + true /* recursive */)) { + if (!file_util::PathExists(src_file_path_)) { + set_error_code(base::PLATFORM_FILE_ERROR_NOT_FOUND); + return; + } + if (src_file_path_.value() == dest_file_path_.value()) { + set_error_code(base::PLATFORM_FILE_ERROR_EXISTS); + return; + } + // Something else went wrong. set_error_code(base::PLATFORM_FILE_ERROR_FAILED); + } } private: @@ -312,12 +278,36 @@ class RelayMove : public RelayWithStatusCallback { protected: virtual void RunWork() { - set_error_code(PerformCommonCheckAndPreparationForMoveAndCopy( - src_file_path_, dest_file_path_)); - if (error_code() != base::PLATFORM_FILE_OK) + bool dest_path_exists = file_util::PathExists(dest_file_path_); + if (!dest_path_exists && + !file_util::DirectoryExists(dest_file_path_.DirName())) { + set_error_code(base::PLATFORM_FILE_ERROR_NOT_FOUND); return; - if (!file_util::Move(src_file_path_, dest_file_path_)) + } + // |src_file_path| exists and is a directory. + // |dest_file_path| exists and is a file. + if (file_util::DirectoryExists(src_file_path_) && + dest_path_exists && + !file_util::DirectoryExists(dest_file_path_)) { + set_error_code(base::PLATFORM_FILE_ERROR_EXISTS); + return; + } + if (file_util::ContainsPath(src_file_path_, dest_file_path_)) { + set_error_code(base::PLATFORM_FILE_ERROR_INVALID_OPERATION); + return; + } + if (!file_util::Move(src_file_path_, dest_file_path_)) { + if (!file_util::PathExists(src_file_path_)) { + set_error_code(base::PLATFORM_FILE_ERROR_NOT_FOUND); + return; + } + if (src_file_path_.value() == dest_file_path_.value()) { + set_error_code(base::PLATFORM_FILE_ERROR_EXISTS); + return; + } + // Something else went wrong. set_error_code(base::PLATFORM_FILE_ERROR_FAILED); + } } private: @@ -713,10 +703,9 @@ bool FileUtilProxy::Close(scoped_refptr<MessageLoopProxy> message_loop_proxy, // static bool FileUtilProxy::Delete(scoped_refptr<MessageLoopProxy> message_loop_proxy, const FilePath& file_path, - bool recursive, StatusCallback* callback) { return Start(FROM_HERE, message_loop_proxy, - new RelayDelete(file_path, recursive, callback)); + new RelayDelete(file_path, false, callback)); } // static diff --git a/base/file_util_proxy.h b/base/file_util_proxy.h index e716ab7..32b78630 100644 --- a/base/file_util_proxy.h +++ b/base/file_util_proxy.h @@ -89,7 +89,6 @@ class FileUtilProxy { // If destination file doesn't exist or destination's parent // doesn't exists. // 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 exists. static bool Copy(scoped_refptr<MessageLoopProxy> message_loop_proxy, @@ -106,11 +105,9 @@ class FileUtilProxy { bool recursive, StatusCallback* callback); - // Deletes a file or a directory. - // It is an error to delete a non-empty directory with recursive=false. + // Deletes a file or empty directory. static bool Delete(scoped_refptr<MessageLoopProxy> message_loop_proxy, const FilePath& file_path, - bool recursive, StatusCallback* callback); // Moves a file or a directory from src_file_path to dest_file_path. diff --git a/base/platform_file.h b/base/platform_file.h index b350f8c..5024717 100644 --- a/base/platform_file.h +++ b/base/platform_file.h @@ -60,9 +60,7 @@ enum PlatformFileError { PLATFORM_FILE_ERROR_NOT_A_DIRECTORY = -9, PLATFORM_FILE_ERROR_INVALID_OPERATION = -10, PLATFORM_FILE_ERROR_SECURITY = -11, - PLATFORM_FILE_ERROR_ABORT = -12, - PLATFORM_FILE_ERROR_NOT_A_FILE = -13, - PLATFORM_FILE_ERROR_NOT_EMPTY = -14, + PLATFORM_FILE_ERROR_ABORT = -12 }; // Used to hold information about a given file. diff --git a/chrome/browser/file_system/file_system_dispatcher_host.cc b/chrome/browser/file_system/file_system_dispatcher_host.cc index 96571ce..6dd6072 100644 --- a/chrome/browser/file_system/file_system_dispatcher_host.cc +++ b/chrome/browser/file_system/file_system_dispatcher_host.cc @@ -161,11 +161,10 @@ void FileSystemDispatcherHost::OnCopy( GetNewOperation(request_id)->Copy(src_path, dest_path); } -void FileSystemDispatcherHost::OnRemove( - int request_id, const FilePath& path, bool recursive) { +void FileSystemDispatcherHost::OnRemove(int request_id, const FilePath& path) { if (!CheckValidFileSystemPath(path, request_id)) return; - GetNewOperation(request_id)->Remove(path, recursive); + GetNewOperation(request_id)->Remove(path); } void FileSystemDispatcherHost::OnReadMetadata( diff --git a/chrome/browser/file_system/file_system_dispatcher_host.h b/chrome/browser/file_system/file_system_dispatcher_host.h index 68110be..22a560a 100644 --- a/chrome/browser/file_system/file_system_dispatcher_host.h +++ b/chrome/browser/file_system/file_system_dispatcher_host.h @@ -21,6 +21,7 @@ namespace base { class Time; } +class ChromeFileSystemOperation; class FileSystemHostContext; class GURL; class HostContentSettingsMap; @@ -49,7 +50,7 @@ class FileSystemDispatcherHost void OnCopy(int request_id, const FilePath& src_path, const FilePath& dest_path); - void OnRemove(int request_id, const FilePath& path, bool recursive); + void OnRemove(int request_id, const FilePath& path); void OnReadMetadata(int request_id, const FilePath& path); void OnCreate(int request_id, const FilePath& path, diff --git a/chrome/common/file_system/file_system_dispatcher.cc b/chrome/common/file_system/file_system_dispatcher.cc index bc52f3a..a8761a2 100644 --- a/chrome/common/file_system/file_system_dispatcher.cc +++ b/chrome/common/file_system/file_system_dispatcher.cc @@ -67,11 +67,10 @@ bool FileSystemDispatcher::Copy( bool FileSystemDispatcher::Remove( const FilePath& path, - bool recursive, fileapi::FileSystemCallbackDispatcher* dispatcher) { int request_id = dispatchers_.Add(dispatcher); return ChildThread::current()->Send( - new ViewHostMsg_FileSystem_Remove(request_id, path, recursive)); + new ViewHostMsg_FileSystem_Remove(request_id, path)); } bool FileSystemDispatcher::ReadMetadata( @@ -220,3 +219,4 @@ void FileSystemDispatcher::DidWrite( if (complete) dispatchers_.Remove(request_id); } + diff --git a/chrome/common/file_system/file_system_dispatcher.h b/chrome/common/file_system/file_system_dispatcher.h index 8802c1e..80443c4 100644 --- a/chrome/common/file_system/file_system_dispatcher.h +++ b/chrome/common/file_system/file_system_dispatcher.h @@ -43,7 +43,6 @@ class FileSystemDispatcher { const FilePath& dest_path, fileapi::FileSystemCallbackDispatcher* dispatcher); bool Remove(const FilePath& path, - bool recursive, fileapi::FileSystemCallbackDispatcher* dispatcher); bool ReadMetadata(const FilePath& path, fileapi::FileSystemCallbackDispatcher* dispatcher); diff --git a/chrome/common/file_system/webfilesystem_impl.cc b/chrome/common/file_system/webfilesystem_impl.cc index 5b180f6..9128cf1 100644 --- a/chrome/common/file_system/webfilesystem_impl.cc +++ b/chrome/common/file_system/webfilesystem_impl.cc @@ -47,16 +47,6 @@ void WebFileSystemImpl::remove(const WebString& path, FileSystemDispatcher* dispatcher = ChildThread::current()->file_system_dispatcher(); dispatcher->Remove(webkit_glue::WebStringToFilePath(path), - false /* recursive */, - new WebFileSystemCallbackDispatcher(callbacks)); -} - -void WebFileSystemImpl::removeRecursively(const WebString& path, - WebFileSystemCallbacks* callbacks) { - FileSystemDispatcher* dispatcher = - ChildThread::current()->file_system_dispatcher(); - dispatcher->Remove(webkit_glue::WebStringToFilePath(path), - true /* recursive */, new WebFileSystemCallbackDispatcher(callbacks)); } @@ -114,3 +104,4 @@ WebKit::WebFileWriter* WebFileSystemImpl::createFileWriter( const WebString& path, WebKit::WebFileWriterClient* client) { return new WebFileWriterImpl(path, client); } + diff --git a/chrome/common/file_system/webfilesystem_impl.h b/chrome/common/file_system/webfilesystem_impl.h index fc051e0..6892eac 100644 --- a/chrome/common/file_system/webfilesystem_impl.h +++ b/chrome/common/file_system/webfilesystem_impl.h @@ -32,10 +32,6 @@ class WebFileSystemImpl : public WebKit::WebFileSystem { const WebKit::WebString& path, WebKit::WebFileSystemCallbacks*); - virtual void removeRecursively( - const WebKit::WebString& path, - WebKit::WebFileSystemCallbacks*); - virtual void readMetadata( const WebKit::WebString& path, WebKit::WebFileSystemCallbacks*); diff --git a/chrome/common/render_messages_internal.h b/chrome/common/render_messages_internal.h index c600dad..b2cb0bf 100644 --- a/chrome/common/render_messages_internal.h +++ b/chrome/common/render_messages_internal.h @@ -2848,10 +2848,9 @@ IPC_BEGIN_MESSAGES(ViewHost) FilePath /* dest path */) // WebFileSystem::remove() message. - IPC_MESSAGE_CONTROL3(ViewHostMsg_FileSystem_Remove, + IPC_MESSAGE_CONTROL2(ViewHostMsg_FileSystem_Remove, int /* request_id */, - FilePath /* path */, - bool /* recursive */) + FilePath /* path */) // WebFileSystem::readMetadata() message. IPC_MESSAGE_CONTROL2(ViewHostMsg_FileSystem_ReadMetadata, diff --git a/chrome/renderer/pepper_plugin_delegate_impl.cc b/chrome/renderer/pepper_plugin_delegate_impl.cc index 8960d29..c3b9f9b 100644 --- a/chrome/renderer/pepper_plugin_delegate_impl.cc +++ b/chrome/renderer/pepper_plugin_delegate_impl.cc @@ -711,7 +711,7 @@ bool PepperPluginDelegateImpl::Delete( fileapi::FileSystemCallbackDispatcher* dispatcher) { FileSystemDispatcher* file_system_dispatcher = ChildThread::current()->file_system_dispatcher(); - return file_system_dispatcher->Remove(path, false /* recursive */, dispatcher); + return file_system_dispatcher->Remove(path, dispatcher); } bool PepperPluginDelegateImpl::Rename( diff --git a/webkit/blob/deletable_file_reference.cc b/webkit/blob/deletable_file_reference.cc index 9f49943..b005eeb 100644 --- a/webkit/blob/deletable_file_reference.cc +++ b/webkit/blob/deletable_file_reference.cc @@ -57,7 +57,7 @@ DeletableFileReference::DeletableFileReference( DeletableFileReference::~DeletableFileReference() { DCHECK(map()->find(path_)->second == this); map()->erase(path_); - base::FileUtilProxy::Delete(file_thread_, path_, false /* recursive */, NULL); + base::FileUtilProxy::Delete(file_thread_, path_, NULL); } } // namespace webkit_blob diff --git a/webkit/fileapi/file_system_operation.cc b/webkit/fileapi/file_system_operation.cc index 29336a8..e636420 100644 --- a/webkit/fileapi/file_system_operation.cc +++ b/webkit/fileapi/file_system_operation.cc @@ -118,15 +118,14 @@ void FileSystemOperation::ReadDirectory(const FilePath& path) { &FileSystemOperation::DidReadDirectory)); } -void FileSystemOperation::Remove(const FilePath& path, bool recursive) { +void FileSystemOperation::Remove(const FilePath& path) { #ifndef NDEBUG DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationRemove; #endif - base::FileUtilProxy::Delete(proxy_, path, recursive, - callback_factory_.NewCallback( - &FileSystemOperation::DidFinishFileOperation)); + base::FileUtilProxy::Delete(proxy_, path, callback_factory_.NewCallback( + &FileSystemOperation::DidFinishFileOperation)); } void FileSystemOperation::Write( diff --git a/webkit/fileapi/file_system_operation.h b/webkit/fileapi/file_system_operation.h index ba65b7d..ab6d56c 100644 --- a/webkit/fileapi/file_system_operation.h +++ b/webkit/fileapi/file_system_operation.h @@ -45,6 +45,9 @@ class FileSystemOperation { void Copy(const FilePath& src_path, const FilePath& dest_path); + // If |dest_path| exists and is a directory, behavior is unspecified or + // varies for different platforms. TODO(kkanetkar): Fix this as per spec + // when it is addressed in spec. void Move(const FilePath& src_path, const FilePath& dest_path); @@ -56,7 +59,7 @@ class FileSystemOperation { void ReadDirectory(const FilePath& path); - void Remove(const FilePath& path, bool recursive); + void Remove(const FilePath& path); void Write(const FilePath& path, const GURL& blob_url, int64 offset); diff --git a/webkit/fileapi/file_system_operation_unittest.cc b/webkit/fileapi/file_system_operation_unittest.cc index 8ee4ac6..8a02ade 100644 --- a/webkit/fileapi/file_system_operation_unittest.cc +++ b/webkit/fileapi/file_system_operation_unittest.cc @@ -89,7 +89,9 @@ class FileSystemOperationTest : public testing::Test { } protected: - // Common temp base for nondestructive uses. + // Common temp base for all non-existing file/dir path test cases. + // This is in case a dir on test machine exists. It's better to + // create temp and then create non-existing file paths under it. ScopedTempDir base_; int request_id_; @@ -118,12 +120,9 @@ TEST_F(FileSystemOperationTest, TestMoveFailureSrcDoesntExist) { #endif TEST_F(FileSystemOperationTest, MAYBE_TestMoveFailureContainsPath) { - ScopedTempDir src_dir; - ASSERT_TRUE(src_dir.CreateUniqueTempDir()); - FilePath dest_dir_path; - ASSERT_TRUE(file_util::CreateTemporaryFileInDir(src_dir.path(), - &dest_dir_path)); - operation()->Move(src_dir.path(), dest_dir_path); + ScopedTempDir dir_under_base; + dir_under_base.CreateUniqueTempDirUnderPath(base_.path()); + operation()->Move(base_.path(), dir_under_base.path()); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_INVALID_OPERATION, mock_dispatcher_->status()); @@ -132,51 +131,14 @@ TEST_F(FileSystemOperationTest, MAYBE_TestMoveFailureContainsPath) { TEST_F(FileSystemOperationTest, TestMoveFailureSrcDirExistsDestFile) { // Src exists and is dir. Dest is a file. - ScopedTempDir src_dir; - ASSERT_TRUE(src_dir.CreateUniqueTempDir()); - ScopedTempDir dest_dir; ASSERT_TRUE(dest_dir.CreateUniqueTempDir()); FilePath dest_file; file_util::CreateTemporaryFileInDir(dest_dir.path(), &dest_file); - operation()->Move(src_dir.path(), dest_file); + operation()->Move(base_.path(), dest_file); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY, - mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); -} - -TEST_F(FileSystemOperationTest, TestMoveFailureSrcFileExistsDestNonEmptyDir) { - // Src exists and is a directory. Dest is a non-empty directory. - ScopedTempDir src_dir; - ASSERT_TRUE(src_dir.CreateUniqueTempDir()); - - ScopedTempDir dest_dir; - ASSERT_TRUE(dest_dir.CreateUniqueTempDir()); - FilePath child_file; - file_util::CreateTemporaryFileInDir(dest_dir.path(), &child_file); - - operation()->Move(src_dir.path(), dest_dir.path()); - MessageLoop::current()->RunAllPending(); - EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_EMPTY, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); -} - -TEST_F(FileSystemOperationTest, TestMoveFailureSrcFileExistsDestDir) { - // Src exists and is a file. Dest is a directory. - ScopedTempDir src_dir; - ASSERT_TRUE(src_dir.CreateUniqueTempDir()); - FilePath src_file; - file_util::CreateTemporaryFileInDir(src_dir.path(), &src_file); - - ScopedTempDir dest_dir; - ASSERT_TRUE(dest_dir.CreateUniqueTempDir()); - - operation()->Move(src_file, dest_dir.path()); - MessageLoop::current()->RunAllPending(); - EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_FILE, - mock_dispatcher_->status()); + EXPECT_EQ(base::PLATFORM_FILE_ERROR_EXISTS, mock_dispatcher_->status()); EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); } @@ -186,7 +148,7 @@ TEST_F(FileSystemOperationTest, TestMoveFailureDestParentDoesntExist) { ASSERT_TRUE(src_dir.CreateUniqueTempDir()); FilePath nonexisting_file = base_.path().Append( FILE_PATH_LITERAL("NonexistingDir")).Append( - FILE_PATH_LITERAL("NonexistingFile")); + FILE_PATH_LITERAL("NonexistingFile"));; operation()->Move(src_dir.path(), nonexisting_file); MessageLoop::current()->RunAllPending(); @@ -229,57 +191,6 @@ TEST_F(FileSystemOperationTest, TestMoveSuccessSrcFileAndNew) { EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); } -TEST_F(FileSystemOperationTest, TestMoveSuccessSrcDirAndOverwrite) { - ScopedTempDir src_dir; - ASSERT_TRUE(src_dir.CreateUniqueTempDir()); - - ScopedTempDir dest_dir; - ASSERT_TRUE(dest_dir.CreateUniqueTempDir()); - - operation()->Move(src_dir.path(), dest_dir.path()); - MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); - EXPECT_FALSE(file_util::DirectoryExists(src_dir.path())); - - // Make sure we've overwritten but not moved the source under the |dest_dir|. - EXPECT_TRUE(file_util::DirectoryExists(dest_dir.path())); - EXPECT_FALSE(file_util::DirectoryExists( - dest_dir.path().Append(src_dir.path().BaseName()))); -} - -TEST_F(FileSystemOperationTest, TestMoveSuccessSrcDirAndNew) { - ScopedTempDir src_dir; - ASSERT_TRUE(src_dir.CreateUniqueTempDir()); - - ScopedTempDir dir; - ASSERT_TRUE(dir.CreateUniqueTempDir()); - FilePath dest_dir_path(dir.path().Append(FILE_PATH_LITERAL("NewDirectory"))); - - operation()->Move(src_dir.path(), dest_dir_path); - MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); - EXPECT_FALSE(file_util::DirectoryExists(src_dir.path())); - EXPECT_TRUE(file_util::DirectoryExists(dest_dir_path)); -} - -TEST_F(FileSystemOperationTest, TestMoveSuccessSrcDirRecursive) { - ScopedTempDir src_dir; - ASSERT_TRUE(src_dir.CreateUniqueTempDir()); - FilePath child_file; - file_util::CreateTemporaryFileInDir(src_dir.path(), &child_file); - - ScopedTempDir dest_dir; - ASSERT_TRUE(dest_dir.CreateUniqueTempDir()); - - operation()->Move(src_dir.path(), dest_dir.path()); - MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); - EXPECT_TRUE(FileExists(dest_dir.path().Append(child_file.BaseName()))); -} - TEST_F(FileSystemOperationTest, TestCopyFailureSrcDoesntExist) { FilePath src(base_.path().Append(FILE_PATH_LITERAL("a"))); FilePath dest(base_.path().Append(FILE_PATH_LITERAL("b"))); @@ -297,68 +208,27 @@ TEST_F(FileSystemOperationTest, TestCopyFailureSrcDoesntExist) { #endif TEST_F(FileSystemOperationTest, MAYBE_TestCopyFailureContainsPath) { - ScopedTempDir src_dir; - ASSERT_TRUE(src_dir.CreateUniqueTempDir()); - FilePath dest_dir_path; - ASSERT_TRUE(file_util::CreateTemporaryFileInDir(src_dir.path(), - &dest_dir_path)); - operation()->Copy(src_dir.path(), dest_dir_path); + FilePath file_under_base = base_.path().Append(FILE_PATH_LITERAL("b")); + operation()->Copy(base_.path(), file_under_base); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(base::PLATFORM_FILE_ERROR_INVALID_OPERATION, - mock_dispatcher_->status()); + EXPECT_EQ(base::PLATFORM_FILE_ERROR_FAILED, mock_dispatcher_->status()); EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); } TEST_F(FileSystemOperationTest, TestCopyFailureSrcDirExistsDestFile) { // Src exists and is dir. Dest is a file. - ScopedTempDir src_dir; - ASSERT_TRUE(src_dir.CreateUniqueTempDir()); - - ScopedTempDir dest_dir; - ASSERT_TRUE(dest_dir.CreateUniqueTempDir()); + ScopedTempDir dir; + ASSERT_TRUE(dir.CreateUniqueTempDir()); FilePath dest_file; - file_util::CreateTemporaryFileInDir(dest_dir.path(), &dest_file); + file_util::CreateTemporaryFileInDir(dir.path(), &dest_file); - operation()->Copy(src_dir.path(), dest_file); + operation()->Copy(base_.path(), dest_file); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY, mock_dispatcher_->status()); EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); } -TEST_F(FileSystemOperationTest, TestCopyFailureSrcFileExistsDestNonEmptyDir) { - // Src exists and is a directory. Dest is a non-empty directory. - ScopedTempDir src_dir; - ASSERT_TRUE(src_dir.CreateUniqueTempDir()); - - ScopedTempDir dest_dir; - ASSERT_TRUE(dest_dir.CreateUniqueTempDir()); - FilePath child_file; - file_util::CreateTemporaryFileInDir(dest_dir.path(), &child_file); - - operation()->Copy(src_dir.path(), dest_dir.path()); - MessageLoop::current()->RunAllPending(); - EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_EMPTY, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); -} - -TEST_F(FileSystemOperationTest, TestCopyFailureSrcFileExistsDestDir) { - // Src exists and is a file. Dest is a directory. - ScopedTempDir src_dir; - ASSERT_TRUE(src_dir.CreateUniqueTempDir()); - FilePath src_file; - file_util::CreateTemporaryFileInDir(src_dir.path(), &src_file); - - ScopedTempDir dest_dir; - ASSERT_TRUE(dest_dir.CreateUniqueTempDir()); - - operation()->Copy(src_file, dest_dir.path()); - MessageLoop::current()->RunAllPending(); - EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_FILE, - mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); -} - TEST_F(FileSystemOperationTest, TestCopyFailureDestParentDoesntExist) { // Dest. parent path does not exist. ScopedTempDir dir; @@ -411,7 +281,7 @@ TEST_F(FileSystemOperationTest, TestCopySuccessSrcFileAndNew) { EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); } -TEST_F(FileSystemOperationTest, TestCopySuccessSrcDirAndOverwrite) { +TEST_F(FileSystemOperationTest, TestCopySuccessSrcDir) { ScopedTempDir src_dir; ASSERT_TRUE(src_dir.CreateUniqueTempDir()); @@ -422,42 +292,24 @@ TEST_F(FileSystemOperationTest, TestCopySuccessSrcDirAndOverwrite) { MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); - - // Make sure we've overwritten but not copied the source under the |dest_dir|. - EXPECT_TRUE(file_util::DirectoryExists(dest_dir.path())); - EXPECT_FALSE(file_util::DirectoryExists( - dest_dir.path().Append(src_dir.path().BaseName()))); -} - -TEST_F(FileSystemOperationTest, TestCopySuccessSrcDirAndNew) { - ScopedTempDir src_dir; - ASSERT_TRUE(src_dir.CreateUniqueTempDir()); - - ScopedTempDir dir; - ASSERT_TRUE(dir.CreateUniqueTempDir()); - FilePath dest_dir(dir.path().Append(FILE_PATH_LITERAL("NewDirectory"))); - - operation()->Copy(src_dir.path(), dest_dir); - MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); - EXPECT_TRUE(file_util::DirectoryExists(dest_dir)); } -TEST_F(FileSystemOperationTest, TestCopySuccessSrcDirRecursive) { +TEST_F(FileSystemOperationTest, TestCopyDestParentExistsSuccess) { ScopedTempDir src_dir; ASSERT_TRUE(src_dir.CreateUniqueTempDir()); - FilePath child_file; - file_util::CreateTemporaryFileInDir(src_dir.path(), &child_file); + FilePath src_file; + file_util::CreateTemporaryFileInDir(src_dir.path(), &src_file); ScopedTempDir dest_dir; ASSERT_TRUE(dest_dir.CreateUniqueTempDir()); - operation()->Copy(src_dir.path(), dest_dir.path()); + operation()->Copy(src_file, dest_dir.path()); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); - EXPECT_TRUE(FileExists(dest_dir.path().Append(child_file.BaseName()))); + + FilePath src_filename(src_file.BaseName()); + EXPECT_TRUE(FileExists(dest_dir.path().Append(src_filename))); } TEST_F(FileSystemOperationTest, TestCreateFileFailure) { @@ -515,6 +367,7 @@ TEST_F(FileSystemOperationTest, // Dest. parent path does not exist. FilePath nonexisting(base_.path().Append( FILE_PATH_LITERAL("DirDoesntExist"))); + file_util::EnsureEndsWithSeparator(&nonexisting); FilePath nonexisting_file = nonexisting.Append( FILE_PATH_LITERAL("FileDoesntExist")); operation()->CreateDirectory(nonexisting_file, false, false); @@ -525,9 +378,7 @@ TEST_F(FileSystemOperationTest, TEST_F(FileSystemOperationTest, TestCreateDirFailureDirExists) { // Exclusive and dir existing at path. - ScopedTempDir src_dir; - ASSERT_TRUE(src_dir.CreateUniqueTempDir()); - operation()->CreateDirectory(src_dir.path(), true, false); + operation()->CreateDirectory(base_.path(), true, false); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_EXISTS, mock_dispatcher_->status()); EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); @@ -555,8 +406,7 @@ TEST_F(FileSystemOperationTest, TestCreateDirSuccess) { EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); // Dir doesn't exist. - FilePath nonexisting_dir_path(base_.path().Append( - FILE_PATH_LITERAL("nonexistingdir"))); + FilePath nonexisting_dir_path(FILE_PATH_LITERAL("nonexistingdir")); operation()->CreateDirectory(nonexisting_dir_path, false, false); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); @@ -566,7 +416,9 @@ TEST_F(FileSystemOperationTest, TestCreateDirSuccess) { TEST_F(FileSystemOperationTest, TestCreateDirSuccessExclusive) { // Dir doesn't exist. - FilePath nonexisting_dir_path(base_.path().Append( + ScopedTempDir dir; + ASSERT_TRUE(dir.CreateUniqueTempDir()); + FilePath nonexisting_dir_path(dir.path().Append( FILE_PATH_LITERAL("nonexistingdir"))); operation()->CreateDirectory(nonexisting_dir_path, true, false); @@ -682,13 +534,12 @@ TEST_F(FileSystemOperationTest, TestRemoveFailure) { FILE_PATH_LITERAL("NonExistingDir"))); file_util::EnsureEndsWithSeparator(&nonexisting); - operation()->Remove(nonexisting, false /* recursive */); + operation()->Remove(nonexisting); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, mock_dispatcher_->status()); EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); - // It's an error to try to remove a non-empty directory if recursive flag - // is false. + // By spec recursive is always false. Non-empty dir should fail. // parent_dir // | | // child_dir child_file @@ -701,9 +552,9 @@ TEST_F(FileSystemOperationTest, TestRemoveFailure) { ASSERT_TRUE(file_util::CreateTemporaryDirInDir( parent_dir.path(), FILE_PATH_LITERAL("child_dir"), &child_dir)); - operation()->Remove(parent_dir.path(), false /* recursive */); + operation()->Remove(parent_dir.path()); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_EMPTY, + EXPECT_EQ(base::PLATFORM_FILE_ERROR_INVALID_OPERATION, mock_dispatcher_->status()); EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); } @@ -713,30 +564,11 @@ TEST_F(FileSystemOperationTest, TestRemoveSuccess) { ASSERT_TRUE(empty_dir.CreateUniqueTempDir()); EXPECT_TRUE(file_util::DirectoryExists(empty_dir.path())); - operation()->Remove(empty_dir.path(), false /* recursive */); + operation()->Remove(empty_dir.path()); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); EXPECT_FALSE(file_util::DirectoryExists(empty_dir.path())); EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); - - // Removing a non-empty directory with recursive flag == true should be ok. - // parent_dir - // | | - // child_dir child_file - // Verify deleting parent_dir. - ScopedTempDir parent_dir; - ASSERT_TRUE(parent_dir.CreateUniqueTempDir()); - FilePath child_file; - file_util::CreateTemporaryFileInDir(parent_dir.path(), &child_file); - FilePath child_dir; - ASSERT_TRUE(file_util::CreateTemporaryDirInDir( - parent_dir.path(), FILE_PATH_LITERAL("child_dir"), &child_dir)); - - operation()->Remove(parent_dir.path(), true /* recursive */); - MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); - EXPECT_FALSE(file_util::DirectoryExists(parent_dir.path())); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); } TEST_F(FileSystemOperationTest, TestTruncate) { diff --git a/webkit/glue/webkit_glue.cc b/webkit/glue/webkit_glue.cc index f7f60aa..661b87b 100644 --- a/webkit/glue/webkit_glue.cc +++ b/webkit/glue/webkit_glue.cc @@ -321,8 +321,6 @@ WebKit::WebFileError PlatformFileErrorToWebFileError( case base::PLATFORM_FILE_ERROR_INVALID_OPERATION: case base::PLATFORM_FILE_ERROR_EXISTS: case base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY: - case base::PLATFORM_FILE_ERROR_NOT_A_FILE: - case base::PLATFORM_FILE_ERROR_NOT_EMPTY: return WebKit::WebFileErrorInvalidModification; case base::PLATFORM_FILE_ERROR_ACCESS_DENIED: return WebKit::WebFileErrorNoModificationAllowed; diff --git a/webkit/tools/test_shell/simple_file_system.cc b/webkit/tools/test_shell/simple_file_system.cc index 6a26615..8e059ea 100644 --- a/webkit/tools/test_shell/simple_file_system.cc +++ b/webkit/tools/test_shell/simple_file_system.cc @@ -130,10 +130,10 @@ void SimpleFileSystem::copy( } void SimpleFileSystem::remove( - const WebString& path, bool recursive, WebFileSystemCallbacks* callbacks) { + const WebString& path, WebFileSystemCallbacks* callbacks) { FilePath filepath(webkit_glue::WebStringToFilePath(path)); - GetNewOperation(callbacks)->Remove(filepath, recursive); + GetNewOperation(callbacks)->Remove(filepath); } void SimpleFileSystem::readMetadata( diff --git a/webkit/tools/test_shell/simple_file_system.h b/webkit/tools/test_shell/simple_file_system.h index bf13935..db13d9c 100644 --- a/webkit/tools/test_shell/simple_file_system.h +++ b/webkit/tools/test_shell/simple_file_system.h @@ -1,6 +1,6 @@ -// Copyright (c) 2010 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. +// Copyright (c) 2010 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_TOOLS_TEST_SHELL_SIMPLE_FILE_SYSTEM_H_ #define WEBKIT_TOOLS_TEST_SHELL_SIMPLE_FILE_SYSTEM_H_ @@ -27,7 +27,6 @@ class SimpleFileSystem : public WebKit::WebFileSystem { const WebKit::WebString& dest_path, WebKit::WebFileSystemCallbacks* callbacks); virtual void remove(const WebKit::WebString& path, - bool recursive, WebKit::WebFileSystemCallbacks* callbacks); virtual void readMetadata(const WebKit::WebString& path, WebKit::WebFileSystemCallbacks* callbacks); |