Allow direct executors in cronet.

components/cronet/android/java/src/org/chromium/net/impl/CronetUrlRequest.java

 // Copyright 2014 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.impl;
 
 import android.os.SystemClock;
 import android.support.annotation.Nullable;
 import android.util.Log;
 
 import org.chromium.base.VisibleForTesting;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNIAdditionalImport;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.base.annotations.NativeClassQualifiedName;
+import org.chromium.net.InlineExecutionProhibitedException;
 import org.chromium.net.Preconditions;
 import org.chromium.net.QuicException;
 import org.chromium.net.RequestFinishedInfo;
 import org.chromium.net.RequestPriority;
 import org.chromium.net.UploadDataProvider;
 import org.chromium.net.UrlRequest;
 import org.chromium.net.UrlRequestException;
 import org.chromium.net.UrlResponseInfo;
 
 import java.nio.ByteBuffer;
@@ skipping 16 matching lines @@
  * native tasks to native network thread. Because Cancel could be called from
  * any thread it is protected by mUrlRequestAdapterLock.
  */
 @JNINamespace("cronet")
 // Qualifies UrlRequest.StatusListener which is used in onStatus, a JNI method.
 @JNIAdditionalImport(UrlRequest.class)
 @VisibleForTesting
 public final class CronetUrlRequest implements UrlRequest {
     private static final RequestFinishedInfo.Metrics EMPTY_METRICS =
             new RequestFinishedInfo.Metrics(null, null, null, null);
+    private final boolean mAllowDirectExecutor;
 
     /* Native adapter object, owned by UrlRequest. */
     @GuardedBy("mUrlRequestAdapterLock")
     private long mUrlRequestAdapter;
 
     @GuardedBy("mUrlRequestAdapterLock")
     private boolean mStarted = false;
     @GuardedBy("mUrlRequestAdapterLock")
     private boolean mWaitingOnRedirect = false;
     @GuardedBy("mUrlRequestAdapterLock")
@@ skipping 40 matching lines @@
     private Runnable mOnDestroyedCallbackForTesting;
 
     private static final class HeadersList extends ArrayList<Map.Entry<String, String>> {}
 
     private final class OnReadCompletedRunnable implements Runnable {
         // Buffer passed back from current invocation of onReadCompleted.
         ByteBuffer mByteBuffer;
 
         @Override
         public void run() {
+            checkCallingThread();
             // Null out mByteBuffer, to pass buffer ownership to callback or release if done.
             ByteBuffer buffer = mByteBuffer;
             mByteBuffer = null;
 
             try {
                 synchronized (mUrlRequestAdapterLock) {
                     if (isDoneLocked()) {
                         return;
                     }
                     mWaitingOnRead = true;
                 }
                 mCallback.onReadCompleted(CronetUrlRequest.this, mResponseInfo, buffer);
             } catch (Exception e) {
                 onCallbackException(e);
             }
         }
     }
 
     CronetUrlRequest(CronetUrlRequestContext requestContext, String url, int priority,
             UrlRequest.Callback callback, Executor executor, Collection<Object> requestAnnotations,
             boolean metricsCollectionEnabled, boolean disableCache,
-            boolean disableConnectionMigration) {
+            boolean disableConnectionMigration, boolean allowDirectExecutor) {
         if (url == null) {
             throw new NullPointerException("URL is required");
         }
         if (callback == null) {
             throw new NullPointerException("Listener is required");
         }
         if (executor == null) {
             throw new NullPointerException("Executor is required");
         }
         if (requestAnnotations == null) {
             throw new NullPointerException("requestAnnotations is required");
         }
 
+        mAllowDirectExecutor = allowDirectExecutor;
         mRequestContext = requestContext;
         mInitialUrl = url;
         mUrlChain.add(url);
         mPriority = convertRequestPriority(priority);
         mCallback = callback;
         mExecutor = executor;
         mRequestAnnotations = requestAnnotations;
         mRequestMetricsAccumulator =
                 metricsCollectionEnabled ? new UrlRequestMetricsAccumulator() : null;
         mDisableCache = disableCache;
@@ skipping 59 matching lines @@
                         throw new IllegalArgumentException(
                                 "Invalid header " + header.getKey() + "=" + header.getValue());
                     }
                 }
                 if (mUploadDataStream != null) {
                     if (!hasContentType) {
                         throw new IllegalArgumentException(
                                 "Requests with upload data must have a Content-Type.");
                     }
                     mStarted = true;
-                    mUploadDataStream.postTaskToExecutor(new Runnable() {
+                    mUploadDataStream.initializeWithRequest(CronetUrlRequest.this, new Runnable() {
                         @Override
                         public void run() {
-                            mUploadDataStream.initializeWithRequest(CronetUrlRequest.this);
                             synchronized (mUrlRequestAdapterLock) {
                                 if (isDoneLocked()) {
                                     return;
                                 }
                                 mUploadDataStream.attachNativeAdapterToRequest(mUrlRequestAdapter);
                                 startInternalLocked();
                             }
                         }
                     });
                     return;
@@ skipping 120 matching lines @@
     /**
      * Posts task to application Executor. Used for Listener callbacks
      * and other tasks that should not be executed on network thread.
      */
     private void postTaskToExecutor(Runnable task) {
         try {
             mExecutor.execute(task);
         } catch (RejectedExecutionException failException) {
             Log.e(CronetUrlRequestContext.LOG_TAG, "Exception posting task to executor",
                     failException);
-            // If posting a task throws an exception, then there is no choice
-            // but to destroy the request without invoking the callback.
-            destroyRequestAdapter(false);
+            // If posting a task throws an exception, then we fail the request. This exception could
+            // be permanent (executor shutdown), transient (AbortPolicy, or CallerRunsPolicy with
+            // direct execution not permitted), or caused the the runnables we submit if

[mef, 2016/08/31 20:24:25]

nit: the the -> by the?

[Charles, 2016/08/31 22:36:41]

Done.

+            // mUserExecutor is a direct executor and direct execution is not permitted. In the
+            // latter two cases, we at least have a chance to inform the embedder of the request's

[mef, 2016/08/31 20:24:25]

nit: We don't use 'we' in Chromium comments. :)

[Charles, 2016/08/31 22:36:41]

Done.

+            // failure, since failWithException does not enforce that onFailed() is not executed
+            // inline.
+            failWithException(
+                    new UrlRequestException("Exception posting task to executor", failException));
         }
     }
 
     private static int convertRequestPriority(int priority) {
         switch (priority) {
             case Builder.REQUEST_PRIORITY_IDLE:
                 return RequestPriority.IDLE;
             case Builder.REQUEST_PRIORITY_LOWEST:
                 return RequestPriority.LOWEST;
             case Builder.REQUEST_PRIORITY_LOW:
@@ skipping 47 matching lines @@
 
     /**
      * If callback method throws an exception, request gets canceled
      * and exception is reported via onFailed listener callback.
      * Only called on the Executor.
      */
     private void onCallbackException(Exception e) {
         UrlRequestException requestError =
                 new UrlRequestException("Exception received from UrlRequest.Callback", e);
         Log.e(CronetUrlRequestContext.LOG_TAG, "Exception in CalledByNative method", e);
-        // Do not call into listener if request is finished.
-        synchronized (mUrlRequestAdapterLock) {
-            if (isDoneLocked()) {
-                return;
-            }
-            destroyRequestAdapter(false);
-        }
-        try {
-            mCallback.onFailed(this, mResponseInfo, requestError);
-        } catch (Exception failException) {
-            Log.e(CronetUrlRequestContext.LOG_TAG, "Exception notifying of failed request",
-                    failException);
-        }
+        failWithException(requestError);
     }
 
     /**
      * Called when UploadDataProvider encounters an error.
      */
     void onUploadException(Throwable e) {
         UrlRequestException uploadError =
                 new UrlRequestException("Exception received from UploadDataProvider", e);
         Log.e(CronetUrlRequestContext.LOG_TAG, "Exception in upload method", e);
         failWithException(uploadError);
     }
 
     /**
      * Fails the request with an exception. Can be called on any thread.
      */
     private void failWithException(final UrlRequestException exception) {
+        synchronized (mUrlRequestAdapterLock) {
+            if (isDoneLocked()) {
+                return;
+            }
+            destroyRequestAdapter(false);
+        }
         Runnable task = new Runnable() {
             @Override
             public void run() {
-                synchronized (mUrlRequestAdapterLock) {
-                    if (isDoneLocked()) {
-                        return;
-                    }
-                    destroyRequestAdapter(false);
-                }
                 try {
                     mCallback.onFailed(CronetUrlRequest.this, mResponseInfo, exception);
                 } catch (Exception e) {
-                    Log.e(CronetUrlRequestContext.LOG_TAG, "Exception in onError method", e);
+                    Log.e(CronetUrlRequestContext.LOG_TAG, "Exception in onFailed method", e);
                 }
             }
         };
-        postTaskToExecutor(task);
+        try {
+            mExecutor.execute(task);
+        } catch (RejectedExecutionException e) {
+            Log.e(CronetUrlRequestContext.LOG_TAG, "Exception posting task to executor", e);
+        }
     }
 
     ////////////////////////////////////////////////
     // Private methods called by the native code.
     // Always called on network thread.
     ////////////////////////////////////////////////
 
     /**
      * Called before following redirects. The redirect will only be followed if
      * {@link #followRedirect()} is called. If the redirect response has a body, it will be ignored.
@@ skipping 14 matching lines @@
                 httpStatusText, headers, wasCached, negotiatedProtocol, proxyServer);
         mReceivedBytesCountFromRedirects += receivedBytesCount;
         responseInfo.setReceivedBytesCount(mReceivedBytesCountFromRedirects);
 
         // Have to do this after creating responseInfo.
         mUrlChain.add(newLocation);
 
         Runnable task = new Runnable() {
             @Override
             public void run() {
+                checkCallingThread();
                 synchronized (mUrlRequestAdapterLock) {
                     if (isDoneLocked()) {
                         return;
                     }
                     mWaitingOnRedirect = true;
                 }
 
                 try {
                     mCallback.onRedirectReceived(CronetUrlRequest.this, responseInfo, newLocation);
                 } catch (Exception e) {
@@ skipping 10 matching lines @@
      */
     @SuppressWarnings("unused")
     @CalledByNative
     private void onResponseStarted(int httpStatusCode, String httpStatusText, String[] headers,
             boolean wasCached, String negotiatedProtocol, String proxyServer) {
         mResponseInfo = prepareResponseInfoOnNetworkThread(httpStatusCode, httpStatusText, headers,
                 wasCached, negotiatedProtocol, proxyServer);
         Runnable task = new Runnable() {
             @Override
             public void run() {
+                checkCallingThread();
                 synchronized (mUrlRequestAdapterLock) {
                     if (isDoneLocked()) {
                         return;
                     }
                     if (mRequestMetricsAccumulator != null) {
                         mRequestMetricsAccumulator.onResponseStarted();
                     }
                     mWaitingOnRead = true;
                 }
 
@@ skipping 169 matching lines @@
             }
         }
 
         private void onResponseStarted() {
             if (mRequestStartTime != null && mTtfbMs == null) {
                 mTtfbMs = SystemClock.elapsedRealtime() - mRequestStartTime;
             }
         }
     }
 
+    /** Enforces prohibition of direct execution. */
+    private void checkCallingThread() {
+        if (!mAllowDirectExecutor && mRequestContext.isNetworkThread(Thread.currentThread())) {
+            throw new InlineExecutionProhibitedException();
+        }
+    }
+
     // Native methods are implemented in cronet_url_request_adapter.cc.
 
     private native long nativeCreateRequestAdapter(long urlRequestContextAdapter, String url,
             int priority, boolean disableCache, boolean disableConnectionMigration);
 
     @NativeClassQualifiedName("CronetURLRequestAdapter")
     private native boolean nativeSetHttpMethod(long nativePtr, String method);
 
     @NativeClassQualifiedName("CronetURLRequestAdapter")
     private native boolean nativeAddRequestHeader(long nativePtr, String name, String value);
 
     @NativeClassQualifiedName("CronetURLRequestAdapter")
     private native void nativeStart(long nativePtr);
 
     @NativeClassQualifiedName("CronetURLRequestAdapter")
     private native void nativeFollowDeferredRedirect(long nativePtr);
 
     @NativeClassQualifiedName("CronetURLRequestAdapter")
     private native boolean nativeReadData(
             long nativePtr, ByteBuffer byteBuffer, int position, int capacity);
 
     @NativeClassQualifiedName("CronetURLRequestAdapter")
     private native void nativeDestroy(long nativePtr, boolean sendOnCanceled);
 
     @NativeClassQualifiedName("CronetURLRequestAdapter")
     private native void nativeGetStatus(long nativePtr, UrlRequest.StatusListener listener);
 }