Revert 61613 - possible culprit for valgrind error. (Support removeRecursively and new copy/move)

BUG=none
TEST=none

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

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

18 files changed, 129 insertions, 325 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 1008de2..c507af6 100644
--- a/chrome/common/render_messages_internal.h
+++ b/chrome/common/render_messages_internal.h
@@ -2858,10 +2858,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 9b07c03..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_;
@@ -110,14 +112,17 @@ TEST_F(FileSystemOperationTest, TestMoveFailureSrcDoesntExist) {
   EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
 }
 
-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);
+// 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());
   MessageLoop::current()->RunAllPending();
   EXPECT_EQ(base::PLATFORM_FILE_ERROR_INVALID_OPERATION,
             mock_dispatcher_->status());
@@ -126,51 +131,14 @@ TEST_F(FileSystemOperationTest, 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);
-  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());
+  operation()->Move(base_.path(), dest_file);
   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());
 }
 
@@ -180,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();
@@ -223,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")));
@@ -283,70 +200,35 @@ TEST_F(FileSystemOperationTest, TestCopyFailureSrcDoesntExist) {
   EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
 }
 
-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);
+// 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);
   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;
@@ -399,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());
 
@@ -410,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) {
+TEST_F(FileSystemOperationTest, TestCopyDestParentExistsSuccess) {
   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) {
-  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) {
@@ -503,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);
@@ -513,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());
@@ -543,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());
@@ -554,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);
@@ -670,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
@@ -689,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());
 }
@@ -701,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 411d987..8e059ea 100644
--- a/webkit/tools/test_shell/simple_file_system.cc
+++ b/webkit/tools/test_shell/simple_file_system.cc
@@ -133,14 +133,7 @@ void SimpleFileSystem::remove(
     const WebString& path, WebFileSystemCallbacks* callbacks) {
   FilePath filepath(webkit_glue::WebStringToFilePath(path));
 
-  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 */);
+  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 031ed8b..2f9eeef 100644
--- a/webkit/tools/test_shell/simple_file_system.h
+++ b/webkit/tools/test_shell/simple_file_system.h
@@ -28,8 +28,6 @@ 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,