Remove PlatformFile from fileapi::FileSystemFileUtil

content/browser/fileapi/obfuscated_file_util_unittest.cc

 // Copyright 2013 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 <set>
 #include <string>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/file_util.h"
 #include "base/files/file.h"
 #include "base/files/file_path.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/memory/scoped_ptr.h"
-#include "base/platform_file.h"
 #include "base/run_loop.h"
 #include "content/browser/fileapi/mock_file_change_observer.h"
 #include "content/public/test/async_file_test_helper.h"
 #include "content/public/test/mock_special_storage_policy.h"
 #include "content/public/test/sandbox_file_system_test_helper.h"
 #include "content/public/test/test_file_system_context.h"
 #include "content/test/fileapi_test_file_set.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "webkit/browser/fileapi/external_mount_points.h"
 #include "webkit/browser/fileapi/file_system_backend.h"
@@ skipping 289 matching lines @@
   }
 
   int64 PathCost(const FileSystemURL& url) {
     return ObfuscatedFileUtil::ComputeFilePathCost(url.path());
   }
 
   FileSystemURL CreateURL(const base::FilePath& path) {
     return sandbox_file_system_.CreateURL(path);
   }
 
-  void CheckFileAndCloseHandle(
-      const FileSystemURL& url, base::PlatformFile file_handle) {
+  void CheckFileAndCloseHandle(const FileSystemURL& url, base::File file) {
     scoped_ptr<FileSystemOperationContext> context(NewContext(NULL));
     base::FilePath local_path;
     EXPECT_EQ(base::File::FILE_OK,
               ofu()->GetLocalFilePath(context.get(), url, &local_path));
 
     base::File::Info file_info0;
     base::FilePath data_path;
     EXPECT_EQ(base::File::FILE_OK,
               ofu()->GetFileInfo(context.get(), url, &file_info0, &data_path));
     EXPECT_EQ(data_path, local_path);
     EXPECT_TRUE(FileExists(data_path));
     EXPECT_EQ(0, GetSize(data_path));
 
     const char data[] = "test data";
     const int length = arraysize(data) - 1;
 
-    if (base::kInvalidPlatformFileValue == file_handle) {
-      base::File file(data_path,
+    if (!file.IsValid()) {
+      file.Initialize(data_path,
                       base::File::FLAG_OPEN | base::File::FLAG_WRITE);
       ASSERT_TRUE(file.IsValid());
       EXPECT_FALSE(file.created());
-      file_handle = file.TakePlatformFile();
     }
-    ASSERT_EQ(length, base::WritePlatformFile(file_handle, 0, data, length));
-    EXPECT_TRUE(base::ClosePlatformFile(file_handle));

[kinuko, 2014/05/12 05:39:57]

I think this code probably expects a flush happens here (though closing a file
doesn't necessarily guarantee it). Could we explicitly reset file or call
file.Close() (or file.Flush()) to avoid potential flakiness?

[rvargas (doing something else), 2014/05/12 23:39:59]

Done (IMO, if this is racing it needs more than Close/Flush to work :( ).

+    ASSERT_EQ(length, file.Write(0, data, length));
 
     base::File::Info file_info1;
     EXPECT_EQ(length, GetSize(data_path));
     context.reset(NewContext(NULL));
     EXPECT_EQ(base::File::FILE_OK,
               ofu()->GetFileInfo(context.get(), url, &file_info1, &data_path));
     EXPECT_EQ(data_path, local_path);
 
     EXPECT_FALSE(file_info0.is_directory);
     EXPECT_FALSE(file_info1.is_directory);
@@ skipping 453 matching lines @@
   quota::QuotaStatusCode quota_status_;
   int64 usage_;
   fileapi::MockFileChangeObserver change_observer_;
   fileapi::ChangeObserverList change_observers_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ObfuscatedFileUtilTest);
 };
 
 TEST_F(ObfuscatedFileUtilTest, TestCreateAndDeleteFile) {
-  base::PlatformFile file_handle = base::kInvalidPlatformFileValue;
-  bool created;
   FileSystemURL url = CreateURLFromUTF8("fake/file");
   scoped_ptr<FileSystemOperationContext> context(NewContext(NULL));
-  int file_flags = base::PLATFORM_FILE_CREATE | base::PLATFORM_FILE_WRITE;
+  int file_flags = base::File::FLAG_CREATE | base::File::FLAG_WRITE;
 
-  EXPECT_EQ(base::File::FILE_ERROR_NOT_FOUND,
-            ofu()->CreateOrOpen(context.get(), url, file_flags, &file_handle,
-                                &created));
+  base::File file = ofu()->CreateOrOpen(context.get(), url, file_flags);
+  EXPECT_FALSE(file.IsValid());
+  EXPECT_EQ(base::File::FILE_ERROR_NOT_FOUND, file.error_details());
 
   context.reset(NewContext(NULL));
   EXPECT_EQ(base::File::FILE_ERROR_NOT_FOUND,
             ofu()->DeleteFile(context.get(), url));
 
   url = CreateURLFromUTF8("test file");
 
   EXPECT_TRUE(change_observer()->HasNoChange());
 
   // Verify that file creation requires sufficient quota for the path.
   context.reset(NewContext(NULL));
   context->set_allowed_bytes_growth(
       ObfuscatedFileUtil::ComputeFilePathCost(url.path()) - 1);
-  ASSERT_EQ(base::File::FILE_ERROR_NO_SPACE,
-            ofu()->CreateOrOpen(context.get(), url, file_flags,
-                                &file_handle, &created));
+  file = ofu()->CreateOrOpen(context.get(), url, file_flags);
+  EXPECT_FALSE(file.IsValid());
+  ASSERT_EQ(base::File::FILE_ERROR_NO_SPACE, file.error_details());
 
   context.reset(NewContext(NULL));
   context->set_allowed_bytes_growth(
       ObfuscatedFileUtil::ComputeFilePathCost(url.path()));
-  ASSERT_EQ(base::File::FILE_OK,
-            ofu()->CreateOrOpen(context.get(), url, file_flags, &file_handle,
-                                &created));
-  ASSERT_TRUE(created);
+  file = ofu()->CreateOrOpen(context.get(), url, file_flags);
+  EXPECT_TRUE(file.IsValid());
+  ASSERT_TRUE(file.created());
   EXPECT_EQ(1, change_observer()->get_and_reset_create_file_count());
-  EXPECT_NE(base::kInvalidPlatformFileValue, file_handle);
 
-  CheckFileAndCloseHandle(url, file_handle);
+  CheckFileAndCloseHandle(url, file.Pass());
 
   context.reset(NewContext(NULL));
   base::FilePath local_path;
   EXPECT_EQ(base::File::FILE_OK,
             ofu()->GetLocalFilePath(context.get(), url, &local_path));
   EXPECT_TRUE(base::PathExists(local_path));
 
   // Verify that deleting a file isn't stopped by zero quota, and that it frees
   // up quote from its path.
   context.reset(NewContext(NULL));
@@ skipping 10 matching lines @@
   FileSystemURL directory_url = CreateURLFromUTF8(
       "series/of/directories");
   url = FileSystemURLAppendUTF8(directory_url, "file name");
   EXPECT_EQ(base::File::FILE_OK,
             ofu()->CreateDirectory(context.get(), directory_url, exclusive,
                                    recursive));
   // The oepration created 3 directories recursively.
   EXPECT_EQ(3, change_observer()->get_and_reset_create_directory_count());
 
   context.reset(NewContext(NULL));
-  file_handle = base::kInvalidPlatformFileValue;
-  ASSERT_EQ(base::File::FILE_OK,
-            ofu()->CreateOrOpen(context.get(), url, file_flags, &file_handle,
-                                &created));
-  ASSERT_TRUE(created);
+  file = ofu()->CreateOrOpen(context.get(), url, file_flags);
+  ASSERT_TRUE(file.IsValid());
+  ASSERT_TRUE(file.created());
   EXPECT_EQ(1, change_observer()->get_and_reset_create_file_count());
-  EXPECT_NE(base::kInvalidPlatformFileValue, file_handle);
 
-  CheckFileAndCloseHandle(url, file_handle);
+  CheckFileAndCloseHandle(url, file.Pass());
 
   context.reset(NewContext(NULL));
   EXPECT_EQ(base::File::FILE_OK,
             ofu()->GetLocalFilePath(context.get(), url, &local_path));
   EXPECT_TRUE(base::PathExists(local_path));
 
   context.reset(NewContext(NULL));
   EXPECT_EQ(base::File::FILE_OK, ofu()->DeleteFile(context.get(), url));
   EXPECT_EQ(1, change_observer()->get_and_reset_remove_file_count());
   EXPECT_FALSE(base::PathExists(local_path));
@@ skipping 114 matching lines @@
   EXPECT_TRUE(change_observer()->HasNoChange());
 
   context.reset(NewContext(NULL));
   context->set_allowed_bytes_growth(
       ObfuscatedFileUtil::ComputeFilePathCost(url.path()));
   ASSERT_EQ(base::File::FILE_OK,
             ofu()->EnsureFileExists(context.get(), url, &created));
   ASSERT_TRUE(created);
   EXPECT_EQ(1, change_observer()->get_and_reset_create_file_count());
 
-  CheckFileAndCloseHandle(url, base::kInvalidPlatformFileValue);
+  CheckFileAndCloseHandle(url, base::File());
 
   context.reset(NewContext(NULL));
   ASSERT_EQ(base::File::FILE_OK,
             ofu()->EnsureFileExists(context.get(), url, &created));
   ASSERT_FALSE(created);
   EXPECT_TRUE(change_observer()->HasNoChange());
 
   // Also test in a subdirectory.
   url = CreateURLFromUTF8("path/to/file.txt");
   context.reset(NewContext(NULL));
@@ skipping 667 matching lines @@
   EXPECT_EQ(expected_quota, SizeInUsageFile());
   EXPECT_EQ(expected_quota, SizeByQuotaUtil());
   EXPECT_EQ(expected_quota, usage());
 }
 
 TEST_F(ObfuscatedFileUtilTest, TestInconsistency) {
   const FileSystemURL kPath1 = CreateURLFromUTF8("hoge");
   const FileSystemURL kPath2 = CreateURLFromUTF8("fuga");
 
   scoped_ptr<FileSystemOperationContext> context;
-  base::PlatformFile file;
   base::File::Info file_info;
   base::FilePath data_path;
   bool created = false;
 
   // Create a non-empty file.
   context.reset(NewContext(NULL));
   EXPECT_EQ(base::File::FILE_OK,
             ofu()->EnsureFileExists(context.get(), kPath1, &created));
   EXPECT_TRUE(created);
   context.reset(NewContext(NULL));
@@ skipping 40 matching lines @@
 
   // Copy from sound |kPath1| to broken |kPath2|.
   context.reset(NewContext(NULL));
   EXPECT_EQ(base::File::FILE_OK,
             ofu()->CopyOrMoveFile(context.get(), kPath1, kPath2,
                                   FileSystemOperation::OPTION_NONE,
                                   true /* copy */));
 
   ofu()->DestroyDirectoryDatabase(origin(), type_string());
   context.reset(NewContext(NULL));
-  EXPECT_EQ(base::File::FILE_OK,
-            ofu()->CreateOrOpen(
-                context.get(), kPath1,
-                base::PLATFORM_FILE_READ | base::PLATFORM_FILE_CREATE,
-                &file, &created));
-  EXPECT_TRUE(created);
+  base::File file =
+      ofu()->CreateOrOpen(context.get(), kPath1,
+                          base::File::FLAG_READ | base::File::FLAG_CREATE);
+  EXPECT_TRUE(file.IsValid());
+  EXPECT_TRUE(file.created());
 
-  base::File base_file(file);
-  EXPECT_TRUE(base_file.GetInfo(&file_info));
+  EXPECT_TRUE(file.GetInfo(&file_info));
   EXPECT_EQ(0, file_info.size);
 }
 
 TEST_F(ObfuscatedFileUtilTest, TestIncompleteDirectoryReading) {
   const FileSystemURL kPath[] = {
     CreateURLFromUTF8("foo"),
     CreateURLFromUTF8("bar"),
     CreateURLFromUTF8("baz")
   };
   const FileSystemURL empty_path = CreateURL(base::FilePath());
@@ skipping 27 matching lines @@
 
 TEST_F(ObfuscatedFileUtilTest, TestDirectoryTimestampForCreation) {
   scoped_ptr<FileSystemOperationContext> context(NewContext(NULL));
   const FileSystemURL dir_url = CreateURLFromUTF8("foo_dir");
 
   // Create working directory.
   EXPECT_EQ(base::File::FILE_OK,
             ofu()->CreateDirectory(context.get(), dir_url, false, false));
 
   // EnsureFileExists, create case.
-  FileSystemURL url(FileSystemURLAppendUTF8(
-          dir_url, "EnsureFileExists_file"));
+  FileSystemURL url(FileSystemURLAppendUTF8(dir_url, "EnsureFileExists_file"));
   bool created = false;
   ClearTimestamp(dir_url);
   context.reset(NewContext(NULL));
   EXPECT_EQ(base::File::FILE_OK,
             ofu()->EnsureFileExists(context.get(), url, &created));
   EXPECT_TRUE(created);
   EXPECT_NE(base::Time(), GetModifiedTime(dir_url));
 
   // non create case.
   created = true;
@@ skipping 11 matching lines @@
             ofu()->CreateDirectory(context.get(), url, false, false));
 
   ClearTimestamp(dir_url);
   context.reset(NewContext(NULL));
   EXPECT_EQ(base::File::FILE_ERROR_NOT_A_FILE,
             ofu()->EnsureFileExists(context.get(), url, &created));
   EXPECT_EQ(base::Time(), GetModifiedTime(dir_url));
 
   // CreateOrOpen, create case.
   url = FileSystemURLAppendUTF8(dir_url, "CreateOrOpen_file");
-  base::PlatformFile file_handle = base::kInvalidPlatformFileValue;
-  created = false;
   ClearTimestamp(dir_url);
   context.reset(NewContext(NULL));
-  EXPECT_EQ(base::File::FILE_OK,
-            ofu()->CreateOrOpen(
-                context.get(), url,
-                base::PLATFORM_FILE_CREATE | base::PLATFORM_FILE_WRITE,
-                &file_handle, &created));
-  EXPECT_NE(base::kInvalidPlatformFileValue, file_handle);
-  EXPECT_TRUE(created);
-  EXPECT_TRUE(base::ClosePlatformFile(file_handle));
+  base::File file =
+      ofu()->CreateOrOpen(context.get(), url,
+                          base::File::FLAG_CREATE | base::File::FLAG_WRITE);
+
+  EXPECT_TRUE(file.IsValid());
+  EXPECT_TRUE(file.created());
+  file.Close();
   EXPECT_NE(base::Time(), GetModifiedTime(dir_url));
 
   // open case.
-  file_handle = base::kInvalidPlatformFileValue;
-  created = true;
   ClearTimestamp(dir_url);
   context.reset(NewContext(NULL));
-  EXPECT_EQ(base::File::FILE_OK,
-            ofu()->CreateOrOpen(
-                context.get(), url,
-                base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_WRITE,
-                &file_handle, &created));
-  EXPECT_NE(base::kInvalidPlatformFileValue, file_handle);
-  EXPECT_FALSE(created);
-  EXPECT_TRUE(base::ClosePlatformFile(file_handle));
+  file = ofu()->CreateOrOpen(context.get(), url,
+                             base::File::FLAG_OPEN | base::File::FLAG_WRITE);
+  EXPECT_TRUE(file.IsValid());
+  EXPECT_FALSE(file.created());
+  file.Close();
   EXPECT_EQ(base::Time(), GetModifiedTime(dir_url));
 
   // fail case
-  file_handle = base::kInvalidPlatformFileValue;
   ClearTimestamp(dir_url);
   context.reset(NewContext(NULL));
-  EXPECT_EQ(base::File::FILE_ERROR_EXISTS,
-            ofu()->CreateOrOpen(
-                context.get(), url,
-                base::PLATFORM_FILE_CREATE | base::PLATFORM_FILE_WRITE,
-                &file_handle, &created));
-  EXPECT_EQ(base::kInvalidPlatformFileValue, file_handle);
+  file = ofu()->CreateOrOpen(context.get(), url,
+                             base::File::FLAG_CREATE | base::File::FLAG_WRITE);
+  EXPECT_FALSE(file.IsValid());
+  EXPECT_EQ(base::File::FILE_ERROR_EXISTS, file.error_details());
   EXPECT_EQ(base::Time(), GetModifiedTime(dir_url));
 
   // CreateDirectory, create case.
   // Creating CreateDirectory_dir and CreateDirectory_dir/subdir.
   url = FileSystemURLAppendUTF8(dir_url, "CreateDirectory_dir");
   FileSystemURL subdir_url(FileSystemURLAppendUTF8(url, "subdir"));
   ClearTimestamp(dir_url);
   context.reset(NewContext(NULL));
   EXPECT_EQ(base::File::FILE_OK,
             ofu()->CreateDirectory(context.get(), subdir_url,
@@ skipping 438 matching lines @@
                 file));
   ASSERT_EQ(1140, ComputeTotalFileSize());
 
   ASSERT_EQ(base::File::FILE_OK,
             AsyncFileTestHelper::Remove(
                 file_system_context(), dir, true /* recursive */));
   ASSERT_EQ(0, ComputeTotalFileSize());
 }
 
 TEST_F(ObfuscatedFileUtilTest, TestQuotaOnOpen) {
-  FileSystemURL file(CreateURLFromUTF8("file"));
-  base::PlatformFile file_handle;
+  FileSystemURL url(CreateURLFromUTF8("file"));
+
   bool created;
-
   // Creating a file.
   ASSERT_EQ(base::File::FILE_OK,
             ofu()->EnsureFileExists(
-                AllowUsageIncrease(PathCost(file))->context(),
-                file, &created));
+                AllowUsageIncrease(PathCost(url))->context(),
+                url, &created));
   ASSERT_TRUE(created);
   ASSERT_EQ(0, ComputeTotalFileSize());
 
   // Opening it, which shouldn't change the usage.
-  ASSERT_EQ(base::File::FILE_OK,
-            ofu()->CreateOrOpen(
-                AllowUsageIncrease(0)->context(), file,
-                base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_WRITE,
-                &file_handle, &created));
+  base::File file =
+      ofu()->CreateOrOpen(AllowUsageIncrease(0)->context(), url,
+                          base::File::FLAG_OPEN | base::File::FLAG_WRITE);
+  ASSERT_TRUE(file.IsValid());
   ASSERT_EQ(0, ComputeTotalFileSize());
-  EXPECT_TRUE(base::ClosePlatformFile(file_handle));
+  file.Close();
 
   const int length = 33;
   ASSERT_EQ(base::File::FILE_OK,
             ofu()->Truncate(
-                AllowUsageIncrease(length)->context(), file, length));
+                AllowUsageIncrease(length)->context(), url, length));
   ASSERT_EQ(length, ComputeTotalFileSize());
 
   // Opening it with CREATE_ALWAYS flag, which should truncate the file size.
-  ASSERT_EQ(base::File::FILE_OK,
-            ofu()->CreateOrOpen(
-                AllowUsageIncrease(-length)->context(), file,
-                base::PLATFORM_FILE_CREATE_ALWAYS | base::PLATFORM_FILE_WRITE,
-                &file_handle, &created));
+  file = ofu()->CreateOrOpen(
+             AllowUsageIncrease(-length)->context(), url,
+             base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE);
+  ASSERT_TRUE(file.IsValid());
   ASSERT_EQ(0, ComputeTotalFileSize());
-  EXPECT_TRUE(base::ClosePlatformFile(file_handle));
+  file.Close();
 
   // Extending the file again.
   ASSERT_EQ(base::File::FILE_OK,
             ofu()->Truncate(
-                AllowUsageIncrease(length)->context(), file, length));
+                AllowUsageIncrease(length)->context(), url, length));
   ASSERT_EQ(length, ComputeTotalFileSize());
 
   // Opening it with TRUNCATED flag, which should truncate the file size.
