Enable cross-filesystem moves and copies.

BUG=none
TEST=unit tests included; I also tested locally with a cross-filesystem-sync-copy layout test [that I'll add to webkit later], both with and without obfuscation.


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

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

webkit/fileapi/file_system_file_util.cc

@@ -13,6 +13,21 @@
 
 namespace fileapi {
 
+namespace {
+
+// This assumes that the root exists.
+bool ParentExists(FileSystemOperationContext* context,
+    FileSystemFileUtil* file_util, const FilePath& file_path) {
+  // If file_path is in the root, file_path.DirName() will be ".",
+  // since we use paths with no leading '/'.
+  FilePath parent = file_path.DirName();
+  if (parent == FilePath(FILE_PATH_LITERAL(".")))
+    return true;
+  return file_util->DirectoryExists(context, parent);
+}
+
+}
+
 // static
 FileSystemFileUtil* FileSystemFileUtil::GetInstance() {
   return Singleton<FileSystemFileUtil>::get();
@@ -148,9 +163,8 @@ PlatformFileError FileSystemFileUtil::Copy(
   if (DirectoryExists(context, src_file_path))
     return CopyOrMoveDirectory(context, src_file_path, dest_file_path,
                                true /* copy */);
-  else
-    return CopyOrMoveFile(context, src_file_path, dest_file_path,
-                          true /* copy */);
+  return CopyOrMoveFileHelper(context, src_file_path, dest_file_path,
+                              true /* copy */);
 }
 
 PlatformFileError FileSystemFileUtil::Move(
@@ -168,9 +182,8 @@ PlatformFileError FileSystemFileUtil::Move(
   if (DirectoryExists(context, src_file_path))
     return CopyOrMoveDirectory(context, src_file_path, dest_file_path,
                                false /* copy */);
-  else
-    return CopyOrMoveFile(context, src_file_path, dest_file_path,
-                          false /* copy */);
+  return CopyOrMoveFileHelper(context, src_file_path, dest_file_path,
+                              false /* copy */);
 }
 
 PlatformFileError FileSystemFileUtil::Delete(
@@ -222,26 +235,50 @@ FileSystemFileUtil::PerformCommonCheckAndPreparationForMoveAndCopy(
     FileSystemOperationContext* context,
     const FilePath& src_file_path,
     const FilePath& dest_file_path) {
+  bool same_file_system =
+     (context->src_origin_url() == context->dest_origin_url()) &&
+     (context->src_type() == context->dest_type());
+  FileSystemFileUtil* dest_util = context->dest_file_system_file_util();
+  DCHECK(dest_util);
+  FileSystemOperationContext local_dest_context(
+      context->file_system_context(), dest_util);
+  FileSystemOperationContext* dest_context;
+  if (same_file_system) {
+    dest_context = context;
+    DCHECK(context->src_file_system_file_util() ==
+         context->dest_file_system_file_util());
+  } else {
+    // All the single-path virtual FSFU methods expect the context information
+    // to be in the src_* variables, not the dest_* variables, so we have to
+    // make a new context if we want to call them on the dest_file_path.
+    dest_context = &local_dest_context;
+    dest_context->set_src_type(context->dest_type());
+    dest_context->set_src_origin_url(context->dest_origin_url());
+    dest_context->set_src_virtual_path(context->dest_virtual_path());
+    dest_context->set_allowed_bytes_growth(context->allowed_bytes_growth());
+  }
+
   // Exits earlier if the source path does not exist.
   if (!PathExists(context, src_file_path))
     return base::PLATFORM_FILE_ERROR_NOT_FOUND;
 
   // The parent of the |dest_file_path| does not exist.
-  if (!DirectoryExists(context, dest_file_path.DirName()))
+  if (!ParentExists(dest_context, dest_util, dest_file_path))
     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))
+  if (same_file_system && 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 (!PathExists(context, dest_file_path))
+  if (!dest_util->PathExists(dest_context, 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 = DirectoryExists(context, src_file_path);
-  bool dest_is_directory = DirectoryExists(context, dest_file_path);
+  bool dest_is_directory =
+      dest_util->DirectoryExists(dest_context, dest_file_path);
   if (src_is_directory && !dest_is_directory)
     return base::PLATFORM_FILE_ERROR_INVALID_OPERATION;
 
@@ -251,7 +288,7 @@ FileSystemFileUtil::PerformCommonCheckAndPreparationForMoveAndCopy(
     return base::PLATFORM_FILE_ERROR_INVALID_OPERATION;
 
   // It is an error to copy/move an entry into the same path.
-  if (src_file_path.value() == dest_file_path.value())
+  if (same_file_system && (src_file_path.value() == dest_file_path.value()))
     return base::PLATFORM_FILE_ERROR_EXISTS;
 
   if (dest_is_directory) {
@@ -261,8 +298,9 @@ FileSystemFileUtil::PerformCommonCheckAndPreparationForMoveAndCopy(
     // on all platforms, so we delete the destination directory here.
     // TODO(kinuko): may be better to change the file_util::{Copy,Move}.
     if (base::PLATFORM_FILE_OK !=
-        Delete(context, dest_file_path, false /* recursive */)) {
-      if (!IsDirectoryEmpty(context, dest_file_path))
+        dest_util->Delete(dest_context, dest_file_path,
+                          false /* recursive */)) {
+      if (!dest_util->IsDirectoryEmpty(dest_context, dest_file_path))
         return base::PLATFORM_FILE_ERROR_NOT_EMPTY;
       return base::PLATFORM_FILE_ERROR_FAILED;
     }
@@ -286,19 +324,39 @@ PlatformFileError FileSystemFileUtil::CopyOrMoveFile(
   return base::PLATFORM_FILE_ERROR_FAILED;
 }
 
+PlatformFileError FileSystemFileUtil::CopyInForeignFile(
+      FileSystemOperationContext* context,
+      const FilePath& src_file_path,
+      const FilePath& dest_file_path) {
+  return CopyOrMoveFile(context, src_file_path, dest_file_path, true);
+}
+
 PlatformFileError FileSystemFileUtil::CopyOrMoveDirectory(
       FileSystemOperationContext* context,
       const FilePath& src_file_path,
       const FilePath& dest_file_path,
       bool copy) {
+  FileSystemFileUtil* dest_util = context->dest_file_system_file_util();
+  FileSystemOperationContext dest_context(
+      context->file_system_context(), dest_util);
+  // All the single-path virtual FSFU methods expect the context information to
+  // be in the src_* variables, not the dest_* variables, so we have to make a
+  // new context if we want to call them on the dest_file_path.
+  dest_context.set_src_type(context->dest_type());
+  dest_context.set_src_origin_url(context->dest_origin_url());
+  dest_context.set_src_virtual_path(context->dest_virtual_path());
+  dest_context.set_allowed_bytes_growth(context->allowed_bytes_growth());
+
   // Re-check PerformCommonCheckAndPreparationForMoveAndCopy() by DCHECK.
   DCHECK(DirectoryExists(context, src_file_path));
-  DCHECK(DirectoryExists(context, dest_file_path.DirName()));
-  DCHECK(!src_file_path.IsParent(dest_file_path));
-  DCHECK(!PathExists(context, dest_file_path));
-
-  if (!DirectoryExists(context, dest_file_path)) {
-    PlatformFileError error = CreateDirectory(context,
+  DCHECK(ParentExists(&dest_context, dest_util, dest_file_path));
+  DCHECK(!dest_util->PathExists(&dest_context, dest_file_path));
+  if ((context->src_origin_url() == context->dest_origin_url()) &&
+      (context->src_type() == context->dest_type()))
+    DCHECK(!src_file_path.IsParent(dest_file_path));
+
+  if (!dest_util->DirectoryExists(&dest_context, dest_file_path)) {
+    PlatformFileError error = dest_util->CreateDirectory(&dest_context,
         dest_file_path, false, false);
     if (error != base::PLATFORM_FILE_OK)
       return error;
@@ -312,13 +370,12 @@ PlatformFileError FileSystemFileUtil::CopyOrMoveDirectory(
     src_file_path.AppendRelativePath(src_file_path_each, &dest_file_path_each);
 
     if (file_enum->IsDirectory()) {
-      PlatformFileError error = CreateDirectory(context,
+      PlatformFileError error = dest_util->CreateDirectory(&dest_context,
           dest_file_path_each, false, false);
       if (error != base::PLATFORM_FILE_OK)
         return error;
     } else {
-      // CopyOrMoveFile here is the virtual overridden member function.
-      PlatformFileError error = CopyOrMoveFile(
+      PlatformFileError error = CopyOrMoveFileHelper(
           context, src_file_path_each, dest_file_path_each, copy);
       if (error != base::PLATFORM_FILE_OK)
         return error;
@@ -333,6 +390,35 @@ PlatformFileError FileSystemFileUtil::CopyOrMoveDirectory(
   return base::PLATFORM_FILE_OK;
 }
 
+PlatformFileError FileSystemFileUtil::CopyOrMoveFileHelper(
+    FileSystemOperationContext* context,
+    const FilePath& src_file_path,
+    const FilePath& dest_file_path,
+    bool copy) {
+  // CopyOrMoveFile here is the virtual overridden member function.
+  if ((context->src_origin_url() == context->dest_origin_url()) &&
+      (context->src_type() == context->dest_type())) {
+    DCHECK(context->src_file_system_file_util() ==
+           context->dest_file_system_file_util());
+    return CopyOrMoveFile(context, src_file_path, dest_file_path, copy);
+  }
+  base::PlatformFileInfo file_info;
+  FilePath platform_file_path;
+  PlatformFileError error_code;
+  error_code =
+      GetFileInfo(context, src_file_path, &file_info, &platform_file_path);
+  if (error_code != base::PLATFORM_FILE_OK)
+    return error_code;
+
+  DCHECK(context->dest_file_system_file_util());
+  error_code = context->dest_file_system_file_util()->CopyInForeignFile(
+      context, platform_file_path, dest_file_path);
+  if (copy || error_code != base::PLATFORM_FILE_OK)
+    return error_code;
+  return DeleteFile(context, src_file_path);
+}
+
+
 PlatformFileError FileSystemFileUtil::DeleteFile(
     FileSystemOperationContext* unused,
     const FilePath& file_path) {

webkit/fileapi/file_system_file_util.h

@@ -116,6 +116,14 @@ class FileSystemFileUtil {
       const FilePath& dest_file_path,
       bool copy);
 
+  // Copies in a single file from a different filesystem.  The src_file_path is
+  // a true local platform path, regardless of which subclass of
+  // FileSystemFileUtil is being invoked.
+  virtual PlatformFileError CopyInForeignFile(
+        FileSystemOperationContext* context,
+        const FilePath& src_file_path,
+        const FilePath& dest_file_path);
+
   // Copies a file or a directory from |src_file_path| to |dest_file_path|.
   //
   // Error cases:
@@ -254,6 +262,16 @@ class FileSystemFileUtil {
       const FilePath& dest_file_path,
       bool copy);
 
+  // Determines whether a simple same-filesystem move or copy can be done.  If
+  // so, it delegates to CopyOrMoveFile.  Otherwise it looks up the true
+  // platform path of the source file, delegates to CopyInForeignFile, and [for
+  // move] calls DeleteFile on the source file.
+  PlatformFileError CopyOrMoveFileHelper(
+      FileSystemOperationContext* context,
+      const FilePath& src_file_path,
+      const FilePath& dest_file_path,
+      bool copy);
+
   // Returns a pointer to a new instance of AbstractFileEnumerator which is
   // implemented for each FileUtil subclass. The instance needs to be freed
   // by the caller, and its lifetime should not extend past when the current

webkit/fileapi/file_system_file_util_unittest.cc

+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/file_path.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/platform_file.h"
+#include "base/scoped_temp_dir.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "webkit/fileapi/file_system_context.h"
+#include "webkit/fileapi/file_system_operation_context.h"
+#include "webkit/fileapi/file_system_test_helper.h"
+#include "webkit/fileapi/local_file_system_file_util.h"
+#include "webkit/fileapi/obfuscated_file_system_file_util.h"
+
+using namespace fileapi;
+
+namespace {
+
+struct CopyMoveTestCaseRecord {
+  bool is_directory;
+  const FilePath::CharType path[64];
+  int64 data_file_size;
+};
+
+const CopyMoveTestCaseRecord kCopyMoveTestCases[] = {
+  {true, FILE_PATH_LITERAL("dir a"), 0},
+  {true, FILE_PATH_LITERAL("dir a/dir a"), 0},
+  {true, FILE_PATH_LITERAL("dir a/dir d"), 0},
+  {true, FILE_PATH_LITERAL("dir a/dir d/dir e"), 0},
+  {true, FILE_PATH_LITERAL("dir a/dir d/dir e/dir f"), 0},
+  {true, FILE_PATH_LITERAL("dir a/dir d/dir e/dir g"), 0},
+  {true, FILE_PATH_LITERAL("dir a/dir d/dir e/dir h"), 0},
+  {true, FILE_PATH_LITERAL("dir b"), 0},
+  {true, FILE_PATH_LITERAL("dir b/dir a"), 0},
+  {true, FILE_PATH_LITERAL("dir c"), 0},
+  {false, FILE_PATH_LITERAL("file 0"), 38},
+  {false, FILE_PATH_LITERAL("file 2"), 60},
+  {false, FILE_PATH_LITERAL("file 3"), 0},
+  {false, FILE_PATH_LITERAL("dir a/file 0"), 39},
+  {false, FILE_PATH_LITERAL("dir a/dir d/dir e/dir g/file 0"), 40},
+  {false, FILE_PATH_LITERAL("dir a/dir d/dir e/dir g/file 1"), 41},
+  {false, FILE_PATH_LITERAL("dir a/dir d/dir e/dir g/file 2"), 42},
+  {false, FILE_PATH_LITERAL("dir a/dir d/dir e/dir g/file 3"), 50},
+};
+
+}  // namespace (anonymous)
+
+// This is not yet a full unit test for FileSystemFileUtil.  TODO(ericu): Adapt
+// the other subclasses' unit tests, as mentioned in the comments in
+// ObfuscatedFileSystemFileUtil's unit test.
+// Currently this is just a test of cross-filesystem copy and move, which
+// actually exercises subclasses of FileSystemFileUtil as well as the class
+// itself.  We currently only test copies between obfuscated filesystems.
+// TODO(ericu): Add a test for copying between obfuscated and local filesystems,
+// and between different local filesystems.
+class FileSystemFileUtilTest : public testing::Test {
+ public:
+  FileSystemFileUtilTest() {
+  }
+
+  void SetUp() {
+  }
+
+  FileSystemOperationContext* NewContext(FileSystemTestOriginHelper* helper) {
+    FileSystemOperationContext* context = helper->NewOperationContext();
+    context->set_allowed_bytes_growth(1024 * 1024);
+    return context;
+  }
+
+  void TestCrossFileSystemCopyMoveHelper(
+      const GURL& src_origin, fileapi::FileSystemType src_type,
+      const GURL& dest_origin, fileapi::FileSystemType dest_type,
+      bool copy) {
+    ScopedTempDir src_dir;
+    ASSERT_TRUE(src_dir.CreateUniqueTempDir());
+    scoped_refptr<ObfuscatedFileSystemFileUtil> src_util(
+        new ObfuscatedFileSystemFileUtil(src_dir.path()));
+    FileSystemTestOriginHelper src_helper(src_origin, src_type);
+    src_helper.SetUp(src_dir.path(),
+                            false,  // incognito
+                            false,  // unlimited quota
+                            NULL,  // quota::QuotaManagerProxy
+                            src_util.get());
+    ScopedTempDir dest_dir;
+    ASSERT_TRUE(dest_dir.CreateUniqueTempDir());
+    scoped_refptr<ObfuscatedFileSystemFileUtil> dest_util(
+        new ObfuscatedFileSystemFileUtil(dest_dir.path()));
+    FileSystemTestOriginHelper dest_helper(dest_origin, dest_type);
+    dest_helper.SetUp(dest_dir.path(),
+                           false,  // incognito
+                           false,  // unlimited quota
+                           NULL,  // quota::QuotaManagerProxy
+                           dest_util.get());
+
+    // Set up all the source data.
+    scoped_ptr<FileSystemOperationContext> context;
+    FilePath test_root(FILE_PATH_LITERAL("root directory"));
+    for (size_t i = 0; i < arraysize(kCopyMoveTestCases); ++i) {
+      SCOPED_TRACE(testing::Message() << "Creating kCopyMoveTestCases " << i);
+      const CopyMoveTestCaseRecord& test_case = kCopyMoveTestCases[i];
+      FilePath path = test_root.Append(test_case.path);
+      if (test_case.is_directory) {
+        context.reset(NewContext(&src_helper));
+        ASSERT_EQ(base::PLATFORM_FILE_OK,
+            src_util->CreateDirectory(context.get(), path, true, true));
+      } else {
+        context.reset(NewContext(&src_helper));
+        bool created = false;
+        ASSERT_EQ(base::PLATFORM_FILE_OK,
+            src_util->EnsureFileExists(context.get(), path, &created));
+        ASSERT_TRUE(created);
+        context.reset(NewContext(&src_helper));
+        ASSERT_EQ(base::PLATFORM_FILE_OK, src_util->Truncate(
+            context.get(), path, test_case.data_file_size));
+      }
+    }
+
+    // Do the actual copy or move.
+    FileSystemContext* file_system_context = dest_helper.file_system_context();
+    scoped_ptr<FileSystemOperationContext> copy_context(
+        new FileSystemOperationContext(file_system_context, NULL));
+    copy_context->set_src_file_system_file_util(src_util);
+    copy_context->set_dest_file_system_file_util(dest_util);
+    copy_context->set_src_origin_url(src_helper.origin());
+    copy_context->set_dest_origin_url(dest_helper.origin());
+    copy_context->set_src_type(src_helper.type());
+    copy_context->set_dest_type(dest_helper.type());
+    copy_context->set_allowed_bytes_growth(1024 * 1024);
+
+    if (copy)
+      ASSERT_EQ(base::PLATFORM_FILE_OK,
+          src_util->Copy(copy_context.get(), test_root, test_root));
+    else
+      ASSERT_EQ(base::PLATFORM_FILE_OK,
+          src_util->Move(copy_context.get(), test_root, test_root));
+
+    // Validate that the destination paths are correct.
+    for (size_t i = 0; i < arraysize(kCopyMoveTestCases); ++i) {
+      SCOPED_TRACE(testing::Message() << "Validating kCopyMoveTestCases " << i);
+      const CopyMoveTestCaseRecord& test_case = kCopyMoveTestCases[i];
+      FilePath path = test_root.Append(test_case.path);
+      SCOPED_TRACE(testing::Message() << "Path is " << test_case.path);
+
+      base::PlatformFileInfo dest_file_info;
+      FilePath data_path;
+      context.reset(NewContext(&dest_helper));
+      EXPECT_EQ(base::PLATFORM_FILE_OK,
+          dest_util->GetFileInfo(
+              context.get(), path, &dest_file_info, &data_path));
+      if (test_case.is_directory) {
+        EXPECT_TRUE(dest_file_info.is_directory);
+      } else {
+        base::PlatformFileInfo platform_file_info;
+        ASSERT_TRUE(file_util::GetFileInfo(data_path, &platform_file_info));
+        EXPECT_EQ(test_case.data_file_size, platform_file_info.size);
+        EXPECT_FALSE(platform_file_info.is_directory);
+        EXPECT_EQ(platform_file_info.size, dest_file_info.size);
+        EXPECT_FALSE(dest_file_info.is_directory);
+      }
+    }
+
+    // Validate that the source paths are still there [for a copy] or gone [for
+    // a move].
+    for (size_t i = 0; i < arraysize(kCopyMoveTestCases); ++i) {
+      SCOPED_TRACE(testing::Message() << "Validating kCopyMoveTestCases " <<
+          i);
+      const CopyMoveTestCaseRecord& test_case = kCopyMoveTestCases[i];
+      FilePath path = test_root.Append(test_case.path);
+      SCOPED_TRACE(testing::Message() << "Path is " << test_case.path);
+
+      base::PlatformFileInfo src_file_info;
+      FilePath data_path;
+      context.reset(NewContext(&src_helper));
+      base::PlatformFileError expected_result;
+      if (copy)
+        expected_result = base::PLATFORM_FILE_OK;
+      else
+        expected_result = base::PLATFORM_FILE_ERROR_NOT_FOUND;
+      EXPECT_EQ(expected_result,
+          src_util->GetFileInfo(
+              context.get(), path, &src_file_info, &data_path));
+    }
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(FileSystemFileUtilTest);
+};
+
+TEST_F(FileSystemFileUtilTest, TestCrossFileSystemCopyDifferentOrigins) {
+  GURL src_origin("http://www.example.com");
+  fileapi::FileSystemType type = kFileSystemTypePersistent;
+  GURL dest_origin("http://www.not.the.same.domain.com");
+
+  TestCrossFileSystemCopyMoveHelper(src_origin, type, dest_origin, type, true);
+}
+
+TEST_F(FileSystemFileUtilTest, TestCrossFileSystemCopySameOrigin) {
+  GURL origin("http://www.example.com");
+  fileapi::FileSystemType src_type = kFileSystemTypePersistent;
+  fileapi::FileSystemType dest_type = kFileSystemTypeTemporary;
+
+  TestCrossFileSystemCopyMoveHelper(origin, src_type, origin, dest_type, true);
+}
+
+TEST_F(FileSystemFileUtilTest, TestCrossFileSystemMoveDifferentOrigins) {
+  GURL src_origin("http://www.example.com");
+  fileapi::FileSystemType type = kFileSystemTypePersistent;
+  GURL dest_origin("http://www.not.the.same.domain.com");
+
+  TestCrossFileSystemCopyMoveHelper(src_origin, type, dest_origin, type, false);
+}
+
+TEST_F(FileSystemFileUtilTest, TestCrossFileSystemMoveSameOrigin) {
+  GURL origin("http://www.example.com");
+  fileapi::FileSystemType src_type = kFileSystemTypePersistent;
+  fileapi::FileSystemType dest_type = kFileSystemTypeTemporary;
+
+  TestCrossFileSystemCopyMoveHelper(origin, src_type, origin, dest_type, false);
+}

webkit/fileapi/file_system_operation.cc

@@ -148,19 +148,6 @@ void FileSystemOperation::Copy(const GURL& src_path,
     delete this;
     return;
   }
-  if (src_origin_url.GetOrigin() != dest_origin_url.GetOrigin()) {
-    // TODO(ericu): We don't yet support copying across filesystem types, from
-    // extension to sandbox, etc.  From temporary to persistent works, though.
-    // Since the sandbox code isn't in yet, I'm not sure exactly what check
-    // belongs here, but there's also no danger yet.
-    delete this;
-    return;
-  }
-  if (src_file_system_file_util != dest_file_system_file_util) {
-    // TODO(ericu): implement this.
-    delete this;
-    return;
-  }
   file_system_operation_context_.set_src_origin_url(src_origin_url);
   file_system_operation_context_.set_dest_origin_url(dest_origin_url);
   file_system_operation_context_.set_src_type(src_type);
@@ -220,17 +207,6 @@ void FileSystemOperation::Move(const GURL& src_path,
     delete this;
     return;
   }
-  if (src_origin_url.GetOrigin() != dest_origin_url.GetOrigin()) {
-    // TODO(ericu): We don't yet support moving across filesystem types, from
-    // extension to sandbox, etc.  From temporary to persistent works, though.
-    delete this;
-    return;
-  }
-  if (src_file_system_file_util != dest_file_system_file_util) {
-    // TODO(ericu): implement this.
-    delete this;
-    return;
-  }
   file_system_operation_context_.set_src_origin_url(src_origin_url);
   file_system_operation_context_.set_dest_origin_url(dest_origin_url);
   file_system_operation_context_.set_src_type(src_type);

webkit/fileapi/local_file_system_file_util.cc

@@ -121,6 +121,21 @@ PlatformFileError LocalFileSystemFileUtil::CopyOrMoveFile(
       context, local_src_path, local_dest_path, copy);
 }
 
+PlatformFileError LocalFileSystemFileUtil::CopyInForeignFile(
+    FileSystemOperationContext* context,
+    const FilePath& src_file_path,
+    const FilePath& dest_file_path) {
+  if (src_file_path.empty())
+    return base::PLATFORM_FILE_ERROR_INVALID_OPERATION;
+  FilePath local_dest_path =
+      GetLocalPath(context, context->dest_origin_url(), context->dest_type(),
+          dest_file_path);
+  if (local_dest_path.empty())
+    return base::PLATFORM_FILE_ERROR_INVALID_OPERATION;
+  return QuotaFileUtil::GetInstance()->CopyOrMoveFile(
+      context, src_file_path, local_dest_path, true);
+}
+
 PlatformFileError LocalFileSystemFileUtil::DeleteFile(
     FileSystemOperationContext* context,
     const FilePath& file_path) {

webkit/fileapi/local_file_system_file_util.h

@@ -77,6 +77,11 @@ class LocalFileSystemFileUtil : public FileSystemFileUtil {
       const FilePath& dest_file_path,
       bool copy);
 
+  virtual PlatformFileError CopyInForeignFile(
+        FileSystemOperationContext* context,
+        const FilePath& src_file_path,
+        const FilePath& dest_file_path);
+
   virtual PlatformFileError DeleteFile(
       FileSystemOperationContext* context,
       const FilePath& file_path);

webkit/fileapi/obfuscated_file_system_file_util.cc

@@ -284,10 +284,6 @@ PlatformFileError ObfuscatedFileSystemFileUtil::CopyOrMoveFile(
     const FilePath& src_file_path,
     const FilePath& dest_file_path,
     bool copy) {
-  // TODO(ericu): Handle multi-db move+copy, where src and dest aren't in the
-  // same database.  Currently we'll just fail badly.  This may get handled from
-  // higher-level code, though, and as we don't have cross-filesystem
-  // transactions that's not any less efficient than doing it here.
   FileSystemDirectoryDatabase* db =
       GetDirectoryDatabase(context->src_origin_url(), context->src_type());
   if (!db)
@@ -372,6 +368,41 @@ PlatformFileError ObfuscatedFileSystemFileUtil::CopyOrMoveFile(
   return base::PLATFORM_FILE_ERROR_FAILED;
 }
 
+PlatformFileError ObfuscatedFileSystemFileUtil::CopyInForeignFile(
+    FileSystemOperationContext* context,
+    const FilePath& src_file_path,
+    const FilePath& dest_file_path) {
+  FileSystemDirectoryDatabase* db =
+      GetDirectoryDatabase(context->dest_origin_url(), context->dest_type());
+  if (!db)
+    return base::PLATFORM_FILE_ERROR_FAILED;
+  FileId dest_file_id;
+  bool overwrite = db->GetFileWithPath(dest_file_path, &dest_file_id);
+  FileInfo dest_file_info;
+  if (overwrite) {
+    if (!db->GetFileInfo(dest_file_id, &dest_file_info) ||
+        dest_file_info.is_directory()) {
+      NOTREACHED();
+      return base::PLATFORM_FILE_ERROR_FAILED;
+    }
+    FilePath dest_data_path = DataPathToLocalPath(context->dest_origin_url(),
+      context->dest_type(), dest_file_info.data_path);
+    return QuotaFileUtil::GetInstance()->CopyOrMoveFile(context,
+        src_file_path, dest_data_path, true /* copy */);
+  } else {
+    FileId dest_parent_id;
+    if (!db->GetFileWithPath(dest_file_path.DirName(), &dest_parent_id)) {
+      NOTREACHED();  // We shouldn't be called in this case.
+      return base::PLATFORM_FILE_ERROR_NOT_FOUND;
+    }
+    InitFileInfo(&dest_file_info, dest_parent_id,
+        dest_file_path.BaseName().value());
+    return CreateFile(context, context->dest_origin_url(),
+        context->dest_type(), src_file_path, &dest_file_info, 0, NULL);
+  }
+  return base::PLATFORM_FILE_ERROR_FAILED;
+}
+
 PlatformFileError ObfuscatedFileSystemFileUtil::DeleteFile(
     FileSystemOperationContext* context,
     const FilePath& virtual_path) {

webkit/fileapi/obfuscated_file_system_file_util.h

@@ -90,6 +90,11 @@ class ObfuscatedFileSystemFileUtil : public FileSystemFileUtil,
       const FilePath& dest_file_path,
       bool copy) OVERRIDE;
 
+  virtual PlatformFileError CopyInForeignFile(
+        FileSystemOperationContext* context,
+        const FilePath& src_file_path,
+        const FilePath& dest_file_path) OVERRIDE;
+
   virtual base::PlatformFileError DeleteFile(
       FileSystemOperationContext* context,
       const FilePath& file_path) OVERRIDE;

webkit/fileapi/obfuscated_file_system_file_util_unittest.cc

@@ -36,6 +36,12 @@ bool FileExists(const FilePath& path) {
   return file_util::PathExists(path) && !file_util::DirectoryExists(path);
 }
 
+int64 GetSize(const FilePath& path) {
+  int64 size;
+  EXPECT_TRUE(file_util::GetFileSize(path, &size));
+  return size;
+}
+
 // After a move, the dest exists and the source doesn't.
 // After a copy, both source and dest exist.
 struct CopyMoveTestCaseRecord {
@@ -157,12 +163,6 @@ class ObfuscatedFileSystemFileUtilTest : public testing::Test {
     return type_;
   }
 
-  int64 GetSize(const FilePath& path) {
-    int64 size;
-    EXPECT_TRUE(file_util::GetFileSize(path, &size));
-    return size;
-  }
-
   void CheckFileAndCloseHandle(
       const FilePath& virtual_path, PlatformFile file_handle) {
     scoped_ptr<FileSystemOperationContext> context(NewContext());
@@ -189,6 +189,7 @@ class ObfuscatedFileSystemFileUtilTest : public testing::Test {
           base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_WRITE,
           &created,
           &error);
+      ASSERT_NE(base::kInvalidPlatformFileValue, file_handle);
       ASSERT_EQ(base::PLATFORM_FILE_OK, error);
       EXPECT_FALSE(created);
     }
@@ -338,6 +339,55 @@ class ObfuscatedFileSystemFileUtilTest : public testing::Test {
     EXPECT_EQ(file_info.last_modified.ToTimeT(), last_modified_time.ToTimeT());
   }
 
+  void TestCopyInForeignFileHelper(bool overwrite) {
+    ScopedTempDir source_dir;
+    ASSERT_TRUE(source_dir.CreateUniqueTempDir());
+    FilePath root_path = source_dir.path();
+    FilePath src_path = root_path.AppendASCII("file_name");
+    FilePath dest_path(FILE_PATH_LITERAL("new file"));
+    int64 src_file_length = 87;
+
+    base::PlatformFileError error_code;
+    bool created = false;
+    int file_flags = base::PLATFORM_FILE_CREATE | base::PLATFORM_FILE_WRITE;
+    base::PlatformFile file_handle =
+        base::CreatePlatformFile(
+            src_path, file_flags, &created, &error_code);
+    EXPECT_TRUE(created);
+    ASSERT_EQ(base::PLATFORM_FILE_OK, error_code);
+    ASSERT_NE(base::kInvalidPlatformFileValue, file_handle);
+    ASSERT_TRUE(base::TruncatePlatformFile(file_handle, src_file_length));
+    EXPECT_TRUE(base::ClosePlatformFile(file_handle));
+
+    scoped_ptr<FileSystemOperationContext> context;
+
+    if (overwrite) {
+      context.reset(NewContext());
+      EXPECT_EQ(base::PLATFORM_FILE_OK,
+          ofsfu()->EnsureFileExists(context.get(), dest_path, &created));
+      EXPECT_TRUE(created);
+    }
+
+    context.reset(NewContext());
+    EXPECT_EQ(base::PLATFORM_FILE_OK,
+        ofsfu()->CopyInForeignFile(context.get(), src_path, dest_path));
+    context.reset(NewContext());
+    EXPECT_TRUE(ofsfu()->PathExists(context.get(), dest_path));
+    context.reset(NewContext());
+    EXPECT_FALSE(ofsfu()->DirectoryExists(context.get(), dest_path));
+    context.reset(NewContext());
+    base::PlatformFileInfo file_info;
+    FilePath data_path;
+    EXPECT_EQ(base::PLATFORM_FILE_OK, ofsfu()->GetFileInfo(
+        context.get(), dest_path, &file_info, &data_path));
+    EXPECT_NE(data_path, src_path);
+    EXPECT_TRUE(FileExists(data_path));
+    EXPECT_EQ(src_file_length, GetSize(data_path));
+
+    EXPECT_EQ(base::PLATFORM_FILE_OK,
+        ofsfu()->DeleteFile(context.get(), dest_path));
+  }
+
  private:
   ScopedTempDir data_dir_;
   scoped_refptr<ObfuscatedFileSystemFileUtil> obfuscated_file_system_file_util_;
@@ -778,6 +828,11 @@ TEST_F(ObfuscatedFileSystemFileUtilTest, TestCopyOrMoveFileSuccess) {
   }
 }
 
+TEST_F(ObfuscatedFileSystemFileUtilTest, TestCopyInForeignFile) {
+  TestCopyInForeignFileHelper(false /* overwrite */);
+  TestCopyInForeignFileHelper(true /* overwrite */);
+}
+
 TEST_F(ObfuscatedFileSystemFileUtilTest, TestEnumerator) {
   scoped_ptr<FileSystemOperationContext> context(NewContext());
   FilePath src_path = UTF8ToFilePath("source dir");
@@ -832,6 +887,7 @@ TEST_F(ObfuscatedFileSystemFileUtilTest, TestMigration) {
           base::CreatePlatformFile(
               local_src_path, file_flags, &created, &error_code);
       EXPECT_TRUE(created);
+      ASSERT_NE(base::kInvalidPlatformFileValue, file_handle);
       ASSERT_EQ(base::PLATFORM_FILE_OK, error_code);
       ASSERT_TRUE(
           base::TruncatePlatformFile(file_handle, test_case.data_file_size));

webkit/fileapi/quota_file_util_unittest.cc

@@ -32,24 +32,22 @@ class QuotaFileUtilTest : public testing::Test {
     FilePath base_dir = data_dir_.path().AppendASCII("filesystem");
 
     // For path translation we rely on LocalFileSystemFileUtil::GetLocalPath().
-    test_helper_.SetUp(base_dir, LocalFileSystemFileUtil::GetInstance());
+    local_test_helper_.SetUp(base_dir, LocalFileSystemFileUtil::GetInstance());
+    quota_test_helper_.SetUp(base_dir, QuotaFileUtil::GetInstance());
   }
 
   void TearDown() {
-    test_helper_.TearDown();
+    local_test_helper_.TearDown();
+    quota_test_helper_.TearDown();
   }
 
  protected:
   FileSystemOperationContext* NewContext() {
-    return test_helper_.NewOperationContext();
-  }
-
-  QuotaFileUtil* FileUtil() {
-    return QuotaFileUtil::GetInstance();
+    return quota_test_helper_.NewOperationContext();
   }
 
   FilePath Path(const std::string& file_name) {
-    return test_helper_.GetLocalPathFromASCII(file_name);
+    return local_test_helper_.GetLocalPathFromASCII(file_name);
   }
 
   base::PlatformFileError CreateFile(const char* file_name,
@@ -58,28 +56,26 @@ class QuotaFileUtilTest : public testing::Test {
         base::PLATFORM_FILE_WRITE | base::PLATFORM_FILE_ASYNC;
 
     scoped_ptr<FileSystemOperationContext> context(NewContext());
-    return FileUtil()->CreateOrOpen(context.get(), Path(file_name),
-        file_flags, file_handle, created);
+    return QuotaFileUtil::GetInstance()->CreateOrOpen(
+        context.get(), Path(file_name), file_flags, file_handle, created);
   }
 
   base::PlatformFileError EnsureFileExists(const char* file_name,
       bool* created) {
     scoped_ptr<FileSystemOperationContext> context(NewContext());
-    return FileUtil()->EnsureFileExists(context.get(),
-                                        Path(file_name), created);
+    return QuotaFileUtil::GetInstance()->EnsureFileExists(
+        context.get(), Path(file_name), created);
   }
 
   int64 GetCachedUsage() {
-    return FileSystemUsageCache::GetUsage(test_helper_.GetUsageCachePath());
-  }
-
-  FileSystemContext* file_system_context() const {
-    return test_helper_.file_system_context();
+    return FileSystemUsageCache::GetUsage(
+        quota_test_helper_.GetUsageCachePath());
   }
 
  private:
   ScopedTempDir data_dir_;
-  FileSystemTestOriginHelper test_helper_;
+  FileSystemTestOriginHelper local_test_helper_;
+  FileSystemTestOriginHelper quota_test_helper_;
   base::ScopedCallbackFactory<QuotaFileUtilTest> callback_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(QuotaFileUtilTest);
@@ -95,7 +91,7 @@ TEST_F(QuotaFileUtilTest, CreateAndClose) {
 
   scoped_ptr<FileSystemOperationContext> context(NewContext());
   EXPECT_EQ(base::PLATFORM_FILE_OK,
-            FileUtil()->Close(context.get(), file_handle));
+            QuotaFileUtil::GetInstance()->Close(context.get(), file_handle));
 }
 
 TEST_F(QuotaFileUtilTest, EnsureFileExists) {

webkit/tools/test_shell/test_shell.gypi

@@ -384,6 +384,7 @@
         '../../database/quota_table_unittest.cc',
         '../../fileapi/file_system_context_unittest.cc',
         '../../fileapi/file_system_directory_database_unittest.cc',
+        '../../fileapi/file_system_file_util_unittest.cc',
         '../../fileapi/file_system_operation_unittest.cc',
         '../../fileapi/file_system_origin_database_unittest.cc',
         '../../fileapi/file_system_path_manager_unittest.cc',