Don't crash on InputStream exceptions in the android_webview.

The InputStream methods can, under normal operation, throw an IOException.
If such a stream is being read by the WebView it will crash. This change
addresses that by introducing a wrapper class that deals with the exceptions.

BUG=157880

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@286214 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 180 insertions, 23 deletions

diff --git a/android_webview/java/src/org/chromium/android_webview/InputStreamUtil.java b/android_webview/java/src/org/chromium/android_webview/InputStreamUtil.java
new file mode 100644
index 0000000..d4fb749
--- /dev/null
+++ b/android_webview/java/src/org/chromium/android_webview/InputStreamUtil.java
@@ -0,0 +1,70 @@
+// 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.android_webview;
+
+import android.util.Log;
+
+import org.chromium.base.CalledByNative;
+import org.chromium.base.JNINamespace;
+
+import java.io.IOException;
+import java.io.InputStream;
+
+/**
+ * Utility methods for calling InputStream methods. These take care of exception handling.
+ */
+@JNINamespace("android_webview")
+class InputStreamUtil {
+    private static final String LOGTAG = "AwAssets";
+    // The InputStream APIs return -1 in some cases. In order to convey the extra information that
+    // the call had failed due to an exception being thrown we simply map all negative return values
+    // from the original calls to -1 and make -2 mean that an exception has been thrown.
+    private static final int CALL_FAILED_STATUS = -1;
+    private static final int EXCEPTION_THROWN_STATUS = -2;
+
+    private static String logMessage(String method) {
+        return "Got exception when calling " + method + "() on an InputStream returned from " +
+            "shouldInterceptRequest. This will cause the related request to fail.";
+    }
+
+    @CalledByNative
+    public static void close(InputStream stream) {
+        try {
+            stream.close();
+        } catch (IOException e) {
+            Log.e(LOGTAG, logMessage("close"), e);
+        }
+    }
+
+    @CalledByNative
+    public static int available(InputStream stream) {
+        try {
+            return Math.max(CALL_FAILED_STATUS, stream.available());
+        } catch (IOException e) {
+            Log.e(LOGTAG, logMessage("available"), e);
+            return EXCEPTION_THROWN_STATUS;
+        }
+    }
+
+    @CalledByNative
+    public static int read(InputStream stream, byte[] b, int off, int len) {
+        try {
+            return Math.max(CALL_FAILED_STATUS, stream.read(b, off, len));
+        } catch (IOException e) {
+            Log.e(LOGTAG, logMessage("read"), e);
+            return EXCEPTION_THROWN_STATUS;
+        }
+    }
+
+    @CalledByNative
+    public static long skip(InputStream stream, long n) {
+        try {
+            return Math.max(CALL_FAILED_STATUS, stream.skip(n));
+        } catch (IOException e) {
+            Log.e(LOGTAG, logMessage("skip"), e);
+            return EXCEPTION_THROWN_STATUS;
+        }
+    }
+}
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldInterceptRequestTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldInterceptRequestTest.java
index 34c0511..de8e625 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldInterceptRequestTest.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldInterceptRequestTest.java
@@ -355,6 +355,49 @@ public class AwContentsClientShouldInterceptRequestTest extends AwTestBase {
         mContentsClient.getOnPageFinishedHelper().waitForCallback(onPageFinishedCallCount);
     }
 
