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

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

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

Author: rvargas@chromium.org

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) {

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,

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_;

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());