Cherry-pick: [Android WebView] Make InputStreamImpl::Read fill the IOBuffer.
Cherry-pick of chromium crrev.com/24082006
BUG: 10769152
Original description:
[Android WebView] Make InputStreamImpl::Read fill the IOBuffer.
This CL improves the performances of InputStreamImpl::Read. The
previous implementation was limiting each Read to transfer at most
4k. This CL makes it possible to fill, in a single run, the IOBuffer
up to the length provided by the caller.
In practice this will reduces the number of thread hops and IPCs.
Note: the max transfer size, at this point, is imited onlt by the
kMaxAllocationSize constant defined in async_resounrce_handler.cc.
Change-Id: I5c733b845641e98292d78400fefa749c02a8a0ef
diff --git a/android_webview/native/input_stream_impl.cc b/android_webview/native/input_stream_impl.cc
index 24d7be0..fca6a29 100644
--- a/android_webview/native/input_stream_impl.cc
+++ b/android_webview/native/input_stream_impl.cc
@@ -84,46 +84,48 @@
return false;
}
+ int remaining_length = length;
+ char* dest_write_ptr = dest->data();
jbyteArray buffer = buffer_.obj();
*bytes_read = 0;
- 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);
+ while (remaining_length > 0) {
+ const int max_transfer_length = std::min(remaining_length, kBufferSize);
+ const int transfer_length = Java_InputStream_readI_AB_I_I(
+ env, jobject_.obj(), buffer, 0, max_transfer_length);
if (ClearException(env))
return false;
- } while (byte_count == 0);
- // We've reached the end of the stream.
- if (byte_count < 0)
- return true;
+ if (transfer_length < 0) // EOF
+ break;
-#ifndef NDEBUG
- int32_t buffer_length = env->GetArrayLength(buffer);
- DCHECK_GE(read_size, byte_count);
- DCHECK_GE(buffer_length, byte_count);
-#endif // NDEBUG
+ // Note: it is possible, yet unlikely, that the Java InputStream returns
+ // a transfer_length == 0 from time to time. In such cases we just continue
+ // the read until we get either valid data or reach EOF.
+ if (transfer_length == 0)
+ continue;
- // 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;
+ DCHECK_GE(max_transfer_length, transfer_length);
+ DCHECK_GE(env->GetArrayLength(buffer), transfer_length);
- // 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;
+ // This check is to prevent a malicious InputStream implementation from
+ // overrunning the |dest| buffer.
+ if (transfer_length > max_transfer_length)
+ return false;
- *bytes_read = byte_count;
+ // Copy the data over to the provided C++ IOBuffer.
+ DCHECK_GE(remaining_length, transfer_length);
+ env->GetByteArrayRegion(buffer, 0, transfer_length,
+ reinterpret_cast<jbyte*>(dest_write_ptr));
+ if (ClearException(env))
+ return false;
+
+ remaining_length -= transfer_length;
+ dest_write_ptr += transfer_length;
+ }
+ // bytes_read can be strictly less than the req. length if EOF is encountered.
+ DCHECK(remaining_length >= 0 && remaining_length <= length);
+ *bytes_read = length - remaining_length;
return true;
}
diff --git a/android_webview/native/input_stream_unittest.cc b/android_webview/native/input_stream_unittest.cc
index 87b3fe3..76ca823 100644
--- a/android_webview/native/input_stream_unittest.cc
+++ b/android_webview/native/input_stream_unittest.cc
@@ -89,10 +89,10 @@
}
TEST_F(InputStreamTest, TryReadMoreThanBuffer) {
- const int bytes_requested = 3 * InputStreamImpl::kBufferSize;
+ const int buffer_size = 3 * InputStreamImpl::kBufferSize;
int bytes_read = 0;
- DoReadCountedStreamTest(bytes_requested, bytes_requested, &bytes_read);
- EXPECT_EQ(InputStreamImpl::kBufferSize, bytes_read);
+ DoReadCountedStreamTest(buffer_size, buffer_size * 2, &bytes_read);
+ EXPECT_EQ(buffer_size, bytes_read);
}
TEST_F(InputStreamTest, CheckContentsReadCorrectly) {
@@ -105,3 +105,17 @@
EXPECT_EQ(i, (unsigned char)buffer->data()[i]);
}
}
+
+TEST_F(InputStreamTest, ReadLargeStreamPartial) {
+ const int bytes_requested = 3 * InputStreamImpl::kBufferSize;
+ int bytes_read = 0;
+ DoReadCountedStreamTest(bytes_requested + 32, bytes_requested, &bytes_read);
+ EXPECT_EQ(bytes_requested, bytes_read);
+}
+
+TEST_F(InputStreamTest, ReadLargeStreamCompletely) {
+ const int bytes_requested = 3 * InputStreamImpl::kBufferSize;
+ int bytes_read = 0;
+ DoReadCountedStreamTest(bytes_requested, bytes_requested, &bytes_read);
+ EXPECT_EQ(bytes_requested, bytes_read);
+}
