diff options
author | ericu@chromium.org <ericu@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-05-26 00:54:46 +0000 |
---|---|---|
committer | ericu@chromium.org <ericu@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-05-26 00:54:46 +0000 |
commit | 81b7f6675d7296ab536db6bb5bf52d0ff949faad (patch) | |
tree | ea636365d46bfcbe49ff5e11fee09201c09211b3 /webkit/fileapi | |
parent | 03b11bc8e4d4184b897a036ad4dd89c53499f79e (diff) | |
download | chromium_src-81b7f6675d7296ab536db6bb5bf52d0ff949faad.zip chromium_src-81b7f6675d7296ab536db6bb5bf52d0ff949faad.tar.gz chromium_src-81b7f6675d7296ab536db6bb5bf52d0ff949faad.tar.bz2 |
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
Diffstat (limited to 'webkit/fileapi')
-rw-r--r-- | webkit/fileapi/file_system_file_util.cc | 130 | ||||
-rw-r--r-- | webkit/fileapi/file_system_file_util.h | 18 | ||||
-rw-r--r-- | webkit/fileapi/file_system_file_util_unittest.cc | 221 | ||||
-rw-r--r-- | webkit/fileapi/file_system_operation.cc | 24 | ||||
-rw-r--r-- | webkit/fileapi/local_file_system_file_util.cc | 15 | ||||
-rw-r--r-- | webkit/fileapi/local_file_system_file_util.h | 5 | ||||
-rw-r--r-- | webkit/fileapi/obfuscated_file_system_file_util.cc | 39 | ||||
-rw-r--r-- | webkit/fileapi/obfuscated_file_system_file_util.h | 5 | ||||
-rw-r--r-- | webkit/fileapi/obfuscated_file_system_file_util_unittest.cc | 68 | ||||
-rw-r--r-- | webkit/fileapi/quota_file_util_unittest.cc | 34 |
10 files changed, 484 insertions, 75 deletions
diff --git a/webkit/fileapi/file_system_file_util.cc b/webkit/fileapi/file_system_file_util.cc index 9349ad03..3dfe459 100644 --- a/webkit/fileapi/file_system_file_util.cc +++ b/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) { diff --git a/webkit/fileapi/file_system_file_util.h b/webkit/fileapi/file_system_file_util.h index 5df8cd4..d6b72e6 100644 --- a/webkit/fileapi/file_system_file_util.h +++ b/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 diff --git a/webkit/fileapi/file_system_file_util_unittest.cc b/webkit/fileapi/file_system_file_util_unittest.cc new file mode 100644 index 0000000..056aa24 --- /dev/null +++ b/webkit/fileapi/file_system_file_util_unittest.cc @@ -0,0 +1,221 @@ +// 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); +} diff --git a/webkit/fileapi/file_system_operation.cc b/webkit/fileapi/file_system_operation.cc index 098ca75..4aa663c 100644 --- a/webkit/fileapi/file_system_operation.cc +++ b/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); diff --git a/webkit/fileapi/local_file_system_file_util.cc b/webkit/fileapi/local_file_system_file_util.cc index cd605ce..8b9e5d7 100644 --- a/webkit/fileapi/local_file_system_file_util.cc +++ b/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) { diff --git a/webkit/fileapi/local_file_system_file_util.h b/webkit/fileapi/local_file_system_file_util.h index 75dc1a4..a7af395 100644 --- a/webkit/fileapi/local_file_system_file_util.h +++ b/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); diff --git a/webkit/fileapi/obfuscated_file_system_file_util.cc b/webkit/fileapi/obfuscated_file_system_file_util.cc index b0092d9..faaa8f6 100644 --- a/webkit/fileapi/obfuscated_file_system_file_util.cc +++ b/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) { diff --git a/webkit/fileapi/obfuscated_file_system_file_util.h b/webkit/fileapi/obfuscated_file_system_file_util.h index 812db522..e69c49a 100644 --- a/webkit/fileapi/obfuscated_file_system_file_util.h +++ b/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; diff --git a/webkit/fileapi/obfuscated_file_system_file_util_unittest.cc b/webkit/fileapi/obfuscated_file_system_file_util_unittest.cc index 9000028..4d5b1f5 100644 --- a/webkit/fileapi/obfuscated_file_system_file_util_unittest.cc +++ b/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)); diff --git a/webkit/fileapi/quota_file_util_unittest.cc b/webkit/fileapi/quota_file_util_unittest.cc index 7921ae8..3322bd6 100644 --- a/webkit/fileapi/quota_file_util_unittest.cc +++ b/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) { |