NQE: Report H2 ping latency to network quality estimator

Report H2 ping latency to network quality estimator where
it is used for computing the network quality.

Change-Id: Ib25e339ccd3967293ec8be94ef35aaa2a30a0678
Bug: 823322
Reviewed-on: https://chromium-review.googlesource.com/c/1285077Reviewed-by: Ryan Hamilton <rch@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601299}

net/http/http_network_session.cc

@@ -240,7 +240,8 @@ HttpNetworkSession::HttpNetworkSession(const Params& params,
                          params.spdy_session_max_recv_window_size,
                          AddDefaultHttp2Settings(params.http2_settings),
                          params.greased_http2_frame,
-                         params.time_func),
+                         params.time_func,
+                         context.network_quality_estimator),
       http_stream_factory_(std::make_unique<HttpStreamFactory>(this)),
       params_(params),
       context_(context) {

net/nqe/network_quality_estimator.cc

@@ -1692,4 +1692,16 @@ void NetworkQualityEstimator::SimulateNetworkQualityChangeForTesting(
   ComputeEffectiveConnectionType();
 }
 
+void NetworkQualityEstimator::RecordSpdyPingLatency(
+    const HostPortPair& host_port_pair,
+    base::TimeDelta rtt) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  DCHECK_LT(nqe::internal::INVALID_RTT_THROUGHPUT, rtt.InMilliseconds());
+
+  Observation observation(rtt.InMilliseconds(), tick_clock_->NowTicks(),
+                          current_network_id_.signal_strength,
+                          NETWORK_QUALITY_OBSERVATION_SOURCE_H2_PINGS);
+  AddAndNotifyObserversOfRTT(observation);
+}
+
 }  // namespace net

net/nqe/network_quality_estimator.h

@@ -260,6 +260,10 @@ class NET_EXPORT NetworkQualityEstimator
   void SimulateNetworkQualityChangeForTesting(
       net::EffectiveConnectionType type);
 
+  // Notifies |this| of round trip ping latency reported by H2 connections.
+  virtual void RecordSpdyPingLatency(const HostPortPair& host_port_pair,
+                                     base::TimeDelta rtt);
+
   typedef nqe::internal::Observation Observation;
   typedef nqe::internal::ObservationBuffer ObservationBuffer;
 

net/nqe/network_quality_estimator_test_util.cc

