Fix OFSFU's Touch to match the other FSFU subclasses.

Fix its test to enforce that, and add a test for FileSystemOperation::TouchFile.

BUG=none
TEST=test_shell_tests


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

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

4 files changed, 80 insertions, 70 deletions

diff --git a/webkit/fileapi/file_system_file_util.h b/webkit/fileapi/file_system_file_util.h
index 8a46fbd..a2788da 100644
--- a/webkit/fileapi/file_system_file_util.h
+++ b/webkit/fileapi/file_system_file_util.h
@@ -188,6 +188,7 @@ class FileSystemFileUtil {
       const FilePath& file_path);
 
   // Touches a file. The callback can be NULL.
+  // If the file doesn't exist, this fails with PLATFORM_FILE_ERROR_NOT_FOUND.
   virtual PlatformFileError Touch(
       FileSystemOperationContext* context,
       const FilePath& file_path,
diff --git a/webkit/fileapi/file_system_operation_unittest.cc b/webkit/fileapi/file_system_operation_unittest.cc
index f56dfe5..50e5ea8 100644
--- a/webkit/fileapi/file_system_operation_unittest.cc
+++ b/webkit/fileapi/file_system_operation_unittest.cc
@@ -956,4 +956,35 @@ TEST_F(FileSystemOperationTest, TestTruncateFailureByQuota) {
   EXPECT_EQ(10, info.size);
 }
 
+TEST_F(FileSystemOperationTest, TestTouchFile) {
+  FilePath file_path(CreateVirtualTemporaryFileInDir(FilePath()));
+  FilePath platform_path = PlatformPath(file_path);
+
+  base::PlatformFileInfo info;
+
+  EXPECT_TRUE(file_util::GetFileInfo(platform_path, &info));
+  EXPECT_FALSE(info.is_directory);
+  EXPECT_EQ(0, info.size);
+  const base::Time last_modified = info.last_modified;
+  const base::Time last_accessed = info.last_accessed;
+
+  const base::Time new_modified_time = base::Time::UnixEpoch();
+  const base::Time new_accessed_time = new_modified_time +
+    base::TimeDelta::FromHours(77);;
+  ASSERT_NE(last_modified, new_modified_time);
+  ASSERT_NE(last_accessed, new_accessed_time);
+
+  operation()->TouchFile(URLForPath(file_path), new_accessed_time,
+      new_modified_time);
+  MessageLoop::current()->RunAllPending();
+  EXPECT_EQ(base::PLATFORM_FILE_OK, status());
+
+  EXPECT_TRUE(file_util::GetFileInfo(platform_path, &info));
+  // We compare as time_t here to lower our resolution, to avoid false
+  // negatives caused by conversion to the local filesystem's native
+  // representation and back.
+  EXPECT_EQ(new_modified_time.ToTimeT(), info.last_modified.ToTimeT());
+  EXPECT_EQ(new_accessed_time.ToTimeT(), info.last_accessed.ToTimeT());
+}
+
 }  // namespace fileapi
diff --git a/webkit/fileapi/obfuscated_file_system_file_util.cc b/webkit/fileapi/obfuscated_file_system_file_util.cc
index dc6b70e..ee284e2 100644
--- a/webkit/fileapi/obfuscated_file_system_file_util.cc
+++ b/webkit/fileapi/obfuscated_file_system_file_util.cc
@@ -541,46 +541,28 @@ PlatformFileError ObfuscatedFileSystemFileUtil::Touch(
     const base::Time& last_access_time,
     const base::Time& last_modified_time) {
   FileSystemDirectoryDatabase* db = GetDirectoryDatabase(
-      context->src_origin_url(), context->src_type(), true);
+      context->src_origin_url(), context->src_type(), false);
   if (!db)
-    return base::PLATFORM_FILE_ERROR_FAILED;
+    return base::PLATFORM_FILE_ERROR_NOT_FOUND;
   FileId file_id;
