Collect all ConnectionAttempts from both sockets in TransportConnectJob.

net/socket/stream_socket.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.
 
 #ifndef NET_SOCKET_STREAM_SOCKET_H_
 #define NET_SOCKET_STREAM_SOCKET_H_
 
 #include "net/log/net_log.h"
+#include "net/socket/connection_attempts.h"
 #include "net/socket/next_proto.h"
 #include "net/socket/socket.h"
 
 namespace net {
 
 class AddressList;
 class IPEndPoint;
 class SSLInfo;
 
 class NET_EXPORT_PRIVATE StreamSocket : public Socket {
  public:
-  ~StreamSocket() override {}
+  ~StreamSocket() override;
 
   // Called to establish a connection.  Returns OK if the connection could be
   // established synchronously.  Otherwise, ERR_IO_PENDING is returned and the
   // given callback will run asynchronously when the connection is established
   // or when an error occurs.  The result is some other error code if the
   // connection could not be established.
   //
   // The socket's Read and Write methods may not be called until Connect
   // succeeds.
   //
@@ skipping 57 matching lines @@
   virtual bool WasNpnNegotiated() const = 0;
 
   // Returns the protocol negotiated via NPN for this socket, or
   // kProtoUnknown will be returned if NPN is not applicable.
   virtual NextProto GetNegotiatedProtocol() const = 0;
 
   // Gets the SSL connection information of the socket.  Returns false if
   // SSL was not used by this socket.
   virtual bool GetSSLInfo(SSLInfo* ssl_info) = 0;
 
+  virtual void GetConnectionAttempts(ConnectionAttempts* out) const;
+  virtual void ClearConnectionAttempts();

[Randy Smith (Not in Mondays), 2015/05/12 20:11:09]

I'm confused.  My sense (please correct me if I'm wrong) is that in most places
in this set of CLs you've had GetConnectionAttepts() clear the underlying
storage when it's called.  Is there a reason for the inconsistency here?  If I'm
right about the common idiom, I'm tempted to ask for a different name here just
to signal a different behavior.  

[Deprecated (see juliatuttle), 2015/05/13 18:22:28]

I believe you're wrong; can you point me to an instance of where I clear them in
Get? (Note that Get is const.)

+  virtual void AddConnectionAttempts(const ConnectionAttempts& attempts);

[Randy Smith (Not in Mondays), 2015/05/12 20:11:09]

Also note my earlier comment (different CL?) about really wanting interface
class methods to be documented with comments in the interface class (relevant to
my other comment here as well).

(ETA) I'll specifically note that I'd like a specification of whether the
socket-internal set of connection attempts is nulled out on this call, and
whether the argument is assigned to or appended to.  I.e. what happens if you
call GetConnectionAttempts() twice with the same arguments?

[Randy Smith (Not in Mondays), 2015/05/12 20:11:09]

So there's a philosophy issue here.  I know that, at least in some contexts,
Sleevi's pretty adamant that interface classes shouldn't have default
implementations, in order to force each subclass to engage with the question of
how they want to deal with that aspect of the interface (as opposed to allowing
out of sight, out of mind, and making changes to interfaces potentially not be
propagated where they need to be).  I have more mixed feelings on the topic, as
it can create a lot of work for subclasses.  I'm still trying to figure out
whether, or how much, I disagree with Ryan, but while I'm figuring that out I'm
sorta in favor of sticking with the current best practice, which is =0 in ABCs. 
That's also supported by consistency--I think the rest of this class is
predominantly/all =0.  

I don't think pushing that here would create a lot of work for you; would you be
willing to do that?

[Deprecated (see juliatuttle), 2015/05/13 18:22:28]

Done.

[Deprecated (see juliatuttle), 2015/05/13 18:22:28]

Will do. Sending comments in the meantime so you can see them while I go through
the subclasses.

+
  protected:
   // The following class is only used to gather statistics about the history of
   // a socket.  It is only instantiated and used in basic sockets, such as
   // TCPClientSocket* instances.  Other classes that are derived from
   // StreamSocket should forward any potential settings to their underlying
   // transport sockets.
   class UseHistory {
    public:
     UseHistory();
     ~UseHistory();
@@ skipping 26 matching lines @@
     // identify the motivation.
     bool omnibox_speculation_;
     bool subresource_speculation_;
     DISALLOW_COPY_AND_ASSIGN(UseHistory);
   };
 };
 
 }  // namespace net
 
 #endif  // NET_SOCKET_STREAM_SOCKET_H_