NQE: Plumb RTT and throughput estimates to the UI thread

This CL makes the RTT and throughput estimates available on the UI
thread. It also makes it possible to add observers on UI thread that
can listen to changes in the RTT/throughput estimations.

BUG=700215
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
TBR=bmcquade@chromium.org, petewil@chromium.org

Review-Url: https://codereview.chromium.org/2717153002
Cr-Commit-Position: refs/heads/master@{#456252}

chrome/browser/net/nqe/ui_network_quality_estimator_service.cc

@@ -80,7 +80,8 @@ void SetNQEOnIOThread(net::NetworkQualitiesPrefsManager* prefs_manager,
 // to the UI service.
 // It is created on the UI thread, but used and deleted on the IO thread.
 class UINetworkQualityEstimatorService::IONetworkQualityObserver
-    : public net::NetworkQualityEstimator::EffectiveConnectionTypeObserver {
+    : public net::NetworkQualityEstimator::EffectiveConnectionTypeObserver,
+      public net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver {
  public:
   explicit IONetworkQualityObserver(
       base::WeakPtr<UINetworkQualityEstimatorService> service)
@@ -90,8 +91,10 @@ class UINetworkQualityEstimatorService::IONetworkQualityObserver
 
   ~IONetworkQualityObserver() override {
     DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
-    if (network_quality_estimator_)
+    if (network_quality_estimator_) {
       network_quality_estimator_->RemoveEffectiveConnectionTypeObserver(this);
+      network_quality_estimator_->RemoveRTTAndThroughputEstimatesObserver(this);
+    }
   }
 
   void InitializeOnIOThread(IOThread* io_thread) {
@@ -103,6 +106,7 @@ class UINetworkQualityEstimatorService::IONetworkQualityObserver
     if (!network_quality_estimator_)
       return;
     network_quality_estimator_->AddEffectiveConnectionTypeObserver(this);
+    network_quality_estimator_->AddRTTAndThroughputEstimatesObserver(this);
   }
 
   // net::NetworkQualityEstimator::EffectiveConnectionTypeObserver
@@ -117,6 +121,20 @@ class UINetworkQualityEstimatorService::IONetworkQualityObserver
             service_, type));
   }
 
+  // net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver
+  // implementation:
+  void OnRTTOrThroughputEstimatesComputed(
+      base::TimeDelta http_rtt,
+      base::TimeDelta transport_rtt,
+      int32_t downstream_throughput_kbps) override {
+    DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+    content::BrowserThread::PostTask(
+        content::BrowserThread::UI, FROM_HERE,
+        base::Bind(&UINetworkQualityEstimatorService::RTTOrThroughputComputed,
+                   service_, http_rtt, transport_rtt,
+                   downstream_throughput_kbps));
+  }
+
  private:
   base::WeakPtr<UINetworkQualityEstimatorService> service_;
   net::NetworkQualityEstimator* network_quality_estimator_;
@@ -126,7 +144,11 @@ class UINetworkQualityEstimatorService::IONetworkQualityObserver
 
 UINetworkQualityEstimatorService::UINetworkQualityEstimatorService(
     Profile* profile)
-    : type_(net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN), weak_factory_(this) {
+    : type_(net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN),
+      http_rtt_(base::TimeDelta::FromMilliseconds(-1)),
+      transport_rtt_(base::TimeDelta::FromMilliseconds(-1)),
+      downstream_throughput_kbps_(-1),
+      weak_factory_(this) {
   DCHECK(profile);
   // If this is running in a context without an IOThread, don't try to create
   // the IO object.
@@ -182,6 +204,21 @@ void UINetworkQualityEstimatorService::EffectiveConnectionTypeChanged(
     observer.OnEffectiveConnectionTypeChanged(type);
 }
 
+void UINetworkQualityEstimatorService::RTTOrThroughputComputed(
+    base::TimeDelta http_rtt,
+    base::TimeDelta transport_rtt,
+    int32_t downstream_throughput_kbps) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  http_rtt_ = http_rtt;
+  transport_rtt_ = transport_rtt;
+  downstream_throughput_kbps_ = downstream_throughput_kbps;
+
+  for (auto& observer : rtt_throughput_observer_list_) {
+    observer.OnRTTOrThroughputEstimatesComputed(http_rtt, transport_rtt,
+                                                downstream_throughput_kbps);
+  }
+}
+
 void UINetworkQualityEstimatorService::AddEffectiveConnectionTypeObserver(
     net::NetworkQualityEstimator::EffectiveConnectionTypeObserver* observer) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
@@ -202,6 +239,28 @@ void UINetworkQualityEstimatorService::RemoveEffectiveConnectionTypeObserver(
   effective_connection_type_observer_list_.RemoveObserver(observer);
 }
 
+void UINetworkQualityEstimatorService::AddRTTAndThroughputEstimatesObserver(
+    net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver* observer) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  rtt_throughput_observer_list_.AddObserver(observer);
+
+  // Notify the |observer| on the next message pump since |observer| may not
+  // be completely set up for receiving the callbacks.
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::Bind(&UINetworkQualityEstimatorService::
+                     NotifyRTTAndThroughputObserverIfPresent,
+                 weak_factory_.GetWeakPtr(), observer));
+}
+
+// Removes |observer| from the list of RTT and throughput estimate observers.
+// Must be called on the IO thread.
+void UINetworkQualityEstimatorService::RemoveRTTAndThroughputEstimatesObserver(
+    net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver* observer) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  rtt_throughput_observer_list_.RemoveObserver(observer);
+}
+
 void UINetworkQualityEstimatorService::SetEffectiveConnectionTypeForTesting(
     net::EffectiveConnectionType type) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
