Second try for 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.

original issue: http://codereview.chromium.org/3567012

BUG=none
TEST=FileSystemOperationTest.*
TBR=ericu

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

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

18 files changed, 328 insertions, 132 deletions

diff --git a/base/file_util_proxy.cc b/base/file_util_proxy.cc
index d71f374..b3b18f8 100644
--- a/base/file_util_proxy.cc
+++ b/base/file_util_proxy.cc
@@ -10,6 +10,63 @@
 // 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:
@@ -205,7 +262,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_INVALID_OPERATION);
+        set_error_code(base::PLATFORM_FILE_ERROR_NOT_EMPTY);
         return;
       }
       set_error_code(base::PLATFORM_FILE_ERROR_FAILED);
@@ -229,36 +286,13 @@ class RelayCopy : public RelayWithStatusCallback {
 
  protected:
   virtual void RunWork() {
-    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);
+    set_error_code(PerformCommonCheckAndPreparationForMoveAndCopy(
+        src_file_path_, dest_file_path_));
+    if (error_code() != base::PLATFORM_FILE_OK)
       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 */)) {
-      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.
+        true /* recursive */))
       set_error_code(base::PLATFORM_FILE_ERROR_FAILED);
-    }
   }
 
  private:
@@ -278,36 +312,12 @@ class RelayMove : public RelayWithStatusCallback {
 
  protected:
   virtual void RunWork() {
-    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);
+    set_error_code(PerformCommonCheckAndPreparationForMoveAndCopy(
+        src_file_path_, dest_file_path_));
+    if (error_code() != base::PLATFORM_FILE_OK)
       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_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.
+    if (!file_util::Move(src_file_path_, dest_file_path_))
       set_error_code(base::PLATFORM_FILE_ERROR_FAILED);
-    }
   }
 
  private:
@@ -703,9 +713,10 @@ 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, false, callback));
+               new RelayDelete(file_path, recursive, callback));
 }
 
 // static
diff --git a/base/file_util_proxy.h b/base/file_util_proxy.h
index 32b78630..e716ab7 100644
--- a/base/file_util_proxy.h
+++ b/base/file_util_proxy.h
@@ -89,6 +89,7 @@ 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,
@@ -105,9 +106,11 @@ class FileUtilProxy {
       bool recursive,
       StatusCallback* callback);
 
-  // Deletes a file or empty directory.
+  // Deletes a file or a directory.
+  // It is an error to delete a non-empty directory with recursive=false.
   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 5024717..b350f8c 100644
--- a/base/platform_file.h
+++ b/base/platform_file.h
@@ -60,7 +60,9 @@ 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_ABORT = -12,
+  PLATFORM_FILE_ERROR_NOT_A_FILE = -13,
+  PLATFORM_FILE_ERROR_NOT_EMPTY = -14,
 };
 
 // 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 6dd6072..96571ce 100644
--- a/chrome/browser/file_system/file_system_dispatcher_host.cc
+++ b/chrome/browser/file_system/file_system_dispatcher_host.cc
@@ -161,10 +161,11 @@ void FileSystemDispatcherHost::OnCopy(
   GetNewOperation(request_id)->Copy(src_path, dest_path);
 }
 
-void FileSystemDispatcherHost::OnRemove(int request_id, const FilePath& path) {
+void FileSystemDispatcherHost::OnRemove(
+    int request_id, const FilePath& path, bool recursive) {
   if (!CheckValidFileSystemPath(path, request_id))
     return;
-  GetNewOperation(request_id)->Remove(path);
+  GetNewOperation(request_id)->Remove(path, recursive);
 }
 
 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 22a560a..68110be 100644
--- a/chrome/browser/file_system/file_system_dispatcher_host.h
+++ b/chrome/browser/file_system/file_system_dispatcher_host.h
@@ -21,7 +21,6 @@ namespace base {
 class Time;
 }
 
-class ChromeFileSystemOperation;
 class FileSystemHostContext;
 class GURL;
 class HostContentSettingsMap;
@@ -50,7 +49,7 @@ class FileSystemDispatcherHost
   void OnCopy(int request_id,
               const FilePath& src_path,
               const FilePath& dest_path);
-  void OnRemove(int request_id, const FilePath& path);
+  void OnRemove(int request_id, const FilePath& path, bool recursive);
   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 a8761a2..bc52f3a 100644
--- a/chrome/common/file_system/file_system_dispatcher.cc
+++ b/chrome/common/file_system/file_system_dispatcher.cc
@@ -67,10 +67,11 @@ 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));
+      new ViewHostMsg_FileSystem_Remove(request_id, path, recursive));
 }
 
 bool FileSystemDispatcher::ReadMetadata(
@@ -219,4 +220,3 @@ 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 80443c4..8802c1e 100644
--- a/chrome/common/file_system/file_system_dispatcher.h
+++ b/chrome/common/file_system/file_system_dispatcher.h
@@ -43,6 +43,7 @@ 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 9128cf1..5b180f6 100644
--- a/chrome/common/file_system/webfilesystem_impl.cc
+++ b/chrome/common/file_system/webfilesystem_impl.cc
@@ -47,6 +47,16 @@ 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));
 }
 
