Changes connection migration code to migrate on NetworkMadeDefault

net/quic/chromium/quic_stream_factory.cc

Index: net/quic/chromium/quic_stream_factory.cc
diff --git a/net/quic/chromium/quic_stream_factory.cc b/net/quic/chromium/quic_stream_factory.cc
index 944a27f256aecaa422cbc8c0e49e9f0d26c2488e..3e04d77b1452bf1c6431ac68c0611568f17533d1 100644
--- a/net/quic/chromium/quic_stream_factory.cc
+++ b/net/quic/chromium/quic_stream_factory.cc
@@ -1454,24 +1454,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, /*force_close=*/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, /*force_close=*/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) {
@@ -1486,22 +1488,19 @@ NetworkHandle QuicStreamFactory::FindAlternateNetwork(
 }
 
 void QuicStreamFactory::MaybeMigrateOrCloseSessions(
-    NetworkHandle network,
+    NetworkHandle new_network,
     bool force_close,
     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.
+    if (session->GetDefaultSocket()->GetBoundNetwork() == new_network) {
+      // If session is already bound to |new_network|, move on.
       HistogramAndLogMigrationFailure(
           bound_net_log, MIGRATION_STATUS_ALREADY_MIGRATED,
-          session->connection_id(), "Not bound to network");
+          session->connection_id(), "Already bound to new network");
       continue;
     }
     if (session->GetNumActiveStreams() == 0) {
@@ -1523,20 +1522,16 @@ void QuicStreamFactory::MaybeMigrateOrCloseSessions(
           NetLog::TYPE_QUIC_CONNECTION_MIGRATION_FAILURE,
           base::Bind(&NetLogQuicConnectionMigrationFailureCallback,
                      session->connection_id(), "No new network"));
-      if (force_close) {

[Ryan Hamilton, 2016/08/19 20:05:02]

This looks like a change in behavior, but I might be misunderstand it because of
the way the semantics have changed. Can you say more?

[Jana, 2016/08/19 21:44:08]

I've moved the code around a bit, hopefully it'll help explain. I've also
changed the comment in the header file to explain better.

-        session->CloseSessionOnError(ERR_NETWORK_CHANGED,
-                                     QUIC_CONNECTION_MIGRATION_NO_NEW_NETWORK);
-      }
+      session->CloseSessionOnError(ERR_NETWORK_CHANGED,
+                                   QUIC_CONNECTION_MIGRATION_NO_NEW_NETWORK);
       continue;
     }
     if (session->config()->DisableConnectionMigration()) {
-      // Do not migrate sessions where connection migration is disabled by
-      // config.
+      // Do not migrate sessions where connection migration is disabled.
       HistogramAndLogMigrationFailure(bound_net_log, MIGRATION_STATUS_DISABLED,
                                       session->connection_id(),
                                       "Migration disabled");
       if (force_close) {
-        // Close sessions where connection migration is disabled.
         session->CloseSessionOnError(ERR_NETWORK_CHANGED,
                                      QUIC_IP_ADDRESS_CHANGED);
       }
@@ -1548,7 +1543,6 @@ void QuicStreamFactory::MaybeMigrateOrCloseSessions(
           bound_net_log, MIGRATION_STATUS_NON_MIGRATABLE_STREAM,
           session->connection_id(), "Non-migratable stream");
       if (force_close) {
-        // Close sessions with non-migratable streams.
         session->CloseSessionOnError(
             ERR_NETWORK_CHANGED,
             QUIC_CONNECTION_MIGRATION_NON_MIGRATABLE_STREAM);