Fix CronetUrlRequestTest#testFailures flake

components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestTest.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 android.test.MoreAsserts;
 import android.test.suitebuilder.annotation.SmallTest;
@@ skipping 1661 matching lines @@
         assertEquals(1, callback.mRedirectCount);
         if (failureType == FailureType.CANCEL_SYNC || failureType == FailureType.CANCEL_ASYNC) {
             assertResponseStepCanceled(callback);
         } else if (failureType == FailureType.THROW_SYNC) {
             assertEquals(ResponseStep.ON_FAILED, callback.mResponseStep);
         }
         assertTrue(urlRequest.isDone());
         assertEquals(expectResponseInfo, callback.mResponseInfo != null);
         assertEquals(expectError, callback.mError != null);
         assertEquals(expectError, callback.mOnErrorCalled);
-        assertEquals(failureType == FailureType.CANCEL_SYNC
-                        || failureType == FailureType.CANCEL_ASYNC
-                        || failureType == FailureType.CANCEL_ASYNC_WITHOUT_PAUSE,
+        assertEquals(
+                failureType == FailureType.CANCEL_SYNC || failureType == FailureType.CANCEL_ASYNC,
                 callback.mOnCanceledCalled);
+        // When failureType is FailureType.CANCEL_ASYNC_WITHOUT_PAUSE, there
+        // might be an onSucceeded() task already posted. If that's the case,
+        // onCanceled() will not be invoked. See crbug.com/657415.

[pauljensen, 2016/12/14 14:40:31]

I'm confused, we waited for the callback to get either onSucceeded, onFailed or
onCanceled, as the call to callback.blockForDone() ensured.  In the case of the
flakes all of our asserts passed except for this one, which means onFailed was
not called (as per the assertion on line 1681), and which means onSucceeded was
not called (as per the assertion on line 1679) and onCanceled was not called (as
per the assertion on line 1682).  So how can onSucceeded have been called and
not called?

[pauljensen, 2016/12/14 15:17:32]

I was confused because mResponseInfo is set in onResponseStarted as well as
onSucceeded.

[xunjieli, 2016/12/14 15:35:39]

After talking to you, I realize that we should only skip the case of canceling
async without pause during onReadCompleted(). The rest two
CANCEL_ASYNC_WITHOUT_PAUSEs should still have |mOnCanceledCalled| as true.
Hope it's slightly clearer this way.

     }
 
     @SmallTest
     @Feature({"Cronet"})
     public void testFailures() throws Exception {
         throwOrCancel(FailureType.CANCEL_SYNC, ResponseStep.ON_RECEIVED_REDIRECT,
                 false, false);
         throwOrCancel(FailureType.CANCEL_ASYNC, ResponseStep.ON_RECEIVED_REDIRECT,
                 false, false);
         throwOrCancel(FailureType.CANCEL_ASYNC_WITHOUT_PAUSE, ResponseStep.ON_RECEIVED_REDIRECT,
@@ skipping 385 matching lines @@
     }
 
     private void assertResponseStepCanceled(TestUrlRequestCallback callback) {
         if (callback.mResponseStep == ResponseStep.ON_FAILED && callback.mError != null) {
             throw new Error(
                     "Unexpected response state: " + ResponseStep.ON_FAILED, callback.mError);
         }
         assertEquals(ResponseStep.ON_CANCELED, callback.mResponseStep);
     }
 }