-  ASSERT_EQ(base::File::FILE_OK,
-            ofu()->CreateOrOpen(
-                AllowUsageIncrease(-length)->context(), file,
-                base::PLATFORM_FILE_OPEN_TRUNCATED | base::PLATFORM_FILE_WRITE,
-                &file_handle, &created));
+  file = ofu()->CreateOrOpen(
+             AllowUsageIncrease(-length)->context(), url,
+             base::File::FLAG_OPEN_TRUNCATED | base::File::FLAG_WRITE);
+  ASSERT_TRUE(file.IsValid());
   ASSERT_EQ(0, ComputeTotalFileSize());
-  EXPECT_TRUE(base::ClosePlatformFile(file_handle));
+  file.Close();
 }
 
 TEST_F(ObfuscatedFileUtilTest, MaybeDropDatabasesAliveCase) {
   MaybeDropDatabasesAliveCaseTestBody();
 }
 
 TEST_F(ObfuscatedFileUtilTest, MaybeDropDatabasesAlreadyDeletedCase) {
   MaybeDropDatabasesAlreadyDeletedCaseTestBody();
 }
 
 TEST_F(ObfuscatedFileUtilTest, DestroyDirectoryDatabase_Isolated) {
   DestroyDirectoryDatabase_IsolatedTestBody();
 }
 
 TEST_F(ObfuscatedFileUtilTest, GetDirectoryDatabase_Isolated) {
   GetDirectoryDatabase_IsolatedTestBody();
 }
 
 TEST_F(ObfuscatedFileUtilTest, MigrationBackFromIsolated) {
   MigrationBackFromIsolatedTestBody();
 }
 
 TEST_F(ObfuscatedFileUtilTest, OpenPathInNonDirectory) {
-  FileSystemURL file(CreateURLFromUTF8("file"));
+  FileSystemURL url(CreateURLFromUTF8("file"));
   FileSystemURL path_in_file(CreateURLFromUTF8("file/file"));
   bool created;
 
   ASSERT_EQ(base::File::FILE_OK,
-            ofu()->EnsureFileExists(UnlimitedContext().get(), file, &created));
+            ofu()->EnsureFileExists(UnlimitedContext().get(), url, &created));
   ASSERT_TRUE(created);
 
