Introduce ERR_NETWORK_CHANGED and allow URLFetcher to automatically retry on that error.

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 225 matching lines @@
   // See ClientSocketPool::CancelRequest for documentation on this function.
   void CancelRequest(const std::string& group_name,
                      ClientSocketHandle* handle);
 
   // See ClientSocketPool::ReleaseSocket for documentation on this function.
   void ReleaseSocket(const std::string& group_name,
                      StreamSocket* socket,
                      int id);
 
   // See ClientSocketPool::Flush for documentation on this function.
-  void Flush();
+  // Each request aborted by this call will get |error| as the failure reason.
+  void Flush(int error);
 
   // See ClientSocketPool::IsStalled for documentation on this function.
   bool IsStalled() const;
 
   // See ClientSocketPool::CloseIdleSockets for documentation on this function.
   void CloseIdleSockets();
 
   // See ClientSocketPool::IdleSocketCount() for documentation on this function.
   int idle_socket_count() const {
     return idle_socket_count_;
@@ skipping 241 matching lines @@
                      Group* group,
                      const BoundNetLog& net_log);
 
   // Adds |socket| to the list of idle sockets for |group|.
   void AddIdleSocket(StreamSocket* socket, Group* group);
 
   // Iterates through |group_map_|, canceling all ConnectJobs and deleting
   // groups if they are no longer needed.
   void CancelAllConnectJobs();
 
-  // Iterates through |group_map_|, posting ERR_ABORTED callbacks for all
+  // Iterates through |group_map_|, posting |error| callbacks for all
   // requests, and then deleting groups if they are no longer needed.
-  void AbortAllRequests();
+  void AbortAllRequests(int error);
 
   // Returns true if we can't create any more sockets due to the total limit.
   bool ReachedMaxSocketsLimit() const;
 
   // This is the internal implementation of RequestSocket().  It differs in that
   // it does not handle logging into NetLog of the queueing status of
   // |request|.
   int RequestSocketInternal(const std::string& group_name,
                             const Request* request);
 
@@ skipping 180 matching lines @@
   void CancelRequest(const std::string& group_name,
                      ClientSocketHandle* handle) {
     return helper_.CancelRequest(group_name, handle);
   }
 
   void ReleaseSocket(const std::string& group_name, StreamSocket* socket,
                      int id) {
     return helper_.ReleaseSocket(group_name, socket, id);
   }
 
-  void Flush() { helper_.Flush(); }
+  void Flush() { helper_.Flush(ERR_NETWORK_CHANGED); }

[willchan no longer on Chromium, 2012/12/11 17:39:33]

It's non-obvious that Flush() should always mean ERR_NETWORK_CHANGED. Who calls
this? Isn't ClientSocketPoolBaseHelper already listening to the appropriate
signals and sending out the ERR_NETWORK_CHANGED cancellations?

[Joao da Silva, 2012/12/11 17:52:57]

The previous patch sets had ERR_ABORTED here, but Szymon clarified that this is
never triggered by a user action at
https://codereview.chromium.org/11464028/diff/9001/net/socket/client_socket_p....
WDYT?

[szym, 2012/12/11 17:54:45]

I agree it's not obvious. I followed the call graph up and concluded that Flush
is only called (indirectly) by:
ClientSocketPoolManagerImpl::OnCertAdded/OnCertTrustChanged, 
ChromeBenchmarkingMessageFilter and NetInternalsMessageHandler.

None of them are due to user action wanting to abort any requests (except the
net-internals WebUI). I compare those changes to the user changing the network
configuration (e.g., switching DNS servers), so ERR_NETWORK_CHANGED seems more
fitting than ERR_ABORTED.

The same logic applies to the call-sites of
SpdySessionPool::CloseCurrentSessions() which are OnCert* and
SSLConfigServiceManagerPref::OnPreferenceChanged.

[willchan no longer on Chromium, 2012/12/11 18:06:14]

Assuming ERR_NETWORK_CHANGED sounds wrong here then. I'd rather also do
FlushWithError() or something. ChromeBenchmarkingMessageFilter is actually
triggered by user action, since when you start the benchmark, it tries to flush
all network state so it can have a "clean" run.

[Joao da Silva, 2012/12/11 21:14:55]

Flush() has been changed to FlushWithError(int) as suggested.

I've also added the error parameter to FlushSocketPools (as
FlushSocketPoolsWithError), so that HttpNetworkSession::CloseAllConnections()
(which is what ChromeBenchmarkingMessageFilter and NetInternalsMessageHandler
end up calling) invokes it with ERR_ABORTED, and the certs-changes callbacks
invoke it with ERR_NETWORK_CHANGED.

 
   bool IsStalled() const { return helper_.IsStalled(); }
 
   void CloseIdleSockets() { return helper_.CloseIdleSockets(); }
 
   int idle_socket_count() const { return helper_.idle_socket_count(); }
 
   int IdleSocketCountInGroup(const std::string& group_name) const {
     return helper_.IdleSocketCountInGroup(group_name);
   }
@@ skipping 83 matching lines @@
   // Histograms for the pool
   ClientSocketPoolHistograms* const histograms_;
   internal::ClientSocketPoolBaseHelper helper_;
 
   DISALLOW_COPY_AND_ASSIGN(ClientSocketPoolBase);
 };
 
 }  // namespace net
 
 #endif  // NET_SOCKET_CLIENT_SOCKET_POOL_BASE_H_