Add DCHECKs and switch to safe math to avoid arithmetic error.

Bug: 1114590, 1090532
Change-Id: Ic149aa63ee65fe2f78c4bb8f28497677ec4ac4c2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2347325
Commit-Queue: Zhongyi Shi <zhongyi@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#796949}

net/quic/quic_connectivity_monitor.cc

@@ -35,9 +35,7 @@ void QuicConnectivityMonitor::RecordConnectivityStatsToHistograms(
       return;
   }
 
-  UMA_HISTOGRAM_COUNTS_100(
-      "Net.QuicConnectivityMonitor.NumActiveQuicSessionsAtNetworkChange",
-      active_sessions_.size());
+  base::ClampedNumeric<int> num_degrading_sessions = GetNumDegradingSessions();
 
   if (num_sessions_active_during_current_speculative_connectivity_failure_) {
     UMA_HISTOGRAM_COUNTS_100(
@@ -46,6 +44,20 @@ void QuicConnectivityMonitor::RecordConnectivityStatsToHistograms(
             .value());
   }
 
+  UMA_HISTOGRAM_COUNTS_100(
+      "Net.QuicConnectivityMonitor.NumActiveQuicSessionsAtNetworkChange",
+      active_sessions_.size());
+
+  int percentage = 0;
+  if (num_sessions_active_during_current_speculative_connectivity_failure_ &&
+      num_sessions_active_during_current_speculative_connectivity_failure_
+              .value() > 0) {
+    percentage = base::saturated_cast<int>(
+        num_all_degraded_sessions_ * 100.0 /
+        num_sessions_active_during_current_speculative_connectivity_failure_
+            .value());
+  }
+
   UMA_HISTOGRAM_COUNTS_100(
       "Net.QuicConnectivityMonitor.NumAllSessionsDegradedAtNetworkChange",
       num_all_degraded_sessions_);
@@ -55,14 +67,6 @@ void QuicConnectivityMonitor::RecordConnectivityStatsToHistograms(
   base::UmaHistogramCustomCounts(raw_histogram_name1,
                                  num_all_degraded_sessions_, 1, 100, 50);
 
-  int percentage = 0;
-  if (num_sessions_active_during_current_speculative_connectivity_failure_) {
-    percentage =
-        num_all_degraded_sessions_ * 100 /
-        num_sessions_active_during_current_speculative_connectivity_failure_
-            .value();
-  }
-
   const std::string percentage_histogram_name1 =
       "Net.QuicConnectivityMonitor.PercentageAllDegradedSessions." +
       notification;
@@ -70,16 +74,18 @@ void QuicConnectivityMonitor::RecordConnectivityStatsToHistograms(
   base::UmaHistogramPercentage(percentage_histogram_name1, percentage);
 
   // Skip degrading session collection if there are less than two sessions.
-  if (active_sessions_.size() < 2)
+  if (active_sessions_.size() < 2u)
     return;
 
-  size_t num_degrading_sessions = GetNumDegradingSessions();
   const std::string raw_histogram_name =
       "Net.QuicConnectivityMonitor.NumActiveDegradingSessions." + notification;
 
   base::UmaHistogramCustomCounts(raw_histogram_name, num_degrading_sessions, 1,
                                  100, 50);
-  percentage = num_degrading_sessions * 100 / active_sessions_.size();
+
+  percentage = base::saturated_cast<double>(num_degrading_sessions * 100.0 /
+                                            active_sessions_.size());
+
   const std::string percentage_histogram_name =
       "Net.QuicConnectivityMonitor.PercentageActiveDegradingSessions." +
       notification;
@@ -109,6 +115,11 @@ void QuicConnectivityMonitor::OnSessionPathDegrading(
 
   degrading_sessions_.insert(session);
   num_all_degraded_sessions_++;
+  // If the degrading session used to be on the previous default network, it is
+  // possible that the session is no longer tracked in |active_sessions_| due
+  // to the recent default network change.
+  active_sessions_.insert(session);
+
   if (!num_sessions_active_during_current_speculative_connectivity_failure_) {
     num_sessions_active_during_current_speculative_connectivity_failure_ =
         active_sessions_.size();
@@ -127,6 +138,12 @@ void QuicConnectivityMonitor::OnSessionResumedPostPathDegrading(
     return;
 
   degrading_sessions_.erase(session);
+
+  // If the resumed session used to be on the previous default network, it is
+  // possible that the session is no longer tracked in |active_sessions_| due
+  // to the recent default network change.
+  active_sessions_.insert(session);
+
   num_all_degraded_sessions_ = 0u;
   num_sessions_active_during_current_speculative_connectivity_failure_ =
       base::nullopt;
@@ -139,6 +156,11 @@ void QuicConnectivityMonitor::OnSessionEncounteringWriteError(
   if (network != default_network_)
     return;
 
+  // If the session used to be on the previous default network, it is
+  // possible that the session is no longer tracked in |active_sessions_| due
+  // to the recent default network change.
+  active_sessions_.insert(session);
+
   ++write_error_map_[error_code];
 
   bool is_session_degraded =

net/quic/quic_connectivity_monitor.h

@@ -5,6 +5,7 @@
 #ifndef NET_QUIC_QUIC_CONNECTIVITY_MONITOR_H_
 #define NET_QUIC_QUIC_CONNECTIVITY_MONITOR_H_
 
+#include "base/numerics/clamped_math.h"
 #include "net/base/network_change_notifier.h"
 #include "net/quic/quic_chromium_client_session.h"
 #include "net/third_party/quiche/src/quic/platform/api/quic_containers.h"
@@ -104,11 +105,13 @@ class NET_EXPORT_PRIVATE QuicConnectivityMonitor
   // - starts by the earliest detection of path degradation or a connectivity
   //   related packet write error,
   // - ends immediately by the detection of path recovery or a network change.
-  base::Optional<size_t>
+  // Use clamped math to cap number of sessions at INT_MAX.
+  base::Optional<base::ClampedNumeric<int>>
       num_sessions_active_during_current_speculative_connectivity_failure_;
   // Total number of sessions that has been degraded before any recovery,
   // including no longer active sessions.
-  size_t num_all_degraded_sessions_{0u};
+  // Use clamped math to cap number of sessions at INT_MAX.
+  base::ClampedNumeric<int> num_all_degraded_sessions_{0};
 
   // Map from the write error code to the corresponding number of reports.
   WriteErrorMap write_error_map_;