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

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 e35ab29fca11f767943d2ff2a676ac216e3921b4..2fc14321da0450ede00558c5dc8d28d861f7381c 100644
--- a/net/socket/client_socket_pool_base.h
+++ b/net/socket/client_socket_pool_base.h
@@ -243,7 +243,8 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper
                      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;
@@ -505,9 +506,9 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper
   // 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;
@@ -708,7 +709,7 @@ class ClientSocketPoolBase {
     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(); }