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

Unified Diff: net/http/http_stream_factory_impl_job.cc

Issue 1952423002: JobController 2: Remove reference between HttpStreamFactoryImpl::Jobs. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@Job_Controller_1
Patch Set: move resume logic up to controller Created 4 years, 6 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: net/http/http_stream_factory_impl_job.cc
diff --git a/net/http/http_stream_factory_impl_job.cc b/net/http/http_stream_factory_impl_job.cc
index 8ac81e681e670de409449473ebfb84201a763235..6a997b24e3612e2ed1f86341183f8a4dda630f37 100644
--- a/net/http/http_stream_factory_impl_job.cc
+++ b/net/http/http_stream_factory_impl_job.cc
@@ -194,6 +194,7 @@ HttpStreamFactoryImpl::Job::Job(Delegate* delegate,
io_callback_(base::Bind(&Job::OnIOComplete, base::Unretained(this))),
connection_(new ClientSocketHandle),
session_(session),
+ state_(STATE_NONE),
next_state_(STATE_NONE),
pac_request_(NULL),
destination_(destination),
@@ -201,8 +202,6 @@ HttpStreamFactoryImpl::Job::Job(Delegate* delegate,
alternative_service_(alternative_service),
delegate_(delegate),
job_type_(job_type),
- blocking_job_(NULL),
- waiting_job_(NULL),
using_ssl_(false),
using_spdy_(false),
using_quic_(false),
@@ -288,60 +287,35 @@ LoadState HttpStreamFactoryImpl::Job::GetLoadState() const {
}
}
-void HttpStreamFactoryImpl::Job::WaitFor(Job* job) {
- DCHECK_EQ(STATE_NONE, next_state_);
- DCHECK_EQ(STATE_NONE, job->next_state_);
- DCHECK(!blocking_job_);
- DCHECK(!job->waiting_job_);
-
- // Never share connection with other jobs for FTP requests.
- DCHECK(!request_info_.url.SchemeIs("ftp"));
-
- blocking_job_ = job;
- job->waiting_job_ = this;
-}
-
-void HttpStreamFactoryImpl::Job::ResumeAfterDelay() {
- DCHECK(!blocking_job_);
- DCHECK_EQ(STATE_WAIT_FOR_JOB_COMPLETE, next_state_);
+void HttpStreamFactoryImpl::Job::ResumeAfterDelay(
+ const base::TimeDelta& delay) {
+ DCHECK((job_type_ == ALTERNATIVE || !delegate_->blocking()));
net_log_.AddEvent(NetLog::TYPE_HTTP_STREAM_JOB_DELAYED,
- base::Bind(&NetLogHttpStreamJobDelayCallback, wait_time_));
+ base::Bind(&NetLogHttpStreamJobDelayCallback, delay));
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE, base::Bind(&HttpStreamFactoryImpl::Job::OnIOComplete,
ptr_factory_.GetWeakPtr(), OK),
- wait_time_);
+ delay);
}
-void HttpStreamFactoryImpl::Job::Resume(Job* job,
- const base::TimeDelta& delay) {
- DCHECK_EQ(blocking_job_, job);
- blocking_job_ = NULL;
-
- // If |this| job is not past STATE_WAIT_FOR_JOB_COMPLETE state, then it will
- // be delayed by the |wait_time_| when it resumes.
- if (next_state_ == STATE_NONE || next_state_ <= STATE_WAIT_FOR_JOB_COMPLETE)
- wait_time_ = delay;
+void HttpStreamFactoryImpl::Job::Resume(const base::TimeDelta& delay) {
+ DCHECK_EQ(job_type_, MAIN);
+ // If |this| job is not past STATE_WAIT_COMPLETE state, then it will
+ // be delayed by the |delay| when it resumes.
+ if (next_state_ == STATE_NONE || next_state_ <= STATE_WAIT_COMPLETE)
+ delegate_->set_wait_time_for_main_job(delay);
- // We know we're blocked if the next_state_ is STATE_WAIT_FOR_JOB_COMPLETE.
+ // We know we're blocked if the next_state_ is STATE_WAIT_COMPLETE.
// Unblock |this|.
- if (next_state_ == STATE_WAIT_FOR_JOB_COMPLETE)
- ResumeAfterDelay();
+ if (next_state_ == STATE_WAIT_COMPLETE)
+ ResumeAfterDelay(delegate_->wait_time_for_main_job());
Ryan Hamilton 2016/06/29 23:12:28 Can the delegate (the controller) simply call Resu
Zhongyi Shi 2016/06/30 22:53:39 Done. Yay!!
}
void HttpStreamFactoryImpl::Job::Orphan() {
net_log_.AddEvent(NetLog::TYPE_HTTP_STREAM_JOB_ORPHANED);
- if (blocking_job_) {
- // We've been orphaned, but there's a job we're blocked on. Don't bother
- // racing, just cancel ourself.
- DCHECK(blocking_job_->waiting_job_);
- blocking_job_->waiting_job_ = NULL;
- blocking_job_ = NULL;
- if (delegate_->for_websockets() && connection_ && connection_->socket()) {
- connection_->socket()->Disconnect();
- }
- delegate_->OnOrphanedJobComplete(this);
- } else if (delegate_->for_websockets()) {
+
+ if (delegate_->for_websockets()) {
// We cancel this job because a WebSocketHandshakeStream can't be created
// without a WebSocketHandshakeStreamBase::CreateHelper which is stored in
// the Request class and isn't retrievable by this job.
@@ -555,12 +529,13 @@ int HttpStreamFactoryImpl::Job::RunLoop(int result) {
"HttpStreamFactoryImpl::Job::RunLoop");
result = DoLoop(result);
- if (result == ERR_IO_PENDING)
+ if (result == ERR_IO_PENDING) {
+ if (state_ == STATE_INIT_CONNECTION) {
+ delegate_->OnInitConnectionNotSuccessful(this, delay_);
+ delay_ = base::TimeDelta();
+ }
return result;
-
- // If there was an error, we should have already resumed the |waiting_job_|,
- // if there was one.
- DCHECK(result == OK || waiting_job_ == NULL);
+ }
if (job_type_ == PRECONNECT) {
base::ThreadTaskRunnerHandle::Get()->PostTask(
@@ -681,9 +656,9 @@ int HttpStreamFactoryImpl::Job::DoLoop(int result) {
DCHECK_NE(next_state_, STATE_NONE);
int rv = result;
do {
- State state = next_state_;
+ state_ = next_state_;
next_state_ = STATE_NONE;
- switch (state) {
+ switch (state_) {
case STATE_START:
DCHECK_EQ(OK, rv);
rv = DoStart();
@@ -695,12 +670,12 @@ int HttpStreamFactoryImpl::Job::DoLoop(int result) {
case STATE_RESOLVE_PROXY_COMPLETE:
rv = DoResolveProxyComplete(rv);
break;
- case STATE_WAIT_FOR_JOB:
+ case STATE_WAIT:
DCHECK_EQ(OK, rv);
- rv = DoWaitForJob();
+ rv = DoWait();
break;
- case STATE_WAIT_FOR_JOB_COMPLETE:
- rv = DoWaitForJobComplete(rv);
+ case STATE_WAIT_COMPLETE:
+ rv = DoWaitComplete(rv);
break;
case STATE_INIT_CONNECTION:
DCHECK_EQ(OK, rv);
@@ -761,10 +736,6 @@ int HttpStreamFactoryImpl::Job::DoStart() {
// Don't connect to restricted ports.
if (!IsPortAllowedForScheme(destination_.port(),
request_info_.url.scheme())) {
- if (waiting_job_) {
- waiting_job_->Resume(this, base::TimeDelta());
- waiting_job_ = NULL;
- }
return ERR_UNSAFE_PORT;
}
@@ -837,14 +808,10 @@ int HttpStreamFactoryImpl::Job::DoResolveProxyComplete(int result) {
}
if (result != OK) {
- if (waiting_job_) {
- waiting_job_->Resume(this, base::TimeDelta());
- waiting_job_ = NULL;
- }
return result;
}
- next_state_ = STATE_WAIT_FOR_JOB;
+ next_state_ = STATE_WAIT;
return OK;
}
@@ -855,27 +822,20 @@ bool HttpStreamFactoryImpl::Job::ShouldForceQuic() const {
proxy_info_.is_direct() && origin_url_.SchemeIs("https");
}
-int HttpStreamFactoryImpl::Job::DoWaitForJob() {
- if (!blocking_job_ && wait_time_.is_zero()) {
- // There is no |blocking_job_| and there is no |wait_time_|.
- next_state_ = STATE_INIT_CONNECTION;
- return OK;
- }
-
- next_state_ = STATE_WAIT_FOR_JOB_COMPLETE;
- if (!wait_time_.is_zero()) {
- // If there is a waiting_time, then resume the job after the wait_time_.
- DCHECK(!blocking_job_);
- ResumeAfterDelay();
+int HttpStreamFactoryImpl::Job::DoWait() {
+ if (delegate_->ShoudWait(this)) {
+ next_state_ = STATE_WAIT_COMPLETE;
+ return ERR_IO_PENDING;
}
- return ERR_IO_PENDING;
+ next_state_ = STATE_INIT_CONNECTION;
+ return OK;
}
-int HttpStreamFactoryImpl::Job::DoWaitForJobComplete(int result) {
- DCHECK(!blocking_job_);
+int HttpStreamFactoryImpl::Job::DoWaitComplete(int result) {
+ DCHECK((job_type_ == ALTERNATIVE || !delegate_->blocking()));
DCHECK_EQ(OK, result);
- wait_time_ = base::TimeDelta();
+ delegate_->set_wait_time_for_main_job(base::TimeDelta());
next_state_ = STATE_INIT_CONNECTION;
return OK;
}
@@ -885,7 +845,7 @@ int HttpStreamFactoryImpl::Job::DoInitConnection() {
tracked_objects::ScopedTracker tracking_profile(
FROM_HERE_WITH_EXPLICIT_FUNCTION(
"462812 HttpStreamFactoryImpl::Job::DoInitConnection"));
- DCHECK(!blocking_job_);
+ DCHECK((job_type_ == ALTERNATIVE || !delegate_->blocking()));
DCHECK(!connection_->is_initialized());
DCHECK(proxy_info_.proxy_server().is_valid());
next_state_ = STATE_INIT_CONNECTION_COMPLETE;
@@ -957,21 +917,16 @@ int HttpStreamFactoryImpl::Job::DoInitConnection() {
if (rv == OK) {
using_existing_quic_session_ = true;
} else {
- // OK, there's no available QUIC session. Let |waiting_job_| resume
- // if it's paused.
- if (waiting_job_) {
- if (rv == ERR_IO_PENDING) {
- // Start the |waiting_job_| after the delay returned by
- // GetTimeDelayForWaitingJob().
- //
- // If QUIC request fails during handshake, then
- // DoInitConnectionComplete() will start the |waiting_job_|.
- waiting_job_->Resume(this, quic_request_.GetTimeDelayForWaitingJob());
- } else {
- // QUIC request has failed, resume the |waiting_job_|.
- waiting_job_->Resume(this, base::TimeDelta());
- }
- waiting_job_ = NULL;
+ base::TimeDelta delay;
+ // OK, there's no available QUIC session. Let JobController to resume
+ // the waiting job if it's paused.
+ if (rv == ERR_IO_PENDING) {
+ // Start the waiting job after the delay returned by
+ // GetTimeDelayForWaitingJob().
+ //
+ // If QUIC request fails during handshake, then
+ // DoInitConnectionComplete() will start the waiting job.
+ delay_ = quic_request_.GetTimeDelayForWaitingJob();
}
}
return rv;
@@ -1004,13 +959,6 @@ int HttpStreamFactoryImpl::Job::DoInitConnection() {
delegate_->SetSpdySessionKey(this, spdy_session_key);
}
- // OK, there's no available SPDY session. Let |waiting_job_| resume if it's
- // paused.
- if (waiting_job_) {
- waiting_job_->Resume(this, base::TimeDelta());
- waiting_job_ = NULL;
- }
-
if (proxy_info_.is_http() || proxy_info_.is_https())
establishing_tunnel_ = using_ssl_;
@@ -1063,10 +1011,6 @@ int HttpStreamFactoryImpl::Job::DoInitConnection() {
}
int HttpStreamFactoryImpl::Job::DoInitConnectionComplete(int result) {
- if (using_quic_ && result < 0 && waiting_job_) {
- waiting_job_->Resume(this, base::TimeDelta());
- waiting_job_ = NULL;
- }
if (job_type_ == PRECONNECT) {
if (using_quic_)
return result;
@@ -1075,6 +1019,7 @@ int HttpStreamFactoryImpl::Job::DoInitConnectionComplete(int result) {
}
if (result == ERR_SPDY_SESSION_ALREADY_EXISTS) {
+ DCHECK(!using_quic_);
// We found a SPDY connection after resolving the host. This is
// probably an IP pooled connection.
SpdySessionKey spdy_session_key = GetSpdySessionKey();
@@ -1091,6 +1036,15 @@ int HttpStreamFactoryImpl::Job::DoInitConnectionComplete(int result) {
return OK;
}
+ // TODO(willchan): Make this a bit more exact. Maybe there are recoverable
+ // errors, such as ignoring certificate errors for Alternate-Protocol.
+ // Report failure to delegate as there's error in DoInitConnection. Delegate
+ // might be able to resume the other job.
+ if (result < 0) {
+ delegate_->OnInitConnectionNotSuccessful(this, delay_);
+ delay_ = base::TimeDelta();
+ }
+
if (proxy_info_.is_quic() && using_quic_) {
// Mark QUIC proxy as bad if QUIC got disabled on the destination port.
// Underlying QUIC layer would have closed the connection.
@@ -1101,13 +1055,6 @@ int HttpStreamFactoryImpl::Job::DoInitConnectionComplete(int result) {
}
}
- // TODO(willchan): Make this a bit more exact. Maybe there are recoverable
- // errors, such as ignoring certificate errors for Alternate-Protocol.
- if (result < 0 && waiting_job_) {
- waiting_job_->Resume(this, base::TimeDelta());
- waiting_job_ = NULL;
- }
-
// |result| may be the result of any of the stacked pools. The following
// logic is used when determining how to interpret an error.
// If |result| < 0:

Powered by Google App Engine
This is Rietveld 408576698