Implementation of network level throttler.

net/http/http_network_transaction.cc

Index: net/http/http_network_transaction.cc
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index 6414180bfc3747ae240e275b5b500e36b61688aa..5480e60597d77d719fd6bb312d4c7984d78269ae 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -161,7 +161,7 @@ int HttpNetworkTransaction::Start(const HttpRequestInfo* request_info,
   if (request_->load_flags & LOAD_PREFETCH)
     response_.unused_since_prefetch = true;
 
-  next_state_ = STATE_NOTIFY_BEFORE_CREATE_STREAM;
+  next_state_ = STATE_THROTTLE;
   int rv = DoLoop(OK);
   if (rv == ERR_IO_PENDING)
     callback_ = callback;
@@ -379,6 +379,8 @@ LoadState HttpNetworkTransaction::GetLoadState() const {
   // TODO(wtc): Define a new LoadState value for the
   // STATE_INIT_CONNECTION_COMPLETE state, which delays the HTTP request.
   switch (next_state_) {
+    case STATE_THROTTLE_COMPLETE:
+      return LOAD_STATE_THROTTLED;
     case STATE_CREATE_STREAM:
       return LOAD_STATE_WAITING_FOR_DELEGATE;
     case STATE_CREATE_STREAM_COMPLETE:
@@ -435,11 +437,18 @@ void HttpNetworkTransaction::PopulateNetErrorDetails(
 }
 
 void HttpNetworkTransaction::SetPriority(RequestPriority priority) {
+  const std::string priority_string(RequestPriorityToString(priority));

[Randy Smith (Not in Mondays), 2016/05/24 21:38:50]

I don't intend the above code to be what lands (why do a std::string
construction when the netlog isn't active?) but wanted to check in with you
about alternative strategies before I implemented them.  What I *want* to do is
to create a variation on NetLog::StringCallback that takes a char*, initialize a
char* variable with the results of RequestPriorityToString, and pass that to
StringCallback, as I could imagine that being useful in other places similar to
this one.  But I wanted to check with you as to whether there were gotchas to
that approach, or whether you'd just prefer me to create a new callback for this
particular event.

[mmenke, 2016/05/24 21:52:50]

I think you mean takes a RequestPriority, not a char*?  I'm fine with that.

[mmenke, 2016/05/24 22:59:52]

Actually, you can just use an int called "priority", and net-internals will
convert it to a string, magically.

[Randy Smith (Not in Mondays), 2016/05/24 23:20:30]

Ok, I'll take your word for it, but FWIW, I looked and couldn't find the code
that would do that.  Done.

[mmenke, 2016/05/25 15:32:15]

Oops, looks like you're right about this!  It's never logged along anywhere, so
we just use RequestPriorityToString.

That having been said, it seems like this should really be logged in
URLRequest::SetPriority...Which...Already logs priority changes.  So I guess we
don't need this.

[Randy Smith (Not in Mondays), 2016/05/25 16:57:48]

I'm amused by how resistant I feel removing it, but yes, there's never a
many->one mapping from URLRequests to HttpNetworkTransactions, so it's probably
fine to not log it here.  Removed. 

+  net_log_.AddEvent(NetLog::TYPE_HTTP_TRANSACTION_SET_PRIORITY,
+                    NetLog::StringCallback("priority", &priority_string));
+
   priority_ = priority;
   if (stream_request_)
     stream_request_->SetPriority(priority);
   if (stream_)
     stream_->SetPriority(priority);

[Randy Smith (Not in Mondays), 2016/05/24 21:38:50]

I find myself worried about the future, if we ever implement throttling at other
layers of the stack and one of the above results in unthrottling, synchronous
completion, and callback, like the call below might.  I presume it's a worry for
the future, but if you have any thoughts on the topic, I'm curious about them.

[mmenke, 2016/05/24 21:52:50]

Very good point.  I think only one of these should ever have an effect, so we're
safe early returning at each branch....If we have a stream_, we're not waiting
on a stream request.  If we have a stream request, we're not waiting on a
throttle, etc...

Think this requires some more thought.

[Randy Smith (Not in Mondays), 2016/05/24 23:20:30]

*nod*  I'll put a comment here noting potential future problems and not worry
about it further for this CL.

+  if (throttle_)
+    throttle_->SetPriority(priority);
+  // The above call may have resulted in deleting |*this|.
 }
 
 void HttpNetworkTransaction::SetWebSocketHandshakeStreamCreateHelper(
@@ -595,6 +604,17 @@ void HttpNetworkTransaction::GetConnectionAttempts(
   *out = connection_attempts_;
 }
 
+void HttpNetworkTransaction::OnThrottleStateChanged() {
+  // TODO(rdsmith): This DCHECK is dependent on the throttle state
+  // initializing as throttled only transition from throttled->unthrottled.
+  // That is true right now, but may not be so in the future.
+  DCHECK_EQ(STATE_THROTTLE_COMPLETE, next_state_);
+
+  net_log_.EndEvent(NetLog::TYPE_HTTP_TRANSACTION_THROTTLED);
+
+  DoLoop(OK);
+}
+
 bool HttpNetworkTransaction::IsSecureRequest() const {
   return request_->url.SchemeIsCryptographic();
 }
@@ -665,6 +685,14 @@ int HttpNetworkTransaction::DoLoop(int result) {
     State state = next_state_;
     next_state_ = STATE_NONE;
     switch (state) {
+      case STATE_THROTTLE:
+        DCHECK_EQ(OK, rv);
+        rv = DoThrottle();
+        break;
+      case STATE_THROTTLE_COMPLETE:
+        DCHECK_EQ(OK, rv);
+        rv = DoThrottleComplete();
+        break;
       case STATE_NOTIFY_BEFORE_CREATE_STREAM:
         DCHECK_EQ(OK, rv);
         rv = DoNotifyBeforeCreateStream();
@@ -776,6 +804,28 @@ int HttpNetworkTransaction::DoLoop(int result) {
   return rv;
 }
 
+int HttpNetworkTransaction::DoThrottle() {
+  throttle_ = session_->throttler()->CreateThrottle(
+      this, priority_, (request_->load_flags & LOAD_IGNORE_LIMITS) != 0);
+  next_state_ = STATE_THROTTLE_COMPLETE;
+
+  if (throttle_->IsThrottled()) {
+    net_log_.BeginEvent(NetLog::TYPE_HTTP_TRANSACTION_THROTTLED);
+    return ERR_IO_PENDING;
+  }
+
+  return OK;
+}
+
+int HttpNetworkTransaction::DoThrottleComplete() {
+  DCHECK(throttle_);
+  DCHECK(!throttle_->IsThrottled());
+
+  next_state_ = STATE_NOTIFY_BEFORE_CREATE_STREAM;
+
+  return OK;
+}
+
 int HttpNetworkTransaction::DoNotifyBeforeCreateStream() {
   next_state_ = STATE_CREATE_STREAM;
   bool defer = false;