Revert of Base: Make FileProxy automaticaly close the file on a worker thread. (https://codereview.chromium.org/231703002/)
Reason for revert:
Triggering AssertIOAllowed on a handful of Blink LayoutTests.
Original issue's description:
> 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
TBR=willchan@chromium.org,rvargas@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=322664
Review URL: https://codereview.chromium.org/238383003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@263758 0039d316-1c4b-4281-b951-d872f2087c98
CrOS-Libchrome-Original-Commit: 850a6e979ec81047da2f126c11f620e77e1f3ac9
diff --git a/base/files/file.cc b/base/files/file.cc
index da261d6..2a2f843 100644
--- a/base/files/file.cc
+++ b/base/files/file.cc
@@ -60,8 +60,6 @@
}
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 fa04d7c..b517761 100644
--- a/base/files/file_proxy.cc
+++ b/base/files/file_proxy.cc
@@ -13,38 +13,27 @@
#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()),
- error_(File::FILE_ERROR_FAILED),
- task_runner_(proxy->task_runner()),
- proxy_(AsWeakPtr(proxy)) {
+ proxy_(AsWeakPtr(proxy)),
+ error_(File::FILE_ERROR_FAILED) {
}
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);
};
@@ -230,12 +219,13 @@
} // 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 3c834f6..f02960b 100644
--- a/base/files/file_proxy.h
+++ b/base/files/file_proxy.h
@@ -25,8 +25,11 @@
// same rules of the equivalent File method, as they are implemented by bouncing
// the operation to File using a TaskRunner.
//
-// This class performs automatic proxying to close the underlying file at
-// destruction.
+// 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.
//
// 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.
@@ -128,7 +131,6 @@
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 7748923..bb7e6c3 100644
--- a/base/files/file_proxy_unittest.cc
+++ b/base/files/file_proxy_unittest.cc
@@ -14,7 +14,6 @@
#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 {
@@ -144,21 +143,6 @@
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());