Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(187)

Unified Diff: content/browser/loader/resource_dispatcher_host_unittest.cc

Issue 25772002: Allows prefetch requests to live beyond the renderer by delaying (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Addressing great comments from mmenke. Created 7 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 4a9e27553dd59efdc099870f899e4872016bbef9..be633ae7ad62778509fde49ed079eee713b2172c 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"
@@ -15,6 +16,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"
@@ -26,6 +28,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"
@@ -600,12 +603,19 @@ class ResourceDispatcherHostTest : public testing::Test,
const GURL& url);
// Generates a request using the given filter. This will probably be a
- // ForwardingFilter.
+ // ForwardingFilter. Uses the resource_type_ as the resource type.
void MakeTestRequest(ResourceMessageFilter* filter,
asanka 2013/10/11 21:01:20 Nit: Can you get rid of MakeTestRequest and resour
jkarlin2 2013/10/14 14:30:00 Done.
int render_view_id,
int request_id,
const GURL& url);
+ // Generates a request using the given filter. This will probably be a
+ // ForwardingFilter.
+ void MakeTestRequestWithResourceType(ResourceMessageFilter* filter,
+ int render_view_id, int request_id,
+ const GURL& url,
+ ResourceType::Type type);
+
void CancelRequest(int request_id);
void CompleteStartRequest(int request_id);
@@ -634,11 +644,6 @@ class ResourceDispatcherHostTest : public testing::Test,
SetResponse(headers, std::string());
}
- // Sets a particular resource type for any request from now on.
- void SetResourceType(ResourceType::Type type) {
- resource_type_ = type;
- }
-
void SendDataReceivedACKs(bool send_acks) {
send_data_received_acks_ = send_acks;
}
@@ -749,11 +754,21 @@ void ResourceDispatcherHostTest::MakeTestRequest(
int render_view_id,
int request_id,
const GURL& url) {
+ MakeTestRequestWithResourceType(filter, render_view_id, request_id, url,
+ resource_type_);
+}
+
+void ResourceDispatcherHostTest::MakeTestRequestWithResourceType(
+ ResourceMessageFilter* filter,
+ int render_view_id,
+ int request_id,
+ const GURL& url,
+ ResourceType::Type type) {
// 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);
@@ -908,6 +923,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);
+ MakeTestRequestWithResourceType(filter_.get(), 0, 1,
+ net::URLRequestTestJob::test_url_1(),
+ ResourceType::PREFETCH);
+ MakeTestRequestWithResourceType(filter_.get(), 0, 2,
+ net::URLRequestTestJob::test_url_2(),
+ ResourceType::PREFETCH);
+ MakeTestRequest(0, 3, net::URLRequestTestJob::test_url_3());
+
+ // test_url_1 is synchronous and already complete.
+ EXPECT_EQ(2, host_.pending_requests());
+ // We need to make sure that the request comes from the renderer, else it
+ // won't delay.
+ host_.CancelRequest(filter_->child_id(), 2, true);
+ host_.CancelRequest(filter_->child_id(), 3, true);
+
+ // 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->AppendSwitchASCII(switches::kDelayPrefetchCancellation, "200");
+
+ MakeTestRequestWithResourceType(filter_.get(), 0, 1,
+ net::URLRequestTestJob::test_url_2(),
+ ResourceType::PREFETCH);
+ host_.CancelRequest(filter_->child_id(), 1, true);
+ base::MessageLoop::current()->RunUntilIdle();
+ 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(210),
+ 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;
@@ -1544,11 +1654,11 @@ TEST_F(ResourceDispatcherHostTest, ForbiddenDownload) {
std::string response_data("<html><title>Test One</title></html>");
SetResponse(raw_headers, response_data);
- // Only MAIN_FRAMEs can trigger a download.
- SetResourceType(ResourceType::MAIN_FRAME);
-
HandleScheme("http");
- MakeTestRequest(0, 1, GURL("http:bla"));
+
+ // Only MAIN_FRAMEs can trigger a download.
+ MakeTestRequestWithResourceType(filter_.get(), 0, 1, GURL("http:bla"),
+ ResourceType::MAIN_FRAME);
// Flush all pending requests.
while (net::URLRequestTestJob::ProcessOnePendingMessage()) {}
@@ -1595,11 +1705,12 @@ TEST_F(ResourceDispatcherHostTest, IgnoreCancelForDownloads) {
response_data.resize(1025, ' ');
SetResponse(raw_headers, response_data);
- SetResourceType(ResourceType::MAIN_FRAME);
SetDelayedCompleteJobGeneration(true);
HandleScheme("http");
- MakeTestRequest(render_view_id, request_id, GURL("http://example.com/blah"));
+ MakeTestRequestWithResourceType(filter_.get(), render_view_id, request_id,
+ GURL("http://example.com/blah"),
+ ResourceType::MAIN_FRAME);
// Return some data so that the request is identified as a download
// and the proper resource handlers are created.
EXPECT_TRUE(net::URLRequestTestJob::ProcessOnePendingMessage());
@@ -1630,11 +1741,12 @@ TEST_F(ResourceDispatcherHostTest, CancelRequestsForContext) {
response_data.resize(1025, ' ');
SetResponse(raw_headers, response_data);
- SetResourceType(ResourceType::MAIN_FRAME);
SetDelayedCompleteJobGeneration(true);
HandleScheme("http");
- MakeTestRequest(render_view_id, request_id, GURL("http://example.com/blah"));
+ MakeTestRequestWithResourceType(filter_.get(), render_view_id, request_id,
+ GURL("http://example.com/blah"),
+ ResourceType::MAIN_FRAME);
// Return some data so that the request is identified as a download
// and the proper resource handlers are created.
EXPECT_TRUE(net::URLRequestTestJob::ProcessOnePendingMessage());
@@ -1671,10 +1783,12 @@ TEST_F(ResourceDispatcherHostTest, CancelRequestsForContextTransferred) {
std::string response_data("<html>foobar</html>");
SetResponse(raw_headers, response_data);
- SetResourceType(ResourceType::MAIN_FRAME);
HandleScheme("http");
- MakeTestRequest(render_view_id, request_id, GURL("http://example.com/blah"));
+ MakeTestRequestWithResourceType(filter_.get(), render_view_id, request_id,
+ GURL("http://example.com/blah"),
+ ResourceType::MAIN_FRAME);
+
GlobalRequestID global_request_id(filter_->child_id(), request_id);
host_.MarkAsTransferredNavigation(global_request_id,
@@ -1711,7 +1825,6 @@ TEST_F(ResourceDispatcherHostTest, TransferNavigationHtml) {
SetResponse("HTTP/1.1 302 Found\n"
"Location: http://other.com/blech\n\n");
- SetResourceType(ResourceType::MAIN_FRAME);
HandleScheme("http");
// Temporarily replace ContentBrowserClient with one that will trigger the
@@ -1719,7 +1832,8 @@ TEST_F(ResourceDispatcherHostTest, TransferNavigationHtml) {
TransfersAllNavigationsContentBrowserClient new_client;
ContentBrowserClient* old_client = SetBrowserClientForTesting(&new_client);
- MakeTestRequest(render_view_id, request_id, GURL("http://example.com/blah"));
+ MakeTestRequestWithResourceType(filter_.get(), render_view_id, request_id,
+ GURL("http://example.com/blah"), ResourceType::MAIN_FRAME);
// Now that we're blocked on the redirect, update the response and unblock by
// telling the AsyncResourceHandler to follow the redirect.
@@ -1782,7 +1896,6 @@ TEST_F(ResourceDispatcherHostTest, TransferNavigationText) {
SetResponse("HTTP/1.1 302 Found\n"
"Location: http://other.com/blech\n\n");
- SetResourceType(ResourceType::MAIN_FRAME);
HandleScheme("http");
// Temporarily replace ContentBrowserClient with one that will trigger the
@@ -1790,7 +1903,9 @@ TEST_F(ResourceDispatcherHostTest, TransferNavigationText) {
TransfersAllNavigationsContentBrowserClient new_client;
ContentBrowserClient* old_client = SetBrowserClientForTesting(&new_client);
- MakeTestRequest(render_view_id, request_id, GURL("http://example.com/blah"));
+ MakeTestRequestWithResourceType(filter_.get(), render_view_id, request_id,
+ GURL("http://example.com/blah"),
+ ResourceType::MAIN_FRAME);
// Now that we're blocked on the redirect, update the response and unblock by
// telling the AsyncResourceHandler to follow the redirect. Use a text/plain
@@ -1854,7 +1969,6 @@ TEST_F(ResourceDispatcherHostTest, TransferNavigationWithProcessCrash) {
"Location: http://other.com/blech\n\n");
const std::string kResponseBody = "hello world";
- SetResourceType(ResourceType::MAIN_FRAME);
HandleScheme("http");
// Temporarily replace ContentBrowserClient with one that will trigger the
@@ -1943,7 +2057,6 @@ TEST_F(ResourceDispatcherHostTest, TransferNavigationWithTwoRedirects) {
SetResponse("HTTP/1.1 302 Found\n"
"Location: http://other.com/blech\n\n");
- SetResourceType(ResourceType::MAIN_FRAME);
HandleScheme("http");
// Temporarily replace ContentBrowserClient with one that will trigger the
@@ -1951,7 +2064,9 @@ TEST_F(ResourceDispatcherHostTest, TransferNavigationWithTwoRedirects) {
TransfersAllNavigationsContentBrowserClient new_client;
ContentBrowserClient* old_client = SetBrowserClientForTesting(&new_client);
- MakeTestRequest(render_view_id, request_id, GURL("http://example.com/blah"));
+ MakeTestRequestWithResourceType(filter_.get(), render_view_id, request_id,
+ GURL("http://example.com/blah"),
+ ResourceType::MAIN_FRAME);
// Now that we're blocked on the redirect, simulate hitting another redirect.
SetResponse("HTTP/1.1 302 Found\n"
@@ -2024,10 +2139,10 @@ TEST_F(ResourceDispatcherHostTest, TransferNavigationWithTwoRedirects) {
TEST_F(ResourceDispatcherHostTest, UnknownURLScheme) {
EXPECT_EQ(0, host_.pending_requests());
- SetResourceType(ResourceType::MAIN_FRAME);
HandleScheme("http");
- MakeTestRequest(0, 1, GURL("foo://bar"));
+ MakeTestRequestWithResourceType(filter_.get(), 0, 1, GURL("foo://bar"),
+ ResourceType::MAIN_FRAME);
// Flush all pending requests.
while (net::URLRequestTestJob::ProcessOnePendingMessage()) {}
@@ -2074,6 +2189,30 @@ 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).
+ MakeTestRequestWithResourceType(filter_.get(), 0, 1,
+ GURL("big-job:0123456789,1000000"),
+ ResourceType::PREFETCH);
+
+ // Sort all the messages we saw by request.
+ ResourceIPCAccumulator::ClassifiedMessages msgs;
+ accum_.GetClassifiedMessages(&msgs);
+
+ EXPECT_EQ(2u, 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());

Powered by Google App Engine
This is Rietveld 408576698