3 files changed, 60 insertions, 50 deletions
diff --git a/android_webview/browser/input_stream.h b/android_webview/browser/input_stream.h
index 8ef4765..9205a12 100644
--- a/android_webview/browser/input_stream.h
+++ b/android_webview/browser/input_stream.h
@@ -34,9 +34,9 @@ class InputStream {
   // thrown.
   virtual bool Skip(int64_t n, int64_t* bytes_skipped) = 0;
 
-  // Reads |length| bytes into |dest|. Sets |bytes_read| to the total number of
-  // bytes read into |dest| or -1 if there is no more data because the end of
-  // the stream was reached.
+  // Reads at most |length| bytes into |dest|. Sets |bytes_read| to the total
+  // number of bytes read into |dest| or 0 if there is no more data because the
+  // end of the stream was reached.
   // |dest| must be at least |length| in size.
   // Returns true if completed successfully or false if an exception was
   // thrown.
diff --git a/android_webview/native/input_stream_impl.cc b/android_webview/native/input_stream_impl.cc
index 2647c7f..5838650 100644
--- a/android_webview/native/input_stream_impl.cc
+++ b/android_webview/native/input_stream_impl.cc
@@ -81,40 +81,43 @@ bool InputStreamImpl::Read(net::IOBuffer* dest, int length, int* bytes_read) {
   jbyteArray buffer = buffer_.obj();
   *bytes_read = 0;
 
-  while (length > 0) {
-    const int read_size = std::min(length, kBufferSize);
-    int32_t byte_count =
-      Java_InputStream_readI_AB_I_I(
-          env, jobject_.obj(), buffer, 0, read_size);
-
+  const int read_size = std::min(length, kBufferSize);
+  int32_t byte_count;
+  do {
+    // Unfortunately it is valid for the Java InputStream to read 0 bytes some
+    // number of times before returning any more data. Because this method
+    // signals EOF by setting |bytes_read| to 0 and returning true necessary to
+    // call the Java-side read method until it returns something other than 0.
+    byte_count = Java_InputStream_readI_AB_I_I(
+        env, jobject_.obj(), buffer, 0, read_size);
     if (ClearException(env))
       return false;
+  } while (byte_count == 0);
 
-    if (byte_count <= 0)
-      break;
+  // We've reached the end of the stream.
+  if (byte_count < 0)
+    return true;
 
 #ifndef NDEBUG
-    int32_t buffer_length = env->GetArrayLength(buffer);
-    DCHECK_GE(read_size, byte_count);
-    DCHECK_GE(buffer_length, byte_count);
+  int32_t buffer_length = env->GetArrayLength(buffer);
+  DCHECK_GE(read_size, byte_count);
+  DCHECK_GE(buffer_length, byte_count);
 #endif // NDEBUG
 
-    // The DCHECKs are in place to help Chromium developers in case of bugs,
-    // this check is to prevent a malicious InputStream implementation from
-    // overrunning the |dest| buffer.
-    if (byte_count > read_size)
-      return false;
+  // The DCHECKs are in place to help Chromium developers in case of bugs,
+  // this check is to prevent a malicious InputStream implementation from
+  // overrunning the |dest| buffer.
+  if (byte_count > read_size)
+    return false;
 
-    // Copy the data over to the provided C++ side buffer.
-    DCHECK_GE(length, byte_count);
-    env->GetByteArrayRegion(buffer, 0, byte_count,
-        reinterpret_cast<jbyte*>(dest->data() + *bytes_read));
-    if (ClearException(env))
-      return false;
+  // Copy the data over to the provided C++ side buffer.
+  DCHECK_GE(length, byte_count);
+  env->GetByteArrayRegion(buffer, 0, byte_count,
+      reinterpret_cast<jbyte*>(dest->data() + *bytes_read));
+  if (ClearException(env))
+    return false;
 
-    *bytes_read += byte_count;
-    length -= byte_count;
-  }
+  *bytes_read = byte_count;
   return true;
 }
 
diff --git a/android_webview/native/input_stream_unittest.cc b/android_webview/native/input_stream_unittest.cc
index 86ffb84..59a326d 100644
--- a/android_webview/native/input_stream_unittest.cc
+++ b/android_webview/native/input_stream_unittest.cc
@@ -42,19 +42,18 @@ class InputStreamTest : public Test {
     ASSERT_TRUE(RegisterNativesImpl(env_));
   }
 
-  scoped_refptr<IOBuffer> DoReadCountedStreamTest(int streamSize,
-                                                  int bytesToRead) {
+  scoped_refptr<IOBuffer> DoReadCountedStreamTest(int stream_size,
+                                                  int bytes_requested,
+                                                  int* bytes_read) {
     ScopedJavaLocalRef<jobject> counting_jstream =
-        Java_InputStreamUnittest_getCountingStream(env_, streamSize);
+        Java_InputStreamUnittest_getCountingStream(env_, stream_size);
     EXPECT_FALSE(counting_jstream.is_null());
 
     scoped_ptr<InputStream> input_stream(
         new InputStreamImpl(counting_jstream));
-    int bytesRead = 0;
-    scoped_refptr<IOBuffer> buffer = new IOBuffer(bytesToRead);
+    scoped_refptr<IOBuffer> buffer = new IOBuffer(bytes_requested);
 
-    EXPECT_TRUE(input_stream->Read(buffer.get(), bytesToRead, &bytesRead));
-    EXPECT_EQ(bytesToRead, bytesRead);
+    EXPECT_TRUE(input_stream->Read(buffer.get(), bytes_requested, bytes_read));
     return buffer;
   }
 
@@ -67,34 +66,42 @@ TEST_F(InputStreamTest, ReadEmptyStream) {
   EXPECT_FALSE(empty_jstream.is_null());
 
   scoped_ptr<InputStream> input_stream(new InputStreamImpl(empty_jstream));
-  const int bytesToRead = 10;
-  int bytesRead = 0;
-  scoped_refptr<IOBuffer> buffer = new IOBuffer(bytesToRead);
+  const int bytes_requested = 10;
+  int bytes_read = 0;
+  scoped_refptr<IOBuffer> buffer = new IOBuffer(bytes_requested);
 
-  EXPECT_TRUE(input_stream->Read(buffer.get(), bytesToRead, &bytesRead));
-  EXPECT_EQ(0, bytesRead);
+  EXPECT_TRUE(input_stream->Read(buffer.get(), bytes_requested, &bytes_read));
+  EXPECT_EQ(0, bytes_read);
 }
 
 TEST_F(InputStreamTest, ReadStreamPartial) {
-  const int bytesToRead = 128;
-  DoReadCountedStreamTest(bytesToRead * 2, bytesToRead);
+  const int bytes_requested = 128;
+  int bytes_read = 0;
+  DoReadCountedStreamTest(bytes_requested * 2, bytes_requested, &bytes_read);
+  EXPECT_EQ(bytes_requested, bytes_read);
 }
 
 TEST_F(InputStreamTest, ReadStreamCompletely) {
-  const int bytesToRead = 42;
-  DoReadCountedStreamTest(bytesToRead, bytesToRead);
+  const int bytes_requested = 42;
+  int bytes_read = 0;
+  DoReadCountedStreamTest(bytes_requested, bytes_requested, &bytes_read);
+  EXPECT_EQ(bytes_requested, bytes_read);
 }
 
-TEST_F(InputStreamTest, ReadStreamInChunks) {
-  const int bytesToRead = 3 * InputStreamImpl::kBufferSize;
-  DoReadCountedStreamTest(bytesToRead, bytesToRead);
+TEST_F(InputStreamTest, TryReadMoreThanBuffer) {
+  const int bytes_requested = 3 * InputStreamImpl::kBufferSize;
+  int bytes_read = 0;
+  DoReadCountedStreamTest(bytes_requested, bytes_requested, &bytes_read);
+  EXPECT_EQ(InputStreamImpl::kBufferSize, bytes_read);
 }
 
 TEST_F(InputStreamTest, CheckContentsReadCorrectly) {
-  const int bytesToRead = 256;
+  const int bytes_requested = 256;
+  int bytes_read = 0;
   scoped_refptr<IOBuffer> buffer =
-      DoReadCountedStreamTest(bytesToRead, bytesToRead);
-  for (int i = 0; i < bytesToRead; ++i) {
+      DoReadCountedStreamTest(bytes_requested, bytes_requested, &bytes_read);
+  EXPECT_EQ(bytes_requested, bytes_read);
+  for (int i = 0; i < bytes_requested; ++i) {
     EXPECT_EQ(i, buffer->data()[i]);
   }
 }