Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(737)

Unified Diff: components/cronet/android/java/src/org/chromium/net/CronetUploadDataStream.java

Issue 1801943002: [Cronet] Fix race in UploadDataStream.attachToRequest. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@2661
Patch Set: Created 4 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: components/cronet/android/java/src/org/chromium/net/CronetUploadDataStream.java
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 6256d46a80979812729329f57619aebd276b9de8..a1a6e4fca1a2d178661d6cc7e57fed9688dbc167 100644
--- a/components/cronet/android/java/src/org/chromium/net/CronetUploadDataStream.java
+++ b/components/cronet/android/java/src/org/chromium/net/CronetUploadDataStream.java
@@ -232,7 +232,7 @@ final class CronetUploadDataStream implements UploadDataSink {
/**
* Posts task to application Executor.
*/
- private void postTaskToExecutor(Runnable task) {
+ void postTaskToExecutor(Runnable task) {
try {
mExecutor.execute(task);
} catch (Throwable e) {
@@ -293,34 +293,35 @@ final class CronetUploadDataStream implements UploadDataSink {
}
/**
+ * Initializes upload length by getting it from data provider. Always called
+ * on executor thread to allow getLength() to block and/or report errors.
+ * If data provider throws an exception, then it is reported to the request.
+ * No native calls to urlRequest are allowed as this is done before request
+ * start, so native object may not exist.
+ */
+ void initializeWithRequest(final CronetUrlRequest urlRequest) {
+ synchronized (mLock) {
+ mRequest = urlRequest;
+ mInWhichUserCallback = UserCallback.GET_LENGTH;
+ }
+ try {
+ mLength = mDataProvider.getLength();
+ } catch (Throwable t) {
+ onError(t);
+ }
+ synchronized (mLock) {
+ mInWhichUserCallback = UserCallback.NOT_IN_CALLBACK;
+ }
+ }
+
+ /**
* Creates native objects and attaches them to the underlying request
- * adapter object.
- * TODO(mmenke): If more types of native upload streams are needed, create
- * an interface with just this method, to minimize CronetURLRequest's
- * dependencies on each upload stream type.
+ * adapter object. Always called on executor thread.
*/
- void attachToRequest(final CronetUrlRequest request, final long requestAdapter,
- final Runnable afterAttachCallback) {
- mRequest = request;
- postTaskToExecutor(new Runnable() {
- @Override
- public void run() {
- synchronized (mLock) {
- mInWhichUserCallback = UserCallback.GET_LENGTH;
- }
- try {
- mLength = mDataProvider.getLength();
- } catch (Throwable t) {
- onError(t);
- }
- synchronized (mLock) {
- mInWhichUserCallback = UserCallback.NOT_IN_CALLBACK;
- mUploadDataStreamAdapter =
- nativeAttachUploadDataToRequest(requestAdapter, mLength);
- }
- afterAttachCallback.run();
- }
- });
+ void attachNativeAdapterToRequest(final long requestAdapter) {
+ synchronized (mLock) {
+ mUploadDataStreamAdapter = nativeAttachUploadDataToRequest(requestAdapter, mLength);
+ }
}
/**
@@ -330,10 +331,11 @@ final class CronetUploadDataStream implements UploadDataSink {
*/
@VisibleForTesting
long createUploadDataStreamForTesting() throws IOException {
- mUploadDataStreamAdapter = nativeCreateAdapterForTesting();
- mLength = mDataProvider.getLength();
- return nativeCreateUploadDataStreamForTesting(mLength,
- mUploadDataStreamAdapter);
+ synchronized (mLock) {
+ mUploadDataStreamAdapter = nativeCreateAdapterForTesting();
+ mLength = mDataProvider.getLength();
+ return nativeCreateUploadDataStreamForTesting(mLength, mUploadDataStreamAdapter);
+ }
}
@VisibleForTesting
« no previous file with comments | « no previous file | components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698