Add reprioritization to socket pools.

net/http/http_network_transaction.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_network_transaction.h"
 
 #include <memory>
 #include <set>
 #include <utility>
 #include <vector>
@@ skipping 403 matching lines @@
 void HttpNetworkTransaction::PopulateNetErrorDetails(
     NetErrorDetails* details) const {
   *details = net_error_details_;
   if (stream_)
     stream_->PopulateNetErrorDetails(details);
 }
 
 void HttpNetworkTransaction::SetPriority(RequestPriority priority) {
   priority_ = priority;
 
-  // TODO: Note that if throttling is ever implemented below this
+  // TODO(rdsmith): Handle race between SetPriority and
+  // HttpStreamFactoryImpl::Job notifying this class of stream
+  // creation (where this class will have set the priority on the
+  // stream_request_ but that will be a no-op because the stream has
+  // already been created).

[mmenke, 2017/01/03 22:51:22]

Think this should be clearer.  I assume the issue is that everything will wok,
except when this class gets the stream_, it will still have the old priority.

[Randy Smith (Not in Mondays), 2017/01/05 03:47:18]

Yep.  I've reworded the comment; let me know what you think.

+
+  // TODO(rdsmith): Note that if throttling is ever implemented below this

[mmenke, 2017/01/03 22:51:22]

Know this is an old comment, but I can't remember the meaning...the socket pool
is below this layer, and implements a form of throttling.

[Randy Smith (Not in Mondays), 2017/01/05 03:47:18]

I've added " ... if *priority based* throttling ..." to the comment; let me know
if you need more.  The basic issue is that if the SetPriority calls can result
in upcalls that nuke requests, this code will have problems.  Currently this
routine depends on the only thing that can delete *this being the
throttle->SetPriority() call.

   // level, either of the two below calls may result in request
   // completion, callbacks, and the potential deletion of this object
   // (like the call below to throttle_->SetPriority()).  In that case,
   // this code will need to be modified.
   if (stream_request_)
     stream_request_->SetPriority(priority);
   if (stream_)
     stream_->SetPriority(priority);
 
   if (throttle_)
@@ skipping 1265 matching lines @@
   DCHECK(stream_request_);
 
   // Since the transaction can restart with auth credentials, it may create a
   // stream more than once. Accumulate all of the connection attempts across
   // those streams by appending them to the vector:
   for (const auto& attempt : stream_request_->connection_attempts())
     connection_attempts_.push_back(attempt);
 }
 
 }  // namespace net