From b8354ea3897f5d67a3cf4328ecf2489b356332a9 Mon Sep 17 00:00:00 2001 From: Yuri Lin Date: Tue, 2 May 2023 16:53:28 -0400 Subject: [PATCH] Move zen mode evaluation back into setConfig. The process of setting zen mode during a config change was separated into two pieces because of a cross-lock held with AudioManager. To avoid having lock contention, it suffices to bring the ringer mode changes out to the handler while having the zen mode evaluations happen in one pass. Also remove an unnecessary call to evaluateZenMode in onSystemReady (it's called right after cleanUpZenRules, which will already call setConfig -> evaluateZenMode). Bug: 280333605 Test: ZenModeHelperTest Change-Id: Icc7615e2d02fc8b83613687c8e2bed8e99fa9613 --- .../server/notification/ZenModeHelper.java | 61 ++++++------------- .../server/notification/RoleObserverTest.java | 5 +- .../notification/ZenModeHelperTest.java | 35 ++++++++++- 3 files changed, 56 insertions(+), 45 deletions(-) diff --git a/services/core/java/com/android/server/notification/ZenModeHelper.java b/services/core/java/com/android/server/notification/ZenModeHelper.java index 8277041596283..f38c6c1aa827c 100644 --- a/services/core/java/com/android/server/notification/ZenModeHelper.java +++ b/services/core/java/com/android/server/notification/ZenModeHelper.java @@ -226,7 +226,6 @@ public class ZenModeHelper { mPm = mContext.getPackageManager(); mHandler.postMetricsTimer(); cleanUpZenRules(); - evaluateZenMode("onSystemReady", true); mIsBootComplete = true; showZenUpgradeNotification(mZenMode); } @@ -936,7 +935,10 @@ public class ZenModeHelper { if (policyChanged) { dispatchOnPolicyChanged(); } - mHandler.postApplyConfig(config, reason, triggeringComponent, setRingerMode); + final String val = Integer.toString(config.hashCode()); + Global.putString(mContext.getContentResolver(), Global.ZEN_MODE_CONFIG_ETAG, val); + evaluateZenMode(reason, setRingerMode); + mConditions.evaluateConfig(config, triggeringComponent, true /*processSubscriptions*/); return true; } catch (SecurityException e) { Log.wtf(TAG, "Invalid rule in config", e); @@ -946,14 +948,6 @@ public class ZenModeHelper { } } - private void applyConfig(ZenModeConfig config, String reason, - ComponentName triggeringComponent, boolean setRingerMode) { - final String val = Integer.toString(config.hashCode()); - Global.putString(mContext.getContentResolver(), Global.ZEN_MODE_CONFIG_ETAG, val); - evaluateZenMode(reason, setRingerMode); - mConditions.evaluateConfig(config, triggeringComponent, true /*processSubscriptions*/); - } - private int getZenModeSetting() { return Global.getInt(mContext.getContentResolver(), Global.ZEN_MODE, Global.ZEN_MODE_OFF); } @@ -987,21 +981,23 @@ public class ZenModeHelper { mZenMode = zen; setZenModeSetting(mZenMode); updateConsolidatedPolicy(reason); - updateRingerModeAffectedStreams(); - if (setRingerMode && (zen != zenBefore || (zen == Global.ZEN_MODE_IMPORTANT_INTERRUPTIONS - && policyHashBefore != mConsolidatedPolicy.hashCode()))) { - applyZenToRingerMode(); - } - applyRestrictions(); + boolean shouldApplyToRinger = setRingerMode && (zen != zenBefore || ( + zen == Global.ZEN_MODE_IMPORTANT_INTERRUPTIONS + && policyHashBefore != mConsolidatedPolicy.hashCode())); + mHandler.postUpdateRingerAndAudio(shouldApplyToRinger); if (zen != zenBefore) { mHandler.postDispatchOnZenModeChanged(); } } - private void updateRingerModeAffectedStreams() { + private void updateRingerAndAudio(boolean shouldApplyToRinger) { if (mAudioManager != null) { mAudioManager.updateRingerModeAffectedStreamsInternal(); } + if (shouldApplyToRinger) { + applyZenToRingerMode(); + } + applyRestrictions(); } private int computeZenMode() { @@ -1620,22 +1616,7 @@ public class ZenModeHelper { private final class H extends Handler { private static final int MSG_DISPATCH = 1; private static final int MSG_METRICS = 2; - private static final int MSG_APPLY_CONFIG = 4; - - private final class ConfigMessageData { - public final ZenModeConfig config; - public ComponentName triggeringComponent; - public final String reason; - public final boolean setRingerMode; - - ConfigMessageData(ZenModeConfig config, String reason, - ComponentName triggeringComponent, boolean setRingerMode) { - this.config = config; - this.reason = reason; - this.setRingerMode = setRingerMode; - this.triggeringComponent = triggeringComponent; - } - } + private static final int MSG_RINGER_AUDIO = 5; private static final long METRICS_PERIOD_MS = 6 * 60 * 60 * 1000; @@ -1653,10 +1634,9 @@ public class ZenModeHelper { sendEmptyMessageDelayed(MSG_METRICS, METRICS_PERIOD_MS); } - private void postApplyConfig(ZenModeConfig config, String reason, - ComponentName triggeringComponent, boolean setRingerMode) { - sendMessage(obtainMessage(MSG_APPLY_CONFIG, - new ConfigMessageData(config, reason, triggeringComponent, setRingerMode))); + private void postUpdateRingerAndAudio(boolean shouldApplyToRinger) { + removeMessages(MSG_RINGER_AUDIO); + sendMessage(obtainMessage(MSG_RINGER_AUDIO, shouldApplyToRinger)); } @Override @@ -1668,10 +1648,9 @@ public class ZenModeHelper { case MSG_METRICS: mMetrics.emit(); break; - case MSG_APPLY_CONFIG: - ConfigMessageData applyConfigData = (ConfigMessageData) msg.obj; - applyConfig(applyConfigData.config, applyConfigData.reason, - applyConfigData.triggeringComponent, applyConfigData.setRingerMode); + case MSG_RINGER_AUDIO: + boolean shouldApplyToRinger = (boolean) msg.obj; + updateRingerAndAudio(shouldApplyToRinger); } } } diff --git a/services/tests/uiservicestests/src/com/android/server/notification/RoleObserverTest.java b/services/tests/uiservicestests/src/com/android/server/notification/RoleObserverTest.java index d4a2e9aa4f5b5..66c1e35754c57 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/RoleObserverTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/RoleObserverTest.java @@ -55,6 +55,7 @@ import android.telecom.TelecomManager; import android.telephony.TelephonyManager; import android.test.suitebuilder.annotation.SmallTest; import android.testing.AndroidTestingRunner; +import android.testing.TestableLooper; import android.testing.TestableLooper.RunWithLooper; import android.util.ArraySet; import android.util.AtomicFile; @@ -103,6 +104,7 @@ public class RoleObserverTest extends UiServiceTestCase { private RoleManager mRoleManager; @Mock private Looper mMainLooper; + private TestableLooper mTestableLooper; NotificationRecordLoggerFake mNotificationRecordLogger = new NotificationRecordLoggerFake(); private InstanceIdSequence mNotificationInstanceIdSequence = new InstanceIdSequenceFake( 1 << 30); @@ -144,9 +146,10 @@ public class RoleObserverTest extends UiServiceTestCase { mService = new TestableNotificationManagerService(mContext, mNotificationRecordLogger, mNotificationInstanceIdSequence); mRoleObserver = mService.new RoleObserver(mContext, mRoleManager, mPm, mMainLooper); + mTestableLooper = TestableLooper.get(this); try { - mService.init(mService.new WorkerHandler(mock(Looper.class)), + mService.init(mService.new WorkerHandler(mTestableLooper.getLooper()), mock(RankingHandler.class), mock(IPackageManager.class), mock(PackageManager.class), mock(LightsManager.class), diff --git a/services/tests/uiservicestests/src/com/android/server/notification/ZenModeHelperTest.java b/services/tests/uiservicestests/src/com/android/server/notification/ZenModeHelperTest.java index cb41769c3619b..2c95bdee34540 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/ZenModeHelperTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/ZenModeHelperTest.java @@ -175,6 +175,7 @@ public class ZenModeHelperTest extends UiServiceTestCase { mConditionProviders = new ConditionProviders(mContext, new UserProfiles(), AppGlobals.getPackageManager()); mConditionProviders.addSystemProvider(new CountdownConditionProvider()); + mConditionProviders.addSystemProvider(new ScheduleConditionProvider()); mZenModeHelper = new ZenModeHelper(mContext, mTestableLooper.getLooper(), mConditionProviders, mStatsEventBuilderFactory); @@ -815,6 +816,32 @@ public class ZenModeHelperTest extends UiServiceTestCase { mZenModeHelper.TAG); } + @Test + public void testSetConfig_updatesAudioEventually() { + AudioManagerInternal mAudioManager = mock(AudioManagerInternal.class); + mZenModeHelper.mAudioManager = mAudioManager; + setupZenConfig(); + + // Change the config a little bit, but enough that it would turn zen mode on + ZenModeConfig newConfig = mZenModeHelper.mConfig.copy(); + newConfig.manualRule = new ZenModeConfig.ZenRule(); + newConfig.manualRule.zenMode = ZEN_MODE_IMPORTANT_INTERRUPTIONS; + newConfig.manualRule.enabled = true; + mZenModeHelper.setConfig(newConfig, null, "test"); + + // audio manager shouldn't do anything until the handler processes its messagse + verify(mAudioManager, never()).updateRingerModeAffectedStreamsInternal(); + + // now process the looper's messages + mTestableLooper.processAllMessages(); + + // Expect calls to audio manager + verify(mAudioManager, times(1)).updateRingerModeAffectedStreamsInternal(); + + // called during applyZenToRingerMode(), which should be true since zen changed + verify(mAudioManager, atLeastOnce()).getRingerModeInternal(); + } + @Test public void testParcelConfig() { mZenModeHelper.mZenMode = ZEN_MODE_IMPORTANT_INTERRUPTIONS; @@ -1827,7 +1854,11 @@ public class ZenModeHelperTest extends UiServiceTestCase { @Test public void testRulesWithSameUri() { - Uri sharedUri = ZenModeConfig.toScheduleConditionId(new ScheduleInfo()); + // needs to be a valid schedule info object for the subscription to happen properly + ScheduleInfo scheduleInfo = new ScheduleInfo(); + scheduleInfo.days = new int[]{1, 2}; + scheduleInfo.endHour = 1; + Uri sharedUri = ZenModeConfig.toScheduleConditionId(scheduleInfo); AutomaticZenRule zenRule = new AutomaticZenRule("name", new ComponentName("android", "ScheduleConditionProvider"), sharedUri, @@ -1874,14 +1905,12 @@ public class ZenModeHelperTest extends UiServiceTestCase { // note that caller=null because that's how it comes in from NMS.setZenMode mZenModeHelper.setManualZenMode(ZEN_MODE_IMPORTANT_INTERRUPTIONS, null, null, ""); - mTestableLooper.processAllMessages(); // confirm that setting zen mode via setManualZenMode changed the zen mode correctly assertEquals(ZEN_MODE_IMPORTANT_INTERRUPTIONS, mZenModeHelper.mZenMode); // and also that it works to turn it back off again mZenModeHelper.setManualZenMode(Global.ZEN_MODE_OFF, null, null, ""); - mTestableLooper.processAllMessages(); assertEquals(Global.ZEN_MODE_OFF, mZenModeHelper.mZenMode); }