-  if (db->GetFileWithPath(virtual_path, &file_id)) {
-    FileInfo file_info;
-    if (!db->GetFileInfo(file_id, &file_info)) {
-      NOTREACHED();
-      return base::PLATFORM_FILE_ERROR_FAILED;
-    }
-    if (file_info.is_directory()) {
-      file_info.modification_time = last_modified_time;
-      if (!db->UpdateFileInfo(file_id, file_info))
-        return base::PLATFORM_FILE_ERROR_FAILED;
-      return base::PLATFORM_FILE_OK;
-    }
-    FilePath data_path = DataPathToLocalPath(context->src_origin_url(),
-      context->src_type(), file_info.data_path);
-    return underlying_file_util_->Touch(
-        context, data_path, last_access_time, last_modified_time);
-  }
-  FileId parent_id;
-  if (!db->GetFileWithPath(virtual_path.DirName(), &parent_id))
+  if (!db->GetFileWithPath(virtual_path, &file_id))
     return base::PLATFORM_FILE_ERROR_NOT_FOUND;
 
   FileInfo file_info;
-  InitFileInfo(&file_info, parent_id, virtual_path.BaseName().value());
-  // In the event of a sporadic underlying failure, we might create a new file,
-  // but fail to update its mtime + atime.
-  if (!AllocateQuotaForPath(context, 1, file_info.name.size()))
-    return base::PLATFORM_FILE_ERROR_NO_SPACE;
-  PlatformFileError error = CreateFile(context, context->src_origin_url(),
-      context->src_type(), FilePath(), &file_info, 0, NULL);
-  if (base::PLATFORM_FILE_OK != error)
-    return error;
-
+  if (!db->GetFileInfo(file_id, &file_info)) {
+    NOTREACHED();
+    return base::PLATFORM_FILE_ERROR_FAILED;
+  }
+  if (file_info.is_directory()) {
+    file_info.modification_time = last_modified_time;
+    if (!db->UpdateFileInfo(file_id, file_info))
+      return base::PLATFORM_FILE_ERROR_FAILED;
+    return base::PLATFORM_FILE_OK;
+  }
   FilePath data_path = DataPathToLocalPath(context->src_origin_url(),
     context->src_type(), file_info.data_path);
-  return underlying_file_util_->Touch(context, data_path,
-    last_access_time, last_modified_time);
+  return underlying_file_util_->Touch(
+      context, data_path, last_access_time, last_modified_time);
 }
 
 PlatformFileError ObfuscatedFileSystemFileUtil::Truncate(
diff --git a/webkit/fileapi/obfuscated_file_system_file_util_unittest.cc b/webkit/fileapi/obfuscated_file_system_file_util_unittest.cc
index ca8a179..509f806 100644
--- a/webkit/fileapi/obfuscated_file_system_file_util_unittest.cc
+++ b/webkit/fileapi/obfuscated_file_system_file_util_unittest.cc
@@ -386,24 +386,11 @@ class ObfuscatedFileSystemFileUtilTest : public testing::Test {
     }
   }
 
