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

components/cronet/android/java/src/org/chromium/net/impl/CronetBidirectionalStream.java

 // Copyright 2015 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.impl;
 
 import org.chromium.base.Log;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
@@ skipping 528 matching lines @@
         postTaskToExecutor(mOnReadCompletedTask);
     }
 
     @SuppressWarnings("unused")
     @CalledByNative
     private void onWritevCompleted(final ByteBuffer[] byteBuffers, int[] initialPositions,
             int[] initialLimits, boolean endOfStream) {
         assert byteBuffers.length == initialPositions.length;
         assert byteBuffers.length == initialLimits.length;
         synchronized (mNativeStreamLock) {
+            if (isDoneLocked()) return;

[mef, 2017/02/14 21:02:47]

Just checking: Does this syntax conform to coding guidelines?

[xunjieli, 2017/02/14 21:14:34]

Yes, this is listed under "Use Standard Brace Style" on
http://source.android.com/source/code-style.html

[mef, 2017/02/14 21:19:54]

Cool, thanks for the reference!

             mWriteState = State.WAITING_FOR_FLUSH;
             // Flush if there is anything in the flush queue mFlushData.
             if (!mFlushData.isEmpty()) {
                 sendFlushDataLocked();
             }
         }
         for (int i = 0; i < byteBuffers.length; i++) {
             ByteBuffer buffer = byteBuffers[i];
             if (buffer.position() != initialPositions[i] || buffer.limit() != initialLimits[i]) {
                 failWithException(new CronetExceptionImpl(
                         "ByteBuffer modified externally during write", null));
                 return;
             }
             // Current implementation always writes the complete buffer.
             buffer.position(buffer.limit());
             postTaskToExecutor(new OnWriteCompletedRunnable(buffer,

[mef, 2017/02/14 21:02:47]

I think if you use direct executor and cancel stream from very first
OnWriteCompleted callback (which will cause native adapter to be set to 0) then
the next completion of nativeWritevData should consistently trigger the issue. 

[xunjieli, 2017/02/14 21:14:34]

Right, but how do we reliably trigger nativeWritevData?

[mef, 2017/02/14 21:19:54]

Would the way you had it in the test (write / flush, write / flush, etc) work?
That way once first write completes there will be another portion of flush data
ready.

                     // Only set endOfStream flag if this buffer is the last in byteBuffers.
                     endOfStream && i == byteBuffers.length - 1));
         }
     }
 
     @SuppressWarnings("unused")
     @CalledByNative
     private void onResponseTrailersReceived(String[] trailers) {
         final UrlResponseInfo.HeaderBlock trailersBlock =
                 new UrlResponseInfoImpl.HeaderBlockImpl(headersListFromStrings(trailers));
@@ skipping 228 matching lines @@
     private native boolean nativeReadData(
             long nativePtr, ByteBuffer byteBuffer, int position, int limit);
 
     @NativeClassQualifiedName("CronetBidirectionalStreamAdapter")
     private native boolean nativeWritevData(long nativePtr, ByteBuffer[] buffers, int[] positions,
             int[] limits, boolean endOfStream);
 
     @NativeClassQualifiedName("CronetBidirectionalStreamAdapter")
     private native void nativeDestroy(long nativePtr, boolean sendOnCanceled);
 }