Introduces ability for session to wait on a migration trigger for a new

net/quic/chromium/quic_chromium_client_session.cc

Index: net/quic/chromium/quic_chromium_client_session.cc
diff --git a/net/quic/chromium/quic_chromium_client_session.cc b/net/quic/chromium/quic_chromium_client_session.cc
index c2bdf2b1df8f3b622d829ceb9a7f644fbf9fe9f5..aeae564ba897b4d1e5a6642674c437f5f39cf272 100644
--- a/net/quic/chromium/quic_chromium_client_session.cc
+++ b/net/quic/chromium/quic_chromium_client_session.cc
@@ -57,6 +57,10 @@ const size_t kMaxReadersPerQuicSession = 5;
 // SSLClientSocketImpl.
 const size_t kTokenBindingSignatureMapSize = 10;
 
+// Time to wait (in seconds) when no networks are available and
+// migrating sessions need to wait for a new network to connect.
+const size_t kWaitTimeForNewNetworkSecs = 10;
+
 // Histograms for tracking down the crashes from http://crbug.com/354669
 // Note: these values must be kept in sync with the corresponding values in:
 // tools/metrics/histograms/histograms.xml
@@ -999,13 +1003,16 @@ void QuicChromiumClientSession::MigrateSessionOnWriteError() {
   // If migration_pending_ is false, an earlier task completed migration.
   if (!migration_pending_)
     return;
-
-  if (stream_factory_ != nullptr &&
-      stream_factory_->MaybeMigrateSingleSession(this, WRITE_ERROR) ==
-          MigrationResult::SUCCESS) {
+  MigrationResult result = MigrationResult::FAILURE;
+  if (stream_factory_ != nullptr) {
+    result = stream_factory_->MaybeMigrateSingleSession(this, WRITE_ERROR);
+  }
+  if (result == MigrationResult::SUCCESS)
+    return;
+  if (result == MigrationResult::NO_NEW_NETWORK) {
+    OnNoNewNetwork();
     return;
   }
-
   // Close the connection if migration failed. Do not cause a
   // connection close packet to be sent since socket may be borked.
   connection()->CloseConnection(QUIC_PACKET_WRITE_ERROR,
@@ -1013,14 +1020,27 @@ void QuicChromiumClientSession::MigrateSessionOnWriteError() {
                                 ConnectionCloseBehavior::SILENT_CLOSE);
 }
 
+void QuicChromiumClientSession::OnNoNewNetwork() {
+  // When called from a network notification, migration_pending_ may
+  // be false.  When a new network connects later, migration will
+  // proceed via OnNetworkConnected only if a migration is pending, so
+  // ensure that migration is pending.
+  migration_pending_ = true;
+  // Block the packet writer to avoid any writes while migration is in progress.
+  static_cast<QuicChromiumPacketWriter*>(connection()->writer())
+      ->set_write_blocked(true);
+  // Post a task to maybe close the session if the alarm fires.
+  task_runner_->PostDelayedTask(
+      FROM_HERE, base::Bind(&QuicChromiumClientSession::OnMigrationTimeout,
+                            weak_factory_.GetWeakPtr(), sockets_.size()),
+      base::TimeDelta::FromSeconds(kWaitTimeForNewNetworkSecs));
+}
+
 void QuicChromiumClientSession::WriteToNewSocket() {
   // Prevent any pending migration from executing.
   migration_pending_ = false;
-
   static_cast<QuicChromiumPacketWriter*>(connection()->writer())
       ->set_write_blocked(false);
-  DCHECK(!connection()->writer()->IsWriteBlocked());
-
   if (packet_ == nullptr) {
     // Unblock the connection before sending a PING packet, since it
     // may have been blocked before the migration started.
@@ -1033,7 +1053,6 @@ void QuicChromiumClientSession::WriteToNewSocket() {
   // that method may set packet_ if there is a write error.
   scoped_refptr<StringIOBuffer> packet = packet_;
   packet_ = nullptr;
-
   // The connection is waiting for the original write to complete
   // asynchronously. The new writer will notify the connection if the
   // write below completes asynchronously, but a synchronous competion
@@ -1041,7 +1060,6 @@ void QuicChromiumClientSession::WriteToNewSocket() {
   WriteResult result =
       static_cast<QuicChromiumPacketWriter*>(connection()->writer())
           ->WritePacketToSocket(packet);
-
   if (result.error_code == ERR_IO_PENDING)
     return;
   // All write errors should be mapped into ERR_IO_PENDING by
@@ -1050,6 +1068,33 @@ void QuicChromiumClientSession::WriteToNewSocket() {
   connection()->OnCanWrite();
 }
 
+void QuicChromiumClientSession::OnMigrationTimeout(size_t num_sockets) {
+  // If number of sockets has increased, this migration task is stale.

[Ryan Hamilton, 2016/09/13 02:57:31]

s/increased/changed/ I'd say. (And "<" => "!=" below)

[Jana, 2016/09/13 03:12:14]

Done. Just curious: why? We aren't expecting this to go in any other direction,
are we?

[Ryan Hamilton, 2016/09/13 03:15:32]

Agreed that "we" are not expecting it to go backwards. 

But as far as this method is concerned, if it *did* go backwards we probably
wouldn't want to proceed, would we? Alternatively, DCHECK_GE(num_sockets,
socket_Size()) or some such?

It really doesn't matter... I'm just neurotic!

+  if (num_sockets < sockets_.size())
+    return;
+  UMA_HISTOGRAM_ENUMERATION("Net.QuicSession.ConnectionMigration",
+                            MIGRATION_STATUS_NO_ALTERNATE_NETWORK,
+                            MIGRATION_STATUS_MAX);
+  CloseSessionOnError(ERR_NETWORK_CHANGED,
+                      QUIC_CONNECTION_MIGRATION_NO_NEW_NETWORK);
+}
+
+void QuicChromiumClientSession::OnNetworkConnected(
+    NetworkChangeNotifier::NetworkHandle network,
+    const BoundNetLog& bound_net_log) {
+  // If migration_pending_ is false, there was no migration pending or
+  // an earlier task completed migration.
+  if (!migration_pending_)
+    return;
+

[Ryan Hamilton, 2016/09/13 02:57:31]

Yey, vertical white space!

[Jana, 2016/09/13 03:12:14]

:-)

+  // TODO(jri): Ensure that OnSessionGoingAway is called consistently,
+  // and that it's always called at the same time in the whole
+  // migration process. Allows tests to be more uniform.
+  stream_factory_->OnSessionGoingAway(this);
+  stream_factory_->MigrateSessionToNewNetwork(
+      this, network, /*close_session_on_error=*/true, net_log_);
+}
+
 void QuicChromiumClientSession::OnWriteError(int error_code) {
   DCHECK_NE(ERR_IO_PENDING, error_code);
   DCHECK_GT(0, error_code);