[Cronet] Move initialization to a new thread rather than the UI thread.

net/android/java/src/org/chromium/net/NetworkChangeNotifierAutoDetect.java

 // Copyright 2012 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 static android.net.ConnectivityManager.TYPE_VPN;
 import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET;
 import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_VPN;
 import static android.net.NetworkCapabilities.TRANSPORT_VPN;
 
 import android.Manifest.permission;
 import android.annotation.SuppressLint;
 import android.annotation.TargetApi;
 import android.content.BroadcastReceiver;
 import android.content.Context;
 import android.content.Intent;
 import android.content.IntentFilter;
 import android.content.pm.PackageManager;
 import android.net.ConnectivityManager;
 import android.net.ConnectivityManager.NetworkCallback;
 import android.net.Network;
 import android.net.NetworkCapabilities;
 import android.net.NetworkInfo;
 import android.net.NetworkRequest;
 import android.net.wifi.WifiInfo;
 import android.net.wifi.WifiManager;
 import android.os.Build;
+import android.os.Handler;
+import android.os.Looper;
 import android.telephony.TelephonyManager;
 
 import org.chromium.base.ApplicationState;
 import org.chromium.base.ApplicationStatus;
-import org.chromium.base.ThreadUtils;
+import org.chromium.base.BuildConfig;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.net.ConnectionType.ConnectionTypeEnum;
 
 import java.io.IOException;
 import java.util.Arrays;
 
 import javax.annotation.concurrent.GuardedBy;
 
 /**
@@ skipping 314 matching lines @@
                                          permission.ACCESS_WIFI_STATE, mContext.getPackageName())
                     == PackageManager.PERMISSION_GRANTED;
             mWifiManager = mHasWifiPermission
                     ? (WifiManager) mContext.getSystemService(Context.WIFI_SERVICE)
                     : null;
             mHasWifiPermissionComputed = true;
             return mHasWifiPermission;
         }
 
         String getWifiSsid() {
-            // Synchronized because this method can be called on multiple threads (e.g. UI thread
+            // Synchronized because this method can be called on multiple threads (e.g. mLooper
             // from a private caller, and another thread calling a public API like
             // getCurrentNetworkState) and is otherwise racy.
             synchronized (mLock) {
                 // If app has permission it's faster to query WifiManager directly.
                 if (hasPermissionLocked()) {
                     WifiInfo wifiInfo = getWifiInfoLocked();
                     if (wifiInfo != null) {
                         return wifiInfo.getSSID();
                     }
                     return "";
@@ skipping 21 matching lines @@
                     RecordHistogram.recordBooleanHistogram("NCN.getWifiInfo2ndSuccess", false);
                     return null;
                 }
             }
         }
     }
 
     // This class gets called back by ConnectivityManager whenever networks come
     // and go. It gets called back on a special handler thread
     // ConnectivityManager creates for making the callbacks. The callbacks in
-    // turn post to the UI thread where mObserver lives.
+    // turn post to mLooper where mObserver lives.
     @TargetApi(Build.VERSION_CODES.LOLLIPOP)
     private class MyNetworkCallback extends NetworkCallback {
         // If non-null, this indicates a VPN is in place for the current user, and no other
         // networks are accessible.
         private Network mVpnInPlace;
 
         // Initialize mVpnInPlace.
         void initializeVpnInPlace() {
             final Network[] networks = getAllNetworksFiltered(mConnectivityManagerDelegate, null);
             mVpnInPlace = null;
@@ skipping 54 matching lines @@
             if (ignoreConnectedNetwork(network, capabilities)) {
                 return;
             }
             final boolean makeVpnDefault = capabilities.hasTransport(TRANSPORT_VPN);
             if (makeVpnDefault) {
                 mVpnInPlace = network;
             }
             final long netId = networkToNetId(network);
             @ConnectionTypeEnum
             final int connectionType = mConnectivityManagerDelegate.getConnectionType(network);
-            ThreadUtils.postOnUiThread(new Runnable() {
+            runOnThread(new Runnable() {
                 @Override
                 public void run() {
                     mObserver.onNetworkConnect(netId, connectionType);
                     if (makeVpnDefault) {
                         // Make VPN the default network.
                         mObserver.onConnectionTypeChanged(connectionType);
                         // Purge all other networks as they're inaccessible to Chrome now.
                         mObserver.purgeActiveNetworkList(new long[] {netId});
                     }
                 }
             });
         }
 
         @Override
         public void onCapabilitiesChanged(
                 Network network, NetworkCapabilities networkCapabilities) {
             if (ignoreConnectedNetwork(network, networkCapabilities)) {
                 return;
             }
             // A capabilities change may indicate the ConnectionType has changed,
             // so forward the new ConnectionType along to observer.
             final long netId = networkToNetId(network);
             final int connectionType = mConnectivityManagerDelegate.getConnectionType(network);
-            ThreadUtils.postOnUiThread(new Runnable() {
+            runOnThread(new Runnable() {
                 @Override
                 public void run() {
                     mObserver.onNetworkConnect(netId, connectionType);
                 }
             });
         }
 
         @Override
         public void onLosing(Network network, int maxMsToLive) {
             if (ignoreConnectedNetwork(network, null)) {
                 return;
             }
             final long netId = networkToNetId(network);
-            ThreadUtils.postOnUiThread(new Runnable() {
+            runOnThread(new Runnable() {
                 @Override
                 public void run() {
                     mObserver.onNetworkSoonToDisconnect(netId);
                 }
             });
         }
 
         @Override
         public void onLost(final Network network) {
             if (ignoreNetworkDueToVpn(network)) {
                 return;
             }
-            ThreadUtils.postOnUiThread(new Runnable() {
+            runOnThread(new Runnable() {
                 @Override
                 public void run() {
                     mObserver.onNetworkDisconnect(networkToNetId(network));
                 }
             });
             // If the VPN is going away, inform observer that other networks that were previously
             // hidden by ignoreNetworkDueToVpn() are now available for use, now that this user's
             // traffic is not forced into the VPN.
             if (mVpnInPlace != null) {
                 assert network.equals(mVpnInPlace);
                 mVpnInPlace = null;
                 for (Network newNetwork :
                         getAllNetworksFiltered(mConnectivityManagerDelegate, network)) {
                     onAvailable(newNetwork);
                 }
                 @ConnectionTypeEnum
                 final int newConnectionType = convertToConnectionType(getCurrentNetworkState());
-                ThreadUtils.postOnUiThread(new Runnable() {
+                runOnThread(new Runnable() {
                     @Override
                     public void run() {
                         mObserver.onConnectionTypeChanged(newConnectionType);
                     }
                 });
             }
         }
     }
 
     /**
@@ skipping 23 matching lines @@
          * Initializes the policy with the notifier, overriding subclasses should always
          * call this method.
          */
         protected void init(NetworkChangeNotifierAutoDetect notifier) {
             mNotifier = notifier;
         }
 
         protected abstract void destroy();
     }
 
