From f9354793f965ac3bb006dbc12f5c867c7c51f39a Mon Sep 17 00:00:00 2001 From: Remi NGUYEN VAN Date: Tue, 27 Aug 2019 20:21:58 +0900 Subject: [PATCH] Run callbacks on ConnectivityService thread Run MultinetworkPolicyTracker and DataConnectionStats callbacks on the ConnectivityService handler thread. Previously the callbacks would be using the SystemServer foreground thread (Looper.myLooper()), or the broadcast thread for the MultinetworkPolicyTracker BroadcastReceiver. This is error-prone, can cause threading issues and makes it difficult to test the components. Test: atest FrameworksNetTests Change-Id: I189213dd363004abed294659165bf5430d153bba --- .../net/util/MultinetworkPolicyTracker.java | 29 +++++++++++-------- .../android/server/ConnectivityService.java | 2 +- .../connectivity/DataConnectionStats.java | 16 ++++++++-- 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/core/java/android/net/util/MultinetworkPolicyTracker.java b/core/java/android/net/util/MultinetworkPolicyTracker.java index f7e494d830ace..4e88149b80953 100644 --- a/core/java/android/net/util/MultinetworkPolicyTracker.java +++ b/core/java/android/net/util/MultinetworkPolicyTracker.java @@ -64,7 +64,7 @@ public class MultinetworkPolicyTracker { private final Context mContext; private final Handler mHandler; - private final Runnable mReevaluateRunnable; + private final Runnable mAvoidBadWifiCallback; private final List mSettingsUris; private final ContentResolver mResolver; private final SettingObserver mSettingObserver; @@ -81,12 +81,7 @@ public class MultinetworkPolicyTracker { public MultinetworkPolicyTracker(Context ctx, Handler handler, Runnable avoidBadWifiCallback) { mContext = ctx; mHandler = handler; - mReevaluateRunnable = () -> { - if (updateAvoidBadWifi() && avoidBadWifiCallback != null) { - avoidBadWifiCallback.run(); - } - updateMeteredMultipathPreference(); - }; + mAvoidBadWifiCallback = avoidBadWifiCallback; mSettingsUris = Arrays.asList( Settings.Global.getUriFor(NETWORK_AVOID_BAD_WIFI), Settings.Global.getUriFor(NETWORK_METERED_MULTIPATH_PREFERENCE)); @@ -95,15 +90,15 @@ public class MultinetworkPolicyTracker { mBroadcastReceiver = new BroadcastReceiver() { @Override public void onReceive(Context context, Intent intent) { - reevaluate(); + reevaluateInternal(); } }; - TelephonyManager.from(ctx).listen(new PhoneStateListener() { + TelephonyManager.from(ctx).listen(new PhoneStateListener(handler.getLooper()) { @Override public void onActiveDataSubscriptionIdChanged(int subId) { mActiveSubId = subId; - reevaluate(); + reevaluateInternal(); } }, PhoneStateListener.LISTEN_ACTIVE_DATA_SUBSCRIPTION_ID_CHANGE); @@ -119,7 +114,7 @@ public class MultinetworkPolicyTracker { final IntentFilter intentFilter = new IntentFilter(); intentFilter.addAction(Intent.ACTION_CONFIGURATION_CHANGED); mContext.registerReceiverAsUser( - mBroadcastReceiver, UserHandle.ALL, intentFilter, null, null); + mBroadcastReceiver, UserHandle.ALL, intentFilter, null, mHandler); reevaluate(); } @@ -164,7 +159,17 @@ public class MultinetworkPolicyTracker { @VisibleForTesting public void reevaluate() { - mHandler.post(mReevaluateRunnable); + mHandler.post(this::reevaluateInternal); + } + + /** + * Reevaluate the settings. Must be called on the handler thread. + */ + private void reevaluateInternal() { + if (updateAvoidBadWifi() && mAvoidBadWifiCallback != null) { + mAvoidBadWifiCallback.run(); + } + updateMeteredMultipathPreference(); } public boolean updateAvoidBadWifi() { diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 667445b1c58f4..62db5578337c2 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -1101,7 +1101,7 @@ public class ConnectivityService extends IConnectivityManager.Stub mSettingsObserver = new SettingsObserver(mContext, mHandler); registerSettingsCallbacks(); - final DataConnectionStats dataConnectionStats = new DataConnectionStats(mContext); + final DataConnectionStats dataConnectionStats = new DataConnectionStats(mContext, mHandler); dataConnectionStats.startMonitoring(); mKeepaliveTracker = new KeepaliveTracker(mContext, mHandler); diff --git a/services/core/java/com/android/server/connectivity/DataConnectionStats.java b/services/core/java/com/android/server/connectivity/DataConnectionStats.java index 227ab2341012c..e6a4428ef219b 100644 --- a/services/core/java/com/android/server/connectivity/DataConnectionStats.java +++ b/services/core/java/com/android/server/connectivity/DataConnectionStats.java @@ -21,6 +21,8 @@ import android.content.Context; import android.content.Intent; import android.content.IntentFilter; import android.net.ConnectivityManager; +import android.os.Handler; +import android.os.Looper; import android.os.RemoteException; import android.telephony.PhoneStateListener; import android.telephony.ServiceState; @@ -39,15 +41,19 @@ public class DataConnectionStats extends BroadcastReceiver { private final Context mContext; private final IBatteryStats mBatteryStats; + private final Handler mListenerHandler; + private final PhoneStateListener mPhoneStateListener; private IccCardConstants.State mSimState = IccCardConstants.State.READY; private SignalStrength mSignalStrength; private ServiceState mServiceState; private int mDataState = TelephonyManager.DATA_DISCONNECTED; - public DataConnectionStats(Context context) { + public DataConnectionStats(Context context, Handler listenerHandler) { mContext = context; mBatteryStats = BatteryStatsService.getService(); + mListenerHandler = listenerHandler; + mPhoneStateListener = new PhoneStateListenerImpl(listenerHandler.getLooper()); } public void startMonitoring() { @@ -63,7 +69,7 @@ public class DataConnectionStats extends BroadcastReceiver { filter.addAction(TelephonyIntents.ACTION_SIM_STATE_CHANGED); filter.addAction(ConnectivityManager.CONNECTIVITY_ACTION); filter.addAction(ConnectivityManager.INET_CONDITION_ACTION); - mContext.registerReceiver(this, filter); + mContext.registerReceiver(this, filter, null /* broadcastPermission */, mListenerHandler); } @Override @@ -128,7 +134,11 @@ public class DataConnectionStats extends BroadcastReceiver { && mServiceState.getState() != ServiceState.STATE_POWER_OFF; } - private final PhoneStateListener mPhoneStateListener = new PhoneStateListener() { + private class PhoneStateListenerImpl extends PhoneStateListener { + PhoneStateListenerImpl(Looper looper) { + super(looper); + } + @Override public void onSignalStrengthsChanged(SignalStrength signalStrength) { mSignalStrength = signalStrength;