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;
+  {
+    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,
       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