Cleanup: Move more recursive operation logic from FileUtilHelper to FileUtil

- To make each FileUtil methods more well-defined
- To further reduce the number of FileUtil calls (As a preparation to split recursive jobs into multiple tasks)

BUG=146215
TEST=content_unittests:.*File.*,content_browsertests:FileSystemLayoutTest.Op{Copy,Move}


Review URL: https://chromiumcodereview.appspot.com/11960003

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

19 files changed, 200 insertions, 227 deletions

diff --git a/chrome/browser/media_gallery/linux/mtp_device_delegate_impl_linux.cc b/chrome/browser/media_gallery/linux/mtp_device_delegate_impl_linux.cc
index 6a5e2c1..cd1f877 100644
--- a/chrome/browser/media_gallery/linux/mtp_device_delegate_impl_linux.cc
+++ b/chrome/browser/media_gallery/linux/mtp_device_delegate_impl_linux.cc
@@ -133,6 +133,8 @@ PlatformFileError MTPDeviceDelegateImplLinux::CreateSnapshotFile(
   PlatformFileError error = GetFileInfo(device_file_path, file_info);
   if (error != base::PLATFORM_FILE_OK)
     return error;
+  if (file_info->is_directory)
+    return base::PLATFORM_FILE_ERROR_NOT_A_FILE;
 
   if (file_info->size <= 0 || file_info->size > kuint32max)
     return base::PLATFORM_FILE_ERROR_FAILED;
diff --git a/chrome/browser/media_gallery/win/mtp_device_delegate_impl_win.cc b/chrome/browser/media_gallery/win/mtp_device_delegate_impl_win.cc
index 9b79882..ba211af 100644
--- a/chrome/browser/media_gallery/win/mtp_device_delegate_impl_win.cc
+++ b/chrome/browser/media_gallery/win/mtp_device_delegate_impl_win.cc
@@ -167,6 +167,8 @@ base::PlatformFileError MTPDeviceDelegateImplWin::CreateSnapshotFile(
   base::PlatformFileError error = GetFileInfo(device_file_path, file_info);
   if (error != base::PLATFORM_FILE_OK)
     return error;
+  if (file_info->is_directory)
+    return base::PLATFORM_FILE_ERROR_NOT_A_FILE;
   if (!media_transfer_protocol::WriteFileObjectContentToPath(device_.get(),
                                                              file_object_id,
                                                              local_path))
@@ -237,4 +239,4 @@ string16 MTPDeviceDelegateImplWin::GetFileObjectIdFromPath(
   return file_object_id;
 }
 
-}  // namespace chrome
\ No newline at end of file
+}  // namespace chrome
diff --git a/webkit/fileapi/file_system_file_util.h b/webkit/fileapi/file_system_file_util.h
index ef83054..fc1972b 100644
--- a/webkit/fileapi/file_system_file_util.h
+++ b/webkit/fileapi/file_system_file_util.h
@@ -155,12 +155,18 @@ class WEBKIT_STORAGE_EXPORT FileSystemFileUtil {
       const FileSystemURL& url,
       int64 length) = 0;
 
-  // Returns true if a given |url| is an empty directory.
-  virtual bool IsDirectoryEmpty(
-      FileSystemOperationContext* context,
-      const FileSystemURL& url) = 0;
-
   // Copies or moves a single file from |src_url| to |dest_url|.
