Add functionality for embedders to configure NQE

components/cronet/android/test/javatests/src/org/chromium/net/NQETest.java

Index: components/cronet/android/test/javatests/src/org/chromium/net/NQETest.java
diff --git a/components/cronet/android/test/javatests/src/org/chromium/net/NQETest.java b/components/cronet/android/test/javatests/src/org/chromium/net/NQETest.java
index 0d391b041b33539fd19e225b131b632eac191a89..ab0d728fa7d4cef2f4aaffea5729a9d8b2bb3710 100644
--- a/components/cronet/android/test/javatests/src/org/chromium/net/NQETest.java
+++ b/components/cronet/android/test/javatests/src/org/chromium/net/NQETest.java
@@ -7,10 +7,11 @@ package org.chromium.net;
 import android.os.StrictMode;
 import android.support.test.filters.SmallTest;
 
+import org.json.JSONObject;
+
 import org.chromium.base.Log;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.base.annotations.SuppressFBWarnings;
-import org.chromium.base.test.util.DisabledTest;
 import org.chromium.base.test.util.Feature;
 import org.chromium.base.test.util.MetricsUtils.HistogramDelta;
 import org.chromium.net.MetricsTestUtil.TestExecutor;
@@ -108,6 +109,9 @@ public class NQETest extends CronetTestBase {
         TestNetworkQualityRttListener rttListener =
                 new TestNetworkQualityRttListener(networkQualityExecutor);
         cronetEngineBuilder.enableNetworkQualityEstimator(true);
+
+        JSONObject experimentalOptions = new JSONObject();

[mgersh, 2017/06/28 18:26:50]

These two lines shouldn't be necessary.

[tbansal1, 2017/06/29 01:17:58]

Done.

+        cronetEngineBuilder.setExperimentalOptions(experimentalOptions.toString());
         final ExperimentalCronetEngine cronetEngine = cronetEngineBuilder.build();
         cronetEngine.configureNetworkQualityEstimatorForTesting(true, true, false);
 
@@ -126,8 +130,8 @@ public class NQETest extends CronetTestBase {
 
     // Returns whether a file contains a particular string.
     @SuppressFBWarnings("OBL_UNSATISFIED_OBLIGATION_EXCEPTION_EDGE")
-    private boolean fileContainsString(String filename, String content) throws IOException {
-        File file = new File(getTestStorage(getContext()) + "/prefs/" + filename);
+    private boolean prefsFileContainsString(String content) throws IOException {
+        File file = new File(getTestStorage(getContext()) + "/prefs/local_prefs.json");
         FileInputStream fileInputStream = new FileInputStream(file);
         byte[] data = new byte[(int) file.length()];
         fileInputStream.read(data);
@@ -135,9 +139,17 @@ public class NQETest extends CronetTestBase {
         return new String(data, "UTF-8").contains(content);
     }
 
+    @SuppressFBWarnings("OBL_UNSATISFIED_OBLIGATION_EXCEPTION_EDGE")
+    private boolean deletePrefsFile() throws Exception {
+        File file = new File(getTestStorage(getContext()) + "/prefs/local_prefs.json");
+        if (file.exists()) {
+            return file.delete();
+        }
+        return true;
+    }
+
     @SmallTest
     @Feature({"Cronet"})
-    @DisabledTest(message = "Disabled due to flaky assert. See crbug.com/710626")
     public void testQuicDisabled() throws Exception {
         ExperimentalCronetEngine.Builder cronetEngineBuilder =
                 new ExperimentalCronetEngine.Builder(getContext());
@@ -147,6 +159,10 @@ public class NQETest extends CronetTestBase {
                 new TestNetworkQualityRttListener(listenersExecutor);
         TestNetworkQualityThroughputListener throughputListener =
                 new TestNetworkQualityThroughputListener(listenersExecutor);
+
+        JSONObject experimentalOptions = new JSONObject();

[mgersh, 2017/06/28 18:26:50]

Also shouldn't be necessary.

[tbansal1, 2017/06/29 01:17:58]

Done.

+        cronetEngineBuilder.setExperimentalOptions(experimentalOptions.toString());
+
         cronetEngineBuilder.enableNetworkQualityEstimator(true).enableHttp2(true).enableQuic(false);
         cronetEngineBuilder.setStoragePath(getTestStorage(getContext()));
         final ExperimentalCronetEngine cronetEngine = cronetEngineBuilder.build();
@@ -214,7 +230,7 @@ public class NQETest extends CronetTestBase {
             Log.i(TAG, "Still waiting for pref file update.....");
             Thread.sleep(12000);
             try {
-                if (fileContainsString("local_prefs.json", "network_qualities")) {
+                if (prefsFileContainsString("network_qualities")) {
                     break;
                 }
             } catch (FileNotFoundException e) {
@@ -222,9 +238,169 @@ public class NQETest extends CronetTestBase {
                 // flushed to the disk.
             }
         }
-        assertTrue(fileContainsString("local_prefs.json", "network_qualities"));
+        assertTrue(prefsFileContainsString("network_qualities"));
 
         cronetEngine.shutdown();
         assertTrue(writeCountHistogram.getDelta() > 0);
     }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testPrefsWriteRead() throws Exception {
+        deletePrefsFile();

[mgersh, 2017/06/28 18:26:50]

CronetTestBase() does this for you, so no need for it here.

[tbansal1, 2017/06/29 01:17:58]

Added comment.

[mgersh, 2017/06/29 19:02:31]

Oh, I see what's going on. You're running into crbug.com/637972 and these tests
are being run twice with the same configuration. They're testing functionality
that relies on native code, so they should all have the @OnlyRunNativeCronet
annotation. Then they'll only run once and you can remove this line.

[tbansal1, 2017/06/29 22:45:05]

Done.

+
+        // When the loop is run for the first time, network quality is written to the disk. The
+        // test verifies that in the next loop, the network quality is read back.
+        for (int i = 0; i <= 1; ++i) {
+            ExperimentalCronetEngine.Builder cronetEngineBuilder =
+                    new ExperimentalCronetEngine.Builder(getContext());
+            assert RttThroughputValues.INVALID_RTT_THROUGHPUT < 0;
+            Executor listenersExecutor =
+                    Executors.newSingleThreadExecutor(new ExecutorThreadFactory());
+            TestNetworkQualityRttListener rttListener =
+                    new TestNetworkQualityRttListener(listenersExecutor);
+            cronetEngineBuilder.enableNetworkQualityEstimator(true).enableHttp2(true).enableQuic(
+                    false);
+            cronetEngineBuilder.setStoragePath(getTestStorage(getContext()));
+
+            JSONObject nqeOptions =
+                    new JSONObject().put("persistent_cache_reading_enabled", "true");
+            JSONObject experimentalOptions =
+                    new JSONObject().put("NetworkQualityEstimator", nqeOptions);
+
+            cronetEngineBuilder.setExperimentalOptions(experimentalOptions.toString());
+            final ExperimentalCronetEngine cronetEngine = cronetEngineBuilder.build();
+            cronetEngine.configureNetworkQualityEstimatorForTesting(true, true, true);
+            cronetEngine.addRttListener(rttListener);
+
+            HistogramDelta writeCountHistogram = new HistogramDelta("NQE.Prefs.WriteCount", 1);
+            assertEquals(0, writeCountHistogram.getDelta()); // Sanity check.
+
+            HistogramDelta readCountHistogram = new HistogramDelta("NQE.Prefs.ReadCount", 1);
+            assertEquals(0, readCountHistogram.getDelta()); // Sanity check.
+
+            HistogramDelta readPrefsSizeHistogram = new HistogramDelta("NQE.Prefs.ReadSize", 1);
+            assertEquals(0, readPrefsSizeHistogram.getDelta()); // Sanity check.
+
+            // NETWORK_QUALITY_OBSERVATION_SOURCE_HTTP_CACHED_ESTIMATE: 3
+            HistogramDelta cachedRttHistogram = new HistogramDelta("NQE.RTT.ObservationSource", 3);
+            assertEquals(0, cachedRttHistogram.getDelta()); // Sanity check.
+
+            TestUrlRequestCallback callback = new TestUrlRequestCallback();
+            UrlRequest.Builder builder =
+                    cronetEngine.newUrlRequestBuilder(mUrl, callback, callback.getExecutor());
+            UrlRequest urlRequest = builder.build();
+            urlRequest.start();
+            callback.blockForDone();
+
+            // Wait for RTT observation (at the URL request layer) to be posted.
+            rttListener.waitUntilFirstUrlRequestRTTReceived();
+
+            // Prefs must be read at startup.
+            assertTrue(readCountHistogram.getDelta() > 0);
+
+            // Check RTT observation count after throughput observation has been received. This
+            // ensures
+            // that executor has finished posting the RTT observation to the RTT listeners.
+            assertTrue(rttListener.rttObservationCount() > 0);
+
+            // Verify that effective connection type callback is received and
+            // effective connection type is correctly set.
+            assertTrue(cronetEngine.getEffectiveConnectionType()
+                    != EffectiveConnectionType.TYPE_UNKNOWN);
+
+            if (i == 0) {
+                // Verify that the cached estimates were written to the prefs.
+                while (true) {

[mgersh, 2017/06/28 18:26:50]

If you do this after cronetEngine.shutdown(), you won't need the while loop and
you won't have to ignore a FileNotFoundException. You'll still have to wait for
a prefs write before shutting down, but it can probably be a much shorter wait
than 12 seconds without making the test flaky.

[tbansal1, 2017/06/29 01:17:58]

Cool. Done.

+                    Log.i(TAG, "Still waiting for pref file update.....");
+                    Thread.sleep(12000);
+                    try {
+                        if (prefsFileContainsString("network_qualities")) {
+                            break;
+                        }
+                    } catch (FileNotFoundException e) {
+                        // Ignored this exception since the file will only be created when updates
+                        // are flushed to the disk.
+                    }
+                }
+                assertTrue(prefsFileContainsString("network_qualities"));
+            }
+
+            cronetEngine.shutdown();
+            // Stored network quality in the pref should be read in the second iteration.
+            assertEquals(readPrefsSizeHistogram.getDelta() > 0, i > 0);
+            assertEquals(cachedRttHistogram.getDelta() > 0, i > 0);
+        }
+    }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testQuicDisabledWithParams() throws Exception {
+        ExperimentalCronetEngine.Builder cronetEngineBuilder =
+                new ExperimentalCronetEngine.Builder(getContext());
+        Executor listenersExecutor = Executors.newSingleThreadExecutor(new ExecutorThreadFactory());
+        TestNetworkQualityRttListener rttListener =
+                new TestNetworkQualityRttListener(listenersExecutor);
+        TestNetworkQualityThroughputListener throughputListener =
+                new TestNetworkQualityThroughputListener(listenersExecutor);
+
+        // Force the effective connection type to "2G".
+        JSONObject nqeOptions = new JSONObject().put("force_effective_connection_type", "Slow-2G");
+        // Add one more extra param two times to ensure robustness.
+        nqeOptions.put("some_other_param_1", "value1");
+        nqeOptions.put("some_other_param_2", "value2");
+        JSONObject experimentalOptions =
+                new JSONObject().put("NetworkQualityEstimator", nqeOptions);
+        experimentalOptions.put("SomeOtherFieldTrialName", new JSONObject());
+
+        cronetEngineBuilder.enableNetworkQualityEstimator(true).enableHttp2(true).enableQuic(false);
+        cronetEngineBuilder.setExperimentalOptions(experimentalOptions.toString());
+        final ExperimentalCronetEngine cronetEngine = cronetEngineBuilder.build();
+        cronetEngine.configureNetworkQualityEstimatorForTesting(true, true, false);
+
+        cronetEngine.addRttListener(rttListener);
+        cronetEngine.addThroughputListener(throughputListener);
+
+        TestUrlRequestCallback callback = new TestUrlRequestCallback();
+        UrlRequest.Builder builder =
+                cronetEngine.newUrlRequestBuilder(mUrl, callback, callback.getExecutor());
+        UrlRequest urlRequest = builder.build();
+        urlRequest.start();
+        callback.blockForDone();
+
+        // Throughput observation is posted to the network quality estimator on the network thread
+        // after the UrlRequest is completed. The observations are then eventually posted to
+        // throughput listeners on the executor provided to network quality.
+        throughputListener.waitUntilFirstThroughputObservationReceived();
+
+        // Wait for RTT observation (at the URL request layer) to be posted.
+        rttListener.waitUntilFirstUrlRequestRTTReceived();
+
+        assertTrue(throughputListener.throughputObservationCount() > 0);
+
+        // Check RTT observation count after throughput observation has been received. This ensures
+        // that executor has finished posting the RTT observation to the RTT listeners.
+        assertTrue(rttListener.rttObservationCount() > 0);
+
+        // NETWORK_QUALITY_OBSERVATION_SOURCE_URL_REQUEST
+        assertTrue(rttListener.rttObservationCount(0) > 0);
+
+        // NETWORK_QUALITY_OBSERVATION_SOURCE_TCP
+        assertTrue(rttListener.rttObservationCount(1) > 0);
+
+        // NETWORK_QUALITY_OBSERVATION_SOURCE_QUIC
+        assertEquals(0, rttListener.rttObservationCount(2));
+
+        // Verify that the listeners were notified on the expected thread.
+        assertEquals(mNetworkQualityThread, rttListener.getThread());
+        assertEquals(mNetworkQualityThread, throughputListener.getThread());
+
+        // Verify that effective connection type callback is received and effective connection type
+        // is correctly set to the forced value. This also verifies that the configuration params
+        // from Cronet embedders were correctly read by NetworkQualityEstimator.
+        assertEquals(
+                EffectiveConnectionType.TYPE_SLOW_2G, cronetEngine.getEffectiveConnectionType());
+
+        cronetEngine.shutdown();
+    }
 }