Connection migation clean-up: move connection migration logic OnWriteError/PathDegrading

to session.

Bug: 775593
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: If844eb130c63a24d140237498dfba585e665f2e0
Reviewed-on: https://chromium-review.googlesource.com/726940
Commit-Queue: Zhongyi Shi <zhongyi@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513300}

net/quic/chromium/quic_chromium_client_session.cc

@@ -95,6 +95,14 @@ void RecordUnexpectedNotGoingAway(Location location) {
                             NUM_LOCATIONS);
 }
 
+std::unique_ptr<base::Value> NetLogQuicConnectionMigrationTriggerCallback(
+    std::string trigger,
+    NetLogCaptureMode capture_mode) {
+  std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
+  dict->SetString("trigger", trigger);
+  return std::move(dict);
+}
+
 std::unique_ptr<base::Value> NetLogQuicConnectionMigrationFailureCallback(
     QuicConnectionId connection_id,
     std::string reason,
@@ -1467,9 +1475,21 @@ void QuicChromiumClientSession::MigrateSessionOnWriteError(int error_code) {
     return;
 
   MigrationResult result = MigrationResult::FAILURE;
-  if (stream_factory_ != nullptr)
-    result = stream_factory_->MaybeMigrateSingleSessionOnWriteError(this,
-                                                                    error_code);
+  if (stream_factory_ != nullptr) {
+    stream_factory_->CollectDataOnWriteError(error_code);
+
+    const NetLogWithSource migration_net_log = NetLogWithSource::Make(
+        net_log_.net_log(), NetLogSourceType::QUIC_CONNECTION_MIGRATION);
+    migration_net_log.BeginEvent(
+        NetLogEventType::QUIC_CONNECTION_MIGRATION_TRIGGERED,
+        base::Bind(&NetLogQuicConnectionMigrationTriggerCallback,
+                   "WriteError"));
+
+    result = MigrateToAlternateNetwork(/*close_session_on_error*/ false,
+                                       migration_net_log);
+    migration_net_log.EndEvent(
+        NetLogEventType::QUIC_CONNECTION_MIGRATION_TRIGGERED);
+  }
 
   if (result == MigrationResult::SUCCESS)
     return;
@@ -1572,7 +1592,24 @@ void QuicChromiumClientSession::OnWriteUnblocked() {
 
 void QuicChromiumClientSession::OnPathDegrading() {
   if (stream_factory_) {
-    stream_factory_->MaybeMigrateSingleSessionOnPathDegrading(this);
+    stream_factory_->CollectDataOnPathDegrading();
+
+    const NetLogWithSource migration_net_log = NetLogWithSource::Make(
+        net_log_.net_log(), NetLogSourceType::QUIC_CONNECTION_MIGRATION);
+    migration_net_log.BeginEvent(
+        NetLogEventType::QUIC_CONNECTION_MIGRATION_TRIGGERED,
+        base::Bind(&NetLogQuicConnectionMigrationTriggerCallback,
+                   "PathDegrading"));
+    if (migrate_session_early_) {
+      MigrateToAlternateNetwork(/*close_session_on_error*/ true,
+                                migration_net_log);
+    } else {
+      HistogramAndLogMigrationFailure(migration_net_log,
+                                      MIGRATION_STATUS_DISABLED,
+                                      connection_id(), "Migration disabled");
+    }
+    migration_net_log.EndEvent(
+        NetLogEventType::QUIC_CONNECTION_MIGRATION_TRIGGERED);
   }
 }
 

net/quic/chromium/quic_stream_factory.cc

@@ -120,16 +120,6 @@ std::unique_ptr<base::Value> NetLogQuicConnectionMigrationTriggerCallback(
   return std::move(dict);
 }
 
-std::unique_ptr<base::Value> NetLogQuicConnectionMigrationFailureCallback(
-    QuicConnectionId connection_id,
-    std::string reason,
-    NetLogCaptureMode capture_mode) {
-  std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
-  dict->SetString("connection_id", base::Uint64ToString(connection_id));
-  dict->SetString("reason", reason);
-  return std::move(dict);
-}
-
 // Helper class that is used to log a connection migration event.
 class ScopedConnectionMigrationEventLog {
  public:
@@ -157,17 +147,6 @@ void HistogramCreateSessionFailure(enum CreateSessionFailure error) {
                             CREATION_ERROR_MAX);
 }
 
-void HistogramAndLogMigrationFailure(const NetLogWithSource& net_log,
-                                     enum QuicConnectionMigrationStatus status,
-                                     QuicConnectionId connection_id,
-                                     std::string reason) {
-  UMA_HISTOGRAM_ENUMERATION("Net.QuicSession.ConnectionMigration", status,
-                            MIGRATION_STATUS_MAX);
-  net_log.AddEvent(NetLogEventType::QUIC_CONNECTION_MIGRATION_FAILURE,
-                   base::Bind(&NetLogQuicConnectionMigrationFailureCallback,
-                              connection_id, reason));
-}
-
 void LogPlatformNotificationInHistogram(
     enum QuicPlatformNotification notification) {
   UMA_HISTOGRAM_ENUMERATION("Net.QuicSession.PlatformNotification",
@@ -1294,49 +1273,14 @@ void QuicStreamFactory::MaybeMigrateOrCloseSessions(
   }
 }
 
-MigrationResult QuicStreamFactory::MaybeMigrateSingleSessionOnWriteError(
-    QuicChromiumClientSession* session,
-    int error_code) {
+void QuicStreamFactory::CollectDataOnWriteError(int error_code) {
   most_recent_write_error_timestamp_ = base::TimeTicks::Now();
   most_recent_write_error_ = error_code;
-
-  const NetLogWithSource migration_net_log = NetLogWithSource::Make(
-      net_log_, NetLogSourceType::QUIC_CONNECTION_MIGRATION);
-  migration_net_log.BeginEvent(
-      NetLogEventType::QUIC_CONNECTION_MIGRATION_TRIGGERED,
-      base::Bind(&NetLogQuicConnectionMigrationTriggerCallback, "WriteError"));
-
-  MigrationResult result = session->MigrateToAlternateNetwork(
-      /*close_session_on_error*/ false, migration_net_log);
-  migration_net_log.EndEvent(
-      NetLogEventType::QUIC_CONNECTION_MIGRATION_TRIGGERED);
-  return result;
 }
 
-MigrationResult QuicStreamFactory::MaybeMigrateSingleSessionOnPathDegrading(
-    QuicChromiumClientSession* session) {
+void QuicStreamFactory::CollectDataOnPathDegrading() {
   if (most_recent_path_degrading_timestamp_ == base::TimeTicks())
     most_recent_path_degrading_timestamp_ = base::TimeTicks::Now();
-
-  const NetLogWithSource migration_net_log = NetLogWithSource::Make(
-      net_log_, NetLogSourceType::QUIC_CONNECTION_MIGRATION);
-  migration_net_log.BeginEvent(
-      NetLogEventType::QUIC_CONNECTION_MIGRATION_TRIGGERED,
-      base::Bind(&NetLogQuicConnectionMigrationTriggerCallback,
-                 "PathDegrading"));
-
-  MigrationResult result = MigrationResult::FAILURE;
-  if (migrate_sessions_early_) {
-    result = session->MigrateToAlternateNetwork(
-        /*close_session_on_error*/ true, migration_net_log);
-  } else {
-    HistogramAndLogMigrationFailure(
-        migration_net_log, MIGRATION_STATUS_DISABLED, session->connection_id(),
-        "Migration disabled");
-  }
-  migration_net_log.EndEvent(
-      NetLogEventType::QUIC_CONNECTION_MIGRATION_TRIGGERED);
-  return result;
 }
 
 void QuicStreamFactory::MigrateSessionToNewPeerAddress(

net/quic/chromium/quic_stream_factory.h

@@ -303,18 +303,15 @@ class NET_EXPORT_PRIVATE QuicStreamFactory
       bool close_if_cannot_migrate,
       const NetLogWithSource& net_log);
 
-  // Method that attempts migration of |session| on write error with
-  // |error_code| if |session| is active and if there is an alternate network
-  // than the one to which |session| is currently bound.
-  MigrationResult MaybeMigrateSingleSessionOnWriteError(
-      QuicChromiumClientSession* session,
-      int error_code);
-
-  // Method that attempts migration of |session| on path degrading if |session|
-  // is active and if there is an alternate network than the one to which
-  // |session| is currently bound.
-  MigrationResult MaybeMigrateSingleSessionOnPathDegrading(
-      QuicChromiumClientSession* session);
+  // TODO(zhongyi): move metrics collection to session once connection migration
+  // logic is all in QuicChromiumClientSession.
+  // Method that collects error code data on write error.
+  void CollectDataOnWriteError(int error_code);
+
+  // TODO(zhongyi): move metrics collection to session once connection migration
+  // logic is all in QuicChromiumClientSession.
+  // Method that collects timestamp when some session is on path degrading.
+  void CollectDataOnPathDegrading();
 
   // Migrates |session| over to using |peer_address|. Causes a PING frame
   // to be sent to the new peer address.