From 495ad5a3bde13a2938d84862cb841f39220ed4b7 Mon Sep 17 00:00:00 2001 From: xunjieli Date: Fri, 27 Feb 2015 14:56:37 -0800 Subject: [PATCH] [Cronet] Enable chunked upload in CronetUrlRequest Chunked upload support is added for embedded_test_server, so we should enable chunked upload in CronetUrlRequest since now we have testing support. BUG=412942 Review URL: https://codereview.chromium.org/948503004 Cr-Commit-Position: refs/heads/master@{#318536} --- .../android/cronet_upload_data_stream_adapter.cc | 4 +- .../android/cronet_upload_data_stream_delegate.cc | 1 + .../org/chromium/net/CronetUploadDataStream.java | 5 --- .../src/org/chromium/net/CronetUrlRequestTest.java | 49 ++++++++++++++++++---- .../src/org/chromium/net/TestDataProvider.java | 4 +- net/url_request/url_request.cc | 1 - 6 files changed, 47 insertions(+), 17 deletions(-) diff --git a/components/cronet/android/cronet_upload_data_stream_adapter.cc b/components/cronet/android/cronet_upload_data_stream_adapter.cc index 05ca1ce42ad5..4ac38b946723 100644 --- a/components/cronet/android/cronet_upload_data_stream_adapter.cc +++ b/components/cronet/android/cronet_upload_data_stream_adapter.cc @@ -87,7 +87,9 @@ void CronetUploadDataStreamAdapter::OnReadSuccess(int bytes_read, DCHECK(read_in_progress_); DCHECK(!rewind_in_progress_); DCHECK(bytes_read > 0 || (final_chunk && bytes_read == 0)); - DCHECK(!is_chunked() || !final_chunk); + if (!is_chunked()) { + DCHECK(!final_chunk); + } read_in_progress_ = false; diff --git a/components/cronet/android/cronet_upload_data_stream_delegate.cc b/components/cronet/android/cronet_upload_data_stream_delegate.cc index 4792bdfd044c..fb249ab251d8 100644 --- a/components/cronet/android/cronet_upload_data_stream_delegate.cc +++ b/components/cronet/android/cronet_upload_data_stream_delegate.cc @@ -9,6 +9,7 @@ #include "base/android/jni_android.h" #include "base/android/jni_string.h" #include "base/bind.h" +#include "base/logging.h" #include "base/message_loop/message_loop_proxy.h" #include "base/single_thread_task_runner.h" #include "components/cronet/android/cronet_url_request_adapter.h" diff --git a/components/cronet/android/java/src/org/chromium/net/CronetUploadDataStream.java b/components/cronet/android/java/src/org/chromium/net/CronetUploadDataStream.java index 731623da4628..0427c40d9b68 100644 --- a/components/cronet/android/java/src/org/chromium/net/CronetUploadDataStream.java +++ b/components/cronet/android/java/src/org/chromium/net/CronetUploadDataStream.java @@ -71,11 +71,6 @@ final class CronetUploadDataStream implements UploadDataSink { mExecutor = executor; mDataProvider = dataProvider; mLength = mDataProvider.getLength(); - if (mLength < 0) { - // TODO(mmenke): Add tests and remove this line. - throw new IllegalArgumentException( - "Chunked uploads not supported."); - } } /** diff --git a/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestTest.java b/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestTest.java index d3f93ee7c1bb..15f0d92b46f7 100644 --- a/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestTest.java +++ b/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestTest.java @@ -810,20 +810,53 @@ public class CronetUrlRequestTest extends CronetTestBase { @SmallTest @Feature({"Cronet"}) - public void testUploadChunkedNotSupported() throws Exception { + public void testUploadChunked() throws Exception { TestUrlRequestListener listener = new TestUrlRequestListener(); UrlRequest urlRequest = mActivity.mUrlRequestContext.createRequest( - NativeTestServer.getRedirectToEchoBody(), listener, listener.getExecutor()); + NativeTestServer.getEchoBodyURL(), listener, listener.getExecutor()); TestUploadDataProvider dataProvider = new TestUploadDataProvider( TestUploadDataProvider.SuccessCallbackMode.SYNC, listener.getExecutor()); + dataProvider.addRead("test hello".getBytes()); dataProvider.setChunked(true); - try { - urlRequest.setUploadDataProvider(dataProvider, listener.getExecutor()); - fail("Exception not thrown"); - } catch (IllegalArgumentException e) { - assertEquals("Chunked uploads not supported.", e.getMessage()); - } + urlRequest.setUploadDataProvider(dataProvider, listener.getExecutor()); + urlRequest.addHeader("Content-Type", "useless/string"); + + assertEquals(-1, dataProvider.getLength()); + + urlRequest.start(); + listener.blockForDone(); + + // 1 read call for one data chunk. + assertEquals(1, dataProvider.getNumReadCalls()); + assertEquals("test hello", listener.mResponseAsString); + } + + @SmallTest + @Feature({"Cronet"}) + public void testUploadChunkedLastReadZeroLengthBody() throws Exception { + TestUrlRequestListener listener = new TestUrlRequestListener(); + UrlRequest urlRequest = mActivity.mUrlRequestContext.createRequest( + NativeTestServer.getEchoBodyURL(), listener, listener.getExecutor()); + + TestUploadDataProvider dataProvider = new TestUploadDataProvider( + TestUploadDataProvider.SuccessCallbackMode.SYNC, listener.getExecutor()); + // Add 3 reads. The last read has a 0-length body. + dataProvider.addRead("hello there".getBytes()); + dataProvider.addRead("!".getBytes()); + dataProvider.addRead("".getBytes()); + dataProvider.setChunked(true); + urlRequest.setUploadDataProvider(dataProvider, listener.getExecutor()); + urlRequest.addHeader("Content-Type", "useless/string"); + + assertEquals(-1, dataProvider.getLength()); + + urlRequest.start(); + listener.blockForDone(); + + // 2 read call for the first two data chunks, and 1 for final chunk. + assertEquals(3, dataProvider.getNumReadCalls()); + assertEquals("hello there!", listener.mResponseAsString); } private void throwOrCancel(FailureType failureType, ResponseStep failureStep, diff --git a/components/cronet/android/test/javatests/src/org/chromium/net/TestDataProvider.java b/components/cronet/android/test/javatests/src/org/chromium/net/TestDataProvider.java index 7175fbe2ce7a..ecc1d656dff1 100644 --- a/components/cronet/android/test/javatests/src/org/chromium/net/TestDataProvider.java +++ b/components/cronet/android/test/javatests/src/org/chromium/net/TestDataProvider.java @@ -119,7 +119,7 @@ class TestUploadDataProvider implements UploadDataProvider { mReadPending = true; mStarted = true; - final boolean finalChunk = (mChunked && mNextRead == mReads.size()); + final boolean finalChunk = (mChunked && mNextRead == mReads.size() - 1); if (mNextRead < mReads.size()) { if ((byteBuffer.limit() - byteBuffer.position()) < mReads.get(mNextRead).length) { @@ -128,7 +128,7 @@ class TestUploadDataProvider implements UploadDataProvider { } byteBuffer.put(mReads.get(mNextRead)); ++mNextRead; - } else if (!finalChunk) { + } else { throw new IllegalStateException( "Too many reads: " + mNextRead); } diff --git a/net/url_request/url_request.cc b/net/url_request/url_request.cc index 752331cee8de..8351029509a2 100644 --- a/net/url_request/url_request.cc +++ b/net/url_request/url_request.cc @@ -213,7 +213,6 @@ void URLRequest::AppendChunkToUpload(const char* bytes, } void URLRequest::set_upload(scoped_ptr upload) { - DCHECK(!upload->is_chunked()); upload_data_stream_ = upload.Pass(); } -- 2.11.4.GIT