@@ -234,6 +293,17 @@ void UINetworkQualityEstimatorService::
   observer->OnEffectiveConnectionTypeChanged(type_);
 }
 
+void UINetworkQualityEstimatorService::NotifyRTTAndThroughputObserverIfPresent(
+    net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver* observer)
+    const {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  if (!rtt_throughput_observer_list_.HasObserver(observer))
+    return;
+  observer->OnRTTOrThroughputEstimatesComputed(http_rtt_, transport_rtt_,
+                                               downstream_throughput_kbps_);
+}
+
 // static
 void UINetworkQualityEstimatorService::RegisterProfilePrefs(
     PrefRegistrySimple* registry) {

chrome/browser/net/nqe/ui_network_quality_estimator_service.h

@@ -47,6 +47,20 @@ class UINetworkQualityEstimatorService
       net::NetworkQualityEstimator::EffectiveConnectionTypeObserver* observer)
       override;
 
+  // Must be called on the UI thread. |observer| will be notified on the UI
+  // thread. |observer| would be notified of the changes in the HTTP RTT,
+  // transport RTT or throughput. |observer| would be notified of the current
+  // values in the next message pump.
+  void AddRTTAndThroughputEstimatesObserver(
+      net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver* observer)
+      override;
+
+  // Removes |observer| from the list of RTT and throughput estimate observers.
+  // Must be called on the UI thread.
+  void RemoveRTTAndThroughputEstimatesObserver(
+      net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver* observer)
+      override;
+
   // Registers the profile-specific network quality estimator prefs.
   static void RegisterProfilePrefs(PrefRegistrySimple* registry);
 
@@ -72,6 +86,12 @@ class UINetworkQualityEstimatorService
       net::NetworkQualityEstimator::EffectiveConnectionTypeObserver* observer)
       const;
 
+  // Notifies |observer| of the current effective connection type if |observer|
+  // is still registered as an observer.
+  void NotifyRTTAndThroughputObserverIfPresent(
+      net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver* observer)
+      const;
+
   // KeyedService implementation:
   void Shutdown() override;
 
@@ -81,9 +101,20 @@ class UINetworkQualityEstimatorService
   // reported by NetworkchangeNotifier::GetConnectionType.
   void EffectiveConnectionTypeChanged(net::EffectiveConnectionType type);
 
+  // Called when the estimated HTTP RTT, estimated transport RTT or estimated
+  // downstream throughput is computed.
+  void RTTOrThroughputComputed(base::TimeDelta http_rtt,
+                               base::TimeDelta transport_rtt,
+                               int32_t downstream_throughput_kbps);
+
   // The current EffectiveConnectionType.
   net::EffectiveConnectionType type_;
 
+  // Current network quality metrics.
+  base::TimeDelta http_rtt_;
+  base::TimeDelta transport_rtt_;
+  int32_t downstream_throughput_kbps_;
+
   // IO thread based observer that is owned by this service. Created on the UI
   // thread, but used and deleted on the IO thread.
   std::unique_ptr<IONetworkQualityObserver> io_observer_;