+    private static class ThrowingInputStream extends EmptyInputStream {
+        @Override
+        public int available() {
+            return 100;
+        }
+
+        @Override
+        public int read() throws IOException {
+            throw new IOException("test exception");
+        }
+
+        @Override
+        public int read(byte b[]) throws IOException {
+            throw new IOException("test exception");
+        }
+
+        @Override
+        public int read(byte b[], int off, int len) throws IOException {
+            throw new IOException("test exception");
+        }
+
+        @Override
+        public long skip(long n) throws IOException {
+            return n;
+        }
+    }
+
+    @SmallTest
+    @Feature({"AndroidWebView"})
+    public void testDoesNotCrashOnThrowingStream() throws Throwable {
+        final String aboutPageUrl = addAboutPageToTestServer(mWebServer);
+
+        mShouldInterceptRequestHelper.setReturnValue(
+                new AwWebResourceResponse("text/html", "UTF-8", new ThrowingInputStream()));
+        int shouldInterceptRequestCallCount = mShouldInterceptRequestHelper.getCallCount();
+        int onPageFinishedCallCount = mContentsClient.getOnPageFinishedHelper().getCallCount();
+
+        loadUrlAsync(mAwContents, aboutPageUrl);
+
+        mShouldInterceptRequestHelper.waitForCallback(shouldInterceptRequestCallCount);
+        mContentsClient.getOnPageFinishedHelper().waitForCallback(onPageFinishedCallCount);
+    }
+
     private static class SlowAwWebResourceResponse extends AwWebResourceResponse {
         private CallbackHelper mReadStartedCallbackHelper = new CallbackHelper();
         private CountDownLatch mLatch = new CountDownLatch(1);
diff --git a/android_webview/native/input_stream_impl.cc b/android_webview/native/input_stream_impl.cc
index fca6a29..ef5c4f3 100644
--- a/android_webview/native/input_stream_impl.cc
+++ b/android_webview/native/input_stream_impl.cc
@@ -10,22 +10,25 @@
 // even if they're unused.
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wunused-function"
-#include "jni/InputStream_jni.h"
+#include "jni/InputStreamUtil_jni.h"
 #pragma GCC diagnostic pop
 #include "net/base/io_buffer.h"
 
 using base::android::AttachCurrentThread;
 using base::android::ClearException;
 using base::android::JavaRef;
-using JNI_InputStream::Java_InputStream_available;
-using JNI_InputStream::Java_InputStream_close;
-using JNI_InputStream::Java_InputStream_skip;
-using JNI_InputStream::Java_InputStream_readI_AB_I_I;
 
 namespace android_webview {
 
+namespace {
+
+// This should be the same as InputStramUtil.EXCEPTION_THROWN_STATUS.
+const int kExceptionThrownStatusCode = -2;
+
+}
+
 bool RegisterInputStream(JNIEnv* env) {
-  return JNI_InputStream::RegisterNativesImpl(env);
+  return RegisterNativesImpl(env);
 }
 
 // Maximum number of bytes to be read in a single read.
@@ -50,13 +53,13 @@ InputStreamImpl::InputStreamImpl(const JavaRef<jobject>& stream)
 
 InputStreamImpl::~InputStreamImpl() {
   JNIEnv* env = AttachCurrentThread();
-  Java_InputStream_close(env, jobject_.obj());
+  Java_InputStreamUtil_close(env, jobject_.obj());
 }
 
 bool InputStreamImpl::BytesAvailable(int* bytes_available) const {
   JNIEnv* env = AttachCurrentThread();
-  int bytes = Java_InputStream_available(env, jobject_.obj());
-  if (ClearException(env))
+  int bytes = Java_InputStreamUtil_available(env, jobject_.obj());
+  if (bytes == kExceptionThrownStatusCode)
     return false;
   *bytes_available = bytes;
   return true;
@@ -64,8 +67,8 @@ bool InputStreamImpl::BytesAvailable(int* bytes_available) const {
 
 bool InputStreamImpl::Skip(int64_t n, int64_t* bytes_skipped) {
   JNIEnv* env = AttachCurrentThread();
-  int bytes = Java_InputStream_skip(env, jobject_.obj(), n);
-  if (ClearException(env))
+  int bytes = Java_InputStreamUtil_skip(env, jobject_.obj(), n);
+  if (bytes < 0)
     return false;
   if (bytes > n)
     return false;
@@ -91,9 +94,9 @@ bool InputStreamImpl::Read(net::IOBuffer* dest, int length, int* bytes_read) {
 
   while (remaining_length > 0) {
     const int max_transfer_length = std::min(remaining_length, kBufferSize);
-    const int transfer_length = Java_InputStream_readI_AB_I_I(
+    const int transfer_length = Java_InputStreamUtil_read(
         env, jobject_.obj(), buffer, 0, max_transfer_length);
-    if (ClearException(env))
+    if (transfer_length == kExceptionThrownStatusCode)
       return false;
 
     if (transfer_length < 0)  // EOF
diff --git a/android_webview/native/input_stream_unittest.cc b/android_webview/native/input_stream_unittest.cc
index 76ca823..ec4faf6 100644
--- a/android_webview/native/input_stream_unittest.cc
+++ b/android_webview/native/input_stream_unittest.cc
@@ -119,3 +119,27 @@ TEST_F(InputStreamTest, ReadLargeStreamCompletely) {
   DoReadCountedStreamTest(bytes_requested, bytes_requested, &bytes_read);
   EXPECT_EQ(bytes_requested, bytes_read);
 }
+
+TEST_F(InputStreamTest, DoesNotCrashWhenExceptionThrown) {
+  ScopedJavaLocalRef<jobject> throw_jstream =
+      Java_InputStreamUnittest_getThrowingStream(env_);
+  EXPECT_FALSE(throw_jstream.is_null());
+
+  scoped_ptr<InputStream> input_stream(new InputStreamImpl(throw_jstream));
+
+  int64_t bytes_skipped;
+  EXPECT_FALSE(input_stream->Skip(10, &bytes_skipped));
+
+  int bytes_available;
+  EXPECT_FALSE(input_stream->BytesAvailable(&bytes_available));
+
+
+  const int bytes_requested = 10;
+  int bytes_read = 0;
+  scoped_refptr<IOBuffer> buffer = new IOBuffer(bytes_requested);
+  EXPECT_FALSE(input_stream->Read(buffer.get(), bytes_requested, &bytes_read));
+  EXPECT_EQ(0, bytes_read);
+
+  // This closes the stream.
+  input_stream.reset(NULL);
+}
diff --git a/android_webview/native/webview_native.gyp b/android_webview/native/webview_native.gyp
index 2e965e6..f8e836a 100644
--- a/android_webview/native/webview_native.gyp
+++ b/android_webview/native/webview_native.gyp
@@ -105,15 +105,6 @@
       ],
     },
     {
-      'target_name': 'input_stream_android_jar_jni_headers',
-      'type': 'none',
-      'variables': {
-        'jni_gen_package': 'android_webview',
-        'input_java_class': 'java/io/InputStream.class',
-      },
-      'includes': [ '../../build/jar_file_jni_generator.gypi' ],
-    },
-    {
       'target_name': 'cancellation_signal_android_jar_jni_headers',
       'type': 'none',
       'variables': {
@@ -145,6 +136,7 @@
           '../java/src/org/chromium/android_webview/AwWebContentsDelegate.java',
           '../java/src/org/chromium/android_webview/AwWebResourceResponse.java',
           '../java/src/org/chromium/android_webview/ExternalVideoSurfaceContainer.java',
+          '../java/src/org/chromium/android_webview/InputStreamUtil.java',
           '../java/src/org/chromium/android_webview/JavaBrowserViewRendererHelper.java',
           '../java/src/org/chromium/android_webview/permission/AwPermissionRequest.java',
       ],
@@ -153,7 +145,6 @@
       },
       'includes': [ '../../build/jni_generator.gypi' ],
       'dependencies': [
-        'input_stream_android_jar_jni_headers',
         'cancellation_signal_android_jar_jni_headers',
       ],
     },
diff --git a/android_webview/unittestjava/src/org/chromium/android_webview/unittest/InputStreamUnittest.java b/android_webview/unittestjava/src/org/chromium/android_webview/unittest/InputStreamUnittest.java
index 7610942..f6bbaa1 100644
--- a/android_webview/unittestjava/src/org/chromium/android_webview/unittest/InputStreamUnittest.java
+++ b/android_webview/unittestjava/src/org/chromium/android_webview/unittest/InputStreamUnittest.java
@@ -6,6 +6,7 @@ package org.chromium.android_webview.unittest;
 
 import org.chromium.base.CalledByNative;
 
+import java.io.IOException;
 import java.io.InputStream;
 
 class InputStreamUnittest {
@@ -23,6 +24,31 @@ class InputStreamUnittest {
     }
 
     @CalledByNative
+    static InputStream getThrowingStream() {
+        return new InputStream() {
+            @Override
+            public int available() throws IOException {
+                throw new IOException();
+            }
+
+            @Override
+            public void close() throws IOException {
+                throw new IOException();
+            }
+
+            @Override
+            public long skip(long n) throws IOException {
+                throw new IOException();
+            }
+
+            @Override
+            public int read() throws IOException {
+                throw new IOException();
+            }
+        };
+    }
+
+    @CalledByNative
     static InputStream getCountingStream(final int size) {
         return new InputStream() {
             private int count = 0;