FileStream: Wait for IO completion in destructor regardless of auto_closed flag

We used to call WaitForIOCompletion in destructor only if auto_closed_ flag is true but we should do so not to leave stray IO events in non-autoclose cases as well.

BUG=116639
TEST=FileStreamTest.AsyncRead_EarlyDelete_NoAutoClose

Review URL: http://codereview.chromium.org/9595014

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

3 files changed, 56 insertions, 10 deletions

diff --git a/net/base/file_stream_posix.cc b/net/base/file_stream_posix.cc
index 620d35e..6c9b135 100644
--- a/net/base/file_stream_posix.cc
+++ b/net/base/file_stream_posix.cc
@@ -231,12 +231,14 @@ FileStreamPosix::FileStreamPosix(
 }
 
 FileStreamPosix::~FileStreamPosix() {
+  if (open_flags_ & base::PLATFORM_FILE_ASYNC) {
+    // Block until the last open/close/read/write operation is complete.
+    // TODO(satorux): Ideally we should not block. crbug.com/115067
+    WaitForIOCompletion();
+  }
+
   if (auto_closed_) {
     if (open_flags_ & base::PLATFORM_FILE_ASYNC) {
-      // Block until the last open/close/read/write operation is complete.
-      // TODO(satorux): Ideally we should not block. crbug.com/115067
-      WaitForIOCompletion();
-
       // Close the file in the background.
       if (IsOpen()) {
         const bool posted = base::WorkerPool::PostTask(
diff --git a/net/base/file_stream_unittest.cc b/net/base/file_stream_unittest.cc
index 39719e2d..9500709 100644
--- a/net/base/file_stream_unittest.cc
+++ b/net/base/file_stream_unittest.cc
@@ -327,6 +327,47 @@ TEST_F(FileStreamTest, AsyncRead_EarlyDelete) {
   }
 }
 
+// Similar to AsyncRead_EarlyDelete but using a given file handler rather than
+// calling FileStream::Open, to ensure that deleting a stream with in-flight
+// operation without auto-closing feature is also ok.
+TEST_F(FileStreamTest, AsyncRead_EarlyDelete_NoAutoClose) {
+  int64 file_size;
+  bool ok = file_util::GetFileSize(temp_file_path(), &file_size);
+  EXPECT_TRUE(ok);
+
+  bool created = false;
+  int flags = base::PLATFORM_FILE_OPEN |
+              base::PLATFORM_FILE_READ |
+              base::PLATFORM_FILE_ASYNC;
+  base::PlatformFileError error_code = base::PLATFORM_FILE_ERROR_FAILED;
+  base::PlatformFile file = base::CreatePlatformFile(
+      temp_file_path(), flags, &created, &error_code);
+  EXPECT_EQ(base::PLATFORM_FILE_OK, error_code);
+
+  scoped_ptr<FileStream> stream(new FileStream(file, flags, NULL));
+  int64 total_bytes_avail = stream->Available();
+  EXPECT_EQ(file_size, total_bytes_avail);
+
+  TestCompletionCallback callback;
+  scoped_refptr<IOBufferWithSize> buf = new IOBufferWithSize(4);
+  int rv = stream->Read(buf, buf->size(), callback.callback());
+  stream.reset();  // Delete instead of closing it.
+  if (rv < 0) {
+    EXPECT_EQ(ERR_IO_PENDING, rv);
+    // The callback should not be called if the request is cancelled.
+    MessageLoop::current()->RunAllPending();
+    EXPECT_FALSE(callback.have_result());
+  } else {
+    EXPECT_EQ(std::string(kTestData, rv), std::string(buf->data(), rv));
+  }
+
+  base::PlatformFileInfo info;
+  // The file should still be open.
+  EXPECT_TRUE(base::GetPlatformFileInfo(file, &info));
+  // Clean up.
+  EXPECT_TRUE(base::ClosePlatformFile(file));
+}
+
 TEST_F(FileStreamTest, BasicRead_FromOffset) {
   int64 file_size;
   bool ok = file_util::GetFileSize(temp_file_path(), &file_size);
diff --git a/net/base/file_stream_win.cc b/net/base/file_stream_win.cc
index 0754b9f..713e488 100644
--- a/net/base/file_stream_win.cc
+++ b/net/base/file_stream_win.cc
@@ -242,14 +242,17 @@ FileStreamWin::FileStreamWin(
 }
 
 FileStreamWin::~FileStreamWin() {
+  if (open_flags_ & base::PLATFORM_FILE_ASYNC) {
+    // Block until the in-flight open/close operation is complete.
+    // TODO(satorux): Ideally we should not block. crbug.com/115067
+    WaitForIOCompletion();
+
+    // Block until the last read/write operation is complete.
+    async_context_.reset();
+  }
+
   if (auto_closed_) {
     if (open_flags_ & base::PLATFORM_FILE_ASYNC) {
-      // Block until the in-flight open/close operation is complete.
-      // TODO(satorux): Ideally we should not block. crbug.com/115067
-      WaitForIOCompletion();
-      // Block until the last read/write operation is complete.
-      async_context_.reset();
-
       // Close the file in the background.
       if (IsOpen()) {
         const bool posted = base::WorkerPool::PostTask(