Report the connect status of the oldest connection in the socket pool.

net/socket/client_socket_pool_base.h

Index: net/socket/client_socket_pool_base.h
diff --git a/net/socket/client_socket_pool_base.h b/net/socket/client_socket_pool_base.h
index 4892780b4d641f0f52be1f05a73f5225ae8ed333..cd4b4f2a6b59f8198fa507665e6ce1b9c692459b 100644
--- a/net/socket/client_socket_pool_base.h
+++ b/net/socket/client_socket_pool_base.h
@@ -112,6 +112,14 @@ class NET_EXPORT_PRIVATE ConnectJob {
 
   const BoundNetLog& net_log() const { return net_log_; }
 
+  // Used to order ConnectJob in a set with ascending connect time.
+  struct CompareJobConnectStartTime {
+    bool operator()(const ConnectJob* lhs, const ConnectJob* rhs) const {
+      return lhs->connect_timing().connect_start <
+             rhs->connect_timing().connect_start;
+    }

[mmenke, 2015/04/17 15:32:39]

Also, should probably have two tests for this (In both tests, two sockets.  In
one, the first hangs, second connects.  In the other, reverse them.  Then in the
test make sure we get the status of the first socket.  Have to make sure the
sort method distinguished between the two.  Maybe it would be simplest to just
use a list?  Does mean we have to search through it when a socket connects, but
there are generally at most 6 of them, anyways.

[mmenke, 2015/04/17 15:32:40]

If two ConnectJob are created at the same time, doesn't this mean the set will
consider them equivalent?  So when one of two requests with the same start time
connects, we could end up removing the other one from the set of ConnectJobs?

[haavardm, 2015/04/18 07:24:16]

Agree. Using something else than a set here is probably a good idea and a list
seems appropriate.

[haavardm, 2015/04/18 07:24:16]

Yes, I discovered yesterday that that's exactly what seems to happen. The weird
thing is that it should be fixed by adding
if (lhs->connect_timing().connect_start == rhs->connect_timing().connect_start) 
  return lhs < rhs
before comparing the time. However, connect jobs are still removed. This
addition also means that connections created at the same time will be in random
order.
 

[mmenke, 2015/04/18 13:49:57]

That's probably because connect_start times can change (ConnectJobs in higher
layer socket pools copy it from the lower level ones, for instance)

[haavardm, 2015/04/20 13:03:48]

Not sure I get this test. As I understand it, sockets that connect is removed
from the list. So in both those tests only the hanging connection will be left,
not matter how it is sorted?

  Have to make sure the
I implemented the list approach.

[haavardm, 2015/04/20 13:03:48]

I removed the compare function since list is now used instead of a set.

[mmenke, 2015/04/20 14:49:16]

I was thinking SSL sockets, where more is needed after the TCP connection.  That
would coincidentally also exercise the connect time overwriting code, though
unless we inserted some sort of delay, I suspect times would almost always be
the same, anyways.

Anyhow, see (different, less aggressive) suggestions in other file.

+  };
+
  protected:
   RequestPriority priority() const { return priority_; }
   void SetSocket(scoped_ptr<StreamSocket> socket);
@@ -450,7 +458,10 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper
     void DecrementActiveSocketCount() { active_socket_count_--; }
 
     int unassigned_job_count() const { return unassigned_job_count_; }
-    const std::set<ConnectJob*>& jobs() const { return jobs_; }
+    const std::set<ConnectJob*, ConnectJob::CompareJobConnectStartTime>& jobs()
+        const {
+      return jobs_;
+    }
     const std::list<IdleSocket>& idle_sockets() const { return idle_sockets_; }
     int active_socket_count() const { return active_socket_count_; }
     std::list<IdleSocket>* mutable_idle_sockets() { return &idle_sockets_; }
@@ -479,7 +490,7 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper
     size_t unassigned_job_count_;
 
     std::list<IdleSocket> idle_sockets_;
-    std::set<ConnectJob*> jobs_;
+    std::set<ConnectJob*, ConnectJob::CompareJobConnectStartTime> jobs_;
     RequestQueue pending_requests_;
     int active_socket_count_;  // number of active sockets used by clients
     // A timer for when to start the backup job.