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

Side by Side Diff: net/http/http_stream_factory_impl_job_controller_unittest.cc

Issue 2619583002: Clean up HttpStreamFactoryImpl::JobController when Impl::Jobs complete (Closed)
Patch Set: address wez@ comment Created 3 years, 11 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 unified diff | 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 »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright (c) 2016 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2016 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "net/http/http_stream_factory_impl_job_controller.h" 5 #include "net/http/http_stream_factory_impl_job_controller.h"
6 6
7 #include <memory> 7 #include <memory>
8 8
9 #include "base/memory/ptr_util.h" 9 #include "base/memory/ptr_util.h"
10 #include "base/run_loop.h" 10 #include "base/run_loop.h"
(...skipping 353 matching lines...) Expand 10 before | Expand all | Expand 10 after
364 EXPECT_CALL(request_delegate_, OnStreamFailed(_, _)).Times(0); 364 EXPECT_CALL(request_delegate_, OnStreamFailed(_, _)).Times(0);
365 job_controller_->OnStreamFailed(job_factory_.alternative_job(), ERR_FAILED, 365 job_controller_->OnStreamFailed(job_factory_.alternative_job(), ERR_FAILED,
366 SSLConfig()); 366 SSLConfig());
367 367
368 VerifyBrokenAlternateProtocolMapping(request_info, true); 368 VerifyBrokenAlternateProtocolMapping(request_info, true);
369 // Reset the request as it's been successfully served. 369 // Reset the request as it's been successfully served.
370 request_.reset(); 370 request_.reset();
371 EXPECT_TRUE(HttpStreamFactoryImplPeer::IsJobControllerDeleted(factory_)); 371 EXPECT_TRUE(HttpStreamFactoryImplPeer::IsJobControllerDeleted(factory_));
372 } 372 }
373 373
374 // Tests that if alt job succeeds and main job is blocked, |request_| completion
375 // will clean up JobController. Regression test for crbug.com/678768.
376 TEST_F(HttpStreamFactoryImplJobControllerTest,
377 AltJobSucceedsMainJobBlockedControllerDestroyed) {
378 ProxyConfig proxy_config;
379 proxy_config.set_auto_detect(true);
380 MockAsyncProxyResolverFactory* proxy_resolver_factory =
381 new MockAsyncProxyResolverFactory(false);
382 session_deps_.proxy_service.reset(
383 new ProxyService(base::MakeUnique<ProxyConfigServiceFixed>(proxy_config),
384 base::WrapUnique(proxy_resolver_factory), nullptr));
385 Initialize(false);
386
387 HttpRequestInfo request_info;
388 request_info.method = "GET";
389 request_info.url = GURL("https://www.google.com");
390
391 url::SchemeHostPort server(request_info.url);
392 AlternativeService alternative_service(kProtoQUIC, server.host(), 443);
393 SetAlternativeService(request_info, alternative_service);
394 request_.reset(
395 job_controller_->Start(request_info, &request_delegate_, nullptr,
396 NetLogWithSource(), HttpStreamRequest::HTTP_STREAM,
397 DEFAULT_PRIORITY, SSLConfig(), SSLConfig()));
398 EXPECT_TRUE(job_controller_->main_job());
399 EXPECT_TRUE(job_controller_->alternative_job());
400 EXPECT_TRUE(JobControllerPeer::main_job_is_blocked(job_controller_));
401
402 // |alternative_job| succeeds and should report status to Request.
403 HttpStream* http_stream =
404 new HttpBasicStream(base::MakeUnique<ClientSocketHandle>(), false, false);
405 job_factory_.alternative_job()->SetStream(http_stream);
406 EXPECT_CALL(request_delegate_, OnStreamReady(_, _, http_stream))
407 .WillOnce(Invoke(DeleteHttpStreamPointer));
408 job_controller_->OnStreamReady(job_factory_.alternative_job(), SSLConfig());
409
410 EXPECT_TRUE(job_controller_->main_job());
411 EXPECT_TRUE(job_controller_->alternative_job());
412 EXPECT_TRUE(JobControllerPeer::main_job_is_blocked(job_controller_));
413
414 // Invoke OnRequestComplete() which should delete |job_controller_| from
415 // |factory_|.
416 request_.reset();
417 VerifyBrokenAlternateProtocolMapping(request_info, false);
418 // This fails without the fix for crbug.com/678768.
419 EXPECT_TRUE(HttpStreamFactoryImplPeer::IsJobControllerDeleted(factory_));
420 }
421
422 // Tests that if an orphaned job completes after |request_| is gone,
423 // JobController will be cleaned up.
424 TEST_F(HttpStreamFactoryImplJobControllerTest,
425 OrphanedJobCompletesControllerDestroyed) {
426 ProxyConfig proxy_config;
427 proxy_config.set_auto_detect(true);
428 MockAsyncProxyResolverFactory* proxy_resolver_factory =
429 new MockAsyncProxyResolverFactory(false);
430 session_deps_.proxy_service.reset(
431 new ProxyService(base::MakeUnique<ProxyConfigServiceFixed>(proxy_config),
432 base::WrapUnique(proxy_resolver_factory), nullptr));
433 Initialize(false);
434
435 HttpRequestInfo request_info;
436 request_info.method = "GET";
437 request_info.url = GURL("https://www.google.com");
438
439 url::SchemeHostPort server(request_info.url);
440 AlternativeService alternative_service(kProtoQUIC, server.host(), 443);
441 SetAlternativeService(request_info, alternative_service);
442 // Hack to use different URL for the main job to help differentiate the proxy
443 // requests.
444 job_factory_.UseDifferentURLForMainJob(GURL("http://www.google.com"));
445 request_.reset(
446 job_controller_->Start(request_info, &request_delegate_, nullptr,
447 NetLogWithSource(), HttpStreamRequest::HTTP_STREAM,
448 DEFAULT_PRIORITY, SSLConfig(), SSLConfig()));
449 EXPECT_TRUE(job_controller_->main_job());
450 EXPECT_TRUE(job_controller_->alternative_job());
451 EXPECT_TRUE(JobControllerPeer::main_job_is_blocked(job_controller_));
452
453 // Complete main job now.
454 MockAsyncProxyResolver resolver;
455 proxy_resolver_factory->pending_requests()[0]->CompleteNowWithForwarder(
456 net::OK, &resolver);
457 int main_job_request_id =
458 resolver.pending_jobs()[0]->url().SchemeIs("http") ? 0 : 1;
459
460 resolver.pending_jobs()[main_job_request_id]->results()->UseNamedProxy(
461 "result1:80");
462 resolver.pending_jobs()[main_job_request_id]->CompleteNow(net::OK);
463
464 HttpStream* http_stream =
465 new HttpBasicStream(base::MakeUnique<ClientSocketHandle>(), false, false);
466 job_factory_.main_job()->SetStream(http_stream);
467
468 EXPECT_CALL(request_delegate_, OnStreamReady(_, _, http_stream))
469 .WillOnce(Invoke(DeleteHttpStreamPointer));
470 job_controller_->OnStreamReady(job_factory_.main_job(), SSLConfig());
471 // Invoke OnRequestComplete() which should not delete |job_controller_| from
472 // |factory_| because alt job is yet to finish.
473 request_.reset();
474 ASSERT_FALSE(HttpStreamFactoryImplPeer::IsJobControllerDeleted(factory_));
475 EXPECT_FALSE(job_controller_->main_job());
476 EXPECT_TRUE(job_controller_->alternative_job());
477
478 // Make |alternative_job| succeed.
479 resolver.pending_jobs()[0]->results()->UseNamedProxy("result1:80");
480 resolver.pending_jobs()[0]->CompleteNow(net::OK);
481 HttpStream* http_stream2 =
482 new HttpBasicStream(base::MakeUnique<ClientSocketHandle>(), false, false);
483 job_factory_.alternative_job()->SetStream(http_stream2);
484 // This should not call request_delegate_::OnStreamReady.
485 job_controller_->OnStreamReady(job_factory_.alternative_job(), SSLConfig());
486 // Make sure that controller does not leak.
487 EXPECT_TRUE(HttpStreamFactoryImplPeer::IsJobControllerDeleted(factory_));
488 }
489
374 TEST_F(HttpStreamFactoryImplJobControllerTest, 490 TEST_F(HttpStreamFactoryImplJobControllerTest,
375 AltJobSucceedsAfterMainJobFailed) { 491 AltJobSucceedsAfterMainJobFailed) {
376 ProxyConfig proxy_config; 492 ProxyConfig proxy_config;
377 proxy_config.set_auto_detect(true); 493 proxy_config.set_auto_detect(true);
378 // Use asynchronous proxy resolver. 494 // Use asynchronous proxy resolver.
379 MockAsyncProxyResolverFactory* proxy_resolver_factory = 495 MockAsyncProxyResolverFactory* proxy_resolver_factory =
380 new MockAsyncProxyResolverFactory(false); 496 new MockAsyncProxyResolverFactory(false);
381 session_deps_.proxy_service.reset( 497 session_deps_.proxy_service.reset(
382 new ProxyService(base::MakeUnique<ProxyConfigServiceFixed>(proxy_config), 498 new ProxyService(base::MakeUnique<ProxyConfigServiceFixed>(proxy_config),
383 base::WrapUnique(proxy_resolver_factory), nullptr)); 499 base::WrapUnique(proxy_resolver_factory), nullptr));
(...skipping 557 matching lines...) Expand 10 before | Expand all | Expand 10 after
941 // Reset the request as it's been successfully served. 1057 // Reset the request as it's been successfully served.
942 request_.reset(); 1058 request_.reset();
943 EXPECT_TRUE(HttpStreamFactoryImplPeer::IsJobControllerDeleted(factory_)); 1059 EXPECT_TRUE(HttpStreamFactoryImplPeer::IsJobControllerDeleted(factory_));
944 1060
945 histogram_tester.ExpectUniqueSample("Net.QuicAlternativeProxy.Usage", 1061 histogram_tester.ExpectUniqueSample("Net.QuicAlternativeProxy.Usage",
946 2 /* ALTERNATIVE_PROXY_USAGE_LOST_RACE */, 1062 2 /* ALTERNATIVE_PROXY_USAGE_LOST_RACE */,
947 1); 1063 1);
948 } 1064 }
949 1065
950 } // namespace net 1066 } // namespace net
OLDNEW
« 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