@@ -93,6 +124,11 @@ class UINetworkQualityEstimatorService
       net::NetworkQualityEstimator::EffectiveConnectionTypeObserver>
       effective_connection_type_observer_list_;
 
+  // Observer list for changes in RTT or throughput values.
+  base::ObserverList<
+      net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver>
+      rtt_throughput_observer_list_;
+
   // Prefs manager that is owned by this service. Created on the UI thread, but
   // used and deleted on the IO thread.
   std::unique_ptr<net::NetworkQualitiesPrefsManager> prefs_manager_;

chrome/browser/net/nqe/ui_network_quality_estimator_service_browsertest.cc

@@ -6,6 +6,7 @@
 #include <string>
 
 #include "base/bind.h"
+#include "base/macros.h"
 #include "base/metrics/field_trial.h"
 #include "base/run_loop.h"
 #include "base/test/histogram_tester.h"
@@ -51,6 +52,42 @@ class TestEffectiveConnectionTypeObserver
 
  private:
   net::EffectiveConnectionType effective_connection_type_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestEffectiveConnectionTypeObserver);
+};
+
+class TestRTTAndThroughputEstimatesObserver
+    : public net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver {
+ public:
+  TestRTTAndThroughputEstimatesObserver()
+      : http_rtt_(base::TimeDelta::FromMilliseconds(-1)),
+        transport_rtt_(base::TimeDelta::FromMilliseconds(-1)),
+        downstream_throughput_kbps_(-1) {}
+  ~TestRTTAndThroughputEstimatesObserver() override {}
+
+  // net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver
+  // implementation:
+  void OnRTTOrThroughputEstimatesComputed(
+      base::TimeDelta http_rtt,
+      base::TimeDelta transport_rtt,
+      int32_t downstream_throughput_kbps) override {
+    http_rtt_ = http_rtt;
+    transport_rtt_ = transport_rtt;
+    downstream_throughput_kbps_ = downstream_throughput_kbps;
+  }
+
+  base::TimeDelta http_rtt() const { return http_rtt_; }
+  base::TimeDelta transport_rtt() const { return transport_rtt_; }
+  int32_t downstream_throughput_kbps() const {
+    return downstream_throughput_kbps_;
+  }
+
+ private:
+  base::TimeDelta http_rtt_;
+  base::TimeDelta transport_rtt_;
+  int32_t downstream_throughput_kbps_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestRTTAndThroughputEstimatesObserver);
 };
 
 class UINetworkQualityEstimatorServiceBrowserTest
@@ -205,6 +242,50 @@ IN_PROC_BROWSER_TEST_F(UINetworkQualityEstimatorServiceBrowserTest,
             nqe_observer_3.effective_connection_type());
 }
 
