Fix flaky NQE cronet test

components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestContextTest.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;
 
 import static org.chromium.base.CollectionUtil.newHashSet;
 
 import android.content.Context;
 import android.content.ContextWrapper;
@@ skipping 11 matching lines @@
 import org.chromium.net.test.EmbeddedTestServer;
 
 import java.io.BufferedReader;
 import java.io.File;
 import java.io.FileReader;
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.NoSuchElementException;
 import java.util.concurrent.Executor;
+import java.util.concurrent.Executors;
 
 /**
  * Test CronetEngine.
  */
 @JNINamespace("cronet")
 public class CronetUrlRequestContextTest extends CronetTestBase {
     // URLs used for tests.
     private static final String MOCK_CRONET_TEST_FAILED_URL =
             "http://mock.failed.request/-2";
     private static final String MOCK_CRONET_TEST_SUCCESS_URL =
@@ skipping 77 matching lines @@
                 while (mTaskQueue.size() > 0) {
                     mTaskQueue.remove().run();
                 }
             } catch (NoSuchElementException e) {
             }
         }
     }
 
     static class TestNetworkQualityListener
             implements NetworkQualityRttListener, NetworkQualityThroughputListener {
-        int mRttObservationCount;
-        int mThroughputObservationCount;
+        // Lock to ensure that observation counts can be updated and read by different threads.
+        private final Object mLock = new Object();
+        private final ConditionVariable mWaitForThroughput;
+        private int mRttObservationCount;
+        private int mThroughputObservationCount;
+
+        TestNetworkQualityListener(ConditionVariable waitForThroughput) {
+            mWaitForThroughput = waitForThroughput;
+        }
 
         @Override
         public void onRttObservation(int rttMs, long when, int source) {
-            mRttObservationCount++;
+            synchronized (mLock) {
+                mRttObservationCount++;
+            }
         }
 
         @Override
         public void onThroughputObservation(int throughputKbps, long when, int source) {
-            mThroughputObservationCount++;
+            synchronized (mLock) {
+                if (mWaitForThroughput != null) {
+                    mWaitForThroughput.open();
+                }
+                mThroughputObservationCount++;
+            }
         }
 
         public int rttObservationCount() {
-            return mRttObservationCount;
+            synchronized (mLock) {
+                return mRttObservationCount;
+            }
         }
 
         public int throughputObservationCount() {
-            return mThroughputObservationCount;
+            synchronized (mLock) {
+                return mThroughputObservationCount;
+            }
         }
     }
 
     @SmallTest
     @Feature({"Cronet"})
     public void testConfigUserAgent() throws Exception {
         String userAgentName = "User-Agent";
         String userAgentValue = "User-Agent-Value";
         CronetEngine.Builder cronetEngineBuilder = new CronetEngine.Builder(getContext());
         if (testingJavaImpl()) {
@@ skipping 55 matching lines @@
         // Verify that the request is successful and that the Data Reduction
         // Proxy logic configured to use the test server as its proxy.
         assertEquals(200, callback.mResponseInfo.getHttpStatusCode());
         assertEquals(serverHostPort, callback.mResponseInfo.getProxyServer());
         assertEquals("http://DomainThatDoesnt.Resolve/datareductionproxysuccess.txt",
                 callback.mResponseInfo.getUrl());
     }
 
     @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.");
         } catch (IllegalStateException e) {
         }
         try {
             mTestFramework.mCronetEngine.addThroughputListener(networkQualityListener);
             fail("Should throw an exception.");
         } catch (IllegalStateException e) {
         }
         TestUrlRequestCallback callback = new TestUrlRequestCallback();
         UrlRequest urlRequest =
                 mTestFramework.mCronetEngine.createRequest(mUrl, callback, callback.getExecutor());
         urlRequest.start();
         callback.blockForDone();
         assertEquals(0, networkQualityListener.rttObservationCount());
         assertEquals(0, networkQualityListener.throughputObservationCount());
         mTestFramework.mCronetEngine.shutdown();
     }
 
     @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);
         mTestFramework.mCronetEngine.addThroughputListener(networkQualityListener);
         mTestFramework.mCronetEngine.removeRttListener(networkQualityListener);
         mTestFramework.mCronetEngine.removeThroughputListener(networkQualityListener);
         TestUrlRequestCallback callback = new TestUrlRequestCallback();
         UrlRequest urlRequest =
                 mTestFramework.mCronetEngine.createRequest(mUrl, callback, callback.getExecutor());
         urlRequest.start();
         callback.blockForDone();
         testExecutor.runAllTasks();
         assertEquals(0, networkQualityListener.rttObservationCount());
         assertEquals(0, networkQualityListener.throughputObservationCount());
         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();
-        mTestFramework.mCronetEngine.enableNetworkQualityEstimatorForTesting(
-                true, true, testExecutor);
+        Executor executor = Executors.newSingleThreadExecutor();
+        ConditionVariable waitForThroughput = new ConditionVariable();
+        TestNetworkQualityListener networkQualityListener =
+                new TestNetworkQualityListener(waitForThroughput);
+        mTestFramework.mCronetEngine.enableNetworkQualityEstimatorForTesting(true, true, executor);
         mTestFramework.mCronetEngine.addRttListener(networkQualityListener);
         mTestFramework.mCronetEngine.addThroughputListener(networkQualityListener);
         TestUrlRequestCallback callback = new TestUrlRequestCallback();
         UrlRequest urlRequest =
                 mTestFramework.mCronetEngine.createRequest(mUrl, callback, callback.getExecutor());
         urlRequest.start();
         callback.blockForDone();
-        testExecutor.runAllTasks();
+
+        // 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.
+        waitForThroughput.block();
+        assertTrue(networkQualityListener.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(networkQualityListener.rttObservationCount() > 0);
-        assertTrue(networkQualityListener.throughputObservationCount() > 0);
+
         mTestFramework.mCronetEngine.shutdown();
     }
 
     private static class TestRequestFinishedListener
             implements CronetEngine.RequestFinishedListener {
         private UrlRequestInfo mRequestInfo = null;
 
         @Override
         public void onRequestFinished(UrlRequestInfo requestInfo) {
             assertNull("onRequestFinished called repeatedly", mRequestInfo);
@@ skipping 843 matching lines @@
                     }
                 }.start();
                 otherThreadDone.block();
                 builder.build().shutdown();
                 uiThreadDone.open();
             }
         });
         assertTrue(uiThreadDone.block(1000));
     }
 }