-  created = false;
-  base::PlatformFile file_handle = base::kInvalidPlatformFileValue;
-  int file_flags = base::PLATFORM_FILE_CREATE | base::PLATFORM_FILE_WRITE;
-  ASSERT_EQ(base::File::FILE_ERROR_NOT_A_DIRECTORY,
-            ofu()->CreateOrOpen(UnlimitedContext().get(),
-                                path_in_file,
-                                file_flags,
-                                &file_handle,
-                                &created));
-  ASSERT_FALSE(created);
-  ASSERT_EQ(base::kInvalidPlatformFileValue, file_handle);
+  int file_flags = base::File::FLAG_CREATE | base::File::FLAG_WRITE;
+  base::File file =
+      ofu()->CreateOrOpen(UnlimitedContext().get(), path_in_file, file_flags);
+  ASSERT_FALSE(file.IsValid());
+  ASSERT_EQ(base::File::FILE_ERROR_NOT_A_DIRECTORY, file.error_details());
 
   ASSERT_EQ(base::File::FILE_ERROR_NOT_A_DIRECTORY,
             ofu()->CreateDirectory(UnlimitedContext().get(),
                                    path_in_file,
                                    false /* exclusive */,
                                    false /* recursive */));
 }
 
 TEST_F(ObfuscatedFileUtilTest, CreateDirectory_NotADirectoryInRecursive) {
   FileSystemURL file(CreateURLFromUTF8("file"));
@@ skipping 12 matching lines @@
                                    false /* exclusive */,
                                    true /* recursive */));
   ASSERT_EQ(base::File::FILE_ERROR_NOT_A_DIRECTORY,
             ofu()->CreateDirectory(UnlimitedContext().get(),
                                    path_in_file_in_file,
                                    false /* exclusive */,
                                    true /* recursive */));
 }
 
 }  // namespace content