+IN_PROC_BROWSER_TEST_F(UINetworkQualityEstimatorServiceBrowserTest,
+                       VerifyRTTs) {
+  // Verifies that NQE notifying RTTAndThroughputEstimatesObserver causes the
+  // UINetworkQualityEstimatorService to receive an update.
+  Profile* profile = ProfileManager::GetActiveUserProfile();
+  UINetworkQualityEstimatorService* nqe_service =
+      UINetworkQualityEstimatorServiceFactory::GetForProfile(profile);
+  TestRTTAndThroughputEstimatesObserver nqe_observer;
+  nqe_service->AddRTTAndThroughputEstimatesObserver(&nqe_observer);
+
+  base::TimeDelta rtt_1 = base::TimeDelta::FromMilliseconds(100);
+
+  nqe_test_util::OverrideRTTsAndWait(rtt_1);
+  EXPECT_EQ(rtt_1, nqe_observer.http_rtt());
+
+  base::TimeDelta rtt_2 = base::TimeDelta::FromMilliseconds(200);
+
+  nqe_test_util::OverrideRTTsAndWait(rtt_2);
+  EXPECT_EQ(rtt_2, nqe_observer.http_rtt());
+
+  nqe_service->RemoveRTTAndThroughputEstimatesObserver(&nqe_observer);
+
+  base::TimeDelta rtt_3 = base::TimeDelta::FromMilliseconds(300);
+
+  nqe_test_util::OverrideRTTsAndWait(rtt_3);
+  EXPECT_EQ(rtt_2, nqe_observer.http_rtt());
+
+  // Observer should be notified on addition.
+  TestRTTAndThroughputEstimatesObserver nqe_observer_2;
+  nqe_service->AddRTTAndThroughputEstimatesObserver(&nqe_observer_2);
+  EXPECT_GT(0, nqe_observer_2.http_rtt().InMilliseconds());
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(rtt_3, nqe_observer_2.http_rtt());
+
+  // |nqe_observer_3| should be not notified since it unregisters before the
+  // message loop is run.
+  TestRTTAndThroughputEstimatesObserver nqe_observer_3;
+  nqe_service->AddRTTAndThroughputEstimatesObserver(&nqe_observer_3);
+  EXPECT_GT(0, nqe_observer_3.http_rtt().InMilliseconds());
+  nqe_service->RemoveRTTAndThroughputEstimatesObserver(&nqe_observer_3);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_GT(0, nqe_observer_3.http_rtt().InMilliseconds());
+}
+
 // Verify that prefs are writen and read correctly.
 IN_PROC_BROWSER_TEST_F(UINetworkQualityEstimatorServiceBrowserTest,
                        WritingReadingToPrefsEnabled) {

chrome/browser/net/nqe/ui_network_quality_estimator_service_test_util.cc

@@ -28,6 +28,16 @@ void OverrideEffectiveConnectionTypeOnIO(net::EffectiveConnectionType type,
   network_quality_estimator->ReportEffectiveConnectionTypeForTesting(type);
 }
 
+void OverrideRTTsAndWaitOnIO(base::TimeDelta rtt, IOThread* io_thread) {
+  if (!io_thread->globals()->network_quality_estimator)
+    return;
+  net::NetworkQualityEstimator* network_quality_estimator =
+      io_thread->globals()->network_quality_estimator.get();
+  if (!network_quality_estimator)
+    return;
+  network_quality_estimator->ReportRTTsAndThroughputForTesting(rtt, rtt, -1);
+}
+
 }  // namespace
 
 void OverrideEffectiveConnectionTypeAndWait(net::EffectiveConnectionType type) {
@@ -43,4 +53,16 @@ void OverrideEffectiveConnectionTypeAndWait(net::EffectiveConnectionType type) {
   run_loop.Run();
 }
 
+void OverrideRTTsAndWait(base::TimeDelta rtt) {
+  // Block |run_loop| until OverrideRTTsAndWaitOnIO has completed.
+  // Any UI tasks posted by calling OverrideRTTsAndWaitOnIO will complete before
+  // the reply unblocks |run_loop|.
+  base::RunLoop run_loop;
+  content::BrowserThread::PostTaskAndReply(
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&OverrideRTTsAndWaitOnIO, rtt, g_browser_process->io_thread()),
+      run_loop.QuitClosure());
+  run_loop.Run();
+}
+
 }  // namespace nqe_test_util

chrome/browser/net/nqe/ui_network_quality_estimator_service_test_util.h

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_NET_NQE_UI_NETWORK_QUALITY_ESTIMATOR_SERVICE_TEST_UTIL_H_
 #define CHROME_BROWSER_NET_NQE_UI_NETWORK_QUALITY_ESTIMATOR_SERVICE_TEST_UTIL_H_
 
+#include "base/time/time.h"
 #include "net/nqe/effective_connection_type.h"
 
 namespace nqe_test_util {
@@ -15,6 +16,12 @@ namespace nqe_test_util {
 // replies.
 void OverrideEffectiveConnectionTypeAndWait(net::EffectiveConnectionType type);
 
+// Forces NetworkQualityEstimator to report |rtt| to all its
+// RTTAndThroughputEstimatesObservers.
+// This blocks execution on the calling thread until the IO task runs and
+// replies.
+void OverrideRTTsAndWait(base::TimeDelta rtt);
+
 }  // namespace nqe_test_util
 
 #endif  // CHROME_BROWSER_NET_NQE_UI_NETWORK_QUALITY_ESTIMATOR_SERVICE_TEST_UTIL_H_

chrome/browser/page_load_metrics/observers/ukm_page_load_metrics_observer_unittest.cc

@@ -34,6 +34,12 @@ class MockNetworkQualityProvider
   MOCK_METHOD1(
       RemoveEffectiveConnectionTypeObserver,
       void(net::NetworkQualityEstimator::EffectiveConnectionTypeObserver*));
+  MOCK_METHOD1(
+      AddRTTAndThroughputEstimatesObserver,
+      void(net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver*));
+  MOCK_METHOD1(
+      RemoveRTTAndThroughputEstimatesObserver,
+      void(net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver*));
 };
 
 }  // namespace

components/offline_pages/core/background/network_quality_provider_stub.cc

@@ -36,6 +36,14 @@ void NetworkQualityProviderStub::AddEffectiveConnectionTypeObserver(
 void NetworkQualityProviderStub::RemoveEffectiveConnectionTypeObserver(
     net::NetworkQualityEstimator::EffectiveConnectionTypeObserver* observer) {}
 
+void NetworkQualityProviderStub::AddRTTAndThroughputEstimatesObserver(
+    net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver* observer) {
+}
+
+void NetworkQualityProviderStub::RemoveRTTAndThroughputEstimatesObserver(
+    net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver* observer) {
+}
+
 net::EffectiveConnectionType
 NetworkQualityProviderStub::GetEffectiveConnectionType() const {
   return connection_type_;

components/offline_pages/core/background/network_quality_provider_stub.h

@@ -35,6 +35,14 @@ class NetworkQualityProviderStub
       net::NetworkQualityEstimator::EffectiveConnectionTypeObserver* observer)
       override;
 
+  void AddRTTAndThroughputEstimatesObserver(
+      net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver* observer)
+      override;
+
+  void RemoveRTTAndThroughputEstimatesObserver(
+      net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver* observer)
+      override;
+
   void SetEffectiveConnectionTypeForTest(net::EffectiveConnectionType type) {
     connection_type_ = type;
   }

net/nqe/network_quality_estimator.cc

@@ -808,6 +808,17 @@ void NetworkQualityEstimator::ReportEffectiveConnectionTypeForTesting(
                                   effective_connection_type));
 }
 
+void NetworkQualityEstimator::ReportRTTsAndThroughputForTesting(
+    base::TimeDelta http_rtt,
+    base::TimeDelta transport_rtt,
+    int32_t downstream_throughput_kbps) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  for (auto& observer : rtt_and_throughput_estimates_observer_list_)
+    observer.OnRTTOrThroughputEstimatesComputed(http_rtt, transport_rtt,
+                                                downstream_throughput_kbps);
+}
+
 bool NetworkQualityEstimator::RequestProvidesRTTObservation(
     const URLRequest& request) const {
   DCHECK(thread_checker_.CalledOnValidThread());

net/nqe/network_quality_estimator.h

@@ -160,6 +160,8 @@ class NET_EXPORT NetworkQualityEstimator
     // Returns the current effective connection type.
     virtual EffectiveConnectionType GetEffectiveConnectionType() const = 0;
 
+    virtual ~NetworkQualityProvider() {}
+
     // Adds |observer| to a list of effective connection type observers.
     virtual void AddEffectiveConnectionTypeObserver(
         EffectiveConnectionTypeObserver* observer) = 0;
@@ -168,7 +170,16 @@ class NET_EXPORT NetworkQualityEstimator
     virtual void RemoveEffectiveConnectionTypeObserver(
         EffectiveConnectionTypeObserver* observer) = 0;
 
-    virtual ~NetworkQualityProvider() {}
+    // Adds |observer| to the list of RTT and throughput estimate observers.
+    // |observer| would be notified of the current RTT and throughput estimates
+    // in the next message pump.
+    virtual void AddRTTAndThroughputEstimatesObserver(
+        RTTAndThroughputEstimatesObserver* observer) = 0;
+
+    // Removes |observer| from the list of RTT and throughput estimate
+    // observers.
+    virtual void RemoveRTTAndThroughputEstimatesObserver(
+        RTTAndThroughputEstimatesObserver* observer) = 0;
 
    protected:
     NetworkQualityProvider() {}
@@ -296,6 +307,11 @@ class NET_EXPORT NetworkQualityEstimator
   void ReportEffectiveConnectionTypeForTesting(
       EffectiveConnectionType effective_connection_type);
 
+  // Reports the RTTs and throughput to all RTTAndThroughputEstimatesObservers.
+  void ReportRTTsAndThroughputForTesting(base::TimeDelta http_rtt,
+                                         base::TimeDelta transport_rtt,
+                                         int32_t downstream_throughput_kbps);
+
   // Adds and removes |observer| from the list of cache observers.
   void AddNetworkQualitiesCacheObserver(
       nqe::internal::NetworkQualityStore::NetworkQualitiesCacheObserver*