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

Unified Diff: net/http/http_stream_factory_impl_job_controller_unittest.cc

Issue 2703943002: Cap the delay time for the main job when racing with QUIC for delayed tcp case so that if http_serv… (Closed)
Patch Set: Created 3 years, 10 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
« no previous file with comments | « net/http/http_stream_factory_impl_job_controller.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/http/http_stream_factory_impl_job_controller_unittest.cc
diff --git a/net/http/http_stream_factory_impl_job_controller_unittest.cc b/net/http/http_stream_factory_impl_job_controller_unittest.cc
index 75405d82dd8a895090e35ba0ab67d7711fef891e..d4f94a3e4afd1c704b3f158a873c174346e011b1 100644
--- a/net/http/http_stream_factory_impl_job_controller_unittest.cc
+++ b/net/http/http_stream_factory_impl_job_controller_unittest.cc
@@ -9,6 +9,7 @@
#include "base/memory/ptr_util.h"
#include "base/run_loop.h"
#include "base/test/histogram_tester.h"
+#include "base/test/scoped_mock_time_message_loop_task_runner.h"
#include "base/threading/platform_thread.h"
#include "net/base/test_proxy_delegate.h"
#include "net/dns/mock_host_resolver.h"
@@ -813,6 +814,7 @@ TEST_F(HttpStreamFactoryImplJobControllerTest,
}
TEST_F(HttpStreamFactoryImplJobControllerTest, DelayedTCP) {
+ base::ScopedMockTimeMessageLoopTaskRunner test_task_runner;
HangingResolver* resolver = new HangingResolver();
session_deps_.host_resolver.reset(resolver);
@@ -846,17 +848,11 @@ TEST_F(HttpStreamFactoryImplJobControllerTest, DelayedTCP) {
// The alternative job stalls as host resolution hangs when creating the QUIC
// request and controller should resume the main job after delay.
- base::RunLoop run_loop;
- EXPECT_CALL(*job_factory_.main_job(), Resume())
- .Times(1)
- .WillOnce(testing::DoAll(
- testing::Invoke(testing::CreateFunctor(
- &JobControllerPeer::VerifyWaitingTimeForMainJob, job_controller_,
- base::TimeDelta::FromMicroseconds(15))),
- testing::Invoke([&run_loop]() { run_loop.Quit(); })));
-
- // Wait for the main job to be resumed.
- run_loop.Run();
+ EXPECT_TRUE(test_task_runner->HasPendingTask());
+ EXPECT_EQ(1u, test_task_runner->GetPendingTaskCount());
+ EXPECT_CALL(*job_factory_.main_job(), Resume()).Times(1);
+ test_task_runner->FastForwardBy(base::TimeDelta::FromMicroseconds(15));
+ EXPECT_FALSE(test_task_runner->HasPendingTask());
EXPECT_TRUE(job_controller_->main_job());
EXPECT_TRUE(job_controller_->alternative_job());
@@ -865,16 +861,74 @@ TEST_F(HttpStreamFactoryImplJobControllerTest, DelayedTCP) {
EXPECT_CALL(request_delegate_, OnStreamFailed(_, _)).Times(0);
EXPECT_FALSE(JobControllerPeer::main_job_is_blocked(job_controller_));
- // OnStreamFailed will not resume the main job again since it's been resumed
- // already.
+ // OnStreamFailed will post a task to resume the main job immediately but
+ // won't call Resume() on the main job since it's been resumed already.
EXPECT_CALL(*job_factory_.main_job(), Resume()).Times(0);
-
job_controller_->OnStreamFailed(job_factory_.alternative_job(),
ERR_NETWORK_CHANGED, SSLConfig());
+ EXPECT_EQ(1u, test_task_runner->GetPendingTaskCount());
+ test_task_runner->RunUntilIdle();
+}
+
+// Test that main job is blocked for kMaxDelayTimeForMainJob(3s) if
+// http_server_properties cached an inappropriate large srtt for the server,
+// which would potentially delay the main job for a extremely long time in
+// delayed tcp case.
+TEST_F(HttpStreamFactoryImplJobControllerTest, DelayedTCPWithLargeSrtt) {
+ // Overrides the main thread's message loop with a mock tick clock so that we
+ // could verify the main job is resumed with appropriate delay.
+ base::ScopedMockTimeMessageLoopTaskRunner test_task_runner;
+ // The max delay time should be in sync with .cc file.
+ base::TimeDelta kMaxDelayTimeForMainJob = base::TimeDelta::FromSeconds(3);
+ HangingResolver* resolver = new HangingResolver();
+ session_deps_.host_resolver.reset(resolver);
+
+ HttpRequestInfo request_info;
+ request_info.method = "GET";
+ request_info.url = GURL("https://www.google.com");
+
+ Initialize(false);
+
+ // Enable delayed TCP and set a extremely large time delay for waiting job.
+ QuicStreamFactory* quic_stream_factory = session_->quic_stream_factory();
+ test::QuicStreamFactoryPeer::SetDelayTcpRace(quic_stream_factory, true);
+ quic_stream_factory->set_require_confirmation(false);
+ ServerNetworkStats stats1;
+ stats1.srtt = base::TimeDelta::FromSeconds(100);
+ session_->http_server_properties()->SetServerNetworkStats(
+ url::SchemeHostPort(GURL("https://www.google.com")), stats1);
+
+ // Set a SPDY alternative service for the server.
+ url::SchemeHostPort server(request_info.url);
+ AlternativeService alternative_service(kProtoQUIC, server.host(), 443);
+ SetAlternativeService(request_info, alternative_service);
+
+ request_.reset(
+ job_controller_->Start(request_info, &request_delegate_, nullptr,
+ NetLogWithSource(), HttpStreamRequest::HTTP_STREAM,
+ DEFAULT_PRIORITY, SSLConfig(), SSLConfig()));
+ EXPECT_TRUE(job_controller_->main_job());
+ EXPECT_TRUE(job_controller_->alternative_job());
+ EXPECT_TRUE(job_controller_->main_job()->is_waiting());
+
+ // The alternative job stalls as host resolution hangs when creating the QUIC
+ // request and controller should resume the main job after delay.
+ EXPECT_TRUE(test_task_runner->HasPendingTask());
+ EXPECT_EQ(1u, test_task_runner->GetPendingTaskCount());
+
+ EXPECT_CALL(*job_factory_.main_job(), Resume()).Times(1);
+ // Move forward the task runner with kMaxDelayTimeForMainJob and verify the
+ // main job is resumed.
+ test_task_runner->FastForwardBy(kMaxDelayTimeForMainJob);
+ EXPECT_FALSE(test_task_runner->HasPendingTask());
}
TEST_F(HttpStreamFactoryImplJobControllerTest,
ResumeMainJobImmediatelyOnStreamFailed) {
+ // Overrides the main thread's message loop with a mock tick clock so that we
+ // could verify the main job is resumed with appropriate delay.
+ base::ScopedMockTimeMessageLoopTaskRunner test_task_runner;
+
HangingResolver* resolver = new HangingResolver();
session_deps_.host_resolver.reset(resolver);
@@ -898,6 +952,9 @@ TEST_F(HttpStreamFactoryImplJobControllerTest,
AlternativeService alternative_service(kProtoQUIC, server.host(), 443);
SetAlternativeService(request_info, alternative_service);
+ // The alternative job stalls as host resolution hangs when creating the QUIC
+ // request and controller should resume the main job with delay.
+ // OnStreamFailed should resume the main job immediately.
request_.reset(
job_controller_->Start(request_info, &request_delegate_, nullptr,
NetLogWithSource(), HttpStreamRequest::HTTP_STREAM,
@@ -906,23 +963,30 @@ TEST_F(HttpStreamFactoryImplJobControllerTest,
EXPECT_TRUE(job_controller_->alternative_job());
EXPECT_TRUE(job_controller_->main_job()->is_waiting());
+ EXPECT_TRUE(test_task_runner->HasPendingTask());
+ EXPECT_EQ(1u, test_task_runner->GetPendingTaskCount());
+
// |alternative_job| fails but should not report status to Request.
EXPECT_CALL(request_delegate_, OnStreamFailed(_, _)).Times(0);
-
- // The alternative job stalls as host resolution hangs when creating the QUIC
- // request and controller should resume the main job with delay.
- // OnStreamFailed should resume the main job immediately.
- EXPECT_CALL(*job_factory_.main_job(), Resume())
- .Times(1)
- .WillOnce(Invoke(testing::CreateFunctor(
- &JobControllerPeer::VerifyWaitingTimeForMainJob, job_controller_,
- base::TimeDelta::FromMicroseconds(0))));
-
job_controller_->OnStreamFailed(job_factory_.alternative_job(),
ERR_NETWORK_CHANGED, SSLConfig());
+ EXPECT_EQ(2u, test_task_runner->GetPendingTaskCount());
- base::RunLoop().RunUntilIdle();
+ // Verify the main job will be resumed immediately.
+ EXPECT_CALL(*job_factory_.main_job(), Resume()).Times(1);
+ // Execute tasks that have no remaining delay. Tasks with nonzero delay will
+ // remain queued.
+ test_task_runner->RunUntilIdle();
+
+ // Verify there is another task to resume main job with delay but should
+ // not call Resume() on the main job as main job has been resumed.
+ EXPECT_TRUE(test_task_runner->HasPendingTask());
+ EXPECT_EQ(1u, test_task_runner->GetPendingTaskCount());
+ EXPECT_CALL(*job_factory_.main_job(), Resume()).Times(0);
+ test_task_runner->FastForwardBy(base::TimeDelta::FromMicroseconds(15));
+ EXPECT_FALSE(test_task_runner->HasPendingTask());
}
+
// Verifies that the alternative proxy server job is not created if the URL
// scheme is HTTPS.
TEST_F(HttpStreamFactoryImplJobControllerTest, HttpsURL) {
« no previous file with comments | « net/http/http_stream_factory_impl_job_controller.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698