-    private static final String TAG = "NetworkChangeNotifierAutoDetect";
+    private static final String TAG = NetworkChangeNotifierAutoDetect.class.getSimpleName();
     private static final int UNKNOWN_LINK_SPEED = -1;
 
+    private final Looper mLooper;
+    private final Handler mHandler;
+

[xunjieli, 2017/04/13 18:44:54]

nit: maybe get rid of this blank line.

[pauljensen, 2017/05/01 15:35:53]

Done, I also added some comments.

     private final NetworkConnectivityIntentFilter mIntentFilter;
     private final Observer mObserver;
     private final Context mContext;
     private final RegistrationPolicy mRegistrationPolicy;
 
     // mConnectivityManagerDelegates and mWifiManagerDelegate are only non-final for testing.
     private ConnectivityManagerDelegate mConnectivityManagerDelegate;
     private WifiManagerDelegate mWifiManagerDelegate;
     // mNetworkCallback and mNetworkRequest are only non-null in Android L and above.
     private final MyNetworkCallback mNetworkCallback;
@@ skipping 56 matching lines @@
          * Called to cause a purge of cached lists of active networks, of any
          * networks not in the accompanying list of active networks. This is
          * issued if a period elapsed where disconnected notifications may have
          * been missed, and acts to keep cached lists of active networks
          * accurate. Only called on Android L and above.
          */
         public void purgeActiveNetworkList(long[] activeNetIds);
     }
 
     /**
-     * Constructs a NetworkChangeNotifierAutoDetect. Should only be called on UI thread.
+     * Constructs a NetworkChangeNotifierAutoDetect.  Lives on calling thread.

[xunjieli, 2017/04/13 18:44:54]

Could you add the part about receiving notifications from the UI thread?

something like:
Lives on the calling thread, receives broadcast notifications on the UI thread
,and forwards the notifications to the calling thread.

[pauljensen, 2017/05/01 15:35:53]

Done.

      * @param policy The RegistrationPolicy which determines when this class should watch
      *     for network changes (e.g. see (@link RegistrationPolicyAlwaysRegister} and
      *     {@link RegistrationPolicyApplicationStatus}).
      */
     @TargetApi(Build.VERSION_CODES.LOLLIPOP)
     public NetworkChangeNotifierAutoDetect(
             Observer observer, Context context, RegistrationPolicy policy) {
-        // Since BroadcastReceiver is always called back on UI thread, ensure
-        // running on UI thread so notification logic can be single-threaded.
-        ThreadUtils.assertOnUiThread();
+        mLooper = Looper.myLooper();
+        mHandler = new Handler(mLooper);
         mObserver = observer;
         mContext = context.getApplicationContext();
         mConnectivityManagerDelegate = new ConnectivityManagerDelegate(context);
         mWifiManagerDelegate = new WifiManagerDelegate(context);
         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
             mNetworkCallback = new MyNetworkCallback();
             mNetworkRequest = new NetworkRequest.Builder()
                                       .addCapability(NET_CAPABILITY_INTERNET)
                                       // Need to hear about VPNs too.
                                       .removeCapability(NET_CAPABILITY_NOT_VPN)
                                       .build();
         } else {
             mNetworkCallback = null;
             mNetworkRequest = null;
         }
         final NetworkState networkState = getCurrentNetworkState();
         mConnectionType = convertToConnectionType(networkState);
         mWifiSSID = networkState.getWifiSsid();
         mMaxBandwidthMbps = getCurrentMaxBandwidthInMbps(networkState);
         mMaxBandwidthConnectionType = mConnectionType;
         mIntentFilter = new NetworkConnectivityIntentFilter();
         mIgnoreNextBroadcast = false;
         mShouldSignalObserver = false;
         mRegistrationPolicy = policy;
         mRegistrationPolicy.init(this);
         mShouldSignalObserver = true;
     }
 
