[Cronet] Fix race in UploadDataStream.attachToRequest.

components/cronet/android/java/src/org/chromium/net/CronetUploadDataStream.java

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.net;
 
 import android.util.Log;
 
 import org.chromium.base.VisibleForTesting;
 import org.chromium.base.annotations.CalledByNative;
@@ skipping 214 matching lines @@
     public void onRewindError(Exception exception) {
         synchronized (mLock) {
             checkState(UserCallback.REWIND);
             onError(exception);
         }
     }
 
     /**
      * Posts task to application Executor.
      */
-    private void postTaskToExecutor(Runnable task) {
+    void postTaskToExecutor(Runnable task) {
         try {
             mExecutor.execute(task);
         } catch (Throwable e) {
             // Just fail the request. The request is smart enough to handle the
             // case where it was already canceled by the embedder.
             mRequest.onUploadException(e);
         }
     }
 
     /**
@@ skipping 40 matching lines @@
                 throw new IllegalStateException(
                         "Method should not be called when read has not completed.");
             }
             if (mDestroyAdapterPostponed) {
                 destroyAdapter();
             }
         }
     }
 
     /**
+     * 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.

[xunjieli, 2016/03/10 20:32:21]

Could you make a note here to say that that no native function shoudl be
contained in this method?

[mef, 2016/03/10 22:16:30]

Done, although mUploadDataStreamAdapter doesn't exist yet and urlRequest doesn't
expose its native adapter.

+     */
+    void initializeWithRequest(final CronetUrlRequest request) {
+        synchronized (mLock) {
+            mRequest = request;
+            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);
+        }
     }
 
     /**
      * Creates a native CronetUploadDataStreamAdapter and
      * CronetUploadDataStream for testing.
      * @return the address of the native CronetUploadDataStream object.
      */
     @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
     void setOnDestroyedCallbackForTesting(Runnable onDestroyedCallbackForTesting) {
         mOnDestroyedCallbackForTesting = onDestroyedCallbackForTesting;
     }
 
     // Native methods are implemented in upload_data_stream_adapter.cc.
 
     private native long nativeAttachUploadDataToRequest(long urlRequestAdapter,
             long length);
 
     private native long nativeCreateAdapterForTesting();
 
     private native long nativeCreateUploadDataStreamForTesting(long length,
             long adapter);
 
     @NativeClassQualifiedName("CronetUploadDataStreamAdapter")
     private native void nativeOnReadSucceeded(long nativePtr,
             int bytesRead, boolean finalChunk);
 
     @NativeClassQualifiedName("CronetUploadDataStreamAdapter")
     private native void nativeOnRewindSucceeded(long nativePtr);
 
     @NativeClassQualifiedName("CronetUploadDataStreamAdapter")
     private static native void nativeDestroy(long nativePtr);
 }