+  // The filesystem type of |src_url| and |dest_url| MUST be same.
+  //
+  // This returns:
+  // - PLATFORM_FILE_ERROR_NOT_FOUND if |src_file_path|
+  //   or the parent directory of |dest_url| does not exist.
+  // - PLATFORM_FILE_ERROR_NOT_A_FILE if |src_url| exists but is not a file.
+  // - PLATFORM_FILE_ERROR_INVALID_OPERATION if |dest_url| exists and
+  //   is not a file.
+  // - PLATFORM_FILE_ERROR_FAILED if |dest_url| does not exist and
+  //   its parent path is a file.
+  //
   virtual base::PlatformFileError CopyOrMoveFile(
       FileSystemOperationContext* context,
       const FileSystemURL& src_url,
@@ -168,20 +174,38 @@ class WEBKIT_STORAGE_EXPORT FileSystemFileUtil {
       bool copy) = 0;
 
   // Copies in a single file from a different filesystem.
+  //
+  // This returns:
+  // - PLATFORM_FILE_ERROR_NOT_FOUND if |src_file_path|
+  //   or the parent directory of |dest_url| does not exist.
+  // - PLATFORM_FILE_ERROR_INVALID_OPERATION if |dest_url| exists and
+  //   is not a file.
+  // - PLATFORM_FILE_ERROR_FAILED if |dest_url| does not exist and
+  //   its parent path is a file.
+  //
   virtual base::PlatformFileError CopyInForeignFile(
         FileSystemOperationContext* context,
         const FilePath& src_file_path,
         const FileSystemURL& dest_url) = 0;
 
   // Deletes a single file.
-  // It assumes the given url points a file.
+  //
+  // This returns:
+  // - PLATFORM_FILE_ERROR_NOT_FOUND if |url| does not exist.
+  // - PLATFORM_FILE_ERROR_NOT_A_FILE if |url| is not a file.
+  //
   virtual base::PlatformFileError DeleteFile(
       FileSystemOperationContext* context,
       const FileSystemURL& url) = 0;
 
   // Deletes a single empty directory.
-  // It assumes the given url points an empty directory.
-  virtual base::PlatformFileError DeleteSingleDirectory(
+  //
+  // This returns:
+  // - PLATFORM_FILE_ERROR_NOT_FOUND if |url| does not exist.
+  // - PLATFORM_FILE_ERROR_NOT_A_DIRECTORY if |url| is not a directory.
+  // - PLATFORM_FILE_ERROR_NOT_EMPTY if |url| is not empty.
+  //
+  virtual base::PlatformFileError DeleteDirectory(
       FileSystemOperationContext* context,
       const FileSystemURL& url) = 0;
 
@@ -197,6 +221,11 @@ class WEBKIT_STORAGE_EXPORT FileSystemFileUtil {
   // |platform_path| is the path to the snapshot file created.
   // |policy| should indicate the policy how the fileapi backend
   // should handle the returned file.
+  //
+  // This returns:
+  // - PLATFORM_FILE_ERROR_NOT_FOUND if |url| does not exist.
+  // - PLATFORM_FILE_ERROR_NOT_A_FILE if |url| is not a file.
+  //
   virtual base::PlatformFileError CreateSnapshotFile(
       FileSystemOperationContext* context,
       const FileSystemURL& url,
diff --git a/webkit/fileapi/file_util_helper.cc b/webkit/fileapi/file_util_helper.cc
index 550b19b..aed0ba2 100644
--- a/webkit/fileapi/file_util_helper.cc
+++ b/webkit/fileapi/file_util_helper.cc
@@ -110,80 +110,34 @@ CrossFileUtilHelper::CrossFileUtilHelper(
 CrossFileUtilHelper::~CrossFileUtilHelper() {}
 
 base::PlatformFileError CrossFileUtilHelper::DoWork() {
-  base::PlatformFileError error = PerformErrorCheckAndPreparation();
-  if (error != base::PLATFORM_FILE_OK)
-    return error;
-  if (FileUtilHelper::DirectoryExists(context_, src_util_, src_root_url_))
-    return CopyOrMoveDirectory(src_root_url_, dest_root_url_);
-  return CopyOrMoveFile(src_root_url_, dest_root_url_);
-}
-
-PlatformFileError CrossFileUtilHelper::PerformErrorCheckAndPreparation() {
-  FilePath platform_path;
-  base::PlatformFileInfo src_root_info;
-  base::PlatformFileInfo dest_root_info;
-
-  PlatformFileError error = src_util_->GetFileInfo(
-      context_, src_root_url_, &src_root_info, &platform_path);
-
-  if (error != base::PLATFORM_FILE_OK)
-    return error;
-
-  error = dest_util_->GetFileInfo(
-      context_, dest_root_url_, &dest_root_info, &platform_path);
-  bool dest_root_exists = (error == base::PLATFORM_FILE_OK);
-  bool dest_parent_exists = dest_root_exists || IsInRoot(dest_root_url_);
-
-  if (!dest_parent_exists) {
-    base::PlatformFileInfo file_info;
-    FileSystemURL parent_url = dest_root_url_.WithPath(
-        dest_root_url_.path().DirName());
-    error = dest_util_->GetFileInfo(
-        context_, parent_url, &file_info, &platform_path);
-    dest_parent_exists = (error == base::PLATFORM_FILE_OK &&
-                          file_info.is_directory);
-  }
-
-  // The parent of the |dest_root_url_| does not exist.
-  if (!dest_parent_exists)
-    return base::PLATFORM_FILE_ERROR_NOT_FOUND;
-
   // It is an error to try to copy/move an entry into its child.
   if (same_file_system_ && src_root_url_.path().IsParent(dest_root_url_.path()))
     return base::PLATFORM_FILE_ERROR_INVALID_OPERATION;
 
-  // Now it is ok to return if the |dest_root_url_| does not exist.
-  if (!dest_root_exists)
-    return base::PLATFORM_FILE_OK;
-
-  // |src_root_url_| exists and is a directory.
-  // |dest_root_url_| exists and is a file.
-  bool src_is_directory = src_root_info.is_directory;
-  bool dest_is_directory = dest_root_info.is_directory;
-
-  // Either one of |src_root_url_| or |dest_root_url_| is directory,
-  // while the other is not.
-  if (src_is_directory != dest_is_directory)
-    return base::PLATFORM_FILE_ERROR_INVALID_OPERATION;
-
   // It is an error to copy/move an entry into the same path.
   if (same_file_system_ &&
       src_root_url_.path() == dest_root_url_.path())
     return base::PLATFORM_FILE_ERROR_EXISTS;
 
-  if (dest_is_directory) {
-    // It is an error to copy/move an entry to a non-empty directory.
-    // Otherwise the copy/move attempt must overwrite the destination, but
-    // the file_util's Copy or Move method doesn't perform overwrite
-    // on all platforms, so we delete the destination directory here.
-    if (base::PLATFORM_FILE_OK !=
-        dest_util_->DeleteSingleDirectory(context_, dest_root_url_)) {
-      if (!dest_util_->IsDirectoryEmpty(context_, dest_root_url_))
-        return base::PLATFORM_FILE_ERROR_NOT_EMPTY;
-      return base::PLATFORM_FILE_ERROR_FAILED;
-    }
-  }
-  return base::PLATFORM_FILE_OK;
+  // First try to copy/move the file.
+  base::PlatformFileError error = CopyOrMoveFile(src_root_url_, dest_root_url_);
+  if (error == base::PLATFORM_FILE_OK ||
+      error != base::PLATFORM_FILE_ERROR_NOT_A_FILE)
+    return error;
+
+  // Now we should be sure that the source (and destination if exists)
+  // is directory.
+  // Now let's try to remove the destination directory, this must
+  // fail if the directory isn't empty or its parent doesn't exist.
+  error = dest_util_->DeleteDirectory(context_, dest_root_url_);
+  if (error == base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY)
+    return base::PLATFORM_FILE_ERROR_INVALID_OPERATION;
+  if (error != base::PLATFORM_FILE_OK &&
+      error != base::PLATFORM_FILE_ERROR_NOT_FOUND)
+    return error;
+
+  // Perform the actual work for directory copy/move.
+  return CopyOrMoveDirectory(src_root_url_, dest_root_url_);
 }
 
 PlatformFileError CrossFileUtilHelper::CopyOrMoveDirectory(
@@ -284,6 +238,7 @@ PlatformFileError CrossFileUtilHelper::CopyOrMoveFile(
     return error;
 
   // For now we don't support non-snapshot file case.
+  // TODO(kinuko): Address this case too.
   DCHECK(!platform_file_path.empty());
 
   scoped_ptr<ScopedFileDeleter> file_deleter;
@@ -348,7 +303,7 @@ base::PlatformFileError FileUtilHelper::Delete(
     bool recursive) {
   if (DirectoryExists(context, file_util, url)) {
     if (!recursive)
-      return file_util->DeleteSingleDirectory(context, url);
+      return file_util->DeleteDirectory(context, url);
     else
       return DeleteDirectoryRecursive(context, file_util, url);
   } else {
@@ -411,14 +366,14 @@ base::PlatformFileError FileUtilHelper::DeleteDirectoryRecursive(
   }
 
   while (!directories.empty()) {
-    PlatformFileError error = file_util->DeleteSingleDirectory(
+    PlatformFileError error = file_util->DeleteDirectory(
         context, url.WithPath(directories.top()));
     if (error != base::PLATFORM_FILE_ERROR_NOT_FOUND &&
         error != base::PLATFORM_FILE_OK)
       return error;
     directories.pop();
   }
-  return file_util->DeleteSingleDirectory(context, url);
+  return file_util->DeleteDirectory(context, url);
 }
 
 }  // namespace fileapi
diff --git a/webkit/fileapi/isolated_file_util.cc b/webkit/fileapi/isolated_file_util.cc
index d50549e..efe485e 100644
--- a/webkit/fileapi/isolated_file_util.cc
+++ b/webkit/fileapi/isolated_file_util.cc
@@ -179,19 +179,4 @@ scoped_ptr<FileSystemFileUtil::AbstractFileEnumerator>
       .PassAs<FileSystemFileUtil::AbstractFileEnumerator>();
 }
 
-bool DraggedFileUtil::IsDirectoryEmpty(
-    FileSystemOperationContext* context,
-    const FileSystemURL& url) {
-  DCHECK(url.is_valid());
-  if (url.path().empty()) {
-    // The root directory case.
-    std::vector<FileInfo> toplevels;
-    bool success = IsolatedContext::GetInstance()->GetDraggedFileInfo(
-        url.filesystem_id(), &toplevels);
-    DCHECK(success);
-    return toplevels.empty();
-  }
-  return NativeFileUtil::IsDirectoryEmpty(url.path());
-}
-
 }  // namespace fileapi
diff --git a/webkit/fileapi/isolated_file_util.h b/webkit/fileapi/isolated_file_util.h
index d6b06e4..e2f65ee 100644
--- a/webkit/fileapi/isolated_file_util.h
+++ b/webkit/fileapi/isolated_file_util.h
@@ -43,9 +43,6 @@ class WEBKIT_STORAGE_EXPORT_PRIVATE DraggedFileUtil : public IsolatedFileUtil {
       FileSystemOperationContext* context,
       const FileSystemURL& root_url,
       bool recursive) OVERRIDE;
-  virtual bool IsDirectoryEmpty(
-      FileSystemOperationContext* context,
-      const FileSystemURL& url) OVERRIDE;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(DraggedFileUtil);
diff --git a/webkit/fileapi/isolated_file_util_unittest.cc b/webkit/fileapi/isolated_file_util_unittest.cc
index b5e77c5..57ef51a 100644
--- a/webkit/fileapi/isolated_file_util_unittest.cc
+++ b/webkit/fileapi/isolated_file_util_unittest.cc
@@ -50,6 +50,14 @@ FilePath GetTopLevelPath(const FilePath& path) {
   return FilePath(components[0]);
 }
 
+bool IsDirectoryEmpty(FileSystemOperationContext* context,
+                      FileSystemFileUtil* file_util,
+                      const FileSystemURL& url) {
+  scoped_ptr<FileSystemFileUtil::AbstractFileEnumerator> file_enum =
+      file_util->CreateFileEnumerator(context, url, false /* recursive */);
+  return file_enum->Next().empty();
+}
+
 }  // namespace
 
 // TODO(kinuko): we should have separate tests for DraggedFileUtil and
@@ -196,8 +204,8 @@ class IsolatedFileUtilTest : public testing::Test {
       if (file_enum2->IsDirectory()) {
         FileSystemOperationContext context1(file_system_context());
         FileSystemOperationContext context2(file_system_context());
-        EXPECT_EQ(file_util1->IsDirectoryEmpty(&context1, url1),
-                  file_util2->IsDirectoryEmpty(&context2, url2));
+        EXPECT_EQ(IsDirectoryEmpty(&context1, file_util1, url1),
+                  IsDirectoryEmpty(&context2, file_util2, url2));
         continue;
       }
       EXPECT_TRUE(file_set1.find(relative) != file_set1.end());
diff --git a/webkit/fileapi/local_file_util.cc b/webkit/fileapi/local_file_util.cc
index 2e47094..2cae136 100644
--- a/webkit/fileapi/local_file_util.cc
+++ b/webkit/fileapi/local_file_util.cc
@@ -191,15 +191,6 @@ PlatformFileError LocalFileUtil::Truncate(
   return NativeFileUtil::Truncate(file_path, length);
 }
 
-bool LocalFileUtil::IsDirectoryEmpty(
-    FileSystemOperationContext* context,
-    const FileSystemURL& url) {
-  FilePath file_path;
-  if (GetLocalFilePath(context, url, &file_path) != base::PLATFORM_FILE_OK)
-    return true;
-  return NativeFileUtil::IsDirectoryEmpty(file_path);
-}
-
 PlatformFileError LocalFileUtil::CopyOrMoveFile(
     FileSystemOperationContext* context,
     const FileSystemURL& src_url,
@@ -218,7 +209,6 @@ PlatformFileError LocalFileUtil::CopyOrMoveFile(
   return NativeFileUtil::CopyOrMoveFile(src_file_path, dest_file_path, copy);
 }
 
-// TODO(dmikurube): Make it independent from CopyOrMoveFile.
 PlatformFileError LocalFileUtil::CopyInForeignFile(
     FileSystemOperationContext* context,
     const FilePath& src_file_path,
@@ -244,14 +234,14 @@ PlatformFileError LocalFileUtil::DeleteFile(
   return NativeFileUtil::DeleteFile(file_path);
 }
 
-PlatformFileError LocalFileUtil::DeleteSingleDirectory(
+PlatformFileError LocalFileUtil::DeleteDirectory(
     FileSystemOperationContext* context,
     const FileSystemURL& url) {
   FilePath file_path;
   PlatformFileError error = GetLocalFilePath(context, url, &file_path);
   if (error != base::PLATFORM_FILE_OK)
     return error;
-  return NativeFileUtil::DeleteSingleDirectory(file_path);
+  return NativeFileUtil::DeleteDirectory(file_path);
 }
 
 base::PlatformFileError LocalFileUtil::CreateSnapshotFile(
@@ -261,9 +251,14 @@ base::PlatformFileError LocalFileUtil::CreateSnapshotFile(
     FilePath* platform_path,
     SnapshotFilePolicy* policy) {
   DCHECK(policy);
+  DCHECK(file_info);
   // We're just returning the local file information.
   *policy = kSnapshotFileLocal;
-  return GetFileInfo(context, url, file_info, platform_path);
+  base::PlatformFileError error =
+      GetFileInfo(context, url, file_info, platform_path);
+  if (error == base::PLATFORM_FILE_OK && file_info->is_directory)
+    return base::PLATFORM_FILE_ERROR_NOT_A_FILE;
+  return error;
 }
 
 }  // namespace fileapi
diff --git a/webkit/fileapi/local_file_util.h b/webkit/fileapi/local_file_util.h
index f17069f..bbe51f1 100644
--- a/webkit/fileapi/local_file_util.h
+++ b/webkit/fileapi/local_file_util.h
@@ -68,9 +68,6 @@ class WEBKIT_STORAGE_EXPORT_PRIVATE LocalFileUtil : public FileSystemFileUtil {
       FileSystemOperationContext* context,
       const FileSystemURL& url,
       int64 length) OVERRIDE;
-  virtual bool IsDirectoryEmpty(
-      FileSystemOperationContext* context,
-      const FileSystemURL& url) OVERRIDE;
   virtual base::PlatformFileError CopyOrMoveFile(
       FileSystemOperationContext* context,
       const FileSystemURL& src_url,
@@ -83,7 +80,7 @@ class WEBKIT_STORAGE_EXPORT_PRIVATE LocalFileUtil : public FileSystemFileUtil {
   virtual base::PlatformFileError DeleteFile(
       FileSystemOperationContext* context,
       const FileSystemURL& url) OVERRIDE;
-  virtual base::PlatformFileError DeleteSingleDirectory(
+  virtual base::PlatformFileError DeleteDirectory(
       FileSystemOperationContext* context,
       const FileSystemURL& url) OVERRIDE;
   virtual base::PlatformFileError CreateSnapshotFile(
diff --git a/webkit/fileapi/media/device_media_file_util.cc b/webkit/fileapi/media/device_media_file_util.cc
index aaf7a97..3de3012 100644
--- a/webkit/fileapi/media/device_media_file_util.cc
+++ b/webkit/fileapi/media/device_media_file_util.cc
@@ -122,24 +122,6 @@ PlatformFileError DeviceMediaFileUtil::Truncate(
   return base::PLATFORM_FILE_ERROR_FAILED;
 }
 
-bool DeviceMediaFileUtil::IsDirectoryEmpty(
-    FileSystemOperationContext* context,
-    const FileSystemURL& url) {
-  MTPDeviceDelegate* delegate = GetMTPDeviceDelegate(context);
-  if (!delegate)
-    return false;
-
-  scoped_ptr<AbstractFileEnumerator> enumerator(
-      CreateFileEnumerator(context, url, false));
-  FilePath path;
-  while (!(path = enumerator->Next()).empty()) {
-    if (enumerator->IsDirectory() ||
-        context->media_path_filter()->Match(path))
-      return false;
-  }
-  return true;
-}
-
 PlatformFileError DeviceMediaFileUtil::CopyOrMoveFile(
     FileSystemOperationContext* context,
     const FileSystemURL& src_url,
@@ -161,7 +143,7 @@ PlatformFileError DeviceMediaFileUtil::DeleteFile(
   return base::PLATFORM_FILE_ERROR_SECURITY;
 }
 
-PlatformFileError DeviceMediaFileUtil::DeleteSingleDirectory(
+PlatformFileError DeviceMediaFileUtil::DeleteDirectory(
     FileSystemOperationContext* context,
     const FileSystemURL& url) {
   return base::PLATFORM_FILE_ERROR_SECURITY;
diff --git a/webkit/fileapi/media/device_media_file_util.h b/webkit/fileapi/media/device_media_file_util.h
index 4564a88..bd6bb80 100644
--- a/webkit/fileapi/media/device_media_file_util.h
+++ b/webkit/fileapi/media/device_media_file_util.h
@@ -65,9 +65,6 @@ class WEBKIT_STORAGE_EXPORT_PRIVATE DeviceMediaFileUtil
       FileSystemOperationContext* context,
       const FileSystemURL& url,
       int64 length) OVERRIDE;
-  virtual bool IsDirectoryEmpty(
-      FileSystemOperationContext* context,
-      const FileSystemURL& url) OVERRIDE;
   virtual base::PlatformFileError CopyOrMoveFile(
       FileSystemOperationContext* context,
       const FileSystemURL& src_url,
@@ -80,7 +77,7 @@ class WEBKIT_STORAGE_EXPORT_PRIVATE DeviceMediaFileUtil
   virtual base::PlatformFileError DeleteFile(
       FileSystemOperationContext* context,
       const FileSystemURL& url) OVERRIDE;
-  virtual base::PlatformFileError DeleteSingleDirectory(
+  virtual base::PlatformFileError DeleteDirectory(
       FileSystemOperationContext* context,
       const FileSystemURL& url) OVERRIDE;
   virtual base::PlatformFileError CreateSnapshotFile(
diff --git a/webkit/fileapi/media/native_media_file_util.cc b/webkit/fileapi/media/native_media_file_util.cc
index 4e99c82..6f7c53c 100644
--- a/webkit/fileapi/media/native_media_file_util.cc
+++ b/webkit/fileapi/media/native_media_file_util.cc
@@ -83,23 +83,6 @@ PlatformFileError NativeMediaFileUtil::Truncate(
   return NativeFileUtil::Truncate(file_path, length);
 }
 
-bool NativeMediaFileUtil::IsDirectoryEmpty(
-    FileSystemOperationContext* context,
-    const FileSystemURL& url) {
-  DCHECK(context);
-  DCHECK(context->media_path_filter());
-
-  scoped_ptr<AbstractFileEnumerator> enumerator(
-      CreateFileEnumerator(context, url, false));
-  FilePath path;
-  while (!(path = enumerator->Next()).empty()) {
-    if (enumerator->IsDirectory() ||
-        context->media_path_filter()->Match(path))
-      return false;
-  }
-  return true;
-}
-
 PlatformFileError NativeMediaFileUtil::CopyOrMoveFile(
     FileSystemOperationContext* context,
     const FileSystemURL& src_url,
@@ -107,14 +90,28 @@ PlatformFileError NativeMediaFileUtil::CopyOrMoveFile(
     bool copy) {
   FilePath src_file_path;
   PlatformFileError error =
-      GetFilteredLocalFilePath(context, src_url, &src_file_path);
+      GetFilteredLocalFilePathForExistingFileOrDirectory(
+          context, src_url,
+          base::PLATFORM_FILE_ERROR_NOT_FOUND,
+          &src_file_path);
   if (error != base::PLATFORM_FILE_OK)
     return error;
+  if (NativeFileUtil::DirectoryExists(src_file_path))
+    return base::PLATFORM_FILE_ERROR_NOT_A_FILE;
 
   FilePath dest_file_path;
-  error = GetFilteredLocalFilePath(context, dest_url, &dest_file_path);
+  error = GetLocalFilePath(context, dest_url, &dest_file_path);
   if (error != base::PLATFORM_FILE_OK)
     return error;
+  PlatformFileInfo file_info;
+  error = NativeFileUtil::GetFileInfo(dest_file_path, &file_info);
+  if (error != base::PLATFORM_FILE_OK &&
+      error != base::PLATFORM_FILE_ERROR_NOT_FOUND)
+    return error;
+  if (error == base::PLATFORM_FILE_OK && file_info.is_directory)
+    return base::PLATFORM_FILE_ERROR_INVALID_OPERATION;
+  if (!context->media_path_filter()->Match(dest_file_path))
+    return base::PLATFORM_FILE_ERROR_SECURITY;
 
   return NativeFileUtil::CopyOrMoveFile(src_file_path, dest_file_path, copy);
 }
@@ -141,6 +138,12 @@ PlatformFileError NativeMediaFileUtil::DeleteFile(
   PlatformFileError error = GetLocalFilePath(context, url, &file_path);
   if (error != base::PLATFORM_FILE_OK)
     return error;
+  PlatformFileInfo file_info;
+  error = NativeFileUtil::GetFileInfo(file_path, &file_info);
+  if (error != base::PLATFORM_FILE_OK)
+    return error;
+  if (file_info.is_directory)
+    return base::PLATFORM_FILE_ERROR_NOT_A_FILE;
   if (!context->media_path_filter()->Match(file_path))
     return base::PLATFORM_FILE_ERROR_NOT_FOUND;
   return NativeFileUtil::DeleteFile(file_path);
diff --git a/webkit/fileapi/media/native_media_file_util.h b/webkit/fileapi/media/native_media_file_util.h
index 4987a05..97b0999 100644
--- a/webkit/fileapi/media/native_media_file_util.h
+++ b/webkit/fileapi/media/native_media_file_util.h
@@ -41,9 +41,6 @@ class WEBKIT_STORAGE_EXPORT_PRIVATE NativeMediaFileUtil
       FileSystemOperationContext* context,
       const FileSystemURL& url,
       int64 length) OVERRIDE;
-  virtual bool IsDirectoryEmpty(
-      FileSystemOperationContext* context,
-      const FileSystemURL& url) OVERRIDE;
   virtual base::PlatformFileError CopyOrMoveFile(
       FileSystemOperationContext* context,
       const FileSystemURL& src_url,
diff --git a/webkit/fileapi/native_file_util.cc b/webkit/fileapi/native_file_util.cc
index c6bc50b..7ee3b04 100644
--- a/webkit/fileapi/native_file_util.cc
+++ b/webkit/fileapi/native_file_util.cc
@@ -202,20 +202,35 @@ bool NativeFileUtil::DirectoryExists(const FilePath& path) {
   return file_util::DirectoryExists(path);
 }
 
-bool NativeFileUtil::IsDirectoryEmpty(const FilePath& path) {
-  return file_util::IsDirectoryEmpty(path);
-}
-
 PlatformFileError NativeFileUtil::CopyOrMoveFile(
     const FilePath& src_path,
     const FilePath& dest_path,
     bool copy) {
+  base::PlatformFileInfo info;
+  base::PlatformFileError error = NativeFileUtil::GetFileInfo(src_path, &info);
+  if (error != base::PLATFORM_FILE_OK)
+    return error;
+  if (info.is_directory)
+    return base::PLATFORM_FILE_ERROR_NOT_A_FILE;
+
+  error = NativeFileUtil::GetFileInfo(dest_path, &info);
+  if (error != base::PLATFORM_FILE_OK &&
+      error != base::PLATFORM_FILE_ERROR_NOT_FOUND)
+    return error;
+  if (info.is_directory)
+    return base::PLATFORM_FILE_ERROR_INVALID_OPERATION;
+  if (error == base::PLATFORM_FILE_ERROR_NOT_FOUND) {
+    error = NativeFileUtil::GetFileInfo(dest_path.DirName(), &info);
+    if (error != base::PLATFORM_FILE_OK)
+      return error;
+    if (!info.is_directory)
+      return base::PLATFORM_FILE_ERROR_NOT_FOUND;
+  }
+
   if (copy) {
-    if (file_util::CopyFile(src_path,
-                            dest_path))
+    if (file_util::CopyFile(src_path, dest_path))
       return base::PLATFORM_FILE_OK;
   } else {
-    DCHECK(!file_util::DirectoryExists(src_path));
     if (file_util::Move(src_path, dest_path))
       return base::PLATFORM_FILE_OK;
   }
@@ -232,17 +247,13 @@ PlatformFileError NativeFileUtil::DeleteFile(const FilePath& path) {
   return base::PLATFORM_FILE_OK;
 }
 
-PlatformFileError NativeFileUtil::DeleteSingleDirectory(const FilePath& path) {
+PlatformFileError NativeFileUtil::DeleteDirectory(const FilePath& path) {
   if (!file_util::PathExists(path))
     return base::PLATFORM_FILE_ERROR_NOT_FOUND;
-  if (!file_util::DirectoryExists(path)) {
-    // TODO(dmikurube): Check if this error code is appropriate.
+  if (!file_util::DirectoryExists(path))
     return base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY;
-  }
-  if (!file_util::IsDirectoryEmpty(path)) {
-    // TODO(dmikurube): Check if this error code is appropriate.
+  if (!file_util::IsDirectoryEmpty(path))
     return base::PLATFORM_FILE_ERROR_NOT_EMPTY;
-  }
   if (!file_util::Delete(path, false))
     return base::PLATFORM_FILE_ERROR_FAILED;
   return base::PLATFORM_FILE_OK;
diff --git a/webkit/fileapi/native_file_util.h b/webkit/fileapi/native_file_util.h
index ff288e5..f0e1329 100644
--- a/webkit/fileapi/native_file_util.h
+++ b/webkit/fileapi/native_file_util.h
@@ -18,8 +18,18 @@ class Time;
 
 namespace fileapi {
 
+// A thin wrapper class for accessing the OS native filesystem.
+// This performs common error checks necessary to implement FileUtil family
+// in addition to perform native filesystem operations.
+//
+// For the error checks it performs please see the comment for
+// FileSystemFileUtil interface (webkit/fileapi/file_system_file_util.h).
+//
+// Note that all the methods of this class are static and this does NOT
+// inherit from FileSystemFileUtil.
+//
 // TODO(dmikurube): Add unit tests for NativeFileUtil.
-// This class handles accessing the OS native filesystem.
+// http://crbug.com/170564
 class WEBKIT_STORAGE_EXPORT_PRIVATE NativeFileUtil {
  public:
   static base::PlatformFileError CreateOrOpen(
@@ -44,12 +54,11 @@ class WEBKIT_STORAGE_EXPORT_PRIVATE NativeFileUtil {
   static base::PlatformFileError Truncate(const FilePath& path, int64 length);
   static bool PathExists(const FilePath& path);
   static bool DirectoryExists(const FilePath& path);
-  static bool IsDirectoryEmpty(const FilePath& path);
   static base::PlatformFileError CopyOrMoveFile(const FilePath& src_path,
                                                 const FilePath& dest_path,
                                                 bool copy);
   static base::PlatformFileError DeleteFile(const FilePath& path);
-  static base::PlatformFileError DeleteSingleDirectory(const FilePath& path);
+  static base::PlatformFileError DeleteDirectory(const FilePath& path);
 
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(NativeFileUtil);
diff --git a/webkit/fileapi/obfuscated_file_util.cc b/webkit/fileapi/obfuscated_file_util.cc
index 09ed25c..4e743c6 100644
--- a/webkit/fileapi/obfuscated_file_util.cc
+++ b/webkit/fileapi/obfuscated_file_util.cc
@@ -572,31 +572,6 @@ PlatformFileError ObfuscatedFileUtil::Truncate(
   return error;
 }
 
-bool ObfuscatedFileUtil::IsDirectoryEmpty(
-    FileSystemOperationContext* context,
-    const FileSystemURL& url) {
-  FileSystemDirectoryDatabase* db = GetDirectoryDatabase(
-      url.origin(), url.type(), false);
-  if (!db)
-    return true;  // Not a great answer, but it's what others do.
-  FileId file_id;
-  if (!db->GetFileWithPath(url.path(), &file_id))
-    return true;  // Ditto.
-  FileInfo file_info;
-  if (!db->GetFileInfo(file_id, &file_info)) {
-    DCHECK(!file_id);
-    // It's the root directory and the database hasn't been initialized yet.
-    return true;
-  }
-  if (!file_info.is_directory())
-    return true;
-  std::vector<FileId> children;
-  // TODO(ericu): This could easily be made faster with help from the database.
-  if (!db->ListChildren(file_id, &children))
-    return true;
-  return children.empty();
-}
-
 PlatformFileError ObfuscatedFileUtil::CopyOrMoveFile(
     FileSystemOperationContext* context,
     const FileSystemURL& src_url,
@@ -628,7 +603,7 @@ PlatformFileError ObfuscatedFileUtil::CopyOrMoveFile(
   if (error != base::PLATFORM_FILE_OK)
     return error;
   if (src_file_info.is_directory())
-    return base::PLATFORM_FILE_ERROR_FAILED;
+    return base::PLATFORM_FILE_ERROR_NOT_A_FILE;
 
   FileInfo dest_file_info;
   base::PlatformFileInfo dest_platform_file_info;  // overwrite case only
@@ -642,13 +617,12 @@ PlatformFileError ObfuscatedFileUtil::CopyOrMoveFile(
     else if (error != base::PLATFORM_FILE_OK)
       return error;
     else if (dest_file_info.is_directory())
-      return base::PLATFORM_FILE_ERROR_FAILED;
+      return base::PLATFORM_FILE_ERROR_INVALID_OPERATION;
   }
   if (!overwrite) {
     FileId dest_parent_id;
     if (!db->GetFileWithPath(dest_url.path().DirName(),
                              &dest_parent_id)) {
-      NOTREACHED();  // We shouldn't be called in this case.
       return base::PLATFORM_FILE_ERROR_NOT_FOUND;
     }
 
@@ -768,16 +742,16 @@ PlatformFileError ObfuscatedFileUtil::CopyInForeignFile(
     else if (error != base::PLATFORM_FILE_OK)
       return error;
     else if (dest_file_info.is_directory())
-      return base::PLATFORM_FILE_ERROR_FAILED;
+      return base::PLATFORM_FILE_ERROR_INVALID_OPERATION;
   }
   if (!overwrite) {
     FileId dest_parent_id;
     if (!db->GetFileWithPath(dest_url.path().DirName(),
-                             &dest_parent_id) ||
-        !dest_file_info.is_directory()) {
-      NOTREACHED();
+                             &dest_parent_id)) {
       return base::PLATFORM_FILE_ERROR_NOT_FOUND;
     }
+    if (!dest_file_info.is_directory())
+      return base::PLATFORM_FILE_ERROR_FAILED;
     InitFileInfo(&dest_file_info, dest_parent_id,
                  VirtualPath::BaseName(dest_url.path()).value());
   }
@@ -865,7 +839,7 @@ PlatformFileError ObfuscatedFileUtil::DeleteFile(
   return base::PLATFORM_FILE_OK;
 }
 
-PlatformFileError ObfuscatedFileUtil::DeleteSingleDirectory(
+PlatformFileError ObfuscatedFileUtil::DeleteDirectory(
     FileSystemOperationContext* context,
     const FileSystemURL& url) {
   FileSystemDirectoryDatabase* db = GetDirectoryDatabase(
@@ -877,10 +851,12 @@ PlatformFileError ObfuscatedFileUtil::DeleteSingleDirectory(
   if (!db->GetFileWithPath(url.path(), &file_id))
     return base::PLATFORM_FILE_ERROR_NOT_FOUND;
   FileInfo file_info;
-  if (!db->GetFileInfo(file_id, &file_info) || !file_info.is_directory()) {
+  if (!db->GetFileInfo(file_id, &file_info)) {
     NOTREACHED();
     return base::PLATFORM_FILE_ERROR_FAILED;
   }
+  if (!file_info.is_directory())
+    return base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY;
   if (!db->RemoveFileInfo(file_id))
     return base::PLATFORM_FILE_ERROR_NOT_EMPTY;
   int64 growth = -UsageForPath(file_info.name.size());
@@ -901,7 +877,36 @@ base::PlatformFileError ObfuscatedFileUtil::CreateSnapshotFile(
   DCHECK(policy);
   // We're just returning the local file information.
   *policy = kSnapshotFileLocal;
-  return GetFileInfo(context, url, file_info, platform_path);
+  base::PlatformFileError error = GetFileInfo(
+      context, url, file_info, platform_path);
+  if (error == base::PLATFORM_FILE_OK && file_info->is_directory)
+    return base::PLATFORM_FILE_ERROR_NOT_A_FILE;
+  return error;
+}
+
+bool ObfuscatedFileUtil::IsDirectoryEmpty(
+    FileSystemOperationContext* context,
+    const FileSystemURL& url) {
+  FileSystemDirectoryDatabase* db = GetDirectoryDatabase(
+      url.origin(), url.type(), false);
+  if (!db)
+    return true;  // Not a great answer, but it's what others do.
+  FileId file_id;
+  if (!db->GetFileWithPath(url.path(), &file_id))
+    return true;  // Ditto.
+  FileInfo file_info;
+  if (!db->GetFileInfo(file_id, &file_info)) {
+    DCHECK(!file_id);
+    // It's the root directory and the database hasn't been initialized yet.
+    return true;
+  }
+  if (!file_info.is_directory())
+    return true;
+  std::vector<FileId> children;
+  // TODO(ericu): This could easily be made faster with help from the database.
+  if (!db->ListChildren(file_id, &children))
+    return true;
+  return children.empty();
 }
 
 FilePath ObfuscatedFileUtil::GetDirectoryForOriginAndType(
diff --git a/webkit/fileapi/obfuscated_file_util.h b/webkit/fileapi/obfuscated_file_util.h
index 039936a..771f7ce 100644
--- a/webkit/fileapi/obfuscated_file_util.h
+++ b/webkit/fileapi/obfuscated_file_util.h
@@ -98,9 +98,6 @@ class WEBKIT_STORAGE_EXPORT_PRIVATE ObfuscatedFileUtil
       FileSystemOperationContext* context,
       const FileSystemURL& url,
       int64 length) OVERRIDE;
-  virtual bool IsDirectoryEmpty(
-      FileSystemOperationContext* context,
-      const FileSystemURL& url) OVERRIDE;
   virtual base::PlatformFileError CopyOrMoveFile(
       FileSystemOperationContext* context,
       const FileSystemURL& src_url,
@@ -113,7 +110,7 @@ class WEBKIT_STORAGE_EXPORT_PRIVATE ObfuscatedFileUtil
   virtual base::PlatformFileError DeleteFile(
       FileSystemOperationContext* context,
       const FileSystemURL& url) OVERRIDE;
-  virtual base::PlatformFileError DeleteSingleDirectory(
+  virtual base::PlatformFileError DeleteDirectory(
       FileSystemOperationContext* context,
       const FileSystemURL& url) OVERRIDE;
   virtual base::PlatformFileError CreateSnapshotFile(
@@ -123,6 +120,11 @@ class WEBKIT_STORAGE_EXPORT_PRIVATE ObfuscatedFileUtil
       FilePath* platform_path,
       SnapshotFilePolicy* policy) OVERRIDE;
 
+  // Returns true if the directory |url| is empty.
+  bool IsDirectoryEmpty(
+      FileSystemOperationContext* context,
+      const FileSystemURL& url);
+
   // Gets the topmost directory specific to this origin and type.  This will
   // contain both the directory database's files and all the backing file
   // subdirectories.
diff --git a/webkit/fileapi/obfuscated_file_util_unittest.cc b/webkit/fileapi/obfuscated_file_util_unittest.cc
index fea134e..af92761 100644
--- a/webkit/fileapi/obfuscated_file_util_unittest.cc
+++ b/webkit/fileapi/obfuscated_file_util_unittest.cc
@@ -918,7 +918,7 @@ TEST_F(ObfuscatedFileUtilTest, TestDirectoryOps) {
 
   context.reset(NewContext(NULL));
   EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND,
-      ofu()->DeleteSingleDirectory(context.get(), url));
+      ofu()->DeleteDirectory(context.get(), url));
 
   FileSystemURL root = CreateURLFromUTF8("");
   EXPECT_FALSE(DirectoryExists(url));
@@ -947,8 +947,8 @@ TEST_F(ObfuscatedFileUtilTest, TestDirectoryOps) {
   // Can't remove a non-empty directory.
   context.reset(NewContext(NULL));
   EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_EMPTY,
-      ofu()->DeleteSingleDirectory(context.get(),
-                                   url.WithPath(url.path().DirName())));
+      ofu()->DeleteDirectory(context.get(),
+                             url.WithPath(url.path().DirName())));
   EXPECT_TRUE(change_observer()->HasNoChange());
 
   base::PlatformFileInfo file_info;
@@ -976,8 +976,7 @@ TEST_F(ObfuscatedFileUtilTest, TestDirectoryOps) {
   // frees up quota from its path.
   context.reset(NewContext(NULL));
   context->set_allowed_bytes_growth(0);
-  EXPECT_EQ(base::PLATFORM_FILE_OK,
-      ofu()->DeleteSingleDirectory(context.get(), url));
+  EXPECT_EQ(base::PLATFORM_FILE_OK, ofu()->DeleteDirectory(context.get(), url));
   EXPECT_EQ(1, change_observer()->get_and_reset_remove_directory_count());
   EXPECT_EQ(ObfuscatedFileUtil::ComputeFilePathCost(url.path()),
       context->allowed_bytes_growth());
@@ -1823,9 +1822,8 @@ TEST_F(ObfuscatedFileUtilTest, TestDirectoryTimestampForDeletion) {
             ofu()->DeleteFile(context.get(), url));
   EXPECT_EQ(base::Time(), GetModifiedTime(dir_url));
 
-  // DeleteSingleDirectory, fail case.
-  url = dir_url.WithPath(
-      dir_url.path().AppendASCII("DeleteSingleDirectory_dir"));
+  // DeleteDirectory, fail case.
+  url = dir_url.WithPath(dir_url.path().AppendASCII("DeleteDirectory_dir"));
   FileSystemURL file_path(url.WithPath(url.path().AppendASCII("pakeratta")));
   context.reset(NewContext(NULL));
   EXPECT_EQ(base::PLATFORM_FILE_OK,
@@ -1839,7 +1837,7 @@ TEST_F(ObfuscatedFileUtilTest, TestDirectoryTimestampForDeletion) {
   ClearTimestamp(dir_url);
   context.reset(NewContext(NULL));
   EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_EMPTY,
-            ofu()->DeleteSingleDirectory(context.get(), url));
+            ofu()->DeleteDirectory(context.get(), url));
   EXPECT_EQ(base::Time(), GetModifiedTime(dir_url));
 
   // delete case.
@@ -1849,8 +1847,7 @@ TEST_F(ObfuscatedFileUtilTest, TestDirectoryTimestampForDeletion) {
 
   ClearTimestamp(dir_url);
   context.reset(NewContext(NULL));
-  EXPECT_EQ(base::PLATFORM_FILE_OK,
-            ofu()->DeleteSingleDirectory(context.get(), url));
+  EXPECT_EQ(base::PLATFORM_FILE_OK, ofu()->DeleteDirectory(context.get(), url));
   EXPECT_NE(base::Time(), GetModifiedTime(dir_url));
 }
 
diff --git a/webkit/fileapi/syncable/local_file_sync_context_unittest.cc b/webkit/fileapi/syncable/local_file_sync_context_unittest.cc
index 4335fca..0f2ad44 100644
--- a/webkit/fileapi/syncable/local_file_sync_context_unittest.cc
+++ b/webkit/fileapi/syncable/local_file_sync_context_unittest.cc
@@ -572,7 +572,7 @@ TEST_F(LocalFileSyncContextTest, ApplyRemoteChangeForAddOrUpdate) {
   // with wrong file type will result in error.
   change = FileChange(FileChange::FILE_CHANGE_ADD_OR_UPDATE,
                       SYNC_FILE_TYPE_FILE);
-  EXPECT_EQ(SYNC_FILE_ERROR_FAILED,
+  EXPECT_NE(SYNC_STATUS_OK,
             ApplyRemoteChange(file_system.file_system_context(),
                               change, kFilePath1, kDir,
                               SYNC_FILE_TYPE_DIRECTORY));