Move binder calls in NotificationSettingsController to a bg thread
While looking at the traces of a Jank regression we've noticed that
registering the contentObsver for Settings is a binder call, which
takes ~5ms sometimes. Since this is happening in a hot path, part of
the ShadeListBuilder.buildList, we should move it to a bg thread.
Fixes: 314287084
Test: atest SystemUITests:com.android.systemui.statusbar.notification.row.NotificationSettingsControllerTest
Test: observe a perfetto trace of posting, and cancelling Notifications
Flag: NONE
Change-Id: Ib2581e15927ce28352e334af3989da499d25e28d
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationSettingsController.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationSettingsController.java
index 4ace194..a17c066 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationSettingsController.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationSettingsController.java
@@ -130,8 +130,10 @@
}
mListeners.put(uri, currentListeners);
if (currentListeners.size() == 1) {
- mSecureSettings.registerContentObserverForUser(
- uri, false, mContentObserver, mUserTracker.getUserId());
+ mBackgroundHandler.post(() -> {
+ mSecureSettings.registerContentObserverForUser(
+ uri, false, mContentObserver, mUserTracker.getUserId());
+ });
}
}
mBackgroundHandler.post(() -> {
@@ -156,7 +158,9 @@
}
if (mListeners.size() == 0) {
- mSecureSettings.unregisterContentObserver(mContentObserver);
+ mBackgroundHandler.post(() -> {
+ mSecureSettings.unregisterContentObserver(mContentObserver);
+ });
}
}
Trace.traceEnd(Trace.TRACE_TAG_APP);
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationSettingsControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationSettingsControllerTest.kt
index 614995b..8261c1c 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationSettingsControllerTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationSettingsControllerTest.kt
@@ -40,15 +40,17 @@
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.ArgumentCaptor
-import org.mockito.ArgumentMatchers
import org.mockito.ArgumentMatchers.anyInt
import org.mockito.ArgumentMatchers.anyString
import org.mockito.Captor
import org.mockito.Mock
import org.mockito.Mockito
import org.mockito.Mockito.anyBoolean
+import org.mockito.Mockito.clearInvocations
import org.mockito.Mockito.never
import org.mockito.Mockito.verify
+import org.mockito.Mockito.verifyNoMoreInteractions
+import org.mockito.Mockito.verifyZeroInteractions
import org.mockito.MockitoAnnotations
@SmallTest
@@ -61,20 +63,15 @@
val settingUri1: Uri = Secure.getUriFor(setting1)
val settingUri2: Uri = Secure.getUriFor(setting2)
- @Mock
- private lateinit var userTracker: UserTracker
+ @Mock private lateinit var userTracker: UserTracker
private lateinit var mainHandler: Handler
private lateinit var backgroundHandler: Handler
private lateinit var testableLooper: TestableLooper
- @Mock
- private lateinit var secureSettings: SecureSettings
- @Mock
- private lateinit var dumpManager: DumpManager
+ @Mock private lateinit var secureSettings: SecureSettings
+ @Mock private lateinit var dumpManager: DumpManager
- @Captor
- private lateinit var userTrackerCallbackCaptor: ArgumentCaptor<UserTracker.Callback>
- @Captor
- private lateinit var settingsObserverCaptor: ArgumentCaptor<ContentObserver>
+ @Captor private lateinit var userTrackerCallbackCaptor: ArgumentCaptor<UserTracker.Callback>
+ @Captor private lateinit var settingsObserverCaptor: ArgumentCaptor<ContentObserver>
private lateinit var controller: NotificationSettingsController
@@ -86,13 +83,13 @@
backgroundHandler = Handler(testableLooper.looper)
allowTestableLooperAsMainThread()
controller =
- NotificationSettingsController(
- userTracker,
- mainHandler,
- backgroundHandler,
- secureSettings,
- dumpManager
- )
+ NotificationSettingsController(
+ userTracker,
+ mainHandler,
+ backgroundHandler,
+ secureSettings,
+ dumpManager
+ )
}
@After
@@ -116,14 +113,13 @@
// Validate: Nothing to do, since we aren't monitoring settings
verify(secureSettings, never()).unregisterContentObserver(any())
- verify(secureSettings, never()).registerContentObserverForUser(
- any(Uri::class.java), anyBoolean(), any(), anyInt())
+ verify(secureSettings, never())
+ .registerContentObserverForUser(any(Uri::class.java), anyBoolean(), any(), anyInt())
}
@Test
fun updateContentObserverRegistration_onUserChange_withSettingsListeners() {
// When: someone is listening to a setting
- controller.addCallback(settingUri1,
- Mockito.mock(Listener::class.java))
+ controller.addCallback(settingUri1, Mockito.mock(Listener::class.java))
verify(userTracker).addCallback(capture(userTrackerCallbackCaptor), any())
val userCallback = userTrackerCallbackCaptor.value
@@ -134,103 +130,141 @@
// Validate: The tracker is unregistered and re-registered with the new user
verify(secureSettings).unregisterContentObserver(any())
- verify(secureSettings).registerContentObserverForUser(
- eq(settingUri1), eq(false), any(), eq(userId))
+ verify(secureSettings)
+ .registerContentObserverForUser(eq(settingUri1), eq(false), any(), eq(userId))
}
@Test
fun addCallback_onlyFirstForUriRegistersObserver() {
- controller.addCallback(settingUri1,
- Mockito.mock(Listener::class.java))
- verify(secureSettings).registerContentObserverForUser(
- eq(settingUri1), eq(false), any(), eq(ActivityManager.getCurrentUser()))
+ controller.addCallback(settingUri1, Mockito.mock(Listener::class.java))
+ verifyZeroInteractions(secureSettings)
+ testableLooper.processAllMessages()
+ verify(secureSettings)
+ .registerContentObserverForUser(
+ eq(settingUri1),
+ eq(false),
+ any(),
+ eq(ActivityManager.getCurrentUser())
+ )
- controller.addCallback(settingUri1,
- Mockito.mock(Listener::class.java))
- verify(secureSettings).registerContentObserverForUser(
- any(Uri::class.java), anyBoolean(), any(), anyInt())
+ controller.addCallback(settingUri1, Mockito.mock(Listener::class.java))
+ verify(secureSettings)
+ .registerContentObserverForUser(any(Uri::class.java), anyBoolean(), any(), anyInt())
}
@Test
fun addCallback_secondUriRegistersObserver() {
- controller.addCallback(settingUri1,
- Mockito.mock(Listener::class.java))
- verify(secureSettings).registerContentObserverForUser(
- eq(settingUri1), eq(false), any(), eq(ActivityManager.getCurrentUser()))
+ controller.addCallback(settingUri1, Mockito.mock(Listener::class.java))
+ verifyZeroInteractions(secureSettings)
+ testableLooper.processAllMessages()
+ verify(secureSettings)
+ .registerContentObserverForUser(
+ eq(settingUri1),
+ eq(false),
+ any(),
+ eq(ActivityManager.getCurrentUser())
+ )
+ clearInvocations(secureSettings)
- controller.addCallback(settingUri2,
- Mockito.mock(Listener::class.java))
- verify(secureSettings).registerContentObserverForUser(
- eq(settingUri2), eq(false), any(), eq(ActivityManager.getCurrentUser()))
- verify(secureSettings).registerContentObserverForUser(
- eq(settingUri1), anyBoolean(), any(), anyInt())
+ controller.addCallback(settingUri2, Mockito.mock(Listener::class.java))
+ verifyNoMoreInteractions(secureSettings)
+ testableLooper.processAllMessages()
+ verify(secureSettings)
+ .registerContentObserverForUser(
+ eq(settingUri2),
+ eq(false),
+ any(),
+ eq(ActivityManager.getCurrentUser())
+ )
}
@Test
fun removeCallback_lastUnregistersObserver() {
- val listenerSetting1 : Listener = mock()
- val listenerSetting2 : Listener = mock()
+ val listenerSetting1: Listener = mock()
+ val listenerSetting2: Listener = mock()
controller.addCallback(settingUri1, listenerSetting1)
- verify(secureSettings).registerContentObserverForUser(
- eq(settingUri1), eq(false), any(), eq(ActivityManager.getCurrentUser()))
+ verifyZeroInteractions(secureSettings)
+ testableLooper.processAllMessages()
+ verify(secureSettings)
+ .registerContentObserverForUser(
+ eq(settingUri1),
+ eq(false),
+ any(),
+ eq(ActivityManager.getCurrentUser())
+ )
+ clearInvocations(secureSettings)
controller.addCallback(settingUri2, listenerSetting2)
- verify(secureSettings).registerContentObserverForUser(
- eq(settingUri2), anyBoolean(), any(), anyInt())
+ verifyNoMoreInteractions(secureSettings)
+ testableLooper.processAllMessages()
+ verify(secureSettings)
+ .registerContentObserverForUser(eq(settingUri2), anyBoolean(), any(), anyInt())
+ clearInvocations(secureSettings)
controller.removeCallback(settingUri2, listenerSetting2)
+ testableLooper.processAllMessages()
verify(secureSettings, never()).unregisterContentObserver(any())
+ clearInvocations(secureSettings)
controller.removeCallback(settingUri1, listenerSetting1)
+ verifyNoMoreInteractions(secureSettings)
+ testableLooper.processAllMessages()
verify(secureSettings).unregisterContentObserver(any())
}
@Test
fun addCallback_updatesCurrentValue() {
- whenever(secureSettings.getStringForUser(
- setting1, ActivityManager.getCurrentUser())).thenReturn("9")
- whenever(secureSettings.getStringForUser(
- setting2, ActivityManager.getCurrentUser())).thenReturn("5")
+ whenever(secureSettings.getStringForUser(setting1, ActivityManager.getCurrentUser()))
+ .thenReturn("9")
+ whenever(secureSettings.getStringForUser(setting2, ActivityManager.getCurrentUser()))
+ .thenReturn("5")
- val listenerSetting1a : Listener = mock()
- val listenerSetting1b : Listener = mock()
- val listenerSetting2 : Listener = mock()
+ val listenerSetting1a: Listener = mock()
+ val listenerSetting1b: Listener = mock()
+ val listenerSetting2: Listener = mock()
controller.addCallback(settingUri1, listenerSetting1a)
controller.addCallback(settingUri1, listenerSetting1b)
controller.addCallback(settingUri2, listenerSetting2)
+ verifyZeroInteractions(secureSettings)
testableLooper.processAllMessages()
- verify(listenerSetting1a).onSettingChanged(
- settingUri1, ActivityManager.getCurrentUser(), "9")
- verify(listenerSetting1b).onSettingChanged(
- settingUri1, ActivityManager.getCurrentUser(), "9")
- verify(listenerSetting2).onSettingChanged(
- settingUri2, ActivityManager.getCurrentUser(), "5")
+ verify(listenerSetting1a)
+ .onSettingChanged(settingUri1, ActivityManager.getCurrentUser(), "9")
+ verify(listenerSetting1b)
+ .onSettingChanged(settingUri1, ActivityManager.getCurrentUser(), "9")
+ verify(listenerSetting2)
+ .onSettingChanged(settingUri2, ActivityManager.getCurrentUser(), "5")
}
@Test
fun removeCallback_noMoreUpdates() {
- whenever(secureSettings.getStringForUser(
- setting1, ActivityManager.getCurrentUser())).thenReturn("9")
+ whenever(secureSettings.getStringForUser(setting1, ActivityManager.getCurrentUser()))
+ .thenReturn("9")
- val listenerSetting1a : Listener = mock()
- val listenerSetting1b : Listener = mock()
+ val listenerSetting1a: Listener = mock()
+ val listenerSetting1b: Listener = mock()
// First, register
controller.addCallback(settingUri1, listenerSetting1a)
controller.addCallback(settingUri1, listenerSetting1b)
+ verifyZeroInteractions(secureSettings)
testableLooper.processAllMessages()
- verify(secureSettings).registerContentObserverForUser(
- any(Uri::class.java), anyBoolean(), capture(settingsObserverCaptor), anyInt())
- verify(listenerSetting1a).onSettingChanged(
- settingUri1, ActivityManager.getCurrentUser(), "9")
- verify(listenerSetting1b).onSettingChanged(
- settingUri1, ActivityManager.getCurrentUser(), "9")
- Mockito.clearInvocations(listenerSetting1b)
- Mockito.clearInvocations(listenerSetting1a)
+ verify(secureSettings)
+ .registerContentObserverForUser(
+ any(Uri::class.java),
+ anyBoolean(),
+ capture(settingsObserverCaptor),
+ anyInt()
+ )
+ verify(listenerSetting1a)
+ .onSettingChanged(settingUri1, ActivityManager.getCurrentUser(), "9")
+ verify(listenerSetting1b)
+ .onSettingChanged(settingUri1, ActivityManager.getCurrentUser(), "9")
+ clearInvocations(listenerSetting1b)
+ clearInvocations(listenerSetting1a)
// Remove one of them
controller.removeCallback(settingUri1, listenerSetting1a)
@@ -239,10 +273,9 @@
settingsObserverCaptor.value.onChange(false, settingUri1)
testableLooper.processAllMessages()
- verify(listenerSetting1a, never()).onSettingChanged(
- settingUri1, ActivityManager.getCurrentUser(), "9")
- verify(listenerSetting1b).onSettingChanged(
- settingUri1, ActivityManager.getCurrentUser(), "9")
+ verify(listenerSetting1a, never())
+ .onSettingChanged(settingUri1, ActivityManager.getCurrentUser(), "9")
+ verify(listenerSetting1b)
+ .onSettingChanged(settingUri1, ActivityManager.getCurrentUser(), "9")
}
-
-}
\ No newline at end of file
+}
