URLFetcher won't call delegate until file is closed.

net/base/file_stream_unittest.cc

Index: net/base/file_stream_unittest.cc
diff --git a/net/base/file_stream_unittest.cc b/net/base/file_stream_unittest.cc
index 8eed06e0694c408e2b9eb1f65c450d0d1f06302f..25f875b8aac2cd60c0fa6566f09dce7edf17d5d3 100644
--- a/net/base/file_stream_unittest.cc
+++ b/net/base/file_stream_unittest.cc
@@ -78,6 +78,67 @@ TEST_F(FileStreamTest, BasicOpenClose) {
   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,