[Cronet] Add isDone() check in CronetBidirectionalStream#onWritevCompleted

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

Index: components/cronet/android/test/javatests/src/org/chromium/net/BidirectionalStreamTest.java
diff --git a/components/cronet/android/test/javatests/src/org/chromium/net/BidirectionalStreamTest.java b/components/cronet/android/test/javatests/src/org/chromium/net/BidirectionalStreamTest.java
index cd0f40a06b5b64ca3a81e415f4cc095fc9262c75..b981103825eeb253cf5fb7e3e86e44023cbbb38c 100644
--- a/components/cronet/android/test/javatests/src/org/chromium/net/BidirectionalStreamTest.java
+++ b/components/cronet/android/test/javatests/src/org/chromium/net/BidirectionalStreamTest.java
@@ -400,6 +400,58 @@ public class BidirectionalStreamTest extends CronetTestBase {
     @SmallTest
     @Feature({"Cronet"})
     @OnlyRunNativeCronet
+    // Regression test for crbug.com/692168.
+    public void testCancelWhileWriteDataPending() throws Exception {

[xunjieli, 2017/02/15 15:51:43]

Misha, so I came up with this test where we cancel in onReadCompleted() when
there are writes pending. This test crashes with the same callstack almost 100%
of the times locally (though it didn't crash on the bot).

In order to receive onWritevCompleted() after an async cancel(), we need to keep
the send side busy. I achieved so by continuously writing in onWriteCompleted()
user callback without a FIN flag. This is much more reliably than doing a
predefined number of writes. Here we are more confident because when the first
Callback#onReadCompleted() is received, there will be additional native
onWritevCompleted()s.

[mef, 2017/02/15 18:20:14]

Great, thanks for making this work!

+        String url = Http2TestServer.getEchoStreamUrl();
+        // Use a direct executor to avoid race.
+        TestBidirectionalStreamCallback callback = new TestBidirectionalStreamCallback(
+                /*useDirectExecutor*/ true) {
+            @Override
+            public void onStreamReady(BidirectionalStream stream) {
+                // Start the first write.
+                stream.write(getDummyData(), false);
+                stream.flush();
+            }
+            @Override
+            public void onReadCompleted(BidirectionalStream stream, UrlResponseInfo info,
+                    ByteBuffer byteBuffer, boolean endOfStream) {
+                super.onReadCompleted(stream, info, byteBuffer, endOfStream);
+                // Cancel now when the write side is busy.
+                stream.cancel();

[mef, 2017/02/15 18:20:14]

Nice, this is happening while write is in flight.

+            }
+            @Override
+            public void onWriteCompleted(BidirectionalStream stream, UrlResponseInfo info,
+                    ByteBuffer buffer, boolean endOfStream) {
+                // Flush twice to keep the flush queue non-empty.
+                stream.write(getDummyData(), false);
+                stream.flush();
+                stream.write(getDummyData(), false);
+                stream.flush();
+            }
+            // Returns a piece of dummy data to send to the server.
+            private ByteBuffer getDummyData() {
+                byte[] data = new byte[100];
+                for (int i = 0; i < data.length; i++) {
+                    data[i] = 'x';
+                }
+                ByteBuffer dummyData = ByteBuffer.allocateDirect(data.length);

[mef, 2017/02/15 18:20:14]

nit: I think you can do dummyData.put('x') in the loop above and skip data.

[xunjieli, 2017/02/15 20:32:46]

If I put a single byte each time, flip() will only flip the last put() right?
If so, I need to update dummyData's position and limit. That is probably less
readable?

[xunjieli, 2017/02/15 21:32:12]

Sorry, my bad. flip() does reset the position to 0.
Locally it complains that "error: no suitable method found for put(char)" for
dummyData.put('x'). I need to cast 'x' to byte. 
I will keep the intermediate step for readability. Thanks!

+                dummyData.put(data);
+                dummyData.flip();
+                return dummyData;
+            }
+        };
+        CronetBidirectionalStream stream =
+                (CronetBidirectionalStream) mTestFramework.mCronetEngine
+                        .newBidirectionalStreamBuilder(url, callback, callback.getExecutor())
+                        .build();
+        stream.start();
+        callback.blockForDone();
+        assertTrue(callback.mOnCanceledCalled);
+    }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    @OnlyRunNativeCronet
     public void testSimpleGetWithFlush() throws Exception {
         // TODO(xunjieli): Use ParameterizedTest instead of the loop.
         for (int i = 0; i < 2; i++) {