Catch and report exceptions in CalledByNative java methods.

BUG=403515

Review URL: https://codereview.chromium.org/474573003

Cr-Commit-Position: refs/heads/master@{#289971}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@289971 0039d316-1c4b-4281-b951-d872f2087c98

components/cronet/android/java/src/org/chromium/net/ChromiumUrlRequest.java

@@ -4,6 +4,8 @@
 
 package org.chromium.net;
 
+import android.util.Log;
+
 import org.apache.http.conn.ConnectTimeoutException;
 import org.chromium.base.CalledByNative;
 import org.chromium.base.JNINamespace;
@@ -400,42 +402,64 @@ public class ChromiumUrlRequest implements HttpUrlRequest {
 
     // Private methods called by native library.
 
+    /**
+     * If @CalledByNative method throws an exception, request gets cancelled
+     * and exception could be retrieved from request using getException().
+     */
+    private void onCalledByNativeException(Exception e) {
+        mSinkException = new IOException(
+                "CalledByNative method has thrown an exception", e);
+        Log.e(ChromiumUrlRequestContext.LOG_TAG,
+                "Exception in CalledByNative method", e);
+        try {
+            cancel();
+        } catch (Exception cancel_exception) {
+            Log.e(ChromiumUrlRequestContext.LOG_TAG,
+                    "Exception trying to cancel request", cancel_exception);
+        }
+    }
+
     /**
      * A callback invoked when the first chunk of the response has arrived.
      */
     @CalledByNative
     private void onResponseStarted() {
-        mContentType = nativeGetContentType(mUrlRequestAdapter);
-        mContentLength = nativeGetContentLength(mUrlRequestAdapter);
-        mHeadersAvailable = true;
-
-        if (mContentLengthLimit > 0 && mContentLength > mContentLengthLimit
-                && mCancelIfContentLengthOverLimit) {
-            onContentLengthOverLimit();
-            return;
-        }
+        try {
+            mContentType = nativeGetContentType(mUrlRequestAdapter);
+            mContentLength = nativeGetContentLength(mUrlRequestAdapter);
+            mHeadersAvailable = true;
+
+            if (mContentLengthLimit > 0 &&
+                    mContentLength > mContentLengthLimit &&
+                    mCancelIfContentLengthOverLimit) {
+                onContentLengthOverLimit();
+                    return;
+            }
 
-        if (mBufferFullResponse && mContentLength != -1
-                && !mContentLengthOverLimit) {
-            ((ChunkedWritableByteChannel)getSink()).setCapacity(
-                    (int)mContentLength);
-        }
+            if (mBufferFullResponse && mContentLength != -1 &&
+                    !mContentLengthOverLimit) {
+                ((ChunkedWritableByteChannel)getSink()).setCapacity(
+                        (int)mContentLength);
+            }
 
-        if (mOffset != 0) {
-            // The server may ignore the request for a byte range, in which case
-            // status code will be 200, instead of 206. Note that we cannot call
-            // getHttpStatusCode as it rewrites 206 into 200.
-            if (nativeGetHttpStatusCode(mUrlRequestAdapter) == 200) {
-                // TODO(mef): Revisit this logic.
-                if (mContentLength != -1) {
-                    mContentLength -= mOffset;
+            if (mOffset != 0) {
+                // The server may ignore the request for a byte range, in which
+                // case status code will be 200, instead of 206. Note that we
+                // cannot call getHttpStatusCode as it rewrites 206 into 200.
+                if (nativeGetHttpStatusCode(mUrlRequestAdapter) == 200) {
+                    // TODO(mef): Revisit this logic.
+                    if (mContentLength != -1) {
+                        mContentLength -= mOffset;
+                    }
+                    mSkippingToOffset = true;
+                } else {
+                    mSize = mOffset;
                 }
-                mSkippingToOffset = true;
-            } else {
-                mSize = mOffset;
             }
+            mListener.onResponseStarted(this);
+        } catch (Exception e) {
+            onCalledByNativeException(e);
         }
-        mListener.onResponseStarted(this);
     }
 
     /**
@@ -447,37 +471,36 @@ public class ChromiumUrlRequest implements HttpUrlRequest {
      */
     @CalledByNative
     private void onBytesRead(ByteBuffer buffer) {
-        if (mContentLengthOverLimit) {
-            return;
-        }
-
-        int size = buffer.remaining();
-        mSize += size;
-        if (mSkippingToOffset) {
-            if (mSize <= mOffset) {
+        try {
+            if (mContentLengthOverLimit) {
                 return;
-            } else {
-                mSkippingToOffset = false;
-                buffer.position((int)(mOffset - (mSize - size)));
             }
-        }
 
-        boolean contentLengthOverLimit =
-                (mContentLengthLimit != 0 && mSize > mContentLengthLimit);
-        if (contentLengthOverLimit) {
-            buffer.limit(size - (int)(mSize - mContentLengthLimit));
-        }
+            int size = buffer.remaining();
+            mSize += size;
+            if (mSkippingToOffset) {
+                if (mSize <= mOffset) {
+                    return;
+                } else {
+                    mSkippingToOffset = false;
+                    buffer.position((int)(mOffset - (mSize - size)));
+                }
+            }
+
+            boolean contentLengthOverLimit =
+                    (mContentLengthLimit != 0 && mSize > mContentLengthLimit);
+            if (contentLengthOverLimit) {
+                buffer.limit(size - (int)(mSize - mContentLengthLimit));
+            }
 
-        try {
             while (buffer.hasRemaining()) {
                 mSink.write(buffer);
             }
-        } catch (IOException e) {
-            mSinkException = e;
-            cancel();
-        }
-        if (contentLengthOverLimit) {
-            onContentLengthOverLimit();
+            if (contentLengthOverLimit) {
+                onContentLengthOverLimit();
+            }
+        } catch (Exception e) {
+            onCalledByNativeException(e);
         }
     }
 
@@ -487,21 +510,32 @@ public class ChromiumUrlRequest implements HttpUrlRequest {
     @SuppressWarnings("unused")
     @CalledByNative
     private void finish() {
-        synchronized (mLock) {
-            mFinished = true;
+        try {
+            synchronized (mLock) {
+                mFinished = true;
 
-            if (mRecycled) {
-                return;
-            }
-            try {
-                mSink.close();
-            } catch (IOException e) {
-                // Ignore
+                if (mRecycled) {
+                    return;
+                }
+                try {
+                    mSink.close();
+                } catch (IOException e) {
+                    // Ignore
+                }
+                try {
+                    if (mUploadChannel != null && mUploadChannel.isOpen()) {
+                        mUploadChannel.close();
+                    }
+                } catch (IOException e) {
+                    // Ignore
+                }
+                onRequestComplete();
+                nativeDestroyRequestAdapter(mUrlRequestAdapter);
+                mUrlRequestAdapter = 0;
+                mRecycled = true;
             }
-            onRequestComplete();
-            nativeDestroyRequestAdapter(mUrlRequestAdapter);
-            mUrlRequestAdapter = 0;
-            mRecycled = true;
+        } catch (Exception e) {
+            mSinkException = new IOException("Exception in finish", e);
         }
     }
 
@@ -512,10 +546,14 @@ public class ChromiumUrlRequest implements HttpUrlRequest {
     @CalledByNative
     private void onAppendResponseHeader(ResponseHeadersMap headersMap,
             String name, String value) {
-        if (!headersMap.containsKey(name)) {
-            headersMap.put(name, new ArrayList<String>());
+        try {
+            if (!headersMap.containsKey(name)) {
+                headersMap.put(name, new ArrayList<String>());
+            }
+            headersMap.get(name).add(value);
+        } catch (Exception e) {
+            onCalledByNativeException(e);
         }
-        headersMap.get(name).add(value);
     }
 
     /**
@@ -527,25 +565,19 @@ public class ChromiumUrlRequest implements HttpUrlRequest {
     @SuppressWarnings("unused")
     @CalledByNative
     private int readFromUploadChannel(ByteBuffer dest) {
-        if (mUploadChannel == null || !mUploadChannel.isOpen())
-            return -1;
         try {
+            if (mUploadChannel == null || !mUploadChannel.isOpen())
+                return -1;
             int result = mUploadChannel.read(dest);
             if (result < 0) {
                 mUploadChannel.close();
                 return 0;
             }
             return result;
-        } catch (IOException e) {
-            mSinkException = e;
-            try {
-                mUploadChannel.close();
-            } catch (IOException ignored) {
-                // Ignore this exception.
-            }
-            cancel();
-            return -1;
+        } catch (Exception e) {
+            onCalledByNativeException(e);
         }
+        return -1;
     }
 
     // Native methods are implemented in chromium_url_request.cc.

components/cronet/android/java/src/org/chromium/net/ChromiumUrlRequestContext.java

@@ -20,7 +20,7 @@ public class ChromiumUrlRequestContext {
     private static final int LOG_NONE = 3;  // LOG(FATAL), no VLOG.
     private static final int LOG_DEBUG = -1;  // LOG(FATAL...INFO), VLOG(1)
     private static final int LOG_VERBOSE = -2;  // LOG(FATAL...INFO), VLOG(2)
-    private static final String LOG_TAG = "ChromiumNetwork";
+    static final String LOG_TAG = "ChromiumNetwork";
 
     /**
      * Native adapter object, owned by ChromiumUrlRequestContext.

components/cronet/android/sample/javatests/src/org/chromium/cronet_sample_apk/CronetSampleUrlTest.java

@@ -4,12 +4,17 @@
 
 package org.chromium.cronet_sample_apk;
 
+import android.os.ConditionVariable;
+
 import android.test.suitebuilder.annotation.SmallTest;
 
 import org.chromium.base.test.util.Feature;
+import org.chromium.net.HttpUrlRequest;
 import org.chromium.net.HttpUrlRequestFactoryConfig;
+import org.chromium.net.HttpUrlRequestListener;
 
 import java.io.File;
+import java.util.HashMap;
 
 /**
  * Example test that just starts the cronet sample.
@@ -89,6 +94,53 @@ public class CronetSampleUrlTest extends CronetSampleTestBase {
         assertTrue(!file.exists());
     }
 
+    class BadHttpUrlRequestListener implements HttpUrlRequestListener {
+        static final String THROW_TAG = "BadListener";
+        ConditionVariable mComplete = new ConditionVariable();
+
+        public BadHttpUrlRequestListener() {
+        }
+
+        @Override
+        public void onResponseStarted(HttpUrlRequest request) {
+            throw new NullPointerException(THROW_TAG);
+        }
+
+        @Override
+        public void onRequestComplete(HttpUrlRequest request) {
+            mComplete.open();
+            throw new NullPointerException(THROW_TAG);
+        }
+
+        public void blockForComplete() {
+            mComplete.block();
+        }
+    }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testCalledByNativeException() throws Exception {
+        CronetSampleActivity activity = launchCronetSampleWithUrl(URL);
+
+        // Make sure the activity was created as expected.
+        assertNotNull(activity);
+
+        waitForActiveShellToBeDoneLoading();
+
+        HashMap<String, String> headers = new HashMap<String, String>();
+        BadHttpUrlRequestListener listener = new BadHttpUrlRequestListener();
+
+        // Create request with null listener to trigger an exception.
+        HttpUrlRequest request = activity.mChromiumRequestFactory.createRequest(
+                URL, HttpUrlRequest.REQUEST_PRIORITY_MEDIUM, headers, listener);
+        request.start();
+        listener.blockForComplete();
+        assertTrue(request.isCanceled());
+        assertNotNull(request.getException());
+        assertEquals(listener.THROW_TAG, request.getException().getCause().getMessage());
+
+    }
+
     @SmallTest
     @Feature({"Cronet"})
     public void testLegacyLoadUrl() throws Exception {