@@ -61,9 +61,6 @@ TestNetworkQualityEstimator::TestNetworkQualityEstimator(
       suppress_notifications_for_testing_(suppress_notifications_for_testing) {
   SetUseLocalHostRequestsForTesting(allow_local_host_requests_for_tests);
   SetUseSmallResponsesForTesting(allow_smaller_responses_for_tests);
-
-  // Set up the embedded test server.
-  EXPECT_TRUE(embedded_test_server_.Start());
 }
 
 TestNetworkQualityEstimator::TestNetworkQualityEstimator(
@@ -80,13 +77,16 @@ TestNetworkQualityEstimator::TestNetworkQualityEstimator(
       accuracy_recording_intervals_set_(false),
       embedded_test_server_(base::FilePath(kTestFilePath)),
       suppress_notifications_for_testing_(false) {
-  // Set up the embedded test server.
-  EXPECT_TRUE(embedded_test_server_.Start());
 }
 
 TestNetworkQualityEstimator::~TestNetworkQualityEstimator() = default;
 
 void TestNetworkQualityEstimator::RunOneRequest() {
+  // Set up the embedded test server.
+  if (!embedded_test_server_.Started()) {
+    EXPECT_TRUE(embedded_test_server_.Start());
+  }
+
   TestDelegate test_delegate;
   TestURLRequestContext context(true);
   context.set_network_quality_estimator(this);
@@ -107,11 +107,19 @@ void TestNetworkQualityEstimator::SimulateNetworkChange(
   OnConnectionTypeChanged(new_connection_type);
 }
 
-const GURL TestNetworkQualityEstimator::GetEchoURL() const {
+const GURL TestNetworkQualityEstimator::GetEchoURL() {
+  // Set up the embedded test server.
+  if (!embedded_test_server_.Started()) {
+    EXPECT_TRUE(embedded_test_server_.Start());
+  }
   return embedded_test_server_.GetURL("/simple.html");
 }
 
-const GURL TestNetworkQualityEstimator::GetRedirectURL() const {
+const GURL TestNetworkQualityEstimator::GetRedirectURL() {
+  // Set up the embedded test server.
+  if (!embedded_test_server_.Started()) {
+    EXPECT_TRUE(embedded_test_server_.Start());
+  }
   return embedded_test_server_.GetURL("/redirect302-to-https");
 }
 
@@ -322,6 +330,13 @@ void TestNetworkQualityEstimator::NotifyObserversOfEffectiveConnectionType(
     observer.OnEffectiveConnectionTypeChanged(type);
 }
 
+void TestNetworkQualityEstimator::RecordSpdyPingLatency(
+    const HostPortPair& host_port_pair,
+    base::TimeDelta rtt) {
+  ++ping_rtt_received_count_;
+  NetworkQualityEstimator::RecordSpdyPingLatency(host_port_pair, rtt);
+}
+
 const NetworkQualityEstimatorParams* TestNetworkQualityEstimator::params()
     const {
   return params_.get();

net/nqe/network_quality_estimator_test_util.h

@@ -64,11 +64,11 @@ class TestNetworkQualityEstimator : public NetworkQualityEstimator {
       const std::string& network_id);
 
   // Returns a GURL hosted at the embedded test server.
-  const GURL GetEchoURL() const;
+  const GURL GetEchoURL();
 
   // Returns a GURL hosted at the embedded test server which contains redirect
   // to another HTTPS URL.
-  const GURL GetRedirectURL() const;
+  const GURL GetRedirectURL();
 
   void set_effective_connection_type(EffectiveConnectionType type) {
     effective_connection_type_ = type;
@@ -217,6 +217,9 @@ class TestNetworkQualityEstimator : public NetworkQualityEstimator {
     transport_rtt_observation_count_last_ect_computation_ = count;
   }
 
+  // Returns count of ping RTTs received from H2/spdy connections.
+  size_t ping_rtt_received_count() const { return ping_rtt_received_count_; }
+
   const NetworkQualityEstimatorParams* params() const;
 
   using NetworkQualityEstimator::SetTickClockForTesting;
@@ -236,6 +239,9 @@ class TestNetworkQualityEstimator : public NetworkQualityEstimator {
       std::unique_ptr<NetworkQualityEstimatorParams> params,
       std::unique_ptr<BoundTestNetLog> net_log);
 
+  void RecordSpdyPingLatency(const HostPortPair& host_port_pair,
+                             base::TimeDelta rtt) override;
+
   // NetworkQualityEstimator implementation that returns the overridden
   // network
   // id (instead of invoking platform APIs).
@@ -285,6 +291,8 @@ class TestNetworkQualityEstimator : public NetworkQualityEstimator {
   // If true, notifications are not sent to any of the observers.
   const bool suppress_notifications_for_testing_;
 
+  size_t ping_rtt_received_count_ = 0;
+
   base::Optional<size_t> transport_rtt_observation_count_last_ect_computation_;
 
   DISALLOW_COPY_AND_ASSIGN(TestNetworkQualityEstimator);

net/nqe/network_quality_observation.cc

@@ -55,6 +55,7 @@ std::vector<ObservationCategory> Observation::GetObservationCategories() const {
           ObservationCategory::OBSERVATION_CATEGORY_TRANSPORT);
       return observation_categories;
     case NETWORK_QUALITY_OBSERVATION_SOURCE_QUIC:
+    case NETWORK_QUALITY_OBSERVATION_SOURCE_H2_PINGS:
       observation_categories.push_back(
           ObservationCategory::OBSERVATION_CATEGORY_TRANSPORT);
       observation_categories.push_back(

net/nqe/network_quality_observation_source.cc

@@ -18,7 +18,8 @@ static constexpr const char* kObservationSourceMapping[] = {
     "HttpPlatform",
     "HttpExternalEstimate",
     "TransportCachedEstimate",
-    "TransportPlatform"};
+    "TransportPlatform",
+    "H2Pings"};
 
 static_assert(static_cast<size_t>(NETWORK_QUALITY_OBSERVATION_SOURCE_MAX) ==
                   arraysize(kObservationSourceMapping),

net/nqe/network_quality_observation_source.h

@@ -48,6 +48,9 @@ enum NetworkQualityObservationSource {
   // at the transport layer.
   NETWORK_QUALITY_OBSERVATION_SOURCE_DEFAULT_TRANSPORT_FROM_PLATFORM = 7,
 
+  // Round trip ping latency reported by H2 connections.
+  NETWORK_QUALITY_OBSERVATION_SOURCE_H2_PINGS = 8,
+
   NETWORK_QUALITY_OBSERVATION_SOURCE_MAX,
 };
 

net/spdy/spdy_session.cc

@@ -42,6 +42,7 @@
 #include "net/log/net_log_event_type.h"
 #include "net/log/net_log_source_type.h"
 #include "net/log/net_log_with_source.h"
+#include "net/nqe/network_quality_estimator.h"
 #include "net/quic/quic_http_utils.h"
 #include "net/socket/socket.h"
 #include "net/socket/ssl_client_socket.h"
@@ -823,6 +824,7 @@ SpdySession::SpdySession(
         greased_http2_frame,
     TimeFunc time_func,
     ServerPushDelegate* push_delegate,
+    NetworkQualityEstimator* network_quality_estimator,
     NetLog* net_log)
     : in_io_loop_(false),
       spdy_session_key_(spdy_session_key),
@@ -880,6 +882,7 @@ SpdySession::SpdySession(
           base::TimeDelta::FromSeconds(kDefaultConnectionAtRiskOfLossSeconds)),
       hung_interval_(base::TimeDelta::FromSeconds(kHungIntervalSeconds)),
       time_func_(time_func),
+      network_quality_estimator_(network_quality_estimator),
       weak_factory_(this) {
   net_log_.BeginEvent(
       NetLogEventType::HTTP2_SESSION,
@@ -2859,7 +2862,12 @@ void SpdySession::OnPing(spdy::SpdyPingId unique_id, bool is_ack) {
   ping_in_flight_ = false;
 
   // Record RTT in histogram when there are no more pings in flight.
-  RecordPingRTTHistogram(time_func_() - last_ping_sent_time_);
+  base::TimeDelta ping_duration = time_func_() - last_ping_sent_time_;
+  RecordPingRTTHistogram(ping_duration);
+  if (network_quality_estimator_) {
+    network_quality_estimator_->RecordSpdyPingLatency(host_port_pair(),
+                                                      ping_duration);
+  }
 }
 
 void SpdySession::OnRstStream(spdy::SpdyStreamId stream_id,

net/spdy/spdy_session.h

@@ -88,6 +88,7 @@ const spdy::SpdyStreamId kLastStreamId = 0x7fffffff;
 
 struct LoadTimingInfo;
 class NetLog;
+class NetworkQualityEstimator;
 class SpdyStream;
 class SSLInfo;
 class TransportSecurityState;
@@ -304,6 +305,7 @@ class NET_EXPORT SpdySession : public BufferedSpdyFramerVisitorInterface,
                   greased_http2_frame,
               TimeFunc time_func,
               ServerPushDelegate* push_delegate,
+              NetworkQualityEstimator* network_quality_estimator,
               NetLog* net_log);
 
   ~SpdySession() override;
@@ -1140,6 +1142,10 @@ class NET_EXPORT SpdySession : public BufferedSpdyFramerVisitorInterface,
 
   Http2PriorityDependencies priority_dependency_state_;
 
+  // Network quality estimator to which the ping RTTs should be reported. May be
+  // nullptr.
+  NetworkQualityEstimator* network_quality_estimator_;
+
   // Used for posting asynchronous IO tasks.  We use this even though
   // SpdySession is refcounted because we don't need to keep the SpdySession
   // alive if the last reference is within a RunnableMethod.  Just revoke the

net/spdy/spdy_session_pool.cc

@@ -57,7 +57,8 @@ SpdySessionPool::SpdySessionPool(
     size_t session_max_recv_window_size,
     const spdy::SettingsMap& initial_settings,
     const base::Optional<GreasedHttp2Frame>& greased_http2_frame,
-    SpdySessionPool::TimeFunc time_func)
+    SpdySessionPool::TimeFunc time_func,
+    NetworkQualityEstimator* network_quality_estimator)
     : http_server_properties_(http_server_properties),
       transport_security_state_(transport_security_state),
       ssl_config_service_(ssl_config_service),
@@ -71,7 +72,8 @@ SpdySessionPool::SpdySessionPool(
       initial_settings_(initial_settings),
       greased_http2_frame_(greased_http2_frame),
       time_func_(time_func),
-      push_delegate_(nullptr) {
+      push_delegate_(nullptr),
+      network_quality_estimator_(network_quality_estimator) {
   NetworkChangeNotifier::AddIPAddressObserver(this);
   if (ssl_config_service_)
     ssl_config_service_->AddObserver(this);
@@ -113,7 +115,8 @@ base::WeakPtr<SpdySession> SpdySessionPool::CreateAvailableSessionFromSocket(
       enable_sending_initial_data_, enable_ping_based_connection_checking_,
       support_ietf_format_quic_altsvc_, is_trusted_proxy,
       session_max_recv_window_size_, initial_settings_, greased_http2_frame_,
-      time_func_, push_delegate_, net_log.net_log());
+      time_func_, push_delegate_, network_quality_estimator_,
+      net_log.net_log());
 
   new_session->InitializeWithSocket(std::move(connection), this);
 

net/spdy/spdy_session_pool.h

@@ -46,6 +46,7 @@ class HostResolver;
 class HttpServerProperties;
 class HttpStreamRequest;
 class NetLogWithSource;
+class NetworkQualityEstimator;
 class SpdySession;
 class TransportSecurityState;
 
@@ -78,7 +79,8 @@ class NET_EXPORT SpdySessionPool
       size_t session_max_recv_window_size,
       const spdy::SettingsMap& initial_settings,
       const base::Optional<GreasedHttp2Frame>& greased_http2_frame,
-      SpdySessionPool::TimeFunc time_func);
+      SpdySessionPool::TimeFunc time_func,
+      NetworkQualityEstimator* network_quality_estimator);
   ~SpdySessionPool() override;
 
   // In the functions below, a session is "available" if this pool has
@@ -206,6 +208,11 @@ class NET_EXPORT SpdySessionPool
   // not have a SpdySessionKey.
   void RemoveRequestFromSpdySessionRequestMap(HttpStreamRequest* request);
 
+  void set_network_quality_estimator(
+      NetworkQualityEstimator* network_quality_estimator) {
+    network_quality_estimator_ = network_quality_estimator;
+  }
+
  private:
   friend class SpdySessionPoolPeer;  // For testing.
 
@@ -302,6 +309,8 @@ class NET_EXPORT SpdySessionPool
   TimeFunc time_func_;
   ServerPushDelegate* push_delegate_;
 
+  NetworkQualityEstimator* network_quality_estimator_;
+
   DISALLOW_COPY_AND_ASSIGN(SpdySessionPool);
 };
 

net/spdy/spdy_session_unittest.cc

@@ -31,6 +31,7 @@
 #include "net/log/test_net_log.h"
 #include "net/log/test_net_log_entry.h"
 #include "net/log/test_net_log_util.h"
+#include "net/nqe/network_quality_estimator_test_util.h"
 #include "net/socket/client_socket_pool_manager.h"
 #include "net/socket/socket_tag.h"
 #include "net/socket/socket_test_util.h"
@@ -1025,6 +1026,10 @@ TEST_F(SpdySessionTestWithMockTime, ClientPing) {
   AddSSLSocketData();
 
   CreateNetworkSession();
+  TestNetworkQualityEstimator estimator;
+
+  spdy_session_pool_->set_network_quality_estimator(&estimator);
+
   CreateSpdySession();
 
   base::WeakPtr<SpdyStream> spdy_stream1 =
@@ -1075,6 +1080,8 @@ TEST_F(SpdySessionTestWithMockTime, ClientPing) {
 
   EXPECT_TRUE(data.AllWriteDataConsumed());
   EXPECT_TRUE(data.AllReadDataConsumed());
+
+  EXPECT_LE(1u, estimator.ping_rtt_received_count());
 }
 
 TEST_F(SpdySessionTest, ServerPing) {

tools/metrics/histograms/enums.xml

@@ -36290,6 +36290,7 @@ Called by update_net_trust_anchors.py.-->
   <int value="5" label="HTTP-layer external estimate provider"/>
   <int value="6" label="Transport-layer cached"/>
   <int value="7" label="Transport-layer default"/>
+  <int value="8" label="H2 pings"/>
 </enum>
 
 <enum name="NTPBackgroundCustomizationAvailability">