Fix various issues in RedirectToFileResourceHandler.

content/browser/loader/resource_dispatcher_host_unittest.cc

Index: content/browser/loader/resource_dispatcher_host_unittest.cc
diff --git a/content/browser/loader/resource_dispatcher_host_unittest.cc b/content/browser/loader/resource_dispatcher_host_unittest.cc
index 895951bba1e90095b8bab75332df1c73d59898a7..c23d7cb7b7c43f3756c8940aa27c30c67253587c 100644
--- a/content/browser/loader/resource_dispatcher_host_unittest.cc
+++ b/content/browser/loader/resource_dispatcher_host_unittest.cc
@@ -5,6 +5,7 @@
 #include <vector>
 
 #include "base/bind.h"
+#include "base/file_util.h"
 #include "base/files/file_path.h"
 #include "base/memory/scoped_vector.h"
 #include "base/message_loop/message_loop.h"
@@ -31,6 +32,7 @@
 #include "content/public/common/process_type.h"
 #include "content/public/common/resource_response.h"
 #include "content/public/test/test_browser_context.h"
+#include "content/public/test/test_browser_thread_bundle.h"
 #include "content/test/test_content_browser_client.h"
 #include "net/base/net_errors.h"
 #include "net/base/request_priority.h"
@@ -45,10 +47,13 @@
 #include "net/url_request/url_request_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "webkit/common/appcache/appcache_interfaces.h"
+#include "webkit/common/blob/shareable_file_reference.h"
 
 // TODO(eroman): Write unit tests for SafeBrowsing that exercise
 //               SafeBrowsingResourceHandler.
 
+using webkit_blob::ShareableFileReference;
+
 namespace content {
 
 namespace {
@@ -85,6 +90,7 @@ static int RequestIDForMessage(const IPC::Message& msg) {
     case ResourceMsg_ReceivedRedirect::ID:
     case ResourceMsg_SetDataBuffer::ID:
     case ResourceMsg_DataReceived::ID:
+    case ResourceMsg_DataDownloaded::ID:
     case ResourceMsg_RequestComplete::ID: {
       bool result = PickleIterator(msg).ReadInt(&request_id);
       DCHECK(result);
@@ -518,32 +524,45 @@ class TestResourceDispatcherHostDelegate
   scoped_ptr<base::SupportsUserData::Data> user_data_;
 };
 
+class ShareableFileReferenceWaiter {
+ public:
+  ShareableFileReferenceWaiter(const base::FilePath& path) {
+    scoped_refptr<ShareableFileReference> file =
+        ShareableFileReference::Get(path);
+    file->AddFinalReleaseCallback(
+        base::Bind(&ShareableFileReferenceWaiter::Released,
+                   base::Unretained(this)));
+  }
+
+  void Wait() {
+    loop_.Run();
+  }
+
+ private:
+  void Released(const base::FilePath& path) {
+    loop_.Quit();
+  }
+
+  base::RunLoop loop_;
+  DISALLOW_COPY_AND_ASSIGN(ShareableFileReferenceWaiter);
+};
+
 class ResourceDispatcherHostTest : public testing::Test,
                                    public IPC::Sender {
  public:
   ResourceDispatcherHostTest()
-      : ui_thread_(BrowserThread::UI, &message_loop_),
-        file_thread_(BrowserThread::FILE_USER_BLOCKING, &message_loop_),
-        cache_thread_(BrowserThread::CACHE, &message_loop_),
-        io_thread_(BrowserThread::IO, &message_loop_),
+      : thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP),
         old_factory_(NULL),
         send_data_received_acks_(false) {
     browser_context_.reset(new TestBrowserContext());
     BrowserContext::EnsureResourceContextInitialized(browser_context_.get());
-    message_loop_.RunUntilIdle();
+    base::RunLoop().RunUntilIdle();
     ResourceContext* resource_context = browser_context_->GetResourceContext();
     filter_ = new ForwardingFilter(this, resource_context);
     resource_context->GetRequestContext()->set_network_delegate(
         &network_delegate_);
   }
 
-  virtual ~ResourceDispatcherHostTest() {
-    for (std::set<int>::iterator it = child_ids_.begin();
-         it != child_ids_.end(); ++it) {
-      host_.CancelRequestsForProcess(*it);
-    }
-  }
-
   // IPC::Sender implementation
   virtual bool Send(IPC::Message* msg) OVERRIDE {
     accum_.AddMessage(*msg);
@@ -553,6 +572,11 @@ class ResourceDispatcherHostTest : public testing::Test,
       GenerateDataReceivedACK(*msg);
     }
 
+    if (wait_for_request_complete_ &&
+        msg->type() == ResourceMsg_RequestComplete::ID) {
+      wait_for_request_complete_->Quit();
+    }
+
     delete msg;
     return true;
   }
@@ -584,6 +608,11 @@ class ResourceDispatcherHostTest : public testing::Test,
     DCHECK(test_fixture_ == this);
     test_fixture_ = NULL;
 
+    for (std::set<int>::iterator it = child_ids_.begin();
+         it != child_ids_.end(); ++it) {
+      host_.CancelRequestsForProcess(*it);
+    }
+
     host_.Shutdown();
 
     ChildProcessSecurityPolicyImpl::GetInstance()->Remove(0);
@@ -596,7 +625,7 @@ class ResourceDispatcherHostTest : public testing::Test,
     WorkerServiceImpl::GetInstance()->PerformTeardownForTesting();
 
     browser_context_.reset();
-    message_loop_.RunUntilIdle();
+    base::RunLoop().RunUntilIdle();
   }
 
   // Creates a request using the current test object as the filter and
@@ -713,11 +742,14 @@ class ResourceDispatcherHostTest : public testing::Test,
         base::Bind(&GenerateIPCMessage, filter_, base::Passed(&ack)));
   }
 
