QUIC - Whne alternate job decides to delay the main(TCP) job and if the

net/http/http_stream_factory_impl_job.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/http/http_stream_factory_impl_job.h"
 
 #include <algorithm>
 #include <string>
 #include <utility>
 
@@ skipping 58 matching lines @@
   scoped_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
   if (source.IsValid())
     source.AddToEventParameters(dict.get());
   dict->SetString("original_url", original_url->GetOrigin().spec());
   dict->SetString("url", url->GetOrigin().spec());
   dict->SetString("alternative_service", alternative_service->ToString());
   dict->SetString("priority", RequestPriorityToString(priority));
   return std::move(dict);
 }
 
+// Returns parameters associated with the delay of the HTTP stream job.
+scoped_ptr<base::Value> NetLogHttpStreamJobDelayCallback(
+    base::TimeDelta delay,
+    NetLogCaptureMode /* capture_mode */) {
+  scoped_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
+  dict->SetInteger("resume_after_ms", static_cast<int>(delay.InMilliseconds()));
+  return std::move(dict);
+}
+
 // Returns parameters associated with the Proto (with NPN negotiation) of a HTTP
 // stream.
 scoped_ptr<base::Value> NetLogHttpStreamProtoCallback(
     const SSLClientSocket::NextProtoStatus status,
     const std::string* proto,
     NetLogCaptureMode /* capture_mode */) {
   scoped_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
 
   dict->SetString("next_proto_status",
                   SSLClientSocket::NextProtoStatusToString(status));
@@ skipping 138 matching lines @@
   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::ResumeWaitingJobAfterDelay(

[Ryan Hamilton, 2016/01/19 23:23:27]

Does this resume the current job, or a job which is waiting on this job? I
believe it's this job, right? In which case, I think I'd remove "WaitingJob"
from the method name.

[ramant (doing other things), 2016/01/20 04:37:52]

You are right.

Done.

+    const base::TimeDelta& delay) {

[Ryan Hamilton, 2016/01/19 23:23:27]

Instead if passing in delay here, an alternative would be to set wait_time_ and
use that exclusively. That would be nice, because it would explain why
wait_time_ is cleared after the task is posted. (Though perhaps that should be
cleared in the WaitComplete() method.

[ramant (doing other things), 2016/01/20 04:37:52]

Done.

+  DCHECK(!blocking_job_);
+  DCHECK_EQ(STATE_WAIT_FOR_JOB_COMPLETE, next_state_);
+
+  net_log_.AddEvent(NetLog::TYPE_HTTP_STREAM_JOB_DELAYED,
+                    base::Bind(&NetLogHttpStreamJobDelayCallback, delay));
+  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE, base::Bind(&HttpStreamFactoryImpl::Job::OnIOComplete,
+                            ptr_factory_.GetWeakPtr(), OK),
+      delay);
+  wait_time_ = base::TimeDelta();
+}
+
 void HttpStreamFactoryImpl::Job::Resume(Job* job,
                                         const base::TimeDelta& delay) {
   DCHECK_EQ(blocking_job_, job);
   blocking_job_ = NULL;
 
+  // If the job hasn't started, delay that job by |wait_time_| when it starts.

[Ryan Hamilton, 2016/01/19 23:23:27]

"that job" or "this job"?

[ramant (doing other things), 2016/01/20 04:37:52]

:-). It is |this| job. Updated the comment.

+  if (next_state_ == STATE_NONE && !delay.is_zero()) {
+    wait_time_ = delay;
+    return;
+  }
+
   // We know we're blocked if the next_state_ is STATE_WAIT_FOR_JOB_COMPLETE.
   // Unblock |this|.
-  if (next_state_ == STATE_WAIT_FOR_JOB_COMPLETE) {
-    base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-        FROM_HERE, base::Bind(&HttpStreamFactoryImpl::Job::OnIOComplete,
-                              ptr_factory_.GetWeakPtr(), OK),
-        delay);
-  }
+  if (next_state_ == STATE_WAIT_FOR_JOB_COMPLETE)
+    ResumeWaitingJobAfterDelay(delay);

[Ryan Hamilton, 2016/01/19 23:23:27]

Is the only case we need to schedule a future wait the case where we're at
STATE_NONE? What if we've made some progress but not all the way to waiting?

I think I might be inclined to write this as:

if (next_state_ == STATE_WAIT_FOR_JOB_COMPLETE)
    ResumeWaitingJobAfterDelay(delay)

if (next_state_ < STATE_WAIT_FOR_JOB) // or something
    wait_time_ = delay;

[ramant (doing other things), 2016/01/20 04:37:52]

+1 to the above and thanks for the very good point.

Done.

 }
 
 void HttpStreamFactoryImpl::Job::Orphan(const Request* request) {
   DCHECK_EQ(request_, request);
   request_ = NULL;
   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_);
@@ skipping 560 matching lines @@
   }
 
   if (result != OK) {
     if (waiting_job_) {
       waiting_job_->Resume(this, base::TimeDelta());
       waiting_job_ = NULL;
     }
     return result;
   }
 
-  if (blocking_job_)
+  if (blocking_job_ || !wait_time_.is_zero())

[Ryan Hamilton, 2016/01/19 23:23:27]

Alternatively, we could always move to the WAIT_FOR_JOB state, and do this logic
there. I wonder if that might be cleaner. what do you think?

[ramant (doing other things), 2016/01/20 04:37:52]

Done.

     next_state_ = STATE_WAIT_FOR_JOB;
   else
     next_state_ = STATE_INIT_CONNECTION;
   return OK;
 }
 
 bool HttpStreamFactoryImpl::Job::ShouldForceQuic() const {
   return session_->params().enable_quic &&
          session_->params().origin_to_force_quic_on.Equals(server_) &&
          proxy_info_.is_direct() && origin_url_.SchemeIs("https");
 }
 
 int HttpStreamFactoryImpl::Job::DoWaitForJob() {
-  DCHECK(blocking_job_);
   next_state_ = STATE_WAIT_FOR_JOB_COMPLETE;
+  if (wait_time_.is_zero()) {
+    DCHECK(blocking_job_);
+  } else {
+    // If there is a waiting_time, then resume the job after the wait_time_.
+    DCHECK(!blocking_job_);
+    ResumeWaitingJobAfterDelay(wait_time_);
+  }
+
   return ERR_IO_PENDING;
 }
 
 int HttpStreamFactoryImpl::Job::DoWaitForJobComplete(int result) {
   DCHECK(!blocking_job_);
   DCHECK_EQ(OK, result);
   next_state_ = STATE_INIT_CONNECTION;
   return OK;
 }
 
@@ skipping 834 matching lines @@
   if (connection_->socket()) {
     ConnectionAttempts socket_attempts;
     connection_->socket()->GetConnectionAttempts(&socket_attempts);
     request_->AddConnectionAttempts(socket_attempts);
   } else {
     request_->AddConnectionAttempts(connection_->connection_attempts());
   }
 }
 
 }  // namespace net