URLFetcher won't call delegate until file is closed.

net/base/file_stream_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/base/file_stream.h"
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/file_util.h"
 #include "base/message_loop/message_loop.h"
@@ skipping 60 matching lines @@
     EXPECT_EQ(OK, rv);
     EXPECT_TRUE(stream.IsOpen());
     file = stream.GetPlatformFileForTesting();
   }
   EXPECT_NE(base::kInvalidPlatformFileValue, file);
   base::PlatformFileInfo info;
   // The file should be closed.
   EXPECT_FALSE(base::GetPlatformFileInfo(file, &info));
 }
 
+TEST_F(FileStreamTest, BasicOpenExplicitClose) {
+  base::PlatformFile file = base::kInvalidPlatformFileValue;
+  FileStream stream(NULL);
+  int rv = stream.OpenSync(temp_file_path(),
+      base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_READ);
+  EXPECT_EQ(OK, rv);
+  EXPECT_TRUE(stream.IsOpen());
+  file = stream.GetPlatformFileForTesting();
+  EXPECT_NE(base::kInvalidPlatformFileValue, file);
+  EXPECT_EQ(OK, stream.CloseSync());
+  EXPECT_FALSE(stream.IsOpen());
+  base::PlatformFileInfo info;
+  // The file should be closed.
+  EXPECT_FALSE(base::GetPlatformFileInfo(file, &info));
+}
+
+TEST_F(FileStreamTest, AsyncOpenExplicitClose) {
+  base::PlatformFile file = base::kInvalidPlatformFileValue;
+  TestCompletionCallback callback;
+  scoped_ptr<FileStream> stream(new FileStream(NULL));
+  int flags = base::PLATFORM_FILE_OPEN |
+              base::PLATFORM_FILE_READ |
+              base::PLATFORM_FILE_ASYNC;
+  int rv = stream->Open(temp_file_path(), flags, callback.callback());
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+  EXPECT_EQ(OK, callback.WaitForResult());
+  EXPECT_TRUE(stream->IsOpen());
+  file = stream->GetPlatformFileForTesting();
+  EXPECT_EQ(ERR_IO_PENDING, stream->Close(callback.callback()));
+  EXPECT_EQ(OK, callback.WaitForResult());
+  EXPECT_FALSE(stream->IsOpen());
+  base::PlatformFileInfo info;
+  // The file should be closed.
+  EXPECT_FALSE(base::GetPlatformFileInfo(file, &info));
+  stream.reset();

[mmenke, 2013/09/10 22:38:19]

Shouldn't this go above the "base::PlatformFileInfo info;" line?

[waffles, 2013/09/10 22:58:33]

Previously, it did, but while changing it, it seemed to me that this is better.

Before, we checked:
  1) IsOpen() returns false before destroying the stream and
  2) the PlatformFile is closed *after* destroying the stream.

Now, we check: 
  1) IsOpen() returns false before destroying the stream and
  2) the PlatformFile is closed *before* destroying the stream.

The case that we now fail on that passed before is:
* Close() causes IsOpen() to return false but doesn't actually close the file;
running the destructor actually closes the file.

The only case I can think of that we now pass on that failed before is:
* Running the destructor re-opens the file. (seems far-fetched)

Of course, now it occurs to me that I should just remove the reset() altogether,
since I moved it to the end of the scope... and then that the scoped_ptr is
unnecessary after all. So I've now removed it.

Of course, all of the above does not apply to the Orphan test, since we want to
explicitly orphan the stream context before we RunUntilIdle().

+}
+
+TEST_F(FileStreamTest, AsyncOpenExplicitCloseOrphaned) {
+  base::PlatformFile file = base::kInvalidPlatformFileValue;
+  TestCompletionCallback callback;
+  base::PlatformFileInfo info;
+  scoped_ptr<FileStream> stream(new FileStream(
+      NULL, base::MessageLoopProxy::current()));
+  int flags = base::PLATFORM_FILE_OPEN |
+              base::PLATFORM_FILE_READ |
+              base::PLATFORM_FILE_ASYNC;
+  int rv = stream->Open(temp_file_path(), flags, callback.callback());
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+  EXPECT_EQ(OK, callback.WaitForResult());
+  EXPECT_TRUE(stream->IsOpen());
+  file = stream->GetPlatformFileForTesting();
+  EXPECT_EQ(ERR_IO_PENDING, stream->Close(callback.callback()));
+  stream.reset();
+  // File isn't actually closed yet.
+  EXPECT_TRUE(base::GetPlatformFileInfo(file, &info));
+  base::RunLoop runloop;
+  runloop.RunUntilIdle();
+  // The file should now be closed, though the callback has not been called.
+  EXPECT_FALSE(base::GetPlatformFileInfo(file, &info));
+}
+
 TEST_F(FileStreamTest, FileHandleNotLeftOpen) {
   bool created = false;
   ASSERT_EQ(kTestDataSize,
       file_util::WriteFile(temp_file_path(), kTestData, kTestDataSize));
   int flags = base::PLATFORM_FILE_OPEN_ALWAYS | base::PLATFORM_FILE_READ;
   base::PlatformFile file = base::CreatePlatformFile(
       temp_file_path(), flags, &created, NULL);
 
   {
     // Seek to the beginning of the file and read.
@@ skipping 1004 matching lines @@
   scoped_refptr<IOBuffer> buf = new WrappedIOBuffer(NULL);
   rv = stream->Read(buf.get(), 1, callback.callback());
   if (rv == ERR_IO_PENDING)
     rv = callback.WaitForResult();
   EXPECT_LT(rv, 0);
 }
 
 }  // namespace
 
 }  // namespace net