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

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

Issue 1777333002: [Cronet] Fix race in UploadDataStream.attachToRequest. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Address Helen's comments. 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 unified diff | Download patch
OLDNEW
1 // Copyright 2015 The Chromium Authors. All rights reserved. 1 // Copyright 2015 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 package org.chromium.net; 5 package org.chromium.net;
6 6
7 import android.util.Log; 7 import android.util.Log;
8 8
9 import org.chromium.base.VisibleForTesting; 9 import org.chromium.base.VisibleForTesting;
10 import org.chromium.base.annotations.CalledByNative; 10 import org.chromium.base.annotations.CalledByNative;
(...skipping 214 matching lines...) Expand 10 before | Expand all | Expand 10 after
225 public void onRewindError(Exception exception) { 225 public void onRewindError(Exception exception) {
226 synchronized (mLock) { 226 synchronized (mLock) {
227 checkState(UserCallback.REWIND); 227 checkState(UserCallback.REWIND);
228 onError(exception); 228 onError(exception);
229 } 229 }
230 } 230 }
231 231
232 /** 232 /**
233 * Posts task to application Executor. 233 * Posts task to application Executor.
234 */ 234 */
235 private void postTaskToExecutor(Runnable task) { 235 void postTaskToExecutor(Runnable task) {
236 try { 236 try {
237 mExecutor.execute(task); 237 mExecutor.execute(task);
238 } catch (Throwable e) { 238 } catch (Throwable e) {
239 // Just fail the request. The request is smart enough to handle the 239 // Just fail the request. The request is smart enough to handle the
240 // case where it was already canceled by the embedder. 240 // case where it was already canceled by the embedder.
241 mRequest.onUploadException(e); 241 mRequest.onUploadException(e);
242 } 242 }
243 } 243 }
244 244
245 /** 245 /**
(...skipping 41 matching lines...) Expand 10 before | Expand all | Expand 10 after
287 "Method should not be called when read has not completed ."); 287 "Method should not be called when read has not completed .");
288 } 288 }
289 if (mDestroyAdapterPostponed) { 289 if (mDestroyAdapterPostponed) {
290 destroyAdapter(); 290 destroyAdapter();
291 } 291 }
292 } 292 }
293 } 293 }
294 294
295 /** 295 /**
296 * Creates native objects and attaches them to the underlying request 296 * Creates native objects and attaches them to the underlying request
297 * adapter object. 297 * adapter object. Always called on executor thread to allow getLength() to
298 * TODO(mmenke): If more types of native upload streams are needed, create 298 * block and/or report errors.
299 * an interface with just this method, to minimize CronetURLRequest's
300 * dependencies on each upload stream type.
301 */ 299 */
302 void attachToRequest(final CronetUrlRequest request, final long requestAdapt er, 300 void attachToRequest(final CronetUrlRequest request, final long requestAdapt er) {
303 final Runnable afterAttachCallback) {
304 mRequest = request; 301 mRequest = request;
305 postTaskToExecutor(new Runnable() { 302 synchronized (mLock) {
306 @Override 303 mInWhichUserCallback = UserCallback.GET_LENGTH;
307 public void run() { 304 }
308 synchronized (mLock) { 305 try {
309 mInWhichUserCallback = UserCallback.GET_LENGTH; 306 mLength = mDataProvider.getLength();
310 } 307 } catch (Throwable t) {
311 try { 308 onError(t);
312 mLength = mDataProvider.getLength(); 309 }
313 } catch (Throwable t) { 310 synchronized (mLock) {
314 onError(t); 311 mInWhichUserCallback = UserCallback.NOT_IN_CALLBACK;
315 } 312 mUploadDataStreamAdapter = nativeAttachUploadDataToRequest(requestAd apter, mLength);
316 synchronized (mLock) { 313 }
317 mInWhichUserCallback = UserCallback.NOT_IN_CALLBACK;
318 mUploadDataStreamAdapter =
319 nativeAttachUploadDataToRequest(requestAdapter, mLen gth);
320 }
321 afterAttachCallback.run();
322 }
323 });
324 } 314 }
325 315
326 /** 316 /**
327 * Creates a native CronetUploadDataStreamAdapter and 317 * Creates a native CronetUploadDataStreamAdapter and
328 * CronetUploadDataStream for testing. 318 * CronetUploadDataStream for testing.
329 * @return the address of the native CronetUploadDataStream object. 319 * @return the address of the native CronetUploadDataStream object.
330 */ 320 */
331 @VisibleForTesting 321 @VisibleForTesting
332 long createUploadDataStreamForTesting() throws IOException { 322 long createUploadDataStreamForTesting() throws IOException {
333 mUploadDataStreamAdapter = nativeCreateAdapterForTesting(); 323 synchronized (mLock) {
334 mLength = mDataProvider.getLength(); 324 mUploadDataStreamAdapter = nativeCreateAdapterForTesting();
335 return nativeCreateUploadDataStreamForTesting(mLength, 325 mLength = mDataProvider.getLength();
336 mUploadDataStreamAdapter); 326 return nativeCreateUploadDataStreamForTesting(mLength, mUploadDataSt reamAdapter);
327 }
337 } 328 }
338 329
339 @VisibleForTesting 330 @VisibleForTesting
340 void setOnDestroyedCallbackForTesting(Runnable onDestroyedCallbackForTesting ) { 331 void setOnDestroyedCallbackForTesting(Runnable onDestroyedCallbackForTesting ) {
341 mOnDestroyedCallbackForTesting = onDestroyedCallbackForTesting; 332 mOnDestroyedCallbackForTesting = onDestroyedCallbackForTesting;
342 } 333 }
343 334
344 // Native methods are implemented in upload_data_stream_adapter.cc. 335 // Native methods are implemented in upload_data_stream_adapter.cc.
345 336
346 private native long nativeAttachUploadDataToRequest(long urlRequestAdapter, 337 private native long nativeAttachUploadDataToRequest(long urlRequestAdapter,
347 long length); 338 long length);
348 339
349 private native long nativeCreateAdapterForTesting(); 340 private native long nativeCreateAdapterForTesting();
350 341
351 private native long nativeCreateUploadDataStreamForTesting(long length, 342 private native long nativeCreateUploadDataStreamForTesting(long length,
352 long adapter); 343 long adapter);
353 344
354 @NativeClassQualifiedName("CronetUploadDataStreamAdapter") 345 @NativeClassQualifiedName("CronetUploadDataStreamAdapter")
355 private native void nativeOnReadSucceeded(long nativePtr, 346 private native void nativeOnReadSucceeded(long nativePtr,
356 int bytesRead, boolean finalChunk); 347 int bytesRead, boolean finalChunk);
357 348
358 @NativeClassQualifiedName("CronetUploadDataStreamAdapter") 349 @NativeClassQualifiedName("CronetUploadDataStreamAdapter")
359 private native void nativeOnRewindSucceeded(long nativePtr); 350 private native void nativeOnRewindSucceeded(long nativePtr);
360 351
361 @NativeClassQualifiedName("CronetUploadDataStreamAdapter") 352 @NativeClassQualifiedName("CronetUploadDataStreamAdapter")
362 private static native void nativeDestroy(long nativePtr); 353 private static native void nativeDestroy(long nativePtr);
363 } 354 }
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698