Changes connection migration code to migrate on NetworkMadeDefault

instead of on SoonToDisconnect.

The NetworkMadeDefault notification is supposed to always be issued
before SoonToDisconnect, but there seems to be a bug in Android that
causes the order to sometimes be reversed. Current connection migration
looks for an alternate network on SoonToDisconnect, and this may not
work since there may be no alternate networks around. Since
NetworkMadeDefault guarantees that the new default network is around,
this CL changes connection migration code to migrate on
NetworkMadeDefault instead.

BUG=

Review-Url: https://codereview.chromium.org/2258893004
Cr-Commit-Position: refs/heads/master@{#413327}

net/quic/chromium/quic_stream_factory.cc

@@ -1457,24 +1457,26 @@ void QuicStreamFactory::OnNetworkConnected(NetworkHandle network) {
   status_ = OPEN;
 }
 
-void QuicStreamFactory::OnNetworkMadeDefault(NetworkHandle network) {}
+void QuicStreamFactory::OnNetworkMadeDefault(NetworkHandle network) {
+  ScopedConnectionMigrationEventLog scoped_event_log(net_log_,
+                                                     "OnNetworkMadeDefault");
+  DCHECK_NE(NetworkChangeNotifier::kInvalidNetworkHandle, network);
+  MaybeMigrateOrCloseSessions(network, /*close_if_cannot_migrate=*/false,
+                              scoped_event_log.net_log());
+  set_require_confirmation(true);
+}
 
 void QuicStreamFactory::OnNetworkDisconnected(NetworkHandle network) {
   ScopedConnectionMigrationEventLog scoped_event_log(net_log_,
                                                      "OnNetworkDisconnected");
-  MaybeMigrateOrCloseSessions(network, /*force_close=*/true,
+  NetworkHandle new_network = FindAlternateNetwork(network);
+  MaybeMigrateOrCloseSessions(new_network, /*close_if_cannot_migrate=*/true,
                               scoped_event_log.net_log());
-  set_require_confirmation(true);
 }
 
 // This method is expected to only be called when migrating from Cellular to
-// WiFi on Android.
-void QuicStreamFactory::OnNetworkSoonToDisconnect(NetworkHandle network) {
-  ScopedConnectionMigrationEventLog scoped_event_log(
-      net_log_, "OnNetworkSoonToDisconnect");
-  MaybeMigrateOrCloseSessions(network, /*force_close=*/false,
-                              scoped_event_log.net_log());
-}
+// WiFi on Android, and should always be preceded by OnNetworkMadeDefault().
+void QuicStreamFactory::OnNetworkSoonToDisconnect(NetworkHandle network) {}
 
 NetworkHandle QuicStreamFactory::FindAlternateNetwork(
     NetworkHandle old_network) {
@@ -1489,26 +1491,34 @@ NetworkHandle QuicStreamFactory::FindAlternateNetwork(
 }
 
 void QuicStreamFactory::MaybeMigrateOrCloseSessions(
-    NetworkHandle network,
-    bool force_close,
+    NetworkHandle new_network,
+    bool close_if_cannot_migrate,
     const BoundNetLog& bound_net_log) {
-  DCHECK_NE(NetworkChangeNotifier::kInvalidNetworkHandle, network);
-  NetworkHandle new_network = FindAlternateNetwork(network);
-
   QuicStreamFactory::SessionIdMap::iterator it = all_sessions_.begin();
   while (it != all_sessions_.end()) {
     QuicChromiumClientSession* session = it->first;
     ++it;
 
-    if (session->GetDefaultSocket()->GetBoundNetwork() != network) {
-      // If session is not bound to |network|, move on.
+    // Migration attempted, but no new network was found. Close session.
+    if (new_network == NetworkChangeNotifier::kInvalidNetworkHandle) {
+      HistogramAndLogMigrationFailure(
+          bound_net_log, MIGRATION_STATUS_NO_ALTERNATE_NETWORK,
+          session->connection_id(), "No alternate network found");
+      session->CloseSessionOnError(ERR_NETWORK_CHANGED,
+                                   QUIC_CONNECTION_MIGRATION_NO_NEW_NETWORK);
+      continue;
+    }
+
+    // If session is already bound to |new_network|, move on.
+    if (session->GetDefaultSocket()->GetBoundNetwork() == new_network) {
       HistogramAndLogMigrationFailure(
           bound_net_log, MIGRATION_STATUS_ALREADY_MIGRATED,
-          session->connection_id(), "Not bound to network");
+          session->connection_id(), "Already bound to new network");
       continue;
     }
+
+    // Close idle sessions.
     if (session->GetNumActiveStreams() == 0) {
-      // Close idle sessions.
       HistogramAndLogMigrationFailure(
           bound_net_log, MIGRATION_STATUS_NO_MIGRATABLE_STREAMS,
           session->connection_id(), "No active sessions");
@@ -1516,42 +1526,28 @@ void QuicStreamFactory::MaybeMigrateOrCloseSessions(
           ERR_NETWORK_CHANGED, QUIC_CONNECTION_MIGRATION_NO_MIGRATABLE_STREAMS);
       continue;
     }
+
     // If session has active streams, mark it as going away.
     OnSessionGoingAway(session);
 
-    if (new_network == NetworkChangeNotifier::kInvalidNetworkHandle) {
-      // No new network was found.
-      // TODO (jri): Add histogram for this failure case.
-      bound_net_log.AddEvent(
-          NetLog::TYPE_QUIC_CONNECTION_MIGRATION_FAILURE,
-          base::Bind(&NetLogQuicConnectionMigrationFailureCallback,
-                     session->connection_id(), "No new network"));
-      if (force_close) {
-        session->CloseSessionOnError(ERR_NETWORK_CHANGED,
-                                     QUIC_CONNECTION_MIGRATION_NO_NEW_NETWORK);
-      }
-      continue;
-    }
+    // Do not migrate sessions where connection migration is disabled.
     if (session->config()->DisableConnectionMigration()) {
-      // Do not migrate sessions where connection migration is disabled by
-      // config.
       HistogramAndLogMigrationFailure(bound_net_log, MIGRATION_STATUS_DISABLED,
                                       session->connection_id(),
                                       "Migration disabled");
-      if (force_close) {
-        // Close sessions where connection migration is disabled.
+      if (close_if_cannot_migrate) {
         session->CloseSessionOnError(ERR_NETWORK_CHANGED,
                                      QUIC_IP_ADDRESS_CHANGED);
       }
       continue;
     }
+
+    // Do not migrate sessions with non-migratable streams.
     if (session->HasNonMigratableStreams()) {
-      // Do not migrate sessions with non-migratable streams.
       HistogramAndLogMigrationFailure(
           bound_net_log, MIGRATION_STATUS_NON_MIGRATABLE_STREAM,
           session->connection_id(), "Non-migratable stream");
-      if (force_close) {
-        // Close sessions with non-migratable streams.
+      if (close_if_cannot_migrate) {
         session->CloseSessionOnError(
             ERR_NETWORK_CHANGED,
             QUIC_CONNECTION_MIGRATION_NON_MIGRATABLE_STREAM);

net/quic/chromium/quic_stream_factory.h

@@ -283,15 +283,20 @@ class NET_EXPORT_PRIVATE QuicStreamFactory
   NetworkChangeNotifier::NetworkHandle FindAlternateNetwork(
       NetworkChangeNotifier::NetworkHandle old_network);
 
-  // Method that initiates migration of active sessions
-  // currently bound to |network| to an alternate network, if one
-  // exists. Idle sessions bound to |network| are closed. If there is
-  // no alternate network to migrate active sessions onto, active
-  // sessions are closed if |force_close| is true, and continue using
-  // |network| otherwise. Sessions not bound to |network| are left unchanged.
-  void MaybeMigrateOrCloseSessions(NetworkChangeNotifier::NetworkHandle network,
-                                   bool force_close,
-                                   const BoundNetLog& bound_net_log);
+  // Method that initiates migration of active sessions to |new_network|.
+  // If |new_network| is a valid network, sessions that can migrate are
+  // migrated to |new_network|, and sessions not bound to |new_network|
+  // are left unchanged. Sessions with non-migratable streams are closed
+  // if |close_if_cannot_migrate| is true, and continue using their current
+  // network otherwise.
+  //
+  // If |new_network| is NetworkChangeNotifier::kInvalidNetworkHandle,
+  // there is no new network to migrate sessions onto, and all sessions are
+  // closed.
+  void MaybeMigrateOrCloseSessions(
+      NetworkChangeNotifier::NetworkHandle new_network,
+      bool close_if_cannot_migrate,
+      const BoundNetLog& bound_net_log);
 
   // Method that initiates migration of |session| if |session| is
   // active and if there is an alternate network than the one to which