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..a66ade67c81a4f015c79c83f1d0a4a46028ab04f 100644
--- a/net/base/file_stream_unittest.cc
+++ b/net/base/file_stream_unittest.cc
@@ -78,6 +78,68 @@ 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;
+  {
+    FileStream stream(NULL);

[mmenke, 2013/09/10 18:32:52]

nit:  I think it's generally a little clearer to use a scoped_ptr and reset() to
delete instead of a nested block of code.  Also, while one or two other tests in
this file also use this method, it looks to me like scoped_ptrs are the more
common method used to do this in this file as well.  Also seems like there's
less than can go wrong in future refactoring or reuse of the tests (If callback
were declared in the inner code block, for example, things would go badly).

Same goes for the next test as well, of course.

[waffles, 2013/09/10 21:41:34]

I agree that scoped_ptr is better. I had noticed the nested block in
BasicOpenClose, which is highly similar to these tests, so I used it.

+    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));
+}
+
+TEST_F(FileStreamTest, AsyncOpenExplicitCloseOrphaned) {
+  base::PlatformFile file = base::kInvalidPlatformFileValue;
+  TestCompletionCallback callback;
+  base::PlatformFileInfo info;
+  {
+    FileStream stream(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()));
+  }
+  // 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,