Fix flaky tests in CronetUrlRequestContextTest

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 4697f0c7720bff84d7b12109277543445dd65031..f7bd864085d0f0ee2645c3fe9bba7dba463a8b86 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
@@ -13,7 +13,6 @@ import android.os.StrictMode;
 import android.test.suitebuilder.annotation.SmallTest;
 
 import org.chromium.base.FileUtils;
-import org.chromium.base.Log;
 import org.chromium.base.PathUtils;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.base.test.util.Feature;
@@ -42,7 +41,6 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
             "http://mock.failed.request/-2";
     private static final String MOCK_CRONET_TEST_SUCCESS_URL =
             "http://mock.http/success.txt";
-    private static final String TAG = "RequestContextTest";
     private static final int MAX_FILE_SIZE = 1000000000;
     private static final int NUM_EVENT_FILES = 10;
 
@@ -100,25 +98,30 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
      * Callback that shutdowns the request context when request has succeeded
      * or failed.
      */
-    class ShutdownTestUrlRequestCallback extends TestUrlRequestCallback {
+    static class ShutdownTestUrlRequestCallback extends TestUrlRequestCallback {
+        private final CronetEngine mCronetEngine;
+        private ConditionVariable mCallbackCompletionBlock = new ConditionVariable();

[pauljensen, 2016/08/18 15:05:29]

final

[kapishnikov, 2016/08/18 16:18:33]

Done.

+
+        ShutdownTestUrlRequestCallback(CronetEngine cronetEngine) {
+            mCronetEngine = cronetEngine;
+        }
+
         @Override
         public void onSucceeded(UrlRequest request, UrlResponseInfo info) {
-            // TODO: Remove logging when http://crbug.com/596929 & http://crbug.com/635025 is fixed.
-            Log.d(TAG, "ShutdownTestUrlRequestCallback.onSucceeded() has started");
             super.onSucceeded(request, info);
-            Log.d(TAG, "ShutdownTestUrlRequestCallback.onSucceeded() before calling shutdown");
-            mTestFramework.mCronetEngine.shutdown();
-            Log.d(TAG, "ShutdownTestUrlRequestCallback.hasFinished() has finished");
+            mCronetEngine.shutdown();
+            mCallbackCompletionBlock.open();
         }
 
         @Override
         public void onFailed(UrlRequest request, UrlResponseInfo info, UrlRequestException error) {
-            // TODO: Remove logging when http://crbug.com/596929 & http://crbug.com/635025 is fixed.
-            Log.d(TAG, "ShutdownTestUrlRequestCallback.onFailed() has started");
             super.onFailed(request, info, error);
-            Log.d(TAG, "ShutdownTestUrlRequestCallback.onFailed() before calling shutdown");
-            mTestFramework.mCronetEngine.shutdown();
-            Log.d(TAG, "ShutdownTestUrlRequestCallback.onFailed() has finished.");
+            mCronetEngine.shutdown();
+            mCallbackCompletionBlock.open();
+        }
+
+        void blockForCallbackToComplete() {

[pauljensen, 2016/08/18 15:05:29]

add: // Wait for request completion callback.

[kapishnikov, 2016/08/18 16:18:33]

Done.

+            mCallbackCompletionBlock.block();
         }
     }
 
@@ -351,9 +354,9 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
     // TODO: Remove the annotation after fixing http://crbug.com/637979 & http://crbug.com/637972
     @OnlyRunNativeCronet
     public void testShutdown() throws Exception {
-        Log.i(TAG, "testShutdown() has started");
         mTestFramework = startCronetTestFramework();
-        TestUrlRequestCallback callback = new ShutdownTestUrlRequestCallback();
+        ShutdownTestUrlRequestCallback callback =
+                new ShutdownTestUrlRequestCallback(mTestFramework.mCronetEngine);
         // Block callback when response starts to verify that shutdown fails
         // if there are active requests.
         callback.setAutoAdvance(false);
@@ -395,11 +398,8 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
         callback.setAutoAdvance(true);
         callback.startNextRead(urlRequest);
         callback.blockForDone();
-        // TODO: Remove sleep when http://crbug.com/596929 is fixed.
-        // The sleep gives the thread that shuts down the engine time to complete.
-        // See http://crbug.com/596929
-        Thread.sleep(3000);
-        Log.i(TAG, "testShutdown() has finished");
+        callback.blockForCallbackToComplete();
+        callback.shutdownExecutor();
     }
 
     @SmallTest
@@ -493,19 +493,16 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
     // TODO: Remove the annotation after fixing http://crbug.com/637972
     @OnlyRunNativeCronet
     public void testShutdownAfterError() throws Exception {
-        Log.i(TAG, "testShutdownAfterError() has started");
         mTestFramework = startCronetTestFramework();
-        TestUrlRequestCallback callback = new ShutdownTestUrlRequestCallback();
+        ShutdownTestUrlRequestCallback callback =
+                new ShutdownTestUrlRequestCallback(mTestFramework.mCronetEngine);
         UrlRequest.Builder urlRequestBuilder = new UrlRequest.Builder(MOCK_CRONET_TEST_FAILED_URL,
                 callback, callback.getExecutor(), mTestFramework.mCronetEngine);
         urlRequestBuilder.build().start();
         callback.blockForDone();
         assertTrue(callback.mOnErrorCalled);
-        // TODO: Remove sleep when http://crbug.com/635025 is fixed.
-        // The sleep gives the thread that shuts down the engine time to complete.
-        // See http://crbug.com/637986
-        Thread.sleep(3000);
-        Log.i(TAG, "testShutdownAfterError() has finished");
+        callback.blockForCallbackToComplete();
+        callback.shutdownExecutor();
     }
 
     @SmallTest