Allow direct executors in cronet.

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

Index: components/cronet/android/java/src/org/chromium/net/impl/CronetUrlRequest.java
diff --git a/components/cronet/android/java/src/org/chromium/net/impl/CronetUrlRequest.java b/components/cronet/android/java/src/org/chromium/net/impl/CronetUrlRequest.java
index d7da13995e5e93778193615b2acd737971a373fa..818ecb2959671bb2f5a3d0cf7a94059281e58da0 100644
--- a/components/cronet/android/java/src/org/chromium/net/impl/CronetUrlRequest.java
+++ b/components/cronet/android/java/src/org/chromium/net/impl/CronetUrlRequest.java
@@ -13,6 +13,7 @@ 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;
@@ -49,6 +50,7 @@ import javax.annotation.concurrent.GuardedBy;
 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")
@@ -109,6 +111,7 @@ public final class CronetUrlRequest implements UrlRequest {
 
         @Override
         public void run() {
+            checkCallingThread();
             // Null out mByteBuffer, to pass buffer ownership to callback or release if done.
             ByteBuffer buffer = mByteBuffer;
             mByteBuffer = null;
@@ -130,7 +133,7 @@ public final class CronetUrlRequest implements UrlRequest {
     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");
         }
@@ -144,6 +147,7 @@ public final class CronetUrlRequest implements UrlRequest {
             throw new NullPointerException("requestAnnotations is required");
         }
 
+        mAllowDirectExecutor = allowDirectExecutor;
         mRequestContext = requestContext;
         mInitialUrl = url;
         mUrlChain.add(url);
@@ -223,10 +227,9 @@ public final class CronetUrlRequest implements UrlRequest {
                                 "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;
@@ -367,9 +370,15 @@ public final class CronetUrlRequest implements UrlRequest {
         } 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));
         }
     }
 
@@ -437,19 +446,7 @@ public final class CronetUrlRequest implements UrlRequest {
         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);
     }
 
     /**
@@ -466,23 +463,27 @@ public final class CronetUrlRequest implements UrlRequest {
      * 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);
+        }
     }
 
     ////////////////////////////////////////////////
@@ -517,6 +518,7 @@ public final class CronetUrlRequest implements UrlRequest {
         Runnable task = new Runnable() {
             @Override
             public void run() {
+                checkCallingThread();
                 synchronized (mUrlRequestAdapterLock) {
                     if (isDoneLocked()) {
                         return;
@@ -547,6 +549,7 @@ public final class CronetUrlRequest implements UrlRequest {
         Runnable task = new Runnable() {
             @Override
             public void run() {
+                checkCallingThread();
                 synchronized (mUrlRequestAdapterLock) {
                     if (isDoneLocked()) {
                         return;
@@ -736,6 +739,13 @@ public final class CronetUrlRequest implements UrlRequest {
         }
     }
 
+    /** 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,