@@ -104,4 +114,3 @@ 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 6892eac..fc051e0 100644
--- a/chrome/common/file_system/webfilesystem_impl.h
+++ b/chrome/common/file_system/webfilesystem_impl.h
@@ -32,6 +32,10 @@ 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 38f6332..f5643af 100644
--- a/chrome/common/render_messages_internal.h
+++ b/chrome/common/render_messages_internal.h
@@ -2846,9 +2846,10 @@ IPC_BEGIN_MESSAGES(ViewHost)
                        FilePath /* dest path */)
 
   // WebFileSystem::remove() message.
-  IPC_MESSAGE_CONTROL2(ViewHostMsg_FileSystem_Remove,
+  IPC_MESSAGE_CONTROL3(ViewHostMsg_FileSystem_Remove,
                        int /* request_id */,
-                       FilePath /* path */)
+                       FilePath /* path */,
+                       bool /* recursive */)
 
   // 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 c3b9f9b..8960d29 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, dispatcher);
+  return file_system_dispatcher->Remove(path, false /* recursive */, dispatcher);
 }
 
 bool PepperPluginDelegateImpl::Rename(
diff --git a/webkit/blob/deletable_file_reference.cc b/webkit/blob/deletable_file_reference.cc
index b005eeb..9f49943 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_, NULL);
+  base::FileUtilProxy::Delete(file_thread_, path_, false /* recursive */, NULL);
 }
 
 }  // namespace webkit_blob
diff --git a/webkit/fileapi/file_system_operation.cc b/webkit/fileapi/file_system_operation.cc
index e636420..29336a8 100644
--- a/webkit/fileapi/file_system_operation.cc
+++ b/webkit/fileapi/file_system_operation.cc
@@ -118,14 +118,15 @@ void FileSystemOperation::ReadDirectory(const FilePath& path) {
           &FileSystemOperation::DidReadDirectory));
 }
 
