Commit a692903a authored by zhongyi's avatar zhongyi Committed by Commit bot

JobController3: Move MarkAlternativeServiceBroken to job controller

BUG=

Review-Url: https://codereview.chromium.org/2332193003
Cr-Commit-Position: refs/heads/master@{#418768}
parent 55100839
......@@ -206,8 +206,6 @@ HttpStreamFactoryImpl::Job::Job(Delegate* delegate,
negotiated_protocol_(kProtoUnknown),
num_streams_(0),
spdy_session_direct_(false),
job_status_(STATUS_RUNNING),
other_job_status_(STATUS_RUNNING),
stream_type_(HttpStreamRequest::BIDIRECTIONAL_STREAM),
ptr_factory_(this) {
DCHECK(session);
......@@ -581,8 +579,6 @@ int HttpStreamFactoryImpl::Job::RunLoop(int result) {
}
case OK:
job_status_ = STATUS_SUCCEEDED;
MaybeMarkAlternativeServiceBroken();
next_state_ = STATE_DONE;
if (new_spdy_session_.get()) {
base::ThreadTaskRunnerHandle::Get()->PostTask(
......@@ -613,11 +609,6 @@ int HttpStreamFactoryImpl::Job::RunLoop(int result) {
return ERR_IO_PENDING;
default:
if (job_status_ != STATUS_BROKEN) {
DCHECK_EQ(STATUS_RUNNING, job_status_);
job_status_ = STATUS_FAILED;
MaybeMarkAlternativeServiceBroken();
}
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::Bind(&Job::OnStreamFailedCallback,
ptr_factory_.GetWeakPtr(), result));
......@@ -1063,25 +1054,17 @@ int HttpStreamFactoryImpl::Job::DoInitConnectionComplete(int result) {
return ReconsiderProxyAfterError(result);
}
if (IsSpdyAlternative() && !using_spdy_) {
job_status_ = STATUS_BROKEN;
MaybeMarkAlternativeServiceBroken();
if (IsSpdyAlternative() && !using_spdy_)
return ERR_NPN_NEGOTIATION_FAILED;
}
if (!ssl_started && result < 0 &&
(IsSpdyAlternative() || IsQuicAlternative())) {
job_status_ = STATUS_BROKEN;
MaybeMarkAlternativeServiceBroken();
(IsSpdyAlternative() || IsQuicAlternative()))
return result;
}
if (using_quic_) {
if (result < 0) {
job_status_ = STATUS_BROKEN;
MaybeMarkAlternativeServiceBroken();
if (result < 0)
return result;
}
if (stream_type_ == HttpStreamRequest::BIDIRECTIONAL_STREAM) {
bidirectional_stream_impl_ =
quic_request_.CreateBidirectionalStreamImpl();
......@@ -1515,93 +1498,6 @@ void HttpStreamFactoryImpl::Job::ReportJobSucceededForRequest() {
}
}
void HttpStreamFactoryImpl::Job::MarkOtherJobComplete(const Job& job) {
DCHECK_EQ(STATUS_RUNNING, other_job_status_);
DCHECK(!other_job_alternative_proxy_server_.is_valid());
other_job_status_ = job.job_status_;
other_job_alternative_service_ = job.alternative_service_;
other_job_alternative_proxy_server_ = job.alternative_proxy_server_;
// At most one job can have a valid |alternative_proxy_server_|.
DCHECK(!alternative_proxy_server_.is_valid() ||
!other_job_alternative_proxy_server_.is_valid());
MaybeMarkAlternativeServiceBroken();
}
void HttpStreamFactoryImpl::Job::MaybeMarkAlternativeServiceBroken() {
// At least one job should not be an alternative job.
DCHECK(alternative_service_.protocol == UNINITIALIZED_ALTERNATE_PROTOCOL ||
other_job_alternative_service_.protocol ==
UNINITIALIZED_ALTERNATE_PROTOCOL);
if (job_status_ == STATUS_RUNNING || other_job_status_ == STATUS_RUNNING)
return;
MaybeNotifyAlternativeProxyServerBroken();
if (IsSpdyAlternative() || IsQuicAlternative()) {
if (job_status_ == STATUS_BROKEN && other_job_status_ == STATUS_SUCCEEDED) {
HistogramBrokenAlternateProtocolLocation(
BROKEN_ALTERNATE_PROTOCOL_LOCATION_HTTP_STREAM_FACTORY_IMPL_JOB_ALT);
session_->http_server_properties()->MarkAlternativeServiceBroken(
alternative_service_);
}
return;
}
session_->quic_stream_factory()->OnTcpJobCompleted(job_status_ ==
STATUS_SUCCEEDED);
if (job_status_ == STATUS_SUCCEEDED && other_job_status_ == STATUS_BROKEN) {
HistogramBrokenAlternateProtocolLocation(
BROKEN_ALTERNATE_PROTOCOL_LOCATION_HTTP_STREAM_FACTORY_IMPL_JOB_MAIN);
session_->http_server_properties()->MarkAlternativeServiceBroken(
other_job_alternative_service_);
}
}
void HttpStreamFactoryImpl::Job::MaybeNotifyAlternativeProxyServerBroken()
const {
if (!alternative_proxy_server_.is_valid() &&
!other_job_alternative_proxy_server_.is_valid()) {
// Neither of the two jobs used an alternative proxy server.
return;
}
// Neither this job, nor the other job should have used the alternative
// service.
DCHECK_EQ(UNINITIALIZED_ALTERNATE_PROTOCOL, alternative_service_.protocol);
DCHECK_EQ(UNINITIALIZED_ALTERNATE_PROTOCOL,
other_job_alternative_service_.protocol);
ProxyDelegate* proxy_delegate = session_->params().proxy_delegate;
if (!proxy_delegate)
return;
if (alternative_proxy_server_.is_valid()) {
// |this| connected to the alternative proxy server.
if ((job_status_ == STATUS_BROKEN || job_status_ == STATUS_FAILED) &&
other_job_status_ == STATUS_SUCCEEDED) {
// Notify ProxyDelegate.
proxy_delegate->OnAlternativeProxyBroken(alternative_proxy_server_);
}
return;
}
if (other_job_alternative_proxy_server_.is_valid()) {
// Other job connected to the alternative proxy server.
if (job_status_ == STATUS_SUCCEEDED &&
(other_job_status_ == STATUS_BROKEN ||
other_job_status_ == STATUS_FAILED)) {
// Notify ProxyDelegate.
proxy_delegate->OnAlternativeProxyBroken(
other_job_alternative_proxy_server_);
}
return;
}
}
ClientSocketPoolManager::SocketGroupType
HttpStreamFactoryImpl::Job::GetSocketGroup() const {
std::string scheme = origin_url_.scheme();
......
......@@ -235,11 +235,16 @@ class HttpStreamFactoryImpl::Job {
// will be orphaned.
void ReportJobSucceededForRequest();
// Marks that the other |job| has completed.
virtual void MarkOtherJobComplete(const Job& job);
JobType job_type() const { return job_type_; }
const AlternativeService alternative_service() const {
return alternative_service_;
}
const ProxyServer alternative_proxy_server() const {
return alternative_proxy_server_;
}
private:
friend class HttpStreamFactoryImplJobPeer;
......@@ -281,13 +286,6 @@ class HttpStreamFactoryImpl::Job {
STATE_NONE
};
enum JobStatus {
STATUS_RUNNING,
STATUS_FAILED,
STATUS_BROKEN,
STATUS_SUCCEEDED
};
void OnStreamReadyCallback();
void OnBidirectionalStreamImplReadyCallback();
void OnWebSocketHandshakeStreamReadyCallback();
......@@ -380,13 +378,6 @@ class HttpStreamFactoryImpl::Job {
// Should we force QUIC for this stream request.
bool ShouldForceQuic() const;
void MaybeMarkAlternativeServiceBroken();
// May notify proxy delegate that the alternative proxy server is broken. The
// alternative proxy server is considered as broken if the alternative proxy
// server job failed, but the main job was successful.
void MaybeNotifyAlternativeProxyServerBroken() const;
ClientSocketPoolManager::SocketGroupType GetSocketGroup() const;
void MaybeCopyConnectionAttemptsFromSocketOrHandle();
......@@ -429,16 +420,10 @@ class HttpStreamFactoryImpl::Job {
// AlternativeService for this Job if this is an alternative Job.
const AlternativeService alternative_service_;
// AlternativeService for the other Job if this is not an alternative Job.
AlternativeService other_job_alternative_service_;
// Alternative proxy server that should be used by |this| to fetch the
// request.
const ProxyServer alternative_proxy_server_;
// Alternative proxy server for the other job.
ProxyServer other_job_alternative_proxy_server_;
// Unowned. |this| job is owned by |delegate_|.
Delegate* delegate_;
......@@ -493,9 +478,6 @@ class HttpStreamFactoryImpl::Job {
// Only used if |new_spdy_session_| is non-NULL.
bool spdy_session_direct_;
JobStatus job_status_;
JobStatus other_job_status_;
// Type of stream that is requested.
HttpStreamRequest::StreamType stream_type_;
......
......@@ -38,6 +38,7 @@ HttpStreamFactoryImpl::JobController::JobController(
request_(nullptr),
delegate_(delegate),
is_preconnect_(false),
alternative_job_failed_(false),
job_bound_(false),
main_job_is_blocked_(false),
bound_job_(nullptr),
......@@ -219,7 +220,6 @@ void HttpStreamFactoryImpl::JobController::OnWebSocketHandshakeStreamReady(
const ProxyInfo& used_proxy_info,
WebSocketHandshakeStreamBase* stream) {
DCHECK(job);
MarkRequestComplete(job->was_npn_negotiated(), job->negotiated_protocol(),
job->using_spdy());
......@@ -238,6 +238,9 @@ void HttpStreamFactoryImpl::JobController::OnStreamFailed(
Job* job,
int status,
const SSLConfig& used_ssl_config) {
if (job->job_type() == ALTERNATIVE)
OnAlternativeJobFailed(job);
MaybeResumeMainJob(job, base::TimeDelta());
if (job_bound_ && bound_job_ != job) {
......@@ -256,13 +259,10 @@ void HttpStreamFactoryImpl::JobController::OnStreamFailed(
// Hey, we've got other jobs! Maybe one of them will succeed, let's just
// ignore this failure.
factory_->request_map_.erase(job);
// Notify all the other jobs that this one failed.
if (job->job_type() == MAIN) {
alternative_job_->MarkOtherJobComplete(*job);
main_job_.reset();
} else {
DCHECK(job->job_type() == ALTERNATIVE);
main_job_->MarkOtherJobComplete(*job);
alternative_job_.reset();
}
return;
......@@ -420,6 +420,9 @@ void HttpStreamFactoryImpl::JobController::OnNewSpdySessionReady(
// Notify |request_|.
if (!is_preconnect_ && !is_job_orphaned) {
if (job->job_type() == MAIN && alternative_job_failed_)
ReportBrokenAlternativeService();
DCHECK(request_);
// The first case is the usual case.
......@@ -504,6 +507,7 @@ void HttpStreamFactoryImpl::JobController::MaybeResumeMainJob(
Job* job,
const base::TimeDelta& delay) {
DCHECK(job == main_job_.get() || job == alternative_job_.get());
if (!main_job_is_blocked_ || job != alternative_job_.get() || !main_job_)
return;
......@@ -731,17 +735,13 @@ void HttpStreamFactoryImpl::JobController::OnJobSucceeded(Job* job) {
CancelJobs();
return;
}
if (job->job_type() == MAIN && alternative_job_failed_)
ReportBrokenAlternativeService();
if (!bound_job_) {
if (main_job_ && alternative_job_) {
if (main_job_ && alternative_job_)
job->ReportJobSucceededForRequest();
// Notify all the other jobs that this one succeeded.
if (job->job_type() == MAIN) {
alternative_job_->MarkOtherJobComplete(*job);
} else {
DCHECK(job->job_type() == ALTERNATIVE);
main_job_->MarkOtherJobComplete(*job);
}
}
BindJob(job);
return;
}
......@@ -756,6 +756,46 @@ void HttpStreamFactoryImpl::JobController::MarkRequestComplete(
request_->Complete(was_npn_negotiated, negotiated_protocol, using_spdy);
}
void HttpStreamFactoryImpl::JobController::OnAlternativeJobFailed(Job* job) {
DCHECK_EQ(job->job_type(), ALTERNATIVE);
alternative_job_failed_ = true;
if (job->alternative_proxy_server().is_valid()) {
failed_alternative_proxy_server_ = job->alternative_proxy_server();
} else {
DCHECK(!failed_alternative_proxy_server_.is_valid());
failed_alternative_service_ = job->alternative_service();
}
if (!request_ || (job_bound_ && bound_job_ != job)) {
// If |request_| is gone then it must have been successfully served by
// |main_job_|.
// If |request_| is bound to a different job, then it is being
// successfully serverd by the main job.
ReportBrokenAlternativeService();
}
}
void HttpStreamFactoryImpl::JobController::ReportBrokenAlternativeService() {
DCHECK(failed_alternative_service_.protocol !=
UNINITIALIZED_ALTERNATE_PROTOCOL ||
failed_alternative_proxy_server_.is_valid());
if (failed_alternative_proxy_server_.is_valid()) {
ProxyDelegate* proxy_delegate = session_->params().proxy_delegate;
if (proxy_delegate)
proxy_delegate->OnAlternativeProxyBroken(
failed_alternative_proxy_server_);
} else {
HistogramBrokenAlternateProtocolLocation(
BROKEN_ALTERNATE_PROTOCOL_LOCATION_HTTP_STREAM_FACTORY_IMPL_JOB_ALT);
session_->http_server_properties()->MarkAlternativeServiceBroken(
failed_alternative_service_);
}
session_->quic_stream_factory()->OnTcpJobCompleted(true);
}
void HttpStreamFactoryImpl::JobController::MaybeNotifyFactoryOfCompletion() {
if (!request_ && !main_job_ && !alternative_job_) {
DCHECK(!bound_job_);
......
......@@ -201,6 +201,13 @@ class HttpStreamFactoryImpl::JobController
NextProto negotiated_protocol,
bool using_spdy);
// Must be called when |alternative_job_| fails.
void OnAlternativeJobFailed(Job* job);
// Called to report to http_server_properties to mark alternative service
// broken.
void ReportBrokenAlternativeService();
void MaybeNotifyFactoryOfCompletion();
// Called to resume the main job with delay.
......@@ -255,6 +262,15 @@ class HttpStreamFactoryImpl::JobController
std::unique_ptr<Job> main_job_;
std::unique_ptr<Job> alternative_job_;
// True if |alternative_job_| uses alternative service/proxy server and it
// fails.
bool alternative_job_failed_;
// Either and only one of these records failed alternative service/proxy
// server that |alternative_job_| uses.
AlternativeService failed_alternative_service_;
ProxyServer failed_alternative_proxy_server_;
// True if a Job has ever been bound to the |request_|.
bool job_bound_;
......
......@@ -154,6 +154,17 @@ class HttpStreamFactoryImplJobControllerTest
server, alternative_service, expiration);
}
void VerifyBrokenAlternateProtocolMapping(const HttpRequestInfo& request_info,
bool should_mark_broken) {
const url::SchemeHostPort server(request_info.url);
const AlternativeServiceVector alternative_service_vector =
session_->http_server_properties()->GetAlternativeServices(server);
EXPECT_EQ(1u, alternative_service_vector.size());
EXPECT_EQ(should_mark_broken,
session_->http_server_properties()->IsAlternativeServiceBroken(
alternative_service_vector[0]));
}
// Not owned by |this|.
TestProxyDelegate* test_proxy_delegate_;
TestJobFactory job_factory_;
......@@ -262,6 +273,7 @@ TEST_F(HttpStreamFactoryImplJobControllerTest, CancelJobsBeforeBinding) {
// to serve Request yet and JobController will notify the factory to delete
// itself upon completion.
request_.reset();
VerifyBrokenAlternateProtocolMapping(request_info, false);
EXPECT_TRUE(HttpStreamFactoryImplPeer::IsJobControllerDeleted(factory_));
}
......@@ -295,7 +307,6 @@ TEST_F(HttpStreamFactoryImplJobControllerTest, OnStreamFailedForBothJobs) {
// thus should not notify Request of the alternative job's failure. But should
// notify the main job to mark the alternative job failed.
EXPECT_CALL(request_delegate_, OnStreamFailed(_, _)).Times(0);
EXPECT_CALL(*job_factory_.main_job(), MarkOtherJobComplete(_)).Times(1);
job_controller_->OnStreamFailed(job_factory_.alternative_job(), ERR_FAILED,
SSLConfig());
EXPECT_TRUE(!job_controller_->alternative_job());
......@@ -306,10 +317,11 @@ TEST_F(HttpStreamFactoryImplJobControllerTest, OnStreamFailedForBothJobs) {
EXPECT_CALL(request_delegate_, OnStreamFailed(_, _)).Times(1);
job_controller_->OnStreamFailed(job_factory_.main_job(), ERR_FAILED,
SSLConfig());
VerifyBrokenAlternateProtocolMapping(request_info, false);
}
TEST_F(HttpStreamFactoryImplJobControllerTest,
SecondJobFailsAfterFirstJobSucceeds) {
AltJobFailsAfterMainJobSucceeds) {
ProxyConfig proxy_config;
proxy_config.set_auto_detect(true);
// Use asynchronous proxy resolver.
......@@ -344,8 +356,6 @@ TEST_F(HttpStreamFactoryImplJobControllerTest,
EXPECT_CALL(request_delegate_, OnStreamReady(_, _, http_stream))
.WillOnce(Invoke(DeleteHttpStreamPointer));
EXPECT_CALL(*job_factory_.alternative_job(), MarkOtherJobComplete(_))
.Times(1);
job_controller_->OnStreamReady(job_factory_.main_job(), SSLConfig(),
ProxyInfo());
......@@ -355,13 +365,14 @@ TEST_F(HttpStreamFactoryImplJobControllerTest,
job_controller_->OnStreamFailed(job_factory_.alternative_job(), ERR_FAILED,
SSLConfig());
VerifyBrokenAlternateProtocolMapping(request_info, true);
// Reset the request as it's been successfully served.
request_.reset();
EXPECT_TRUE(HttpStreamFactoryImplPeer::IsJobControllerDeleted(factory_));
}
TEST_F(HttpStreamFactoryImplJobControllerTest,
SecondJobSucceedsAfterFirstJobFailed) {
AltJobSucceedsAfterMainJobFailed) {
ProxyConfig proxy_config;
proxy_config.set_auto_detect(true);
// Use asynchronous proxy resolver.
......@@ -388,10 +399,7 @@ TEST_F(HttpStreamFactoryImplJobControllerTest,
EXPECT_TRUE(job_controller_->alternative_job());
// |main_job| fails but should not report status to Request.
// The alternative job will mark the main job complete.
EXPECT_CALL(request_delegate_, OnStreamFailed(_, _)).Times(0);
EXPECT_CALL(*job_factory_.alternative_job(), MarkOtherJobComplete(_))
.Times(1);
job_controller_->OnStreamFailed(job_factory_.main_job(), ERR_FAILED,
SSLConfig());
......@@ -405,6 +413,53 @@ TEST_F(HttpStreamFactoryImplJobControllerTest,
.WillOnce(Invoke(DeleteHttpStreamPointer));
job_controller_->OnStreamReady(job_factory_.alternative_job(), SSLConfig(),
ProxyInfo());
VerifyBrokenAlternateProtocolMapping(request_info, false);
}
TEST_F(HttpStreamFactoryImplJobControllerTest,
MainJobSucceedsAfterAltJobFailed) {
ProxyConfig proxy_config;
proxy_config.set_auto_detect(true);
// Use asynchronous proxy resolver.
MockAsyncProxyResolverFactory* proxy_resolver_factory =
new MockAsyncProxyResolverFactory(false);
session_deps_.proxy_service.reset(
new ProxyService(base::MakeUnique<ProxyConfigServiceFixed>(proxy_config),
base::WrapUnique(proxy_resolver_factory), nullptr));
Initialize(false);
HttpRequestInfo request_info;
request_info.method = "GET";
request_info.url = GURL("https://www.google.com");
url::SchemeHostPort server(request_info.url);
AlternativeService alternative_service(QUIC, server.host(), 443);
SetAlternativeService(request_info, alternative_service);
request_.reset(
job_controller_->Start(request_info, &request_delegate_, nullptr,
BoundNetLog(), HttpStreamRequest::HTTP_STREAM,
DEFAULT_PRIORITY, SSLConfig(), SSLConfig()));
EXPECT_TRUE(job_controller_->main_job());
EXPECT_TRUE(job_controller_->alternative_job());
// |alternative_job| fails but should not report status to Request.
EXPECT_CALL(request_delegate_, OnStreamFailed(_, _)).Times(0);
job_controller_->OnStreamFailed(job_factory_.alternative_job(), ERR_FAILED,
SSLConfig());
// |main_job| succeeds and should report status to Request.
HttpStream* http_stream =
new HttpBasicStream(base::MakeUnique<ClientSocketHandle>(), false, false);
job_factory_.main_job()->SetStream(http_stream);
EXPECT_CALL(request_delegate_, OnStreamReady(_, _, http_stream))
.WillOnce(Invoke(DeleteHttpStreamPointer));
job_controller_->OnStreamReady(job_factory_.main_job(), SSLConfig(),
ProxyInfo());
VerifyBrokenAlternateProtocolMapping(request_info, true);
}
// Regression test for crbug/621069.
......@@ -438,8 +493,6 @@ TEST_F(HttpStreamFactoryImplJobControllerTest, GetLoadStateAfterMainJobFailed) {
// |main_job| fails but should not report status to Request.
// The alternative job will mark the main job complete.
EXPECT_CALL(request_delegate_, OnStreamFailed(_, _)).Times(0);
EXPECT_CALL(*job_factory_.alternative_job(), MarkOtherJobComplete(_))
.Times(1);
job_controller_->OnStreamFailed(job_factory_.main_job(), ERR_FAILED,
SSLConfig());
......@@ -488,7 +541,6 @@ TEST_F(HttpStreamFactoryImplJobControllerTest, DoNotResumeMainJobBeforeWait) {
// Wait until OnStreamFailedCallback is executed on the alternative job.
EXPECT_CALL(request_delegate_, OnStreamFailed(_, _)).Times(1);
EXPECT_CALL(*job_factory_.main_job(), MarkOtherJobComplete(_)).Times(1);
base::RunLoop().RunUntilIdle();
}
......@@ -514,7 +566,6 @@ TEST_F(HttpStreamFactoryImplJobControllerTest, InvalidPortForQuic) {
// Wait until OnStreamFailedCallback is executed on the alternative job.
EXPECT_CALL(*job_factory_.main_job(), Resume()).Times(1);
EXPECT_CALL(*job_factory_.main_job(), MarkOtherJobComplete(_)).Times(1);
base::RunLoop().RunUntilIdle();
}
......@@ -580,7 +631,6 @@ TEST_F(HttpStreamFactoryImplJobControllerTest,
resolver.pending_requests()[0]->CompleteNow(net::OK);
EXPECT_CALL(request_delegate_, OnStreamFailed(_, _)).Times(0);
EXPECT_CALL(*job_factory_.main_job(), Resume()).Times(1);
EXPECT_CALL(*job_factory_.main_job(), MarkOtherJobComplete(_)).Times(1);
base::RunLoop().RunUntilIdle();
}
......@@ -645,7 +695,6 @@ TEST_F(HttpStreamFactoryImplJobControllerTest,
// Request shouldn't be notified as the main job is still pending status.
EXPECT_CALL(request_delegate_, OnStreamFailed(_, _)).Times(0);
EXPECT_CALL(*job_factory_.main_job(), Resume()).Times(1);
EXPECT_CALL(*job_factory_.main_job(), MarkOtherJobComplete(_)).Times(1);
base::RunLoop().RunUntilIdle();
}
......@@ -839,7 +888,6 @@ TEST_F(HttpStreamFactoryImplJobControllerTest, FailAlternativeProxy) {
EXPECT_TRUE(job_controller_->alternative_job());
EXPECT_CALL(request_delegate_, OnStreamReady(_, _, _)).Times(0);
EXPECT_CALL(*job_factory_.main_job(), MarkOtherJobComplete(_)).Times(1);
// Since the alternative proxy server job is started in the next message loop,
// the main job would remain blocked until the alternative proxy starts, and
......
......@@ -120,8 +120,6 @@ class MockHttpStreamFactoryImplJob : public HttpStreamFactoryImpl::Job {
MOCK_METHOD0(Resume, void());
MOCK_METHOD1(MarkOtherJobComplete, void(const Job& job));
MOCK_METHOD0(Orphan, void());
};
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment