Base: Introduce a new version of FileUtilProxy that works with File.

base/files/file_proxy_unittest.cc

Index: base/files/file_proxy_unittest.cc
diff --git a/base/files/file_util_proxy_unittest.cc b/base/files/file_proxy_unittest.cc
similarity index 49%
copy from base/files/file_util_proxy_unittest.cc
copy to base/files/file_proxy_unittest.cc
index 8d8c8eb2708e287f963894902604bf306af2495f..e529c9625fb7a4cb96ce8357f257b621eb0757e6 100644
--- a/base/files/file_util_proxy_unittest.cc
+++ b/base/files/file_proxy_unittest.cc
@@ -2,29 +2,27 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/files/file_util_proxy.h"
+#include "base/files/file_proxy.h"
 
 #include <map>
 
 #include "base/bind.h"
 #include "base/file_util.h"
+#include "base/files/file.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/logging.h"
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop/message_loop.h"
-#include "base/platform_file.h"
 #include "base/threading/thread.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace base {
 
-class FileUtilProxyTest : public testing::Test {
+class FileProxyTest : public testing::Test {
  public:
-  FileUtilProxyTest()
-      : file_thread_("FileUtilProxyTestFileThread"),
+  FileProxyTest()
+      : file_thread_("FileProxyTestFileThread"),
         error_(File::FILE_OK),
-        created_(false),
-        file_(kInvalidPlatformFileValue),
         bytes_written_(-1),
         weak_factory_(this) {}
 
@@ -33,30 +31,19 @@ class FileUtilProxyTest : public testing::Test {
     ASSERT_TRUE(file_thread_.Start());
   }
 
-  virtual void TearDown() OVERRIDE {
-    if (file_ != kInvalidPlatformFileValue)
-      ClosePlatformFile(file_);
-  }
-
   void DidFinish(File::Error error) {
     error_ = error;
     MessageLoop::current()->QuitWhenIdle();
   }
 
-  void DidCreateOrOpen(File::Error error,
-                       PassPlatformFile file,
-                       bool created) {
+  void DidCreateOrOpen(File::Error error) {
     error_ = error;
-    file_ = file.ReleaseValue();
-    created_ = created;
     MessageLoop::current()->QuitWhenIdle();
   }
 
   void DidCreateTemporary(File::Error error,
-                          PassPlatformFile file,
                           const FilePath& path) {
     error_ = error;
-    file_ = file.ReleaseValue();
     path_ = path;
     MessageLoop::current()->QuitWhenIdle();
   }
@@ -85,15 +72,12 @@ class FileUtilProxyTest : public testing::Test {
   }
 
  protected:
-  PlatformFile GetTestPlatformFile(int flags) {
-    if (file_ != kInvalidPlatformFileValue)
-      return file_;
-    bool created;
-    PlatformFileError error;
-    file_ = CreatePlatformFile(test_path(), flags, &created, &error);
-    EXPECT_EQ(PLATFORM_FILE_OK, error);
-    EXPECT_NE(kInvalidPlatformFileValue, file_);
-    return file_;
+  void CreateProxy(uint32 flags, FileProxy* proxy) {
+    proxy->CreateOrOpen(
+        test_path(), flags,
+        Bind(&FileProxyTest::DidCreateOrOpen, weak_factory_.GetWeakPtr()));
+    MessageLoop::current()->Run();
+    EXPECT_TRUE(proxy->IsValid());
   }
 
   TaskRunner* file_task_runner() const {
@@ -107,110 +91,96 @@ class FileUtilProxyTest : public testing::Test {
 
   ScopedTempDir dir_;
   File::Error error_;
-  bool created_;
-  PlatformFile file_;
   FilePath path_;
   File::Info file_info_;
   std::vector<char> buffer_;
   int bytes_written_;
-  WeakPtrFactory<FileUtilProxyTest> weak_factory_;
+  WeakPtrFactory<FileProxyTest> weak_factory_;
 };
 
-TEST_F(FileUtilProxyTest, CreateOrOpen_Create) {
-  FileUtilProxy::CreateOrOpen(
-      file_task_runner(),
+TEST_F(FileProxyTest, CreateOrOpen_Create) {
+  FileProxy proxy(file_task_runner());
+  proxy.CreateOrOpen(
       test_path(),
-      PLATFORM_FILE_CREATE | PLATFORM_FILE_READ,
-      Bind(&FileUtilProxyTest::DidCreateOrOpen, weak_factory_.GetWeakPtr()));
+      File::FLAG_CREATE | File::FLAG_READ,
+      Bind(&FileProxyTest::DidCreateOrOpen, weak_factory_.GetWeakPtr()));
   MessageLoop::current()->Run();
 
   EXPECT_EQ(File::FILE_OK, error_);
-  EXPECT_TRUE(created_);
-  EXPECT_NE(kInvalidPlatformFileValue, file_);
+  EXPECT_TRUE(proxy.IsValid());
+  EXPECT_TRUE(proxy.created());
   EXPECT_TRUE(PathExists(test_path()));
 }
 
-TEST_F(FileUtilProxyTest, CreateOrOpen_Open) {
+TEST_F(FileProxyTest, CreateOrOpen_Open) {
   // Creates a file.
-  WriteFile(test_path(), NULL, 0);
+  base::WriteFile(test_path(), NULL, 0);
   ASSERT_TRUE(PathExists(test_path()));
 
   // Opens the created file.
-  FileUtilProxy::CreateOrOpen(
-      file_task_runner(),
+  FileProxy proxy(file_task_runner());
+  proxy.CreateOrOpen(
       test_path(),
-      PLATFORM_FILE_OPEN | PLATFORM_FILE_READ,
-      Bind(&FileUtilProxyTest::DidCreateOrOpen, weak_factory_.GetWeakPtr()));
+      File::FLAG_OPEN | File::FLAG_READ,
+      Bind(&FileProxyTest::DidCreateOrOpen, weak_factory_.GetWeakPtr()));
   MessageLoop::current()->Run();
 
   EXPECT_EQ(File::FILE_OK, error_);
-  EXPECT_FALSE(created_);
-  EXPECT_NE(kInvalidPlatformFileValue, file_);
+  EXPECT_TRUE(proxy.IsValid());
+  EXPECT_FALSE(proxy.created());
 }
 
-TEST_F(FileUtilProxyTest, CreateOrOpen_OpenNonExistent) {
-  FileUtilProxy::CreateOrOpen(
-      file_task_runner(),
+TEST_F(FileProxyTest, CreateOrOpen_OpenNonExistent) {
+  FileProxy proxy(file_task_runner());
+  proxy.CreateOrOpen(
       test_path(),
-      PLATFORM_FILE_OPEN | PLATFORM_FILE_READ,
-      Bind(&FileUtilProxyTest::DidCreateOrOpen, weak_factory_.GetWeakPtr()));
+      File::FLAG_OPEN | File::FLAG_READ,
+      Bind(&FileProxyTest::DidCreateOrOpen, weak_factory_.GetWeakPtr()));
   MessageLoop::current()->Run();
   EXPECT_EQ(File::FILE_ERROR_NOT_FOUND, error_);
-  EXPECT_FALSE(created_);
-  EXPECT_EQ(kInvalidPlatformFileValue, file_);
+  EXPECT_FALSE(proxy.IsValid());
+  EXPECT_FALSE(proxy.created());
   EXPECT_FALSE(PathExists(test_path()));
 }
 
-TEST_F(FileUtilProxyTest, Close) {
+TEST_F(FileProxyTest, Close) {
   // Creates a file.
-  PlatformFile file = GetTestPlatformFile(
-      PLATFORM_FILE_CREATE | PLATFORM_FILE_WRITE);
+  FileProxy proxy(file_task_runner());
+  CreateProxy(File::FLAG_CREATE | File::FLAG_WRITE, &proxy);
 
 #if defined(OS_WIN)
   // This fails on Windows if the file is not closed.
-  EXPECT_FALSE(base::Move(test_path(),
-                               test_dir_path().AppendASCII("new")));
+  EXPECT_FALSE(base::Move(test_path(), test_dir_path().AppendASCII("new")));
 #endif
 
-  FileUtilProxy::Close(
-      file_task_runner(),
-      file,
-      Bind(&FileUtilProxyTest::DidFinish, weak_factory_.GetWeakPtr()));
+  proxy.Close(Bind(&FileProxyTest::DidFinish, weak_factory_.GetWeakPtr()));
   MessageLoop::current()->Run();
   EXPECT_EQ(File::FILE_OK, error_);
+  EXPECT_FALSE(proxy.IsValid());
 
   // Now it should pass on all platforms.
   EXPECT_TRUE(base::Move(test_path(), test_dir_path().AppendASCII("new")));
 }
 
-TEST_F(FileUtilProxyTest, CreateTemporary) {
-  FileUtilProxy::CreateTemporary(
-      file_task_runner(), 0 /* additional_file_flags */,
-      Bind(&FileUtilProxyTest::DidCreateTemporary, weak_factory_.GetWeakPtr()));
-  MessageLoop::current()->Run();
-  EXPECT_EQ(File::FILE_OK, error_);
-  EXPECT_TRUE(PathExists(path_));
-  EXPECT_NE(kInvalidPlatformFileValue, file_);
-
-  // The file should be writable.
-#if defined(OS_WIN)
-  HANDLE hEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
-  OVERLAPPED overlapped = {0};
-  overlapped.hEvent = hEvent;
-  DWORD bytes_written;
-  if (!::WriteFile(file_, "test", 4, &bytes_written, &overlapped)) {
-    // Temporary file is created with ASYNC flag, so WriteFile may return 0
-    // with ERROR_IO_PENDING.
-    EXPECT_EQ(ERROR_IO_PENDING, GetLastError());
-    GetOverlappedResult(file_, &overlapped, &bytes_written, TRUE);
+TEST_F(FileProxyTest, CreateTemporary) {
+  {
+    FileProxy proxy(file_task_runner());
+    proxy.CreateTemporary(
+        0 /* additional_file_flags */,
+        Bind(&FileProxyTest::DidCreateTemporary, weak_factory_.GetWeakPtr()));
+    MessageLoop::current()->Run();
+
+    EXPECT_TRUE(proxy.IsValid());
+    EXPECT_EQ(File::FILE_OK, error_);
+    EXPECT_TRUE(PathExists(path_));
+
+    // The file should be writable.
+    proxy.Write(0, "test", 4,
+                Bind(&FileProxyTest::DidWrite, weak_factory_.GetWeakPtr()));
+    MessageLoop::current()->Run();
+    EXPECT_EQ(File::FILE_OK, error_);
+    EXPECT_EQ(4, bytes_written_);
   }
-  EXPECT_EQ(4, bytes_written);
-#else
-  // On POSIX ASYNC flag does not affect synchronous read/write behavior.
-  EXPECT_EQ(4, WritePlatformFile(file_, 0, "test", 4));
-#endif
-  EXPECT_TRUE(ClosePlatformFile(file_));
-  file_ = kInvalidPlatformFileValue;
 
   // Make sure the written data can be read from the returned path.
   std::string data;
@@ -221,17 +191,17 @@ TEST_F(FileUtilProxyTest, CreateTemporary) {
   EXPECT_TRUE(base::DeleteFile(path_, false));
 }
 
-TEST_F(FileUtilProxyTest, GetFileInfo_File) {
+TEST_F(FileProxyTest, GetInfo) {
   // Setup.
-  ASSERT_EQ(4, WriteFile(test_path(), "test", 4));
+  ASSERT_EQ(4, base::WriteFile(test_path(), "test", 4));
   File::Info expected_info;
   GetFileInfo(test_path(), &expected_info);
 
   // Run.
-  FileUtilProxy::GetFileInfo(
-      file_task_runner(),
-      test_path(),
-      Bind(&FileUtilProxyTest::DidGetFileInfo, weak_factory_.GetWeakPtr()));
+  FileProxy proxy(file_task_runner());
+  CreateProxy(File::FLAG_OPEN | File::FLAG_READ, &proxy);
+  proxy.GetInfo(
+      Bind(&FileProxyTest::DidGetFileInfo, weak_factory_.GetWeakPtr()));
   MessageLoop::current()->Run();
 
   // Verify.
@@ -239,48 +209,25 @@ TEST_F(FileUtilProxyTest, GetFileInfo_File) {
   EXPECT_EQ(expected_info.size, file_info_.size);
   EXPECT_EQ(expected_info.is_directory, file_info_.is_directory);
   EXPECT_EQ(expected_info.is_symbolic_link, file_info_.is_symbolic_link);
-  EXPECT_EQ(expected_info.last_modified, file_info_.last_modified);
-  EXPECT_EQ(expected_info.last_accessed, file_info_.last_accessed);
-  EXPECT_EQ(expected_info.creation_time, file_info_.creation_time);
-}
 
-TEST_F(FileUtilProxyTest, GetFileInfo_Directory) {
-  // Setup.
-  ASSERT_TRUE(base::CreateDirectory(test_path()));
-  File::Info expected_info;
-  GetFileInfo(test_path(), &expected_info);
-
-  // Run.
-  FileUtilProxy::GetFileInfo(
-      file_task_runner(),
-      test_path(),
-      Bind(&FileUtilProxyTest::DidGetFileInfo, weak_factory_.GetWeakPtr()));
-  MessageLoop::current()->Run();
-
-  // Verify.
-  EXPECT_EQ(File::FILE_OK, error_);
-  EXPECT_EQ(expected_info.size, file_info_.size);
-  EXPECT_EQ(expected_info.is_directory, file_info_.is_directory);
-  EXPECT_EQ(expected_info.is_symbolic_link, file_info_.is_symbolic_link);
+  File file = proxy.TakeFile();
+  EXPECT_TRUE(file.GetInfo(&expected_info));

[kinuko, 2014/03/17 03:50:28]

Sorry I hadn't responded quickly... I'm ok with this change, but this line'd
likely confuse readers in a future.  Would it be possible to make a small
follow-up change and add a comment why we do here, along with the bug number
you're opening for the issue that we use different impl (which leads to have
different precision)?  Thanks.

   EXPECT_EQ(expected_info.last_modified, file_info_.last_modified);
-  EXPECT_EQ(expected_info.last_accessed, file_info_.last_accessed);
   EXPECT_EQ(expected_info.creation_time, file_info_.creation_time);
 }
 
-TEST_F(FileUtilProxyTest, Read) {
+TEST_F(FileProxyTest, Read) {
   // Setup.
   const char expected_data[] = "bleh";
   int expected_bytes = arraysize(expected_data);
   ASSERT_EQ(expected_bytes,
-            WriteFile(test_path(), expected_data, expected_bytes));
+            base::WriteFile(test_path(), expected_data, expected_bytes));
 
   // Run.
-  FileUtilProxy::Read(
-      file_task_runner(),
-      GetTestPlatformFile(PLATFORM_FILE_OPEN | PLATFORM_FILE_READ),
-      0,  // offset
-      128,
-      Bind(&FileUtilProxyTest::DidRead, weak_factory_.GetWeakPtr()));
+  FileProxy proxy(file_task_runner());
+  CreateProxy(File::FLAG_OPEN | File::FLAG_READ, &proxy);
+
+  proxy.Read(0, 128, Bind(&FileProxyTest::DidRead, weak_factory_.GetWeakPtr()));
   MessageLoop::current()->Run();
 
   // Verify.
@@ -291,29 +238,21 @@ TEST_F(FileUtilProxyTest, Read) {
   }
 }
 
-TEST_F(FileUtilProxyTest, WriteAndFlush) {
+TEST_F(FileProxyTest, WriteAndFlush) {
+  FileProxy proxy(file_task_runner());
+  CreateProxy(File::FLAG_CREATE | File::FLAG_WRITE, &proxy);
+
   const char data[] = "foo!";
   int data_bytes = ARRAYSIZE_UNSAFE(data);
-  PlatformFile file = GetTestPlatformFile(
-      PLATFORM_FILE_CREATE | PLATFORM_FILE_WRITE);
-
-  FileUtilProxy::Write(
-      file_task_runner(),
-      file,
-      0,  // offset
-      data,
-      data_bytes,
-      Bind(&FileUtilProxyTest::DidWrite, weak_factory_.GetWeakPtr()));
+  proxy.Write(0, data, data_bytes,
+              Bind(&FileProxyTest::DidWrite, weak_factory_.GetWeakPtr()));
   MessageLoop::current()->Run();
   EXPECT_EQ(File::FILE_OK, error_);
   EXPECT_EQ(data_bytes, bytes_written_);
 
   // Flush the written data.  (So that the following read should always
   // succeed.  On some platforms it may work with or without this flush.)
-  FileUtilProxy::Flush(
-      file_task_runner(),
-      file,
-      Bind(&FileUtilProxyTest::DidFinish, weak_factory_.GetWeakPtr()));
+  proxy.Flush(Bind(&FileProxyTest::DidFinish, weak_factory_.GetWeakPtr()));
   MessageLoop::current()->Run();
   EXPECT_EQ(File::FILE_OK, error_);
 
@@ -325,18 +264,17 @@ TEST_F(FileUtilProxyTest, WriteAndFlush) {
   }
 }
 
-TEST_F(FileUtilProxyTest, Touch) {
+TEST_F(FileProxyTest, SetTimes) {
+  FileProxy proxy(file_task_runner());
+  CreateProxy(
+      File::FLAG_CREATE | File::FLAG_WRITE | File::FLAG_WRITE_ATTRIBUTES,
+      &proxy);
+
   Time last_accessed_time = Time::Now() - TimeDelta::FromDays(12345);
   Time last_modified_time = Time::Now() - TimeDelta::FromHours(98765);
 
-  FileUtilProxy::Touch(
-      file_task_runner(),
-      GetTestPlatformFile(PLATFORM_FILE_CREATE |
-                          PLATFORM_FILE_WRITE |
-                          PLATFORM_FILE_WRITE_ATTRIBUTES),
-      last_accessed_time,
-      last_modified_time,
-      Bind(&FileUtilProxyTest::DidFinish, weak_factory_.GetWeakPtr()));
+  proxy.SetTimes(last_accessed_time, last_modified_time,
+                 Bind(&FileProxyTest::DidFinish, weak_factory_.GetWeakPtr()));
   MessageLoop::current()->Run();
   EXPECT_EQ(File::FILE_OK, error_);
 
@@ -351,20 +289,19 @@ TEST_F(FileUtilProxyTest, Touch) {
             static_cast<int>(info.last_accessed.ToDoubleT()));
 }
 
-TEST_F(FileUtilProxyTest, Truncate_Shrink) {
+TEST_F(FileProxyTest, SetLength_Shrink) {
   // Setup.
   const char kTestData[] = "0123456789";
-  ASSERT_EQ(10, WriteFile(test_path(), kTestData, 10));
+  ASSERT_EQ(10, base::WriteFile(test_path(), kTestData, 10));
   File::Info info;
   GetFileInfo(test_path(), &info);
   ASSERT_EQ(10, info.size);
 
   // Run.
-  FileUtilProxy::Truncate(
-      file_task_runner(),
-      GetTestPlatformFile(PLATFORM_FILE_OPEN | PLATFORM_FILE_WRITE),
-      7,
-      Bind(&FileUtilProxyTest::DidFinish, weak_factory_.GetWeakPtr()));
+  FileProxy proxy(file_task_runner());
+  CreateProxy(File::FLAG_OPEN | File::FLAG_WRITE, &proxy);
+  proxy.SetLength(7,
+                  Bind(&FileProxyTest::DidFinish, weak_factory_.GetWeakPtr()));
   MessageLoop::current()->Run();
 
   // Verify.
@@ -378,20 +315,19 @@ TEST_F(FileUtilProxyTest, Truncate_Shrink) {
     EXPECT_EQ(kTestData[i], buffer[i]);
 }
 
-TEST_F(FileUtilProxyTest, Truncate_Expand) {
+TEST_F(FileProxyTest, SetLength_Expand) {
   // Setup.
   const char kTestData[] = "9876543210";
-  ASSERT_EQ(10, WriteFile(test_path(), kTestData, 10));
+  ASSERT_EQ(10, base::WriteFile(test_path(), kTestData, 10));
   File::Info info;
   GetFileInfo(test_path(), &info);
   ASSERT_EQ(10, info.size);
 
   // Run.
-  FileUtilProxy::Truncate(
-      file_task_runner(),
-      GetTestPlatformFile(PLATFORM_FILE_OPEN | PLATFORM_FILE_WRITE),
-      53,
-      Bind(&FileUtilProxyTest::DidFinish, weak_factory_.GetWeakPtr()));
+  FileProxy proxy(file_task_runner());
+  CreateProxy(File::FLAG_OPEN | File::FLAG_WRITE, &proxy);
+  proxy.SetLength(53,
+                  Bind(&FileProxyTest::DidFinish, weak_factory_.GetWeakPtr()));
   MessageLoop::current()->Run();
 
   // Verify.