From 618e25ceda758e085c89f50458571a508c23edb3 Mon Sep 17 00:00:00 2001 From: tbansal Date: Thu, 20 Oct 2016 15:06:27 -0700 Subject: [PATCH] Expose RTT and throughput estimates from Cronet The estimates are computed by the network quality estimator (NQE) and are pushed to CronetUrlRequestContext. Embedders can query for the estimates by calling appropriate methods on CronetEngine. This CL is a reland of https://codereview.chromium.org/2417643007/. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.android:android_cronet BUG=654498 Review-Url: https://chromiumcodereview.appspot.com/2439833002 Cr-Commit-Position: refs/heads/master@{#426614} --- components/cronet/android/BUILD.gn | 13 +++- .../src/org/chromium/net/CronetEngine.java | 36 ++++++++++ .../org/chromium/net/JavaCronetEngine.java | 15 +++++ .../cronet_url_request_context_adapter.cc | 21 ++++++ .../cronet_url_request_context_adapter.h | 8 +++ .../net/impl/CronetUrlRequestContext.java | 66 ++++++++++++++++++- .../net/CronetUrlRequestContextTest.java | 21 +++++- net/nqe/network_quality.cc | 4 +- net/nqe/network_quality.h | 14 +++- net/nqe/network_quality_estimator.cc | 15 ++++- 10 files changed, 203 insertions(+), 10 deletions(-) diff --git a/components/cronet/android/BUILD.gn b/components/cronet/android/BUILD.gn index 6b1dc41f7ff6d9..af149a5dd26eaa 100644 --- a/components/cronet/android/BUILD.gn +++ b/components/cronet/android/BUILD.gn @@ -34,6 +34,12 @@ java_cpp_enum("effective_connection_type_java") { ] } +java_cpp_enum("rtt_throughput_values_java") { + sources = [ + "//net/nqe/network_quality.h", + ] +} + java_cpp_enum("chromium_url_request_java") { sources = [ "chromium_url_request.h", @@ -66,6 +72,7 @@ android_library("cronet_javadoc_classpath") { ] srcjar_deps = [ ":effective_connection_type_java", + ":rtt_throughput_values_java", ":url_request_error_java", ] } @@ -308,6 +315,7 @@ android_library("cronet_api") { srcjar_deps = [ ":cronet_api_version_srcjar", ":effective_connection_type_java", + ":rtt_throughput_values_java", ":http_cache_type_java", ":url_request_error_java", ":load_states_list", @@ -864,7 +872,10 @@ jar_src("jar_cronet_api_source") { src_search_dirs = [ "api/src" ] # Include generated Java files which should be a part of the API. - srcjar_deps = [ ":effective_connection_type_java" ] + srcjar_deps = [ + ":effective_connection_type_java", + ":rtt_throughput_values_java", + ] source_deps = [ ":cronet_api" ] jar_path = "$_package_dir/cronet_api-src.jar" } diff --git a/components/cronet/android/api/src/org/chromium/net/CronetEngine.java b/components/cronet/android/api/src/org/chromium/net/CronetEngine.java index e386b670818795..d505e04b9b0cab 100644 --- a/components/cronet/android/api/src/org/chromium/net/CronetEngine.java +++ b/components/cronet/android/api/src/org/chromium/net/CronetEngine.java @@ -992,6 +992,42 @@ public abstract BidirectionalStream createBidirectionalStream(String url, */ public abstract int getEffectiveConnectionType(); + /** + * Returns the HTTP RTT estimate (in milliseconds) computed by the network + * quality estimator. Set to + * {@link RttThroughputValues.INVALID_RTT_THROUGHPUT} if a valid value + * is unavailable. This must be called after + * {@link #enableNetworkQualityEstimator}, and will throw an + * exception otherwise. + * @hide as it's a prototype. + * @return Estimate of the HTTP RTT in milliseconds. + */ + public abstract int getHttpRttMs(); + + /** + * Returns the transport RTT estimate (in milliseconds) computed by the + * network quality estimator. Set to + * {@link RttThroughputValues.INVALID_RTT_THROUGHPUT} if a valid value is + * unavailable. This must be called after + * {@link #enableNetworkQualityEstimator}, and will throw an + * exception otherwise. + * @hide as it's a prototype. + * @return Estimate of the transport RTT in milliseconds. + */ + public abstract int getTransportRttMs(); + + /** + * Returns the downstream throughput estimate (in kilobits per second) + * computed by the network quality estimator. Set to + * {@link RttThroughputValues.INVALID_RTT_THROUGHPUT} if a valid value is + * unavailable. This must be called after + * {@link #enableNetworkQualityEstimator}, and will + * throw an exception otherwise. + * @hide as it's a prototype. + * @return Estimate of the downstream throughput in kilobits per second. + */ + public abstract int getDownstreamThroughputKbps(); + /** * Configures the network quality estimator for testing. This must be called * before round trip time and throughput listeners are added, and after the diff --git a/components/cronet/android/api/src/org/chromium/net/JavaCronetEngine.java b/components/cronet/android/api/src/org/chromium/net/JavaCronetEngine.java index ab19536e7fc181..71b2015b6bf817 100644 --- a/components/cronet/android/api/src/org/chromium/net/JavaCronetEngine.java +++ b/components/cronet/android/api/src/org/chromium/net/JavaCronetEngine.java @@ -112,6 +112,21 @@ public int getEffectiveConnectionType() { return EffectiveConnectionType.TYPE_UNKNOWN; } + @Override + public int getHttpRttMs() { + return RttThroughputValues.INVALID_RTT_THROUGHPUT; + } + + @Override + public int getTransportRttMs() { + return RttThroughputValues.INVALID_RTT_THROUGHPUT; + } + + @Override + public int getDownstreamThroughputKbps() { + return RttThroughputValues.INVALID_RTT_THROUGHPUT; + } + @Override public void configureNetworkQualityEstimatorForTesting( boolean useLocalHostRequests, boolean useSmallerResponses) {} diff --git a/components/cronet/android/cronet_url_request_context_adapter.cc b/components/cronet/android/cronet_url_request_context_adapter.cc index ffe6089a2ec9ba..112762c17d925b 100644 --- a/components/cronet/android/cronet_url_request_context_adapter.cc +++ b/components/cronet/android/cronet_url_request_context_adapter.cc @@ -427,6 +427,7 @@ CronetURLRequestContextAdapter::~CronetURLRequestContextAdapter() { network_quality_estimator_->RemoveRTTObserver(this); network_quality_estimator_->RemoveThroughputObserver(this); network_quality_estimator_->RemoveEffectiveConnectionTypeObserver(this); + network_quality_estimator_->RemoveRTTAndThroughputEstimatesObserver(this); } // Stop NetLog observer if there is one. @@ -626,6 +627,7 @@ void CronetURLRequestContextAdapter::InitializeOnNetworkThread( context_builder.set_socket_performance_watcher_factory( network_quality_estimator_->GetSocketPerformanceWatcherFactory()); network_quality_estimator_->AddEffectiveConnectionTypeObserver(this); + network_quality_estimator_->AddRTTAndThroughputEstimatesObserver(this); } context_ = context_builder.Build(); @@ -871,6 +873,25 @@ void CronetURLRequestContextAdapter::OnEffectiveConnectionTypeChanged( effective_connection_type); } +void CronetURLRequestContextAdapter::OnRTTOrThroughputEstimatesComputed( + base::TimeDelta http_rtt, + base::TimeDelta transport_rtt, + int32_t downstream_throughput_kbps) { + DCHECK(GetNetworkTaskRunner()->BelongsToCurrentThread()); + + int32_t http_rtt_ms = http_rtt.InMilliseconds() <= INT32_MAX + ? static_cast(http_rtt.InMilliseconds()) + : INT32_MAX; + int32_t transport_rtt_ms = + transport_rtt.InMilliseconds() <= INT32_MAX + ? static_cast(transport_rtt.InMilliseconds()) + : INT32_MAX; + + Java_CronetUrlRequestContext_onRTTOrThroughputEstimatesComputed( + base::android::AttachCurrentThread(), jcronet_url_request_context_.obj(), + http_rtt_ms, transport_rtt_ms, downstream_throughput_kbps); +} + void CronetURLRequestContextAdapter::OnRTTObservation( int32_t rtt_ms, const base::TimeTicks& timestamp, diff --git a/components/cronet/android/cronet_url_request_context_adapter.h b/components/cronet/android/cronet_url_request_context_adapter.h index 647cc0630742fe..d880b76d79ef36 100644 --- a/components/cronet/android/cronet_url_request_context_adapter.h +++ b/components/cronet/android/cronet_url_request_context_adapter.h @@ -52,6 +52,7 @@ bool CronetUrlRequestContextAdapterRegisterJni(JNIEnv* env); // Adapter between Java CronetUrlRequestContext and net::URLRequestContext. class CronetURLRequestContextAdapter : public net::NetworkQualityEstimator::EffectiveConnectionTypeObserver, + public net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver, public net::NetworkQualityEstimator::RTTObserver, public net::NetworkQualityEstimator::ThroughputObserver { public: @@ -166,6 +167,13 @@ class CronetURLRequestContextAdapter void OnEffectiveConnectionTypeChanged( net::EffectiveConnectionType effective_connection_type) override; + // net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver + // implementation. + void OnRTTOrThroughputEstimatesComputed( + base::TimeDelta http_rtt, + base::TimeDelta transport_rtt, + int32_t downstream_throughput_kbps) override; + // net::NetworkQualityEstimator::RTTObserver implementation. void OnRTTObservation(int32_t rtt_ms, const base::TimeTicks& timestamp, diff --git a/components/cronet/android/java/src/org/chromium/net/impl/CronetUrlRequestContext.java b/components/cronet/android/java/src/org/chromium/net/impl/CronetUrlRequestContext.java index 819347d57a1cad..6b2022fa9bda9f 100644 --- a/components/cronet/android/java/src/org/chromium/net/impl/CronetUrlRequestContext.java +++ b/components/cronet/android/java/src/org/chromium/net/impl/CronetUrlRequestContext.java @@ -24,6 +24,7 @@ import org.chromium.net.NetworkQualityRttListener; import org.chromium.net.NetworkQualityThroughputListener; import org.chromium.net.RequestFinishedInfo; +import org.chromium.net.RttThroughputValues; import org.chromium.net.UrlRequest; import org.chromium.net.urlconnection.CronetHttpURLConnection; import org.chromium.net.urlconnection.CronetURLStreamHandlerFactory; @@ -94,6 +95,27 @@ public class CronetUrlRequestContext extends CronetEngine { @GuardedBy("mNetworkQualityLock") private int mEffectiveConnectionType = EffectiveConnectionType.TYPE_UNKNOWN; + /** + * Current estimate of the HTTP RTT (in milliseconds) computed by the + * network quality estimator. + */ + @GuardedBy("mNetworkQualityLock") + private int mHttpRttMs = RttThroughputValues.INVALID_RTT_THROUGHPUT; + + /** + * Current estimate of the transport RTT (in milliseconds) computed by the + * network quality estimator. + */ + @GuardedBy("mNetworkQualityLock") + private int mTransportRttMs = RttThroughputValues.INVALID_RTT_THROUGHPUT; + + /** + * Current estimate of the downstream throughput (in kilobits per second) + * computed by the network quality estimator. + */ + @GuardedBy("mNetworkQualityLock") + private int mDownstreamThroughputKbps = RttThroughputValues.INVALID_RTT_THROUGHPUT; + @GuardedBy("mNetworkQualityLock") private final ObserverList mRttListenerList = new ObserverList(); @@ -327,13 +349,40 @@ public int getEffectiveConnectionType() { throw new IllegalStateException("Network quality estimator must be enabled"); } synchronized (mNetworkQualityLock) { - synchronized (mLock) { - checkHaveAdapter(); - } return mEffectiveConnectionType; } } + @Override + public int getHttpRttMs() { + if (!mNetworkQualityEstimatorEnabled) { + throw new IllegalStateException("Network quality estimator must be enabled"); + } + synchronized (mNetworkQualityLock) { + return mHttpRttMs; + } + } + + @Override + public int getTransportRttMs() { + if (!mNetworkQualityEstimatorEnabled) { + throw new IllegalStateException("Network quality estimator must be enabled"); + } + synchronized (mNetworkQualityLock) { + return mTransportRttMs; + } + } + + @Override + public int getDownstreamThroughputKbps() { + if (!mNetworkQualityEstimatorEnabled) { + throw new IllegalStateException("Network quality estimator must be enabled"); + } + synchronized (mNetworkQualityLock) { + return mDownstreamThroughputKbps; + } + } + @VisibleForTesting @Override public void configureNetworkQualityEstimatorForTesting( @@ -525,6 +574,17 @@ private void onEffectiveConnectionTypeChanged(int effectiveConnectionType) { } } + @SuppressWarnings("unused") + @CalledByNative + private void onRTTOrThroughputEstimatesComputed( + final int httpRttMs, final int transportRttMs, final int downstreamThroughputKbps) { + synchronized (mNetworkQualityLock) { + mHttpRttMs = httpRttMs; + mTransportRttMs = transportRttMs; + mDownstreamThroughputKbps = downstreamThroughputKbps; + } + } + @SuppressWarnings("unused") @CalledByNative private void onRttObservation(final int rttMs, final long whenMs, final int source) { diff --git a/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestContextTest.java b/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestContextTest.java index 1e433d90b42169..584d519cac91e7 100644 --- a/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestContextTest.java +++ b/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestContextTest.java @@ -12,6 +12,8 @@ import android.os.StrictMode; import android.test.suitebuilder.annotation.SmallTest; +import org.json.JSONObject; + import org.chromium.base.FileUtils; import org.chromium.base.PathUtils; import org.chromium.base.annotations.JNINamespace; @@ -21,7 +23,6 @@ import org.chromium.net.impl.CronetLibraryLoader; import org.chromium.net.impl.CronetUrlRequestContext; import org.chromium.net.test.EmbeddedTestServer; -import org.json.JSONObject; import java.io.BufferedReader; import java.io.File; @@ -280,6 +281,8 @@ public void testRealTimeNetworkQualityObservationsListenerRemoved() throws Excep @Feature({"Cronet"}) public void testRealTimeNetworkQualityObservationsQuicDisabled() throws Exception { CronetEngine.Builder mCronetEngineBuilder = new CronetEngine.Builder(getContext()); + assert RttThroughputValues.INVALID_RTT_THROUGHPUT < 0; + Executor listenersExecutor = Executors.newSingleThreadExecutor(new ExecutorThreadFactory()); ConditionVariable waitForThroughput = new ConditionVariable(); TestNetworkQualityRttListener rttListener = @@ -330,6 +333,22 @@ public void testRealTimeNetworkQualityObservationsQuicDisabled() throws Exceptio assertTrue(testFramework.mCronetEngine.getEffectiveConnectionType() != EffectiveConnectionType.TYPE_UNKNOWN); + // Verify that the HTTP RTT, transport RTT and downstream throughput + // estimates are available. + if (testFramework.mCronetEngine.getEffectiveConnectionType() + != EffectiveConnectionType.TYPE_OFFLINE) { + assertTrue(testFramework.mCronetEngine.getHttpRttMs() > 0); + assertTrue(testFramework.mCronetEngine.getTransportRttMs() > 0); + assertTrue(testFramework.mCronetEngine.getDownstreamThroughputKbps() > 0); + } else { + assertEquals(RttThroughputValues.INVALID_RTT_THROUGHPUT, + testFramework.mCronetEngine.getHttpRttMs()); + assertEquals(RttThroughputValues.INVALID_RTT_THROUGHPUT, + testFramework.mCronetEngine.getTransportRttMs()); + assertEquals(RttThroughputValues.INVALID_RTT_THROUGHPUT, + testFramework.mCronetEngine.getDownstreamThroughputKbps()); + } + testFramework.mCronetEngine.shutdown(); } diff --git a/net/nqe/network_quality.cc b/net/nqe/network_quality.cc index 6831dd4b2135ed..6c572dc917d9b3 100644 --- a/net/nqe/network_quality.cc +++ b/net/nqe/network_quality.cc @@ -9,7 +9,7 @@ namespace nqe { namespace internal { base::TimeDelta InvalidRTT() { - return base::TimeDelta::Max(); + return base::TimeDelta::FromMilliseconds(INVALID_RTT_THROUGHPUT); } NetworkQuality::NetworkQuality() @@ -21,7 +21,7 @@ NetworkQuality::NetworkQuality(const base::TimeDelta& http_rtt, : http_rtt_(http_rtt), transport_rtt_(transport_rtt), downstream_throughput_kbps_(downstream_throughput_kbps) { - DCHECK_GE(downstream_throughput_kbps_, 0); + DCHECK_GE(downstream_throughput_kbps_, kInvalidThroughput); } NetworkQuality::NetworkQuality(const NetworkQuality& other) diff --git a/net/nqe/network_quality.h b/net/nqe/network_quality.h index 457c71e88878ec..9a7dbe460c4cf7 100644 --- a/net/nqe/network_quality.h +++ b/net/nqe/network_quality.h @@ -16,14 +16,26 @@ namespace net { namespace nqe { namespace internal { +// RTT and throughput values are set to |INVALID_RTT_THROUGHPUT| if a valid +// value is unavailable. +// GENERATED_JAVA_ENUM_PACKAGE: org.chromium.net +enum RttThroughputValues { + // Invalid value. + INVALID_RTT_THROUGHPUT = -1, +}; + // Returns the RTT value to be used when the valid RTT is unavailable. Readers // should discard RTT if it is set to the value returned by |InvalidRTT()|. +// TODO(tbansal): Remove this method, and replace all calls by +// |INVALID_RTT_THROUGHPUT|. NET_EXPORT_PRIVATE base::TimeDelta InvalidRTT(); // Throughput is set to |kInvalidThroughput| if a valid value is // unavailable. Readers should discard throughput value if it is set to // |kInvalidThroughput|. -const int32_t kInvalidThroughput = 0; +// TODO(tbansal): Remove this variable, and replace all calls by +// |INVALID_RTT_THROUGHPUT|. +const int32_t kInvalidThroughput = INVALID_RTT_THROUGHPUT; // NetworkQuality is used to cache the quality of a network connection. class NET_EXPORT_PRIVATE NetworkQuality { diff --git a/net/nqe/network_quality_estimator.cc b/net/nqe/network_quality_estimator.cc index 41947a4f325297..136591d8ac0a0f 100644 --- a/net/nqe/network_quality_estimator.cc +++ b/net/nqe/network_quality_estimator.cc @@ -409,6 +409,14 @@ NetworkQualityEstimator::NetworkQualityEstimator( weak_ptr_factory_(this) { static_assert(kDefaultHalfLifeSeconds > 0, "Default half life duration must be > 0"); + static_assert(kMinimumRTTVariationParameterMsec > 0 && + kMinimumRTTVariationParameterMsec > + nqe::internal::INVALID_RTT_THROUGHPUT, + "kMinimumRTTVariationParameterMsec is set incorrectly"); + static_assert(kMinimumThroughputVariationParameterKbps > 0 && + kMinimumThroughputVariationParameterKbps > + nqe::internal::kInvalidThroughput, + "kMinimumThroughputVariationParameterKbps is set incorrectly"); // None of the algorithms can have an empty name. DCHECK(algorithm_name_to_enum_.end() == algorithm_name_to_enum_.find(std::string())); @@ -714,7 +722,8 @@ void NetworkQualityEstimator::NotifyHeadersReceived(const URLRequest& request) { base::TimeDelta observed_http_rtt = load_timing_info.receive_headers_end - load_timing_info.send_start; DCHECK_GE(observed_http_rtt, base::TimeDelta()); - if (observed_http_rtt < peak_network_quality_.http_rtt()) { + if (observed_http_rtt < peak_network_quality_.http_rtt() || + peak_network_quality_.http_rtt() == nqe::internal::InvalidRTT()) { peak_network_quality_ = nqe::internal::NetworkQuality( observed_http_rtt, peak_network_quality_.transport_rtt(), peak_network_quality_.downstream_throughput_kbps()); @@ -1703,7 +1712,9 @@ void NetworkQualityEstimator::OnNewThroughputObservationAvailable( DCHECK_NE(nqe::internal::kInvalidThroughput, downstream_kbps); - if (downstream_kbps > peak_network_quality_.downstream_throughput_kbps()) { + if (downstream_kbps > peak_network_quality_.downstream_throughput_kbps() || + peak_network_quality_.downstream_throughput_kbps() == + nqe::internal::kInvalidThroughput) { peak_network_quality_ = nqe::internal::NetworkQuality( peak_network_quality_.http_rtt(), peak_network_quality_.transport_rtt(), downstream_kbps);