Changes connection migration code to migrate on NetworkMadeDefault

net/quic/chromium/quic_stream_factory.cc

 // 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.
 
 #include "net/quic/chromium/quic_stream_factory.h"
 
 #include <openssl/aead.h>
 
 #include <algorithm>
 #include <tuple>
@@ skipping 1436 matching lines @@
   status_ = OPEN;
   CloseAllSessions(ERR_NETWORK_CHANGED, QUIC_IP_ADDRESS_CHANGED);
   set_require_confirmation(true);
 }
 
 void QuicStreamFactory::OnNetworkConnected(NetworkHandle network) {
   num_timeouts_with_open_streams_ = 0;
   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) {
   // Find a new network that sessions bound to |old_network| can be migrated to.
   NetworkChangeNotifier::NetworkList network_list;
   NetworkChangeNotifier::GetConnectedNetworks(&network_list);
   for (NetworkHandle new_network : network_list) {
     if (new_network != old_network)
       return new_network;
   }
   return NetworkChangeNotifier::kInvalidNetworkHandle;
 }
 
 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) {
       // Close idle sessions.
       HistogramAndLogMigrationFailure(
           bound_net_log, MIGRATION_STATUS_NO_MIGRATABLE_STREAMS,
           session->connection_id(), "No active sessions");
       session->CloseSessionOnError(
           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) {

[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);
       }
       continue;
     }
     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.
         session->CloseSessionOnError(
             ERR_NETWORK_CHANGED,
             QUIC_CONNECTION_MIGRATION_NON_MIGRATABLE_STREAM);
       }
       continue;
     }
 
     MigrateSessionToNewNetwork(session, new_network, bound_net_log, nullptr);
   }
 }
@@ skipping 476 matching lines @@
   // Since the session was active, there's no longer an
   // HttpStreamFactoryImpl::Job running which can mark it broken, unless the TCP
   // job also fails. So to avoid not using QUIC when we otherwise could, we mark
   // it as recently broken, which means that 0-RTT will be disabled but we'll
   // still race.
   http_server_properties_->MarkAlternativeServiceRecentlyBroken(
       alternative_service);
 }
 
 }  // namespace net