Allow direct executors in cronet.

components/cronet/android/java/src/org/chromium/net/impl/CronetUrlRequestContext.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.content.Context;
 import android.os.Build;
 import android.os.ConditionVariable;
 import android.os.Handler;
@@ skipping 43 matching lines @@
     private static final int LOG_VERBOSE = -2; // LOG(FATAL...INFO), VLOG(2)
     static final String LOG_TAG = "ChromiumNetwork";
 
     /**
      * Synchronize access to mUrlRequestContextAdapter and shutdown routine.
      */
     private final Object mLock = new Object();
     private final ConditionVariable mInitCompleted = new ConditionVariable(false);
     private final AtomicInteger mActiveRequestCount = new AtomicInteger(0);
 
+    @GuardedBy("mLock")
     private long mUrlRequestContextAdapter = 0;
+    /**
+     * This field is accessed without synchronization, but only for the purposes of reference
+     * equality comparison with other threads. If such a comparison is performed on the network
+     * thread, then there is a happens-before edge between the write of this field and the
+     * subsequent read; if it's performed on another thread, then observing a value of null won't
+     * change the result of the comparison.
+     */
     private Thread mNetworkThread;
 
     private boolean mNetworkQualityEstimatorEnabled;
 
     /**
      * Locks operations on network quality listeners, because listener
      * addition and removal may occur on a different thread from notification.
      */
     private final Object mNetworkQualityLock = new Object();
 
@@ skipping 100 matching lines @@
         for (Builder.Pkp pkp : builder.publicKeyPins()) {
             nativeAddPkp(urlRequestContextConfig, pkp.mHost, pkp.mHashes, pkp.mIncludeSubdomains,
                     pkp.mExpirationDate.getTime());
         }
         return urlRequestContextConfig;
     }
 
     @Override
     public UrlRequest createRequest(String url, UrlRequest.Callback callback, Executor executor,
             int priority, Collection<Object> requestAnnotations, boolean disableCache,
-            boolean disableConnectionMigration) {
+            boolean disableConnectionMigration, boolean allowDirectExecutor) {
         synchronized (mLock) {
             checkHaveAdapter();
             boolean metricsCollectionEnabled = false;
             synchronized (mFinishedListenerLock) {
                 metricsCollectionEnabled = !mFinishedListenerList.isEmpty();
             }
             return new CronetUrlRequest(this, url, priority, callback, executor, requestAnnotations,
-                    metricsCollectionEnabled, disableCache, disableConnectionMigration);
+                    metricsCollectionEnabled, disableCache, disableConnectionMigration,
+                    allowDirectExecutor);
         }
     }
 
     @Override
     public BidirectionalStream createBidirectionalStream(String url,
             BidirectionalStream.Callback callback, Executor executor, String httpMethod,
             List<Map.Entry<String, String>> requestHeaders,
             @BidirectionalStream.Builder.StreamPriority int priority,
             boolean delayRequestHeadersUntilFirstFlush) {
         synchronized (mLock) {
@@ skipping 109 matching lines @@
     public byte[] getGlobalMetricsDeltas() {
         return nativeGetHistogramDeltas();
     }
 
     @Override
     public int getEffectiveConnectionType() {
         if (!mNetworkQualityEstimatorEnabled) {
             throw new IllegalStateException("Network quality estimator must be enabled");
         }
         synchronized (mNetworkQualityLock) {
-            checkHaveAdapter();
+            synchronized (mLock) {
+                checkHaveAdapter();
+            }
             return mEffectiveConnectionType;
         }
     }
 
     @VisibleForTesting
     @Override
     public void configureNetworkQualityEstimatorForTesting(
             boolean useLocalHostRequests, boolean useSmallerResponses) {
         if (!mNetworkQualityEstimatorEnabled) {
             throw new IllegalStateException("Network quality estimator must be enabled");
@@ skipping 122 matching lines @@
     }
 
     @VisibleForTesting
     public long getUrlRequestContextAdapter() {
         synchronized (mLock) {
             checkHaveAdapter();
             return mUrlRequestContextAdapter;
         }
     }
 
+    @GuardedBy("mLock")
     private void checkHaveAdapter() throws IllegalStateException {

[mef, 2016/09/01 15:17:18]

We use naming convention XyzLocked in CronetUrlRequest for methods guarded by
the lock (for example isDoneLocked()).
Maybe we should do the same here and below?

[Charles, 2016/09/01 16:45:24]

But why? The compiler checks your locking once you've annotated with @GuardedBy.

         if (!haveRequestContextAdapter()) {
             throw new IllegalStateException("Engine is shut down.");
         }
     }
 
+    @GuardedBy("mLock")
     private boolean haveRequestContextAdapter() {
         return mUrlRequestContextAdapter != 0;
     }
 
     /**
      * @return loggingLevel see {@link #LOG_NONE}, {@link #LOG_DEBUG} and
      *         {@link #LOG_VERBOSE}.
      */
     private int getLoggingLevel() {
         int loggingLevel;
         if (Log.isLoggable(LOG_TAG, Log.VERBOSE)) {
             loggingLevel = LOG_VERBOSE;
         } else if (Log.isLoggable(LOG_TAG, Log.DEBUG)) {
             loggingLevel = LOG_DEBUG;
         } else {
             loggingLevel = LOG_NONE;
         }
         return loggingLevel;
     }
 
     @SuppressWarnings("unused")
     @CalledByNative
     private void initNetworkThread() {
-        synchronized (mLock) {
-            mNetworkThread = Thread.currentThread();
-            mInitCompleted.open();
-        }
+        mNetworkThread = Thread.currentThread();
+        mInitCompleted.open();
         Thread.currentThread().setName("ChromiumNet");
         Process.setThreadPriority(Process.THREAD_PRIORITY_BACKGROUND);
     }
 
     @SuppressWarnings("unused")
     @CalledByNative
     private void onEffectiveConnectionTypeChanged(int effectiveConnectionType) {
         synchronized (mNetworkQualityLock) {
             // Convert the enum returned by the network quality estimator to an enum of type
             // EffectiveConnectionType.
@@ skipping 110 matching lines @@
 
     @NativeClassQualifiedName("CronetURLRequestContextAdapter")
     private native void nativeConfigureNetworkQualityEstimatorForTesting(
             long nativePtr, boolean useLocalHostRequests, boolean useSmallerResponses);
 
     @NativeClassQualifiedName("CronetURLRequestContextAdapter")
     private native void nativeProvideRTTObservations(long nativePtr, boolean should);
 
     @NativeClassQualifiedName("CronetURLRequestContextAdapter")
     private native void nativeProvideThroughputObservations(long nativePtr, boolean should);
+
+    public boolean isNetworkThread(Thread thread) {
+        return thread == mNetworkThread;
+    }
 }