Base: Make FileProxy automaticaly close the file on a worker thread.

This CL removes the restriction that callers should call Close before
deleting the object if they want to make sure the file is not closed
on the current thread.

BUG=322664
TEST=base_unittests

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=263675

Review URL: https://codereview.chromium.org/231703002

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

4 files changed, 37 insertions, 11 deletions

diff --git a/base/files/file.cc b/base/files/file.cc
index 2a2f843..da261d6 100644
--- a/base/files/file.cc
+++ b/base/files/file.cc
@@ -60,6 +60,8 @@ File::File(RValue other)
 }
 
 File::~File() {
+  // Go through the AssertIOAllowed logic.
+  Close();
 }
 
 File& File::operator=(RValue other) {
diff --git a/base/files/file_proxy.cc b/base/files/file_proxy.cc
index b517761..fa04d7c 100644
--- a/base/files/file_proxy.cc
+++ b/base/files/file_proxy.cc
@@ -13,27 +13,38 @@
 #include "base/task_runner.h"
 #include "base/task_runner_util.h"
 
+namespace {
+
+void FileDeleter(base::File file) {
+}
+
+}  // namespace
+
 namespace base {
 
 class FileHelper {
  public:
    FileHelper(FileProxy* proxy, File file)
       : file_(file.Pass()),
-        proxy_(AsWeakPtr(proxy)),
-        error_(File::FILE_ERROR_FAILED) {
+        error_(File::FILE_ERROR_FAILED),
+        task_runner_(proxy->task_runner()),
+        proxy_(AsWeakPtr(proxy)) {
    }
 
    void PassFile() {
      if (proxy_)
        proxy_->SetFile(file_.Pass());
+     else if (file_.IsValid())
+       task_runner_->PostTask(FROM_HERE, Bind(&FileDeleter, Passed(&file_)));
    }
 
  protected:
   File file_;
-  WeakPtr<FileProxy> proxy_;
   File::Error error_;
 
  private:
+  scoped_refptr<TaskRunner> task_runner_;
+  WeakPtr<FileProxy> proxy_;
   DISALLOW_COPY_AND_ASSIGN(FileHelper);
 };
 
@@ -219,13 +230,12 @@ class WriteHelper : public FileHelper {
 
 }  // namespace
 
-FileProxy::FileProxy() : task_runner_(NULL) {
-}
-
 FileProxy::FileProxy(TaskRunner* task_runner) : task_runner_(task_runner) {
 }
 
 FileProxy::~FileProxy() {
+  if (file_.IsValid())
+    task_runner_->PostTask(FROM_HERE, Bind(&FileDeleter, Passed(&file_)));
 }
 
 bool FileProxy::CreateOrOpen(const FilePath& file_path,
diff --git a/base/files/file_proxy.h b/base/files/file_proxy.h
index f02960b..3c834f6 100644
--- a/base/files/file_proxy.h
+++ b/base/files/file_proxy.h
@@ -25,11 +25,8 @@ class Time;
 // same rules of the equivalent File method, as they are implemented by bouncing
 // the operation to File using a TaskRunner.
 //
-// This class does NOT perform automatic proxying to close the underlying file
-// at destruction, which means that it may potentially close the file in the
-// wrong thread (the current thread). If that is not appropriate, the caller
-// must ensure that Close() is called, so that the operation happens on the
-// desired thread.
+// This class performs automatic proxying to close the underlying file at
+// destruction.
 //
 // The TaskRunner is in charge of any sequencing of the operations, but a single
 // operation can be proxied at a time, regardless of the use of a callback.
@@ -131,6 +128,7 @@ class BASE_EXPORT FileProxy : public SupportsWeakPtr<FileProxy> {
  private:
   friend class FileHelper;
   void SetFile(File file);
+  TaskRunner* task_runner() { return task_runner_.get(); }
 
   scoped_refptr<TaskRunner> task_runner_;
   File file_;
diff --git a/base/files/file_proxy_unittest.cc b/base/files/file_proxy_unittest.cc
index bb7e6c3..7748923 100644
--- a/base/files/file_proxy_unittest.cc
+++ b/base/files/file_proxy_unittest.cc
@@ -14,6 +14,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/threading/thread.h"
+#include "base/threading/thread_restrictions.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace base {
@@ -143,6 +144,21 @@ TEST_F(FileProxyTest, CreateOrOpen_OpenNonExistent) {
   EXPECT_FALSE(PathExists(test_path()));
 }
 
+TEST_F(FileProxyTest, CreateOrOpen_AbandonedCreate) {
+  bool prev = ThreadRestrictions::SetIOAllowed(false);
+  {
+    FileProxy proxy(file_task_runner());
+    proxy.CreateOrOpen(
+        test_path(),
+        File::FLAG_CREATE | File::FLAG_READ,
+        Bind(&FileProxyTest::DidCreateOrOpen, weak_factory_.GetWeakPtr()));
+  }
+  MessageLoop::current()->Run();
+  ThreadRestrictions::SetIOAllowed(prev);
+
+  EXPECT_TRUE(PathExists(test_path()));
+}
+
 TEST_F(FileProxyTest, Close) {
   // Creates a file.
   FileProxy proxy(file_task_runner());