Fix flaky NQE cronet test

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

Index: components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestContextTest.java
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 f79e4116ed30e0d559f315583e007aa41c5f6261..ddf087c59f48844008d045810f982b1a02a88814 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
@@ -29,6 +29,7 @@ import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.NoSuchElementException;
 import java.util.concurrent.Executor;
+import java.util.concurrent.Executors;
 
 /**
  * Test CronetEngine.
@@ -128,6 +129,11 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
             implements NetworkQualityRttListener, NetworkQualityThroughputListener {
         int mRttObservationCount;
         int mThroughputObservationCount;
+        ConditionVariable mWaitForThroughput = null;

[xunjieli, 2016/05/26 17:55:30]

nit: maybe get rid of "=null", and add "final" keyword. And move the final
member above the non-final ones.

[tbansal1, 2016/05/26 18:02:59]

Done.

+
+        TestNetworkQualityListener(ConditionVariable waitForThroughput) {
+            mWaitForThroughput = waitForThroughput;
+        }
 
         @Override
         public void onRttObservation(int rttMs, long when, int source) {
@@ -136,6 +142,9 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
 
         @Override
         public void onThroughputObservation(int throughputKbps, long when, int source) {
+            if (mWaitForThroughput != null) {
+                mWaitForThroughput.open();
+            }
             mThroughputObservationCount++;
         }
 
@@ -220,11 +229,9 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
 
     @SmallTest
     @Feature({"Cronet"})
-    // TODO(xunjieli): Remove annotation after crbug.com/539519 is fixed.
-    @SuppressWarnings("deprecation")
     public void testRealTimeNetworkQualityObservationsNotEnabled() throws Exception {
         mTestFramework = startCronetTestFramework();
-        TestNetworkQualityListener networkQualityListener = new TestNetworkQualityListener();
+        TestNetworkQualityListener networkQualityListener = new TestNetworkQualityListener(null);
         try {
             mTestFramework.mCronetEngine.addRttListener(networkQualityListener);
             fail("Should throw an exception.");
@@ -247,12 +254,10 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
 
     @SmallTest
     @Feature({"Cronet"})
-    // TODO(xunjieli): Remove annotation after crbug.com/539519 is fixed.
-    @SuppressWarnings("deprecation")
     public void testRealTimeNetworkQualityObservationsListenerRemoved() throws Exception {
         mTestFramework = startCronetTestFramework();
         TestExecutor testExecutor = new TestExecutor();
-        TestNetworkQualityListener networkQualityListener = new TestNetworkQualityListener();
+        TestNetworkQualityListener networkQualityListener = new TestNetworkQualityListener(null);
         mTestFramework.mCronetEngine.enableNetworkQualityEstimatorForTesting(
                 true, true, testExecutor);
         mTestFramework.mCronetEngine.addRttListener(networkQualityListener);
@@ -270,17 +275,14 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
         mTestFramework.mCronetEngine.shutdown();
     }
 
-    /*
     @SmallTest
     @Feature({"Cronet"})
-    // TODO(xunjieli): Remove annotation after crbug.com/539519 is fixed.
-    @SuppressWarnings("deprecation")
-    */
-    @FlakyTest(message = "http://crbug.com/614227")
     public void testRealTimeNetworkQualityObservations() throws Exception {
         mTestFramework = startCronetTestFramework();
-        TestExecutor testExecutor = new TestExecutor();
-        TestNetworkQualityListener networkQualityListener = new TestNetworkQualityListener();
+        Executor testExecutor = Executors.newSingleThreadExecutor();
+        ConditionVariable waitForThroughput = new ConditionVariable();
+        TestNetworkQualityListener networkQualityListener =
+                new TestNetworkQualityListener(waitForThroughput);
         mTestFramework.mCronetEngine.enableNetworkQualityEstimatorForTesting(
                 true, true, testExecutor);
         mTestFramework.mCronetEngine.addRttListener(networkQualityListener);
@@ -290,7 +292,10 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
                 mTestFramework.mCronetEngine.createRequest(mUrl, callback, callback.getExecutor());
         urlRequest.start();
         callback.blockForDone();
-        testExecutor.runAllTasks();
+
+        // Throughput observation is posted to the network quality estimator and its observers
+        // after the UrlRequest is completed.
+        waitForThroughput.block();

[xunjieli, 2016/05/26 17:55:30]

Is throughtput observation always going to be posted after the RTT observation?

[tbansal1, 2016/05/26 18:02:59]

Yes.

[xunjieli, 2016/05/26 18:06:49]

Please add a comment above in "public void onThroughputObservation" of
TestNetworkQualityListener to make a note of this. It is not obvious why you
want to wait for throughput instead of both rtt and throughput.

[tbansal1, 2016/05/26 18:21:40]

I expanded on the comment here, instead of in onThroughputObservation. Let me
know if this is still vague.

         assertTrue(networkQualityListener.rttObservationCount() > 0);
         assertTrue(networkQualityListener.throughputObservationCount() > 0);
         mTestFramework.mCronetEngine.shutdown();