[Android WebView] Close InputStreams when we're done with them.

android_webview/native/input_stream_impl.cc

 // Copyright (c) 2012 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.
 
 #include "android_webview/native/input_stream_impl.h"
 
 #include "base/android/jni_android.h"
 // Disable "Warnings treated as errors" for input_stream_jni as it's a Java
 // system class and we have to generate C++ hooks for all methods in the class
 // even if they're unused.
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wunused-function"
 #include "jni/InputStream_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 {
 
 bool RegisterInputStream(JNIEnv* env) {
   return JNI_InputStream::RegisterNativesImpl(env);
 }
 
 // Maximum number of bytes to be read in a single read.
 const int InputStreamImpl::kBufferSize = 4096;
 
 //static
 const InputStreamImpl* InputStreamImpl::FromInputStream(
         const InputStream* input_stream) {
     return static_cast<const InputStreamImpl*>(input_stream);
 }
 
 InputStreamImpl::InputStreamImpl() {
 }
 
 InputStreamImpl::InputStreamImpl(const JavaRef<jobject>& stream)
     : jobject_(stream) {
   DCHECK(!stream.is_null());
 }
 
 InputStreamImpl::~InputStreamImpl() {
+  JNIEnv* env = AttachCurrentThread();
+  Java_InputStream_close(env, jobject_.obj());
+  DCHECK(!ClearException(env));

[joth, 2013/01/03 19:18:18]

the ClearException will be removed in release build, so it's better to just call
CheckException() unconditionally.

[mkosiba (inactive), 2013/01/04 12:31:11]

uh, actually due to the way the JNI methods are auto-generated the
Java_InputStream_close will CHECK(false) if we get an exception back.

The bug is crbug.com/157880.

[benm (inactive), 2013/01/04 12:55:11]

I see. So right now in the exception case we'll never hit this; but when that
bug is fixed we would still want to call CheckException() here?

[mkosiba (inactive), 2013/01/04 14:41:01]

When the bug is fixed we'll have an unsafe version of the method
(Java_InputStream_unsafe_close ?) which we would have to use instead in order to
have the ClearException code be reachable.
I'd suggest to file a bug to track this.

[benm (inactive), 2013/01/04 16:59:40]

Gotcha. Follow up in https://codereview.chromium.org/11748032/.

 }
 
 bool InputStreamImpl::BytesAvailable(int* bytes_available) const {
   JNIEnv* env = AttachCurrentThread();
   // TODO: Use unsafe version for all Java_InputStream methods in this file
   // once BUG 157880 is fixed.
   int bytes = Java_InputStream_available(env, jobject_.obj());
   if (ClearException(env))
     return false;
   *bytes_available = bytes;
@@ skipping 57 matching lines @@
   env->GetByteArrayRegion(buffer, 0, byte_count,
       reinterpret_cast<jbyte*>(dest->data() + *bytes_read));
   if (ClearException(env))
     return false;
 
   *bytes_read = byte_count;
   return true;
 }
 
 } // namespace android_webview