-  base::MessageLoopForIO message_loop_;
-  BrowserThreadImpl ui_thread_;
-  BrowserThreadImpl file_thread_;
-  BrowserThreadImpl cache_thread_;
-  BrowserThreadImpl io_thread_;
+  void WaitForRequestComplete() {
+    DCHECK(!wait_for_request_complete_);
+    wait_for_request_complete_.reset(new base::RunLoop);
+    wait_for_request_complete_->Run();
+    wait_for_request_complete_.reset();
+  }
+
+  content::TestBrowserThreadBundle thread_bundle_;
   scoped_ptr<TestBrowserContext> browser_context_;
   scoped_refptr<ForwardingFilter> filter_;
   net::TestNetworkDelegate network_delegate_;
@@ -729,6 +761,7 @@ class ResourceDispatcherHostTest : public testing::Test,
   net::URLRequest::ProtocolFactory* old_factory_;
   bool send_data_received_acks_;
   std::set<int> child_ids_;
+  scoped_ptr<base::RunLoop> wait_for_request_complete_;

[mmenke, 2013/12/04 20:55:48]

This name makes me expect a bool.  Maybe just wait_for_complete_run_loop_?

[davidben, 2013/12/04 22:44:04]

Done.

   static ResourceDispatcherHostTest* test_fixture_;
   static bool delay_start_;
   static bool delay_complete_;
@@ -2605,4 +2638,73 @@ TEST_F(ResourceDispatcherHostTest, DataReceivedUnexpectedACKs) {
   }
 }
 
+TEST_F(ResourceDispatcherHostTest, DownloadToFile) {
+  // Make a request which downloads to file.
+  ResourceHostMsg_Request request = CreateResourceRequest(
+      "GET", ResourceType::SUB_RESOURCE, net::URLRequestTestJob::test_url_1());
+  request.download_to_file = true;
+  ResourceHostMsg_RequestResource request_msg(0, 1, request);
+  bool msg_was_ok;
+  host_.OnMessageReceived(request_msg, filter_, &msg_was_ok);
+  ASSERT_TRUE(msg_was_ok);
+
+  // Running the message loop until idle does not work because
+  // RedirectToFileResourceHandler posts things to base::WorkerPool. Instead,
+  // wait for the ResourceMsg_RequestComplete to go out. Then run the event loop
+  // until idle so the loader is gone.
+  WaitForRequestComplete();
+  base::MessageLoop::current()->RunUntilIdle();
+  EXPECT_EQ(0, host_.pending_requests());
+
+  ResourceIPCAccumulator::ClassifiedMessages msgs;
+  accum_.GetClassifiedMessages(&msgs);
+
+  ASSERT_EQ(1U, msgs.size());
+  const std::vector<IPC::Message>& messages = msgs[0];
+
+  // The request should contain the following messages:
+  //     ReceivedResponse    (indicates headers received and filename)
+  //     DataDownloaded*     (bytes downloaded and total length)
+  //     RequestComplete     (request is done)
+
+  // ReceivedResponse
+  ResourceResponseHead response_head;
+  GetResponseHead(messages, &response_head);
+  ASSERT_FALSE(response_head.download_file_path.empty());
+
+  // DataDownloaded
+  size_t total_len = 0;
+  for (size_t i = 1; i < messages.size() - 1; i++) {
+    ASSERT_EQ(ResourceMsg_DataDownloaded::ID, messages[i].type());
+    PickleIterator iter(messages[i]);
+    int request_id, data_len;
+    ASSERT_TRUE(IPC::ReadParam(&messages[i], &iter, &request_id));
+    ASSERT_TRUE(IPC::ReadParam(&messages[i], &iter, &data_len));
+    total_len += data_len;
+  }
+  EXPECT_EQ(net::URLRequestTestJob::test_data_1().size(), total_len);
+
+  // RequestComplete
+  CheckRequestCompleteErrorCode(messages.back(), net::OK);
+
+  // Verify that the data ended up in the temporary file.
+  std::string contents;
+  ASSERT_TRUE(base::ReadFileToString(response_head.download_file_path,
+                                     &contents));
+  EXPECT_EQ(net::URLRequestTestJob::test_data_1(), contents);
+
+  // When the renderer releases the file, it should be deleted. Again,
+  // RunUntilIdle doesn't work because base::WorkerPool is involved.
+  ShareableFileReferenceWaiter waiter(response_head.download_file_path);
+  ResourceHostMsg_ReleaseDownloadedFile release_msg(1);
+  host_.OnMessageReceived(release_msg, filter_, &msg_was_ok);
+  ASSERT_TRUE(msg_was_ok);
+  waiter.Wait();
+  // The release callback runs before the delete is scheduled, so pump the
+  // message loop for the delete itself.
+  base::MessageLoop::current()->RunUntilIdle();
+
+  EXPECT_FALSE(base::PathExists(response_head.download_file_path));
+}
+
 }  // namespace content