-void FileSystemOperation::Remove(const FilePath& path) {
+void FileSystemOperation::Remove(const FilePath& path, bool recursive) {
 #ifndef NDEBUG
   DCHECK(kOperationNone == pending_operation_);
   pending_operation_ = kOperationRemove;
 #endif
 
-  base::FileUtilProxy::Delete(proxy_, path, callback_factory_.NewCallback(
-      &FileSystemOperation::DidFinishFileOperation));
+  base::FileUtilProxy::Delete(proxy_, path, recursive,
+      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 ab6d56c..ba65b7d 100644
--- a/webkit/fileapi/file_system_operation.h
+++ b/webkit/fileapi/file_system_operation.h
@@ -45,9 +45,6 @@ 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);
 
@@ -59,7 +56,7 @@ class FileSystemOperation {
 
   void ReadDirectory(const FilePath& path);
 
-  void Remove(const FilePath& path);
+  void Remove(const FilePath& path, bool recursive);
 
   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 8a02ade..9b07c03 100644
--- a/webkit/fileapi/file_system_operation_unittest.cc
+++ b/webkit/fileapi/file_system_operation_unittest.cc
@@ -89,9 +89,7 @@ class FileSystemOperationTest : public testing::Test {
   }
 
  protected:
-  // 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.
+  // Common temp base for nondestructive uses.
   ScopedTempDir base_;
 
   int request_id_;
@@ -112,17 +110,14 @@ TEST_F(FileSystemOperationTest, TestMoveFailureSrcDoesntExist) {
   EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
 }
 
-// crbug.com/57940
-#if defined(OS_WINDOWS)
-#define MAYBE_TestMoveFailureContainsPath FAILS_TestMoveFailureContainsPath
-#else
-#define MAYBE_TestMoveFailureContainsPath TestMoveFailureContainsPath
-#endif
-
-TEST_F(FileSystemOperationTest, MAYBE_TestMoveFailureContainsPath) {
-  ScopedTempDir dir_under_base;
-  dir_under_base.CreateUniqueTempDirUnderPath(base_.path());
-  operation()->Move(base_.path(), dir_under_base.path());
+TEST_F(FileSystemOperationTest, TestMoveFailureContainsPath) {
+  ScopedTempDir src_dir;
+  ASSERT_TRUE(src_dir.CreateUniqueTempDir());
+  FilePath dest_dir_path;
+  ASSERT_TRUE(file_util::CreateTemporaryDirInDir(src_dir.path(),
+                                                 FILE_PATH_LITERAL("child_dir"),
+                                                 &dest_dir_path));
+  operation()->Move(src_dir.path(), dest_dir_path);
   MessageLoop::current()->RunAllPending();
   EXPECT_EQ(base::PLATFORM_FILE_ERROR_INVALID_OPERATION,
             mock_dispatcher_->status());
@@ -131,14 +126,51 @@ 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(base_.path(), dest_file);
+  operation()->Move(src_dir.path(), dest_file);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(base::PLATFORM_FILE_ERROR_EXISTS, mock_dispatcher_->status());
+  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(request_id_, mock_dispatcher_->request_id());
 }
 
@@ -148,7 +180,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();
@@ -191,6 +223,57 @@ 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")));
@@ -200,35 +283,70 @@ TEST_F(FileSystemOperationTest, TestCopyFailureSrcDoesntExist) {
   EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
 }
 
-// crbug.com/57940
-#if defined(OS_WINDOWS)
-#define MAYBE_TestCopyFailureContainsPath FAILS_TestCopyFailureContainsPath
-#else
-#define MAYBE_TestCopyFailureContainsPath TestCopyFailureContainsPath
-#endif
-
-TEST_F(FileSystemOperationTest, MAYBE_TestCopyFailureContainsPath) {
-  FilePath file_under_base = base_.path().Append(FILE_PATH_LITERAL("b"));
-  operation()->Copy(base_.path(), file_under_base);
+TEST_F(FileSystemOperationTest, TestCopyFailureContainsPath) {
+  ScopedTempDir src_dir;
+  ASSERT_TRUE(src_dir.CreateUniqueTempDir());
+  FilePath dest_dir_path;
+  ASSERT_TRUE(file_util::CreateTemporaryDirInDir(src_dir.path(),
+                                                 FILE_PATH_LITERAL("child_dir"),
+                                                 &dest_dir_path));
+  operation()->Copy(src_dir.path(), dest_dir_path);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(base::PLATFORM_FILE_ERROR_FAILED, mock_dispatcher_->status());
+  EXPECT_EQ(base::PLATFORM_FILE_ERROR_INVALID_OPERATION,
+            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 dir;
-  ASSERT_TRUE(dir.CreateUniqueTempDir());
+  ScopedTempDir src_dir;
+  ASSERT_TRUE(src_dir.CreateUniqueTempDir());
+
+  ScopedTempDir dest_dir;
+  ASSERT_TRUE(dest_dir.CreateUniqueTempDir());
   FilePath dest_file;
-  file_util::CreateTemporaryFileInDir(dir.path(), &dest_file);
+  file_util::CreateTemporaryFileInDir(dest_dir.path(), &dest_file);
 
-  operation()->Copy(base_.path(), dest_file);
+  operation()->Copy(src_dir.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;
@@ -281,7 +399,7 @@ TEST_F(FileSystemOperationTest, TestCopySuccessSrcFileAndNew) {
   EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
 }
 
-TEST_F(FileSystemOperationTest, TestCopySuccessSrcDir) {
+TEST_F(FileSystemOperationTest, TestCopySuccessSrcDirAndOverwrite) {
   ScopedTempDir src_dir;
   ASSERT_TRUE(src_dir.CreateUniqueTempDir());
 
@@ -292,24 +410,42 @@ TEST_F(FileSystemOperationTest, TestCopySuccessSrcDir) {
   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, TestCopyDestParentExistsSuccess) {
+TEST_F(FileSystemOperationTest, TestCopySuccessSrcDirAndNew) {
   ScopedTempDir src_dir;
   ASSERT_TRUE(src_dir.CreateUniqueTempDir());
-  FilePath src_file;
-  file_util::CreateTemporaryFileInDir(src_dir.path(), &src_file);
+
+  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) {
+  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()->Copy(src_file, dest_dir.path());
+  operation()->Copy(src_dir.path(), dest_dir.path());
   MessageLoop::current()->RunAllPending();
   EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
   EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
-
-  FilePath src_filename(src_file.BaseName());
-  EXPECT_TRUE(FileExists(dest_dir.path().Append(src_filename)));
+  EXPECT_TRUE(FileExists(dest_dir.path().Append(child_file.BaseName())));
 }
 
 TEST_F(FileSystemOperationTest, TestCreateFileFailure) {
@@ -367,7 +503,6 @@ 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);
@@ -378,7 +513,9 @@ TEST_F(FileSystemOperationTest,
 
 TEST_F(FileSystemOperationTest, TestCreateDirFailureDirExists) {
   // Exclusive and dir existing at path.
-  operation()->CreateDirectory(base_.path(), true, false);
+  ScopedTempDir src_dir;
+  ASSERT_TRUE(src_dir.CreateUniqueTempDir());
+  operation()->CreateDirectory(src_dir.path(), true, false);
   MessageLoop::current()->RunAllPending();
   EXPECT_EQ(base::PLATFORM_FILE_ERROR_EXISTS, mock_dispatcher_->status());
   EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
@@ -406,7 +543,8 @@ TEST_F(FileSystemOperationTest, TestCreateDirSuccess) {
   EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
 
   // Dir doesn't exist.
-  FilePath nonexisting_dir_path(FILE_PATH_LITERAL("nonexistingdir"));
+  FilePath nonexisting_dir_path(base_.path().Append(
+      FILE_PATH_LITERAL("nonexistingdir")));
   operation()->CreateDirectory(nonexisting_dir_path, false, false);
   MessageLoop::current()->RunAllPending();
   EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
@@ -416,9 +554,7 @@ TEST_F(FileSystemOperationTest, TestCreateDirSuccess) {
 
 TEST_F(FileSystemOperationTest, TestCreateDirSuccessExclusive) {
   // Dir doesn't exist.
-  ScopedTempDir dir;
-  ASSERT_TRUE(dir.CreateUniqueTempDir());
-  FilePath nonexisting_dir_path(dir.path().Append(
+  FilePath nonexisting_dir_path(base_.path().Append(
       FILE_PATH_LITERAL("nonexistingdir")));
 
   operation()->CreateDirectory(nonexisting_dir_path, true, false);
@@ -534,12 +670,13 @@ TEST_F(FileSystemOperationTest, TestRemoveFailure) {
       FILE_PATH_LITERAL("NonExistingDir")));
   file_util::EnsureEndsWithSeparator(&nonexisting);
 
-  operation()->Remove(nonexisting);
+  operation()->Remove(nonexisting, false /* recursive */);
   MessageLoop::current()->RunAllPending();
   EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, mock_dispatcher_->status());
   EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
 
-  // By spec recursive is always false. Non-empty dir should fail.
+  // It's an error to try to remove a non-empty directory if recursive flag
+  // is false.
   //      parent_dir
   //       |       |
   //  child_dir  child_file
@@ -552,9 +689,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());
+  operation()->Remove(parent_dir.path(), false /* recursive */);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(base::PLATFORM_FILE_ERROR_INVALID_OPERATION,
+  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_EMPTY,
             mock_dispatcher_->status());
   EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
 }
@@ -564,11 +701,30 @@ TEST_F(FileSystemOperationTest, TestRemoveSuccess) {
   ASSERT_TRUE(empty_dir.CreateUniqueTempDir());
   EXPECT_TRUE(file_util::DirectoryExists(empty_dir.path()));
 
-  operation()->Remove(empty_dir.path());
+  operation()->Remove(empty_dir.path(), false /* recursive */);
   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 661b87b..f7f60aa 100644
--- a/webkit/glue/webkit_glue.cc
+++ b/webkit/glue/webkit_glue.cc
@@ -321,6 +321,8 @@ 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 8e059ea..411d987 100644
--- a/webkit/tools/test_shell/simple_file_system.cc
+++ b/webkit/tools/test_shell/simple_file_system.cc
@@ -133,7 +133,14 @@ void SimpleFileSystem::remove(
     const WebString& path, WebFileSystemCallbacks* callbacks) {
   FilePath filepath(webkit_glue::WebStringToFilePath(path));
 
-  GetNewOperation(callbacks)->Remove(filepath);
+  GetNewOperation(callbacks)->Remove(filepath, false /* recursive */);
+}
+
+void SimpleFileSystem::removeRecursively(
+    const WebString& path, WebFileSystemCallbacks* callbacks) {
+  FilePath filepath(webkit_glue::WebStringToFilePath(path));
+
+  GetNewOperation(callbacks)->Remove(filepath, true /* recursive */);
 }
 
 void SimpleFileSystem::readMetadata(
diff --git a/webkit/tools/test_shell/simple_file_system.h b/webkit/tools/test_shell/simple_file_system.h
index db13d9c..031ed8b 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_
@@ -28,6 +28,8 @@ class SimpleFileSystem : public WebKit::WebFileSystem {
                     WebKit::WebFileSystemCallbacks* callbacks);
   virtual void remove(const WebKit::WebString& path,
                       WebKit::WebFileSystemCallbacks* callbacks);
+  virtual void removeRecursively(const WebKit::WebString& path,
+                                 WebKit::WebFileSystemCallbacks* callbacks);
   virtual void readMetadata(const WebKit::WebString& path,
                             WebKit::WebFileSystemCallbacks* callbacks);
   virtual void createFile(const WebKit::WebString& path,