From 49325ea1c36d0284555a510583745b6407637984 Mon Sep 17 00:00:00 2001 From: Fabian Kozynski Date: Tue, 12 May 2020 12:02:40 -0400 Subject: [PATCH] Move IPC calls to background thread in CarrierTextController In CarrierTextController, move call to ConnectivityManager to the background thread and cache its value. Move handleSetListening completely to background thread (make sure that callback call is posted in main thread). Also, get KeyguardUpdateMonitor only once, not necessary to keep track of whether we can listen to calls (as we have a cached value now). Test: atest CarrierTextControllerTest Test: manual (start listening), does not trigger StrictMode Fixes: 140034799 Change-Id: Iaf0771e8d71e07212998f44d73465b1a81b2e268 --- .../keyguard/CarrierTextController.java | 77 +++++++++++-------- .../keyguard/CarrierTextControllerTest.java | 2 + 2 files changed, 49 insertions(+), 30 deletions(-) diff --git a/packages/SystemUI/src/com/android/keyguard/CarrierTextController.java b/packages/SystemUI/src/com/android/keyguard/CarrierTextController.java index 0106609b92712..b1e14346c3fab 100644 --- a/packages/SystemUI/src/com/android/keyguard/CarrierTextController.java +++ b/packages/SystemUI/src/com/android/keyguard/CarrierTextController.java @@ -19,8 +19,6 @@ package com.android.keyguard; import static android.telephony.PhoneStateListener.LISTEN_ACTIVE_DATA_SUBSCRIPTION_ID_CHANGE; import static android.telephony.PhoneStateListener.LISTEN_NONE; -import static com.android.systemui.DejankUtils.whitelistIpcs; - import android.content.Context; import android.content.Intent; import android.content.IntentFilter; @@ -47,6 +45,7 @@ import com.android.systemui.keyguard.WakefulnessLifecycle; import java.util.List; import java.util.Objects; +import java.util.concurrent.atomic.AtomicBoolean; import javax.inject.Inject; @@ -61,9 +60,11 @@ public class CarrierTextController { private final boolean mIsEmergencyCallCapable; private final Handler mMainHandler; + private final Handler mBgHandler; private boolean mTelephonyCapable; private boolean mShowMissingSim; private boolean mShowAirplaneMode; + private final AtomicBoolean mNetworkSupported = new AtomicBoolean(); @VisibleForTesting protected KeyguardUpdateMonitor mKeyguardUpdateMonitor; private WifiManager mWifiManager; @@ -78,12 +79,14 @@ public class CarrierTextController { new WakefulnessLifecycle.Observer() { @Override public void onFinishedWakingUp() { - if (mCarrierTextCallback != null) mCarrierTextCallback.finishedWakingUp(); + final CarrierTextCallback callback = mCarrierTextCallback; + if (callback != null) callback.finishedWakingUp(); } @Override public void onStartedGoingToSleep() { - if (mCarrierTextCallback != null) mCarrierTextCallback.startedGoingToSleep(); + final CarrierTextCallback callback = mCarrierTextCallback; + if (callback != null) callback.startedGoingToSleep(); } }; @@ -131,7 +134,7 @@ public class CarrierTextController { @Override public void onActiveDataSubscriptionIdChanged(int subId) { mActiveMobileDataSubscription = subId; - if (mKeyguardUpdateMonitor != null) { + if (mNetworkSupported.get() && mCarrierTextCallback != null) { updateCarrierText(); } } @@ -173,6 +176,17 @@ public class CarrierTextController { mSimSlotsNumber = getTelephonyManager().getSupportedModemCount(); mSimErrorState = new boolean[mSimSlotsNumber]; mMainHandler = Dependency.get(Dependency.MAIN_HANDLER); + mBgHandler = new Handler(Dependency.get(Dependency.BG_LOOPER)); + mKeyguardUpdateMonitor = Dependency.get(KeyguardUpdateMonitor.class); + mBgHandler.post(() -> { + boolean supported = ConnectivityManager.from(mContext).isNetworkSupported( + ConnectivityManager.TYPE_MOBILE); + if (supported && mNetworkSupported.compareAndSet(false, supported)) { + // This will set/remove the listeners appropriately. Note that it will never double + // add the listeners. + handleSetListening(mCarrierTextCallback); + } + }); } private TelephonyManager getTelephonyManager() { @@ -221,48 +235,51 @@ public class CarrierTextController { } /** - * Sets the listening status of this controller. If the callback is null, it is set to - * not listening + * This may be called internally after retrieving the correct value of {@code mNetworkSupported} + * (assumed false to start). In that case, the following happens: + * * - * @param callback Callback to provide text updates + * This call will always be processed in a background thread. */ - public void setListening(CarrierTextCallback callback) { + private void handleSetListening(CarrierTextCallback callback) { TelephonyManager telephonyManager = getTelephonyManager(); if (callback != null) { mCarrierTextCallback = callback; - // TODO(b/140034799) - if (whitelistIpcs(() -> ConnectivityManager.from(mContext).isNetworkSupported( - ConnectivityManager.TYPE_MOBILE))) { - mKeyguardUpdateMonitor = Dependency.get(KeyguardUpdateMonitor.class); + if (mNetworkSupported.get()) { // Keyguard update monitor expects callbacks from main thread - mMainHandler.post(() -> { - if (mKeyguardUpdateMonitor != null) { - mKeyguardUpdateMonitor.registerCallback(mCallback); - } - }); + mMainHandler.post(() -> mKeyguardUpdateMonitor.registerCallback(mCallback)); mWakefulnessLifecycle.addObserver(mWakefulnessObserver); telephonyManager.listen(mPhoneStateListener, LISTEN_ACTIVE_DATA_SUBSCRIPTION_ID_CHANGE); } else { // Don't listen and clear out the text when the device isn't a phone. - mKeyguardUpdateMonitor = null; - callback.updateCarrierInfo(new CarrierTextCallbackInfo("", null, false, null)); + mMainHandler.post(() -> callback.updateCarrierInfo( + new CarrierTextCallbackInfo("", null, false, null) + )); } } else { mCarrierTextCallback = null; - if (mKeyguardUpdateMonitor != null) { - // Keyguard update monitor expects callbacks from main thread - mMainHandler.post(() -> { - if (mKeyguardUpdateMonitor != null) { - mKeyguardUpdateMonitor.removeCallback(mCallback); - } - }); - mWakefulnessLifecycle.removeObserver(mWakefulnessObserver); - } + mMainHandler.post(() -> mKeyguardUpdateMonitor.removeCallback(mCallback)); + mWakefulnessLifecycle.removeObserver(mWakefulnessObserver); telephonyManager.listen(mPhoneStateListener, LISTEN_NONE); } } + /** + * Sets the listening status of this controller. If the callback is null, it is set to + * not listening. + * + * @param callback Callback to provide text updates + */ + public void setListening(CarrierTextCallback callback) { + mBgHandler.post(() -> handleSetListening(callback)); + } + protected List getSubscriptionInfo() { return mKeyguardUpdateMonitor.getFilteredSubscriptionInfo(false); } @@ -500,7 +517,7 @@ public class CarrierTextController { */ private CarrierTextController.StatusMode getStatusForIccState(int simState) { final boolean missingAndNotProvisioned = - !Dependency.get(KeyguardUpdateMonitor.class).isDeviceProvisioned() + !mKeyguardUpdateMonitor.isDeviceProvisioned() && (simState == TelephonyManager.SIM_STATE_ABSENT || simState == TelephonyManager.SIM_STATE_PERM_DISABLED); diff --git a/packages/SystemUI/tests/src/com/android/keyguard/CarrierTextControllerTest.java b/packages/SystemUI/tests/src/com/android/keyguard/CarrierTextControllerTest.java index 870010a1fbad2..aa4122fd190a6 100644 --- a/packages/SystemUI/tests/src/com/android/keyguard/CarrierTextControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/keyguard/CarrierTextControllerTest.java @@ -134,6 +134,7 @@ public class CarrierTextControllerTest extends SysuiTestCase { mDependency.injectMockDependency(WakefulnessLifecycle.class); mDependency.injectTestDependency(Dependency.MAIN_HANDLER, new Handler(mTestableLooper.getLooper())); + mDependency.injectTestDependency(Dependency.BG_LOOPER, mTestableLooper.getLooper()); mDependency.injectTestDependency(KeyguardUpdateMonitor.class, mKeyguardUpdateMonitor); doAnswer(this::checkMainThread).when(mKeyguardUpdateMonitor) @@ -150,6 +151,7 @@ public class CarrierTextControllerTest extends SysuiTestCase { // This should not start listening on any of the real dependencies but will test that // callbacks in mKeyguardUpdateMonitor are done in the mTestableLooper thread mCarrierTextController.setListening(mCarrierTextCallback); + mTestableLooper.processAllMessages(); } @Test