-  void TestTouchHelper(const FilePath& path, bool new_file) {
-    base::Time last_access_time = base::Time::Now();  // Ignored, so not tested.
+  void TestTouchHelper(const FilePath& path, bool is_file) {
+    base::Time last_access_time = base::Time::Now();
     base::Time last_modified_time = base::Time::Now();
-    scoped_ptr<FileSystemOperationContext> context;
-
-    if (new_file) {
-      // Verify that file creation requires sufficient quota for the path.
-      context.reset(NewContext(NULL));
-      context->set_allowed_bytes_growth(
-          ObfuscatedFileSystemFileUtil::ComputeFilePathCost(path) - 1);
-      EXPECT_EQ(base::PLATFORM_FILE_ERROR_NO_SPACE,
-                ofsfu()->Touch(
-                    context.get(), path, last_access_time, last_modified_time));
-    }
 
-    context.reset(NewContext(NULL));
-    context->set_allowed_bytes_growth(
-        ObfuscatedFileSystemFileUtil::ComputeFilePathCost(path));
+    scoped_ptr<FileSystemOperationContext> context(NewContext(NULL));
     EXPECT_EQ(base::PLATFORM_FILE_OK,
               ofsfu()->Touch(
                   context.get(), path, last_access_time, last_modified_time));
@@ -419,6 +406,7 @@ class ObfuscatedFileSystemFileUtilTest : public testing::Test {
 
     context.reset(NewContext(NULL));
     last_modified_time += base::TimeDelta::FromHours(1);
+    last_access_time += base::TimeDelta::FromHours(14);
     EXPECT_EQ(base::PLATFORM_FILE_OK,
               ofsfu()->Touch(
                   context.get(), path, last_access_time, last_modified_time));
@@ -426,6 +414,8 @@ class ObfuscatedFileSystemFileUtilTest : public testing::Test {
     EXPECT_EQ(base::PLATFORM_FILE_OK, ofsfu()->GetFileInfo(
         context.get(), path, &file_info, &local_path));
     EXPECT_EQ(file_info.last_modified.ToTimeT(), last_modified_time.ToTimeT());
+    if (is_file)  // Directories in OFSFU don't support atime.
+      EXPECT_EQ(file_info.last_accessed.ToTimeT(), last_access_time.ToTimeT());
   }
 
   void TestCopyInForeignFileHelper(bool overwrite) {
@@ -844,45 +834,51 @@ TEST_F(ObfuscatedFileSystemFileUtilTest, TestReadDirectoryOnFile) {
 }
 
 TEST_F(ObfuscatedFileSystemFileUtilTest, TestTouch) {
-  FilePath path = UTF8ToFilePath("fake/file");
-  base::Time last_access_time = base::Time::Now();  // Ignored, so not tested.
-  base::Time last_modified_time = base::Time::Now();
+  FilePath path = UTF8ToFilePath("file");
   scoped_ptr<FileSystemOperationContext> context(NewContext(NULL));
+
+  base::Time last_access_time = base::Time::Now();
+  base::Time last_modified_time = base::Time::Now();
+
+  // It's not there yet.
   EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND,
             ofsfu()->Touch(
                 context.get(), path, last_access_time, last_modified_time));
 
-  // Touch will create a file if it's not there but its parent is.
-  bool new_file = true;
-  path = UTF8ToFilePath("file name");
-  TestTouchHelper(path, new_file);
+  // OK, now create it.
+  context.reset(NewContext(NULL));
+  bool created = false;
+  ASSERT_EQ(base::PLATFORM_FILE_OK,
+            ofsfu()->EnsureFileExists(context.get(), path, &created));
+  ASSERT_TRUE(created);
+  TestTouchHelper(path, true);
 
-  bool exclusive = true;
-  bool recursive = true;
-  path = UTF8ToFilePath("directory/to/use");
+  // Now test a directory:
   context.reset(NewContext(NULL));
-  EXPECT_EQ(base::PLATFORM_FILE_OK, ofsfu()->CreateDirectory(
-      context.get(), path, exclusive, recursive));
-  new_file = false;
-  TestTouchHelper(path, new_file);
+  bool exclusive = true;
+  bool recursive = false;
+  path = UTF8ToFilePath("dir");
+  ASSERT_EQ(base::PLATFORM_FILE_OK, ofsfu()->CreateDirectory(context.get(),
+      path, exclusive, recursive));
+  TestTouchHelper(path, false);
 }
 
 TEST_F(ObfuscatedFileSystemFileUtilTest, TestPathQuotas) {
   FilePath path = UTF8ToFilePath("fake/file");
-  base::Time last_access_time = base::Time::Now();
-  base::Time last_modified_time = base::Time::Now();
   scoped_ptr<FileSystemOperationContext> context(NewContext(NULL));
 
-  // Touch will create a file if it's not there but its parent is.
   path = UTF8ToFilePath("file name");
   context->set_allowed_bytes_growth(5);
+  bool created = false;
   EXPECT_EQ(base::PLATFORM_FILE_ERROR_NO_SPACE,
-            ofsfu()->Touch(
-                context.get(), path, last_access_time, last_modified_time));
+            ofsfu()->EnsureFileExists(
+                context.get(), path, &created));
+  EXPECT_FALSE(created);
   context->set_allowed_bytes_growth(1024);
   EXPECT_EQ(base::PLATFORM_FILE_OK,
-            ofsfu()->Touch(
-                context.get(), path, last_access_time, last_modified_time));
+            ofsfu()->EnsureFileExists(
+                context.get(), path, &created));
+  EXPECT_TRUE(created);
   int64 path_cost = ObfuscatedFileSystemFileUtil::ComputeFilePathCost(path);
   EXPECT_EQ(1024 - path_cost, context->allowed_bytes_growth());