Add reprioritization to socket pools.

net/socket/client_socket_handle.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/socket/client_socket_handle.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 13 matching lines @@
       higher_pool_(NULL),
       reuse_type_(ClientSocketHandle::UNUSED),
       callback_(base::Bind(&ClientSocketHandle::OnIOComplete,
                            base::Unretained(this))),
       is_ssl_error_(false) {}
 
 ClientSocketHandle::~ClientSocketHandle() {
   Reset();
 }
 
+void ClientSocketHandle::SetPriority(RequestPriority priority) {
+  if (socket_) {
+    // The priority of the handle is no longer relevant to the socket pool;
+    // just return.
+    //
+    // TODO (rdsmith): Incorporate change in priority information into TCP
+    // params on this socket.

[mmenke, 2017/01/05 20:18:35]

TransportSocketParams don't have a priority, no socket params classes do,
actually, I believe.  The only socket class where priority matters would be for
SpdyProxyClientSocket (And the QUIC equivalent, if there is one), since they can
set their priority on the undlerying SPDY/QUIC session.

[Randy Smith (Not in Mondays), 2017/01/13 23:05:44]

Sorry, didn't mean TransportSocketParams, more TCP Window Size and things like
that.  But I'm going to remove the TODO; integrating priority into things like
TCP Window Size would be its own, large, project.

+    return;
+  }
+
+  if (pool_)

[mmenke, 2017/01/05 20:18:35]

Just destroying the socket seems to return it to the same state as a newly
created socket, so I can't see any reason why we can't just delete it.  There
may once have been a reason for it, which no longer exists.  It could even have
something to do with the old callback system, and/or the fact most net objects
were once refcounted.  I think I may try the cleanup, and see what happens
(Mostly because it seems really simple just to change usage from the Job and see
what happens - if things look good, Reset() is only used in about a dozen files,
though those files do have a lot of references)
Hrm...  You're right that a ClientSocketHandle for a failed socket request
without a socket is indistinguishable from one that has yet to be initialized,
which seems rather unfortunate.  Requiring a client not call a method on a
handle that's not in a connecting state (from its viewpoint) seems like a pretty
reasonable contract to me, the only problem I see with it is that we can't
enforce it.
From my standpoint, something with is_initialized == false is logically
equivalent to nullptr, so modifying it is an anti-pattern.  Also, other than
this method, I think ClientSocketHandles, once connected (And even before,
actually) are basically immutable from the point of view of the consumer, other
than Reset, and method calls on the socket.

[Randy Smith (Not in Mondays), 2017/01/13 23:05:44]

Believe the result of this comment was tracked at the top level, and was "Eh, I
guess the current situation is preferable to the large refactor involved in
cleaning up the ClientSocketHandle API"; let me know if that's wrong.

[mmenke, 2017/01/17 18:56:31]

That's right.

+    pool_->SetPriority(group_name_, this, priority);

[mmenke, 2017/01/05 20:18:35]

Let me try and get a better...handle...  on the life[and]times of
ClientSocketHandles, and I'll get back to you on that.

[Randy Smith (Not in Mondays), 2017/01/13 23:05:44]

See last comment response :-}.

+}
+
 void ClientSocketHandle::Reset() {
   ResetInternal(true);
   ResetErrorState();
 }
 
 void ClientSocketHandle::ResetInternal(bool cancel) {
   // Was Init called?
   if (!group_name_.empty()) {
     // If so, we must have a pool.
     CHECK(pool_);
@@ skipping 126 matching lines @@
   // Broadcast that the socket has been acquired.
   // TODO(eroman): This logging is not complete, in particular set_socket() and
   // release() socket. It ends up working though, since those methods are being
   // used to layer sockets (and the destination sources are the same).
   DCHECK(socket_.get());
   socket_->NetLog().BeginEvent(NetLogEventType::SOCKET_IN_USE,
                                requesting_source_.ToEventParametersCallback());
 }
 
 }  // namespace net