+    private boolean onThread() {
+        return mLooper == Looper.myLooper();
+    }
+
+    private void assertOnThread() {
+        if (BuildConfig.DCHECK_IS_ON && !onThread()) {
+            throw new IllegalStateException(
+                    "Must be called on NetworkChangeNotifierAutoDetect thread.");
+        }
+    }
+
+    private void runOnThread(Runnable r) {
+        if (onThread()) {
+            r.run();
+        } else {
+            mHandler.post(r);
+        }
+    }
+
     /**
      * Allows overriding the ConnectivityManagerDelegate for tests.
      */
     void setConnectivityManagerDelegateForTests(ConnectivityManagerDelegate delegate) {
         mConnectivityManagerDelegate = delegate;
     }
 
     /**
      * Allows overriding the WifiManagerDelegate for tests.
      */
     void setWifiManagerDelegateForTests(WifiManagerDelegate delegate) {
         mWifiManagerDelegate = delegate;
     }
 
     @VisibleForTesting
     RegistrationPolicy getRegistrationPolicy() {
         return mRegistrationPolicy;
     }
 
     /**
      * Returns whether the object has registered to receive network connectivity intents.
      */
     @VisibleForTesting
     boolean isReceiverRegisteredForTesting() {
         return mRegistered;
     }
 
     public void destroy() {
+        assertOnThread();
         mRegistrationPolicy.destroy();
         unregister();
     }
 
     /**
      * Registers a BroadcastReceiver in the given context.
      */
     public void register() {
-        ThreadUtils.assertOnUiThread();
+        assertOnThread();
         if (mRegistered) return;
 
         if (mShouldSignalObserver) {
             final NetworkState networkState = getCurrentNetworkState();
             connectionTypeChanged(networkState);
             maxBandwidthChanged(networkState);
         }
         // When registering for a sticky broadcast, like CONNECTIVITY_ACTION, if registerReceiver
         // returns non-null, it means the broadcast was previously issued and onReceive() will be
         // immediately called with this previous Intent. Since this initial callback doesn't
@@ skipping 19 matching lines @@
                 }
                 mObserver.purgeActiveNetworkList(netIds);
             }
         }
     }
 
     /**
      * Unregisters a BroadcastReceiver in the given context.
      */
     public void unregister() {
+        assertOnThread();
         if (!mRegistered) return;
         mContext.unregisterReceiver(this);
         mRegistered = false;
         if (mNetworkCallback != null) {
             mConnectivityManagerDelegate.unregisterNetworkCallback(mNetworkCallback);
         }
     }
 
     public NetworkState getCurrentNetworkState() {
         return mConnectivityManagerDelegate.getNetworkState(mWifiManagerDelegate);
@@ skipping 187 matching lines @@
      * instead. For more on NetInfo, see http://w3c.github.io/netinfo/.
      */
     public double getCurrentMaxBandwidthInMbps(NetworkState networkState) {
         return NetworkChangeNotifier.getMaxBandwidthForConnectionSubtype(
                 convertToConnectionSubtype(networkState));
     }
 
     // BroadcastReceiver
     @Override
     public void onReceive(Context context, Intent intent) {
-        if (mIgnoreNextBroadcast) {
-            mIgnoreNextBroadcast = false;
-            return;
-        }
-        final NetworkState networkState = getCurrentNetworkState();
-        if (ConnectivityManager.CONNECTIVITY_ACTION.equals(intent.getAction())) {
-            connectionTypeChanged(networkState);
-            maxBandwidthChanged(networkState);
-        }
+        runOnThread(new Runnable() {
+            @Override
+            public void run() {
+                if (!mRegistered) {

[xunjieli, 2017/04/13 18:44:54]

I think this needs a comment to note why this check is needed. It's not obvious
from a glance that this receiver might be unregistered when the task runs.

[pauljensen, 2017/05/01 15:35:53]

Done.

+                    return;
+                }
+                if (mIgnoreNextBroadcast) {
+                    mIgnoreNextBroadcast = false;
+                    return;
+                }
+                final NetworkState networkState = getCurrentNetworkState();
+                connectionTypeChanged(networkState);
+                maxBandwidthChanged(networkState);
+            }
+        });
     }
 
     private void connectionTypeChanged(NetworkState networkState) {
         @ConnectionTypeEnum
         int newConnectionType = convertToConnectionType(networkState);
         String newWifiSSID = networkState.getWifiSsid();
         if (newConnectionType == mConnectionType && newWifiSSID.equals(mWifiSSID)) return;
 
         mConnectionType = newConnectionType;
         mWifiSSID = newWifiSSID;
@@ skipping 30 matching lines @@
             return network.getNetworkHandle();
         } else {
             // NOTE(pauljensen): This depends on Android framework implementation details. These
             // details cannot change because Lollipop is long since released.
             // NetIDs are only 16-bit so use parseInt. This function returns a long because
             // getNetworkHandle() returns a long.
             return Integer.parseInt(network.toString());
         }
     }
 }