Implement finished reason and exception for BidirectionalStream

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 68 matching lines @@
 
     private final CronetUrlRequestContext mRequestContext;
     private final Executor mExecutor;
     private final Callback mCallback;
     private final String mInitialUrl;
     private final int mInitialPriority;
     private final String mInitialMethod;
     private final String mRequestHeaders[];
     private final boolean mDelayRequestHeadersUntilFirstFlush;
     private final Collection<Object> mRequestAnnotations;
+    private UrlRequestException mException;
 
     /*
      * Synchronizes access to mNativeStream, mReadState and mWriteState.
      */
     private final Object mNativeStreamLock = new Object();
 
     @GuardedBy("mNativeStreamLock")
     // Pending write data.
     private LinkedList<ByteBuffer> mPendingData;
 
@@ skipping 533 matching lines @@
             long responseStartMs, long requestEndMs, boolean socketReused, long sentBytesCount,
             long receivedBytesCount) {
         synchronized (mNativeStreamLock) {
             if (mMetrics != null) {
                 throw new IllegalStateException("Metrics collection should only happen once.");
             }
             mMetrics = new CronetMetrics(requestStartMs, dnsStartMs, dnsEndMs, connectStartMs,
                     connectEndMs, sslStartMs, sslEndMs, sendingStartMs, sendingEndMs, pushStartMs,
                     pushEndMs, responseStartMs, requestEndMs, socketReused, sentBytesCount,
                     receivedBytesCount);
-            // TODO(xunjieli): Fill this with real values.
-            final RequestFinishedInfo requestFinishedInfo =
-                    new RequestFinishedInfo(mInitialUrl, mRequestAnnotations, mMetrics,
-                            RequestFinishedInfo.SUCCEEDED, mResponseInfo, null);
+            int finishedReason;
+            if (mReadState == State.SUCCESS && mWriteState == State.SUCCESS) {
+                finishedReason = RequestFinishedInfo.SUCCEEDED;
+            } else if (mReadState == State.ERROR && mWriteState == State.ERROR) {
+                finishedReason = RequestFinishedInfo.FAILED;
+            } else if (mReadState == State.CANCELED && mWriteState == State.CANCELED) {
+                finishedReason = RequestFinishedInfo.CANCELED;
+            } else {
+                throw new IllegalStateException("Unexpected state when collecting metrics.");

[xunjieli, 2016/10/17 21:33:43]

Instead of doing a runtime exception that can't be turned off, could you do an
"assert mReadState == mWriteState" at the start, and then do if-conditionals on
|mReadState| ?

[mgersh, 2016/10/17 22:20:45]

Done.

+            }
+            final RequestFinishedInfo requestFinishedInfo = new RequestFinishedInfo(mInitialUrl,
+                    mRequestAnnotations, mMetrics, finishedReason, mResponseInfo, mException);
             mRequestContext.reportFinished(requestFinishedInfo);
         }
     }
 
     @VisibleForTesting
     public void setOnDestroyedCallbackForTesting(Runnable onDestroyedCallbackForTesting) {
         mOnDestroyedCallbackForTesting = onDestroyedCallbackForTesting;
     }
 
     private static boolean doesMethodAllowWriteData(String methodName) {
@@ skipping 75 matching lines @@
         mNativeStream = 0;
         if (mOnDestroyedCallbackForTesting != null) {
             mOnDestroyedCallbackForTesting.run();
         }
     }
 
     /**
      * Fails the stream with an exception. Only called on the Executor.
      */
     private void failWithExceptionOnExecutor(CronetException e) {
+        mException = e;
         // Do not call into mCallback if request is complete.
         synchronized (mNativeStreamLock) {
             if (isDoneLocked()) {
                 return;
             }
             mReadState = mWriteState = State.ERROR;
             destroyNativeStreamLocked(false);
         }
         try {
             mCallback.onFailed(this, mResponseInfo, e);
@@ skipping 41 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);
 }