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

net/socket/client_socket_pool_base.h

 // 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.
 //
 // A ClientSocketPoolBase is used to restrict the number of sockets open at
 // a time.  It also maintains a list of idle persistent sockets for reuse.
 // Subclasses of ClientSocketPool should compose ClientSocketPoolBase to handle
 // the core logic of (1) restricting the number of active (connected or
 // connecting) sockets per "group" (generally speaking, the hostname), (2)
 // maintaining a per-group list of idle, persistent sockets for reuse, and (3)
@@ skipping 94 matching lines @@
   // result) this method will be called, allowing the pool to add
   // additional error state to the ClientSocketHandle (post late-binding).
   virtual void GetAdditionalErrorState(ClientSocketHandle* handle) {}
 
   const LoadTimingInfo::ConnectTiming& connect_timing() const {
     return connect_timing_;
   }
 
   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);
   StreamSocket* socket() { return socket_.get(); }
   void NotifyDelegateOfCompletion(int rv);
   void ResetTimer(base::TimeDelta remainingTime);
 
   // Connection establishment timing information.
   LoadTimingInfo::ConnectTiming connect_timing_;
 
@@ skipping 318 matching lines @@
 
     // Finds the pending request for |handle| and removes it. Returns
     // the removed pending request, or NULL if there was none.
     scoped_ptr<const Request> FindAndRemovePendingRequest(
         ClientSocketHandle* handle);
 
     void IncrementActiveSocketCount() { active_socket_count_++; }
     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_; }
 
    private:
     // Returns the iterator's pending request after removing it from
     // the queue.
     scoped_ptr<const Request> RemovePendingRequest(
         const RequestQueue::Pointer& pointer);
 
     // Called when the backup socket timer fires.
     void OnBackupJobTimerFired(
         std::string group_name,
         ClientSocketPoolBaseHelper* pool);
 
     // Checks that |unassigned_job_count_| does not execeed the number of
     // ConnectJobs.
     void SanityCheck();
 
     // Total number of ConnectJobs that have never been assigned to a Request.
     // Since jobs use late binding to requests, which ConnectJobs have or have
     // not been assigned to a request are not tracked.  This is incremented on
     // preconnect and decremented when a preconnect is assigned, or when there
     // are fewer than |unassigned_job_count_| ConnectJobs.  Not incremented
     // when a request is cancelled.
     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.
     base::OneShotTimer<Group> backup_job_timer_;
   };
 
   typedef std::map<std::string, Group*> GroupMap;
 
   typedef std::set<ConnectJob*> ConnectJobSet;
 
@@ skipping 372 matching lines @@
   };
 
   internal::ClientSocketPoolBaseHelper helper_;
 
   DISALLOW_COPY_AND_ASSIGN(ClientSocketPoolBase);
 };
 
 }  // namespace net
 
 #endif  // NET_SOCKET_CLIENT_SOCKET_POOL_BASE_H_