Fix CronetUrlRequestTest#testFailures flake

components/cronet/android/test/javatests/src/org/chromium/net/TestUrlRequestCallback.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 android.os.ConditionVariable;
 import android.os.StrictMode;
 
 import static junit.framework.Assert.assertEquals;
@@ skipping 27 matching lines @@
     public boolean mOnCanceledCalled = false;
 
     public int mHttpResponseDataLength = 0;
     public String mResponseAsString = "";
 
     private static final int READ_BUFFER_SIZE = 32 * 1024;
 
     // When false, the consumer is responsible for all calls into the request
     // that advance it.
     private boolean mAutoAdvance = true;
+    // Whether an exception is thrown by maybeThrowCancelOrPause().
+    private boolean mListenerExceptionThrown;
 
     // Conditionally fail on certain steps.
     private FailureType mFailureType = FailureType.NONE;
     private ResponseStep mFailureStep = ResponseStep.NOTHING;
 
     // Signals when request is done either successfully or not.
     private final ConditionVariable mDone = new ConditionVariable();
 
     // Signaled on each step when mAutoAdvance is false.
     private final ConditionVariable mStepBlock = new ConditionVariable();
@@ skipping 26 matching lines @@
             });
             return mExecutorThread;
         }
     }
 
     public enum ResponseStep {
         NOTHING,
         ON_RECEIVED_REDIRECT,
         ON_RESPONSE_STARTED,
         ON_READ_COMPLETED,
-        ON_SUCCEEDED
+        ON_SUCCEEDED,
+        ON_FAILED,
+        ON_CANCELED,
     }
 
     public enum FailureType {
         NONE,
         CANCEL_SYNC,
         CANCEL_ASYNC,
         // Same as above, but continues to advance the request after posting
         // the cancellation task.
         CANCEL_ASYNC_WITHOUT_PAUSE,
         THROW_SYNC
@@ skipping 106 matching lines @@
     @Override
     public void onFailed(UrlRequest request, UrlResponseInfo info, UrlRequestException error) {
         assertEquals(mExecutorThread, Thread.currentThread());
         assertTrue(request.isDone());
         // Shouldn't happen after success.
         assertTrue(mResponseStep != ResponseStep.ON_SUCCEEDED);
         // Should happen at most once for a single request.
         assertFalse(mOnErrorCalled);
         assertFalse(mOnCanceledCalled);
         assertNull(mError);
-        if (mFailureType == FailureType.THROW_SYNC) {
+        if (mListenerExceptionThrown) {
             assertEquals(UrlRequestError.LISTENER_EXCEPTION_THROWN, error.getErrorCode());
             assertEquals(0, error.getCronetInternalErrorCode());
             assertEquals("Exception received from UrlRequest.Callback", error.getMessage());
             assertNotNull(error.getCause());
             assertTrue(error.getCause() instanceof IllegalStateException);
             assertEquals("Listener Exception.", error.getCause().getMessage());
             assertFalse(error.immediatelyRetryable());
         }
 
+        mResponseStep = ResponseStep.ON_FAILED;
         mOnErrorCalled = true;
         mError = error;
         openDone();
         maybeThrowCancelOrPause(request);
     }
 
     @Override
     public void onCanceled(UrlRequest request, UrlResponseInfo info) {
         assertEquals(mExecutorThread, Thread.currentThread());
         assertTrue(request.isDone());
         // Should happen at most once for a single request.
         assertFalse(mOnCanceledCalled);
         assertFalse(mOnErrorCalled);
         assertNull(mError);
 
+        mResponseStep = ResponseStep.ON_CANCELED;
         mOnCanceledCalled = true;
         openDone();
         maybeThrowCancelOrPause(request);
     }
 
     public void startNextRead(UrlRequest request) {
         startNextRead(request, ByteBuffer.allocateDirect(READ_BUFFER_SIZE));
     }
 
     public void startNextRead(UrlRequest request, ByteBuffer buffer) {
         mBufferPositionBeforeRead = buffer.position();
         request.read(buffer);
     }
 
     public boolean isDone() {
         // It's not mentioned by the Android docs, but block(0) seems to block
         // indefinitely, so have to block for one millisecond to get state
         // without blocking.
         return mDone.block(1);
     }
 
     protected void openDone() {
         mDone.open();
     }
 
     /**
      * Returns {@code false} if the listener should continue to advance the
      * request.
      */
     private boolean maybeThrowCancelOrPause(final UrlRequest request) {

[kapishnikov, 2016/08/31 18:10:27]

Maybe we should add
assertEquals(mExecutorThread, Thread.currentThread())
here just in case if somebody, in the future, calls this method from a method
that does not do this validation. It is crucial for the method to be called on
the executor's thread. It will also make the code analysis easier.

[xunjieli, 2016/08/31 20:03:07]

Done.

         if (mResponseStep != mFailureStep || mFailureType == FailureType.NONE) {
             if (!mAutoAdvance) {
                 mStepBlock.open();
                 return true;
             }
             return false;
         }
 
         if (mFailureType == FailureType.THROW_SYNC) {
+            assertFalse(mListenerExceptionThrown);
+            mListenerExceptionThrown = true;
             throw new IllegalStateException("Listener Exception.");
         }
         Runnable task = new Runnable() {
             public void run() {
                 request.cancel();
             }
         };
         if (mFailureType == FailureType.CANCEL_ASYNC
                 || mFailureType == FailureType.CANCEL_ASYNC_WITHOUT_PAUSE) {
             getExecutor().execute(task);
         } else {
             task.run();
         }
         return mFailureType != FailureType.CANCEL_ASYNC_WITHOUT_PAUSE;
     }
 }