net/android/java/src/org/chromium/net/NetworkChangeNotifierAutoDetect.java - Issue 2812963002: [Cronet] Move initialization to a new thread rather than the UI thread.

Unified Diff: net/android/java/src/org/chromium/net/NetworkChangeNotifierAutoDetect.java

Issue 2812963002: [Cronet] Move initialization to a new thread rather than the UI thread. (Closed)

Patch Set: mostly renaming Created 3 years, 8 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

« components/cronet/android/java/src/org/chromium/net/impl/CronetLibraryLoader.java ('K') | « net/android/BUILD.gn ('k') | net/android/java/src/org/chromium/net/ProxyChangeListener.java » ('j') | net/android/java/src/org/chromium/net/ProxyChangeListener.java » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

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

diff --git a/net/android/java/src/org/chromium/net/NetworkChangeNotifierAutoDetect.java b/net/android/java/src/org/chromium/net/NetworkChangeNotifierAutoDetect.java

index 19a3e33512af38609d7f94b396a4edbd9c58afc2..25e752c23b44f7c83fa046a09efc3ec977cf7689 100644

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

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

@@ -26,11 +26,13 @@ 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;

@@ -365,7 +367,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {

}

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) {

@@ -407,7 +409,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {

// 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

@@ -482,7 +484,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {

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);

@@ -506,7 +508,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {

// 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);

@@ -520,7 +522,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {

return;

}

final long netId = networkToNetId(network);

- ThreadUtils.postOnUiThread(new Runnable() {

+ runOnThread(new Runnable() {

@Override

public void run() {

mObserver.onNetworkSoonToDisconnect(netId);

@@ -533,7 +535,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {

if (ignoreNetworkDueToVpn(network)) {

return;

}

- ThreadUtils.postOnUiThread(new Runnable() {

+ runOnThread(new Runnable() {

@Override

public void run() {

mObserver.onNetworkDisconnect(networkToNetId(network));

@@ -551,7 +553,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {

}

@ConnectionTypeEnum

final int newConnectionType = convertToConnectionType(getCurrentNetworkState());

- ThreadUtils.postOnUiThread(new Runnable() {

+ runOnThread(new Runnable() {

@Override

public void run() {

mObserver.onConnectionTypeChanged(newConnectionType);

@@ -595,9 +597,12 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {

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;

@@ -674,7 +679,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {

}

/**

- * 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 notificatio

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}).

@@ -682,9 +687,8 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {

@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);

@@ -713,6 +717,25 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {

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.

@@ -741,6 +764,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {

}

public void destroy() {

+ assertOnThread();

mRegistrationPolicy.destroy();

unregister();

}

@@ -749,7 +773,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {

* Registers a BroadcastReceiver in the given context.

public void register() {

- ThreadUtils.assertOnUiThread();

+ assertOnThread();

if (mRegistered) return;

if (mShouldSignalObserver) {

@@ -789,6 +813,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {

* Unregisters a BroadcastReceiver in the given context.

public void unregister() {

+ assertOnThread();

if (!mRegistered) return;

mContext.unregisterReceiver(this);

mRegistered = false;

@@ -996,15 +1021,21 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {

// 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 chec

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) {