Allows prefetch requests to live beyond the renderer by delaying

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 75f3c7b2dccbf07517dba4a54bc83529e9b2618e..4ede3e3d0f2964b2f03cee10ac260d619a074431 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/command_line.h"
 #include "base/files/file_path.h"
 #include "base/memory/scoped_vector.h"
 #include "base/message_loop/message_loop.h"
@@ -14,6 +15,7 @@
 #include "content/browser/browser_thread_impl.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/loader/resource_dispatcher_host_impl.h"
+#include "content/browser/loader/resource_loader.h"
 #include "content/browser/loader/resource_message_filter.h"
 #include "content/browser/loader/resource_request_info_impl.h"
 #include "content/browser/worker_host/worker_service_impl.h"
@@ -25,6 +27,7 @@
 #include "content/public/browser/resource_dispatcher_host_delegate.h"
 #include "content/public/browser/resource_request_info.h"
 #include "content/public/browser/resource_throttle.h"
+#include "content/public/common/content_switches.h"
 #include "content/public/common/process_type.h"
 #include "content/public/common/resource_response.h"
 #include "content/public/test/test_browser_context.h"
@@ -599,12 +602,21 @@ class ResourceDispatcherHostTest : public testing::Test,
                        const GURL& url);
 
   // Generates a request using the given filter. This will probably be a
+  // ForwardingFilter. Uses the resource_type_ as the resource type.
+  void MakeTestRequest(ResourceMessageFilter* filter,
+                       int render_view_id,
+                       int request_id,
+                       const GURL& url);
+
+  // Generates a request using the given filter. This will probably be a
   // ForwardingFilter.
   void MakeTestRequest(ResourceMessageFilter* filter,
                        int render_view_id,
                        int request_id,
+                       ResourceType::Type type,
                        const GURL& url);

[mmenke, 2013/10/07 19:16:24]

I think it's a bad idea to have two ways to set the resource type.  I'd suggest
getting rid of the old method, updating old callers and getting rid of
resource_type_.  Old one is only used 8 or 9 times, so not a huge effort.

[mmenke, 2013/10/07 19:16:24]

Google style guide discourages operator overloading.  I'd suggest just renaming
this to MakeTestRequestWithResourceType.  Also suggest making type the last
parameter, to keep consistent parameter ordering between functions.

[mmenke, 2013/10/07 19:28:28]

-operator + function

[jkarlin2, 2013/10/08 11:53:01]

Done.

[jkarlin2, 2013/10/08 11:53:01]

Done, though the precedent is already set in the file to overload that function.
 I think the code was cleaner before, though it is probably a little clearer
now.

 
+

[mmenke, 2013/10/07 19:16:24]

nit:  Remove extra blank line.

[jkarlin2, 2013/10/08 11:53:01]

Done.

   void CancelRequest(int request_id);
 
   void CompleteStartRequest(int request_id);
@@ -748,11 +760,20 @@ void ResourceDispatcherHostTest::MakeTestRequest(
     int render_view_id,
     int request_id,
     const GURL& url) {
+  MakeTestRequest(filter, render_view_id, request_id, resource_type_, url);
+}
+
+void ResourceDispatcherHostTest::MakeTestRequest(
+    ResourceMessageFilter* filter,
+    int render_view_id,
+    int request_id,
+    ResourceType::Type type,
+    const GURL& url) {
   // If it's already there, this'll be dropped on the floor, which is fine.
   child_ids_.insert(filter->child_id());
 
   ResourceHostMsg_Request request =
-      CreateResourceRequest("GET", resource_type_, url);
+      CreateResourceRequest("GET", type, url);
   ResourceHostMsg_RequestResource msg(render_view_id, request_id, request);
   bool msg_was_ok;
   host_.OnMessageReceived(msg, filter, &msg_was_ok);
@@ -907,6 +928,101 @@ TEST_F(ResourceDispatcherHostTest, Cancel) {
   CheckCancelledRequestCompleteMessage(msgs[1][1]);
 }
 
+// Shows that unlike normal requests, prefetched async requests are not
+// immediately canceled, and will complete within a timeout period.
+TEST_F(ResourceDispatcherHostTest, PrefetchIgnoreCancel) {
+  CommandLine* command_line = CommandLine::ForCurrentProcess();
+  command_line->AppendSwitch(switches::kDelayPrefetchCancellation);
+  MakeTestRequest(filter_.get(), 0, 1, ResourceType::PREFETCH,
+                  net::URLRequestTestJob::test_url_1());
+  MakeTestRequest(filter_.get(), 0, 2, ResourceType::PREFETCH,
+                  net::URLRequestTestJob::test_url_2());
+  MakeTestRequest(0, 3, net::URLRequestTestJob::test_url_3());
+
+  // test_url_1 is synchronous and already complete.
+  EXPECT_EQ(2, host_.pending_requests());
+  CancelRequest(2);
+  CancelRequest(3);
+
+  // Process any completion messages from cancelling.
+  base::MessageLoop::current()->RunUntilIdle();
+
+  EXPECT_EQ(1, host_.pending_requests());
+
+  // Run the requests to completion.
+  while (net::URLRequestTestJob::ProcessOnePendingMessage()) {}
+  base::MessageLoop::current()->RunUntilIdle();
+
+  EXPECT_EQ(0, host_.pending_requests());
+
+  ResourceIPCAccumulator::ClassifiedMessages msgs;
+  accum_.GetClassifiedMessages(&msgs);
+
+  ASSERT_EQ(3U, msgs.size());
+
+  // The first fetch was synchronous and should have completed successfully.
+  ASSERT_EQ(ResourceMsg_ReceivedResponse::ID, msgs[0][0].type());
+  ASSERT_EQ(ResourceMsg_RequestComplete::ID, msgs[0][1].type());
+
+  // The prefetch should have ignored the cancel and completed.  Note that
+  // prefetches run detached and do not receive data notifications.
+  ASSERT_EQ(ResourceMsg_ReceivedResponse::ID, msgs[1][0].type());
+  ASSERT_EQ(ResourceMsg_RequestComplete::ID, msgs[1][1].type());
+  // Verify that there was no error.
+  int request_id;
+  int error_code;
+  PickleIterator iter(msgs[1][1]);
+  ASSERT_TRUE(IPC::ReadParam(&msgs[1][1], &iter, &request_id));
+  ASSERT_TRUE(IPC::ReadParam(&msgs[1][1], &iter, &error_code));
+  EXPECT_EQ(0, error_code);
+
+  // Request 3 should have been cancelled immediately.
+  ASSERT_EQ(2U, msgs[2].size());
+  ASSERT_EQ(ResourceMsg_ReceivedResponse::ID, msgs[2][0].type());
+  CheckCancelledRequestCompleteMessage(msgs[2][1]);
+}
+
+// Shows that prefetched requests will timeout if the request takes too long
+// to complete.
+TEST_F(ResourceDispatcherHostTest, PrefetchIgnoreCancelTimesOut) {
+  CommandLine* command_line = CommandLine::ForCurrentProcess();
+  command_line->AppendSwitch(switches::kDelayPrefetchCancellation);
+
+  // Reduce the cancel timeout so that our test doesn't take forever.
+  const int kDelay = 200;
+  ResourceLoader::set_delay_prefetch_cancel_ms(kDelay);
+
+  MakeTestRequest(filter_.get(), 0, 1, ResourceType::PREFETCH,
+                  net::URLRequestTestJob::test_url_2());
+  CancelRequest(1);
+
+  EXPECT_EQ(1, host_.pending_requests());
+
+
+  // Wait until after the delay timer times before letting any Reads complete.
+  base::OneShotTimer<base::MessageLoop> timer;
+  timer.Start(FROM_HERE, base::TimeDelta::FromMilliseconds(kDelay + 10),
+              base::MessageLoop::current(), &base::MessageLoop::QuitWhenIdle);
+
+  // We should have cancelled the prefetch by now.
+  base::MessageLoop::current()->Run();
+  EXPECT_EQ(0, host_.pending_requests());
+
+  // In case any messages are still to be processed.
+  while (net::URLRequestTestJob::ProcessOnePendingMessage()) {}
+  base::MessageLoop::current()->RunUntilIdle();
+
+  ResourceIPCAccumulator::ClassifiedMessages msgs;
+  accum_.GetClassifiedMessages(&msgs);
+
+  ASSERT_EQ(1U, msgs.size());
+
+  // The request should have cancelled.
+  ASSERT_EQ(2U, msgs[0].size());
+  ASSERT_EQ(ResourceMsg_ReceivedResponse::ID, msgs[0][0].type());
+  CheckCancelledRequestCompleteMessage(msgs[0][1]);
+}
+
 TEST_F(ResourceDispatcherHostTest, CancelWhileStartIsDeferred) {
   bool was_deleted = false;
 
@@ -1992,6 +2108,29 @@ TEST_F(ResourceDispatcherHostTest, DataReceivedACKs) {
   EXPECT_EQ(ResourceMsg_RequestComplete::ID, msgs[0][size - 1].type());
 }
 
+// Request a very large prefetch.  This tests to make sure that the data
+// is not sent to the render process and verifies that the async handler
+// doesn't fill up its pending buffers and block.
+TEST_F(ResourceDispatcherHostTest, DetachedNoDataSentOrReceived) {
+  EXPECT_EQ(0, host_.pending_requests());
+
+  SendDataReceivedACKs(true);
+
+  HandleScheme("big-job");
+
+  // This request would normally result in many messages (>300).
+  MakeTestRequest(filter_.get(), 0, 1, ResourceType::PREFETCH,
+                  GURL("big-job:0123456789,1000000"));
+
+  // Sort all the messages we saw by request.
+  ResourceIPCAccumulator::ClassifiedMessages msgs;
+  accum_.GetClassifiedMessages(&msgs);
+
+  EXPECT_EQ(size_t(2), msgs[0].size());
+  ASSERT_EQ(ResourceMsg_ReceivedResponse::ID, msgs[0][0].type());
+  ASSERT_EQ(ResourceMsg_RequestComplete::ID, msgs[0][1].type());
+}
+
 TEST_F(ResourceDispatcherHostTest, DelayedDataReceivedACKs) {
   EXPECT_EQ(0, host_.pending_requests());