Expose RTT and throughput estimates from Cronet

components/cronet/android/java/src/org/chromium/net/impl/CronetUrlRequestContext.java

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.net.impl;
 
 import android.content.Context;
 import android.os.Build;
 import android.os.ConditionVariable;
 import android.os.Handler;
 import android.os.Looper;
 import android.os.Process;
 import android.util.Log;
 
 import org.chromium.base.ObserverList;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.base.annotations.NativeClassQualifiedName;
 import org.chromium.base.annotations.UsedByReflection;
 import org.chromium.net.BidirectionalStream;
 import org.chromium.net.CronetEngine;
 import org.chromium.net.EffectiveConnectionType;
 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;
 
 import java.net.Proxy;
 import java.net.URL;
 import java.net.URLConnection;
 import java.net.URLStreamHandlerFactory;
 import java.util.ArrayList;
 import java.util.Collection;
@@ skipping 50 matching lines @@
      */
     private final Object mFinishedListenerLock = new Object();
 
     /**
      * Current effective connection type as computed by the network quality
      * estimator.
      */
     @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_VALUE;
+
+    /**
+     * Current estimate of the transport RTT (in milliseconds) computed by the
+     * network quality estimator.
+     */
+    @GuardedBy("mNetworkQualityLock")
+    private int mTransportRttMs = RttThroughputValues.INVALID_RTT_THROUGHPUT_VALUE;
+
+    /**
+     * 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_VALUE;
+
     @GuardedBy("mNetworkQualityLock")
     private final ObserverList<NetworkQualityRttListener> mRttListenerList =
             new ObserverList<NetworkQualityRttListener>();
 
     @GuardedBy("mNetworkQualityLock")
     private final ObserverList<NetworkQualityThroughputListener> mThroughputListenerList =
             new ObserverList<NetworkQualityThroughputListener>();
 
     @GuardedBy("mFinishedListenerLock")
     private final List<RequestFinishedInfo.Listener> mFinishedListenerList =
@@ skipping 220 matching lines @@
             throw new IllegalStateException("Network quality estimator must be enabled");
         }
         synchronized (mNetworkQualityLock) {
             synchronized (mLock) {
                 checkHaveAdapter();
             }
             return mEffectiveConnectionType;
         }
     }
 
+    @Override
+    public int getHttpRttMsec() {
+        if (!mNetworkQualityEstimatorEnabled) {
+            throw new IllegalStateException("Network quality estimator must be enabled");
+        }
+        synchronized (mNetworkQualityLock) {
+            synchronized (mLock) {

[RyanSturm, 2016/10/14 21:21:32]

I'm not happy with the locking order pattern in this file. Could 

synchronized (mLock) {
 checkHaveAdapter();
}

happen before

synchronized (mNetworkQualityLock)

instead of inside it?

[tbansal1, 2016/10/14 21:50:29]

This pattern was introduced in
https://codereview.chromium.org/2283243002/diff/120001/components/cronet/andr....
Not sure why it is this way.

[RyanSturm, 2016/10/14 22:19:12]

Any chance you want to try to fix it in this CL? There doesn't seem to be a need
to have nested locks, and I would hate to block the IO thread if mLock is stuck
doing something.

[tbansal1, 2016/10/14 22:44:43]

If we do want to fix it, I would prefer to do it in a separate CL.
I am not sure how separating the two locks makes it faster. If mLock is stuck,
then
even if the locks are separated, NQE functions still have to wait.

[RyanSturm, 2016/10/14 22:55:38]

If getEffectiveConnectionTypeChanged gets blocked on mLock on the UI thread,
then calling onEffectiveConnectionTypeChanged will block the IO thread, but if
mNetworkQualityLock weren't held then onEffectiveConnectionTypeChanged could
just set it and leave.

Change it in another CL. Doesn't seem high priority.

[tbansal1, 2016/10/14 23:31:49]

Yeah, I need to check with clm@ why it was done this way.

+                checkHaveAdapter();
+            }
+            return mHttpRttMs;
+        }
+    }
+
+    @Override
+    public int getTransportRttMsec() {
+        if (!mNetworkQualityEstimatorEnabled) {
+            throw new IllegalStateException("Network quality estimator must be enabled");
+        }
+        synchronized (mNetworkQualityLock) {
+            synchronized (mLock) {
+                checkHaveAdapter();
+            }
+            return mTransportRttMs;
+        }
+    }
+
+    @Override
+    public int getDownstreamThroughputKbps() {
+        if (!mNetworkQualityEstimatorEnabled) {
+            throw new IllegalStateException("Network quality estimator must be enabled");
+        }
+        synchronized (mNetworkQualityLock) {
+            synchronized (mLock) {
+                checkHaveAdapter();
+            }
+            return mDownstreamThroughputKbps;
+        }
+    }
+
     @VisibleForTesting
     @Override
     public void configureNetworkQualityEstimatorForTesting(
             boolean useLocalHostRequests, boolean useSmallerResponses) {
         if (!mNetworkQualityEstimatorEnabled) {
             throw new IllegalStateException("Network quality estimator must be enabled");
         }
         synchronized (mLock) {
             checkHaveAdapter();
             nativeConfigureNetworkQualityEstimatorForTesting(
@@ skipping 173 matching lines @@
     private void onEffectiveConnectionTypeChanged(int effectiveConnectionType) {
         synchronized (mNetworkQualityLock) {
             // Convert the enum returned by the network quality estimator to an enum of type
             // EffectiveConnectionType.
             mEffectiveConnectionType = 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) {
         synchronized (mNetworkQualityLock) {
             for (final NetworkQualityRttListener listener : mRttListenerList) {
                 Runnable task = new Runnable() {
                     @Override
                     public void run() {
                         listener.onRttObservation(rttMs, whenMs, source);
                     }
                 };
                 postObservationTaskToExecutor(listener.getExecutor(), task);
@@ skipping 98 matching lines @@
     @NativeClassQualifiedName("CronetURLRequestContextAdapter")
     private native void nativeProvideRTTObservations(long nativePtr, boolean should);
 
     @NativeClassQualifiedName("CronetURLRequestContextAdapter")
     private native void nativeProvideThroughputObservations(long nativePtr, boolean should);
 
     public boolean isNetworkThread(Thread thread) {
         return thread == mNetworkThread;
     }
 }