From 1a479f696062fadb6fda102a53fb5f501cc6346a Mon Sep 17 00:00:00 2001 From: Bryce Lee Date: Wed, 3 Nov 2021 16:33:19 -0700 Subject: [PATCH] Marshal CommunalCoordinator callbacks to main thread. This changelist makes sure callbacks in CommunalCoordinator happen on the main thread, preventing thread assertion issues in the notification logic. Bug: 205047954 Test: atest CommunalCoordinatorTest Change-Id: I6e3c2f2ef5d81baa253913f91e0f915754cf3746 --- .../coordinator/CommunalCoordinator.java | 14 +++++++++++--- .../coordinator/CommunalCoordinatorTest.java | 13 ++++++++++++- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/CommunalCoordinator.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/CommunalCoordinator.java index 992d898531b80..bd011c3847cee 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/CommunalCoordinator.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/CommunalCoordinator.java @@ -19,6 +19,7 @@ package com.android.systemui.statusbar.notification.collection.coordinator; import androidx.annotation.NonNull; import com.android.systemui.communal.CommunalStateController; +import com.android.systemui.dagger.qualifiers.Main; import com.android.systemui.statusbar.NotificationLockscreenUserManager; import com.android.systemui.statusbar.notification.NotificationEntryManager; import com.android.systemui.statusbar.notification.collection.NotifPipeline; @@ -26,6 +27,8 @@ import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.statusbar.notification.collection.coordinator.dagger.CoordinatorScope; import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter; +import java.util.concurrent.Executor; + import javax.inject.Inject; /** @@ -34,14 +37,17 @@ import javax.inject.Inject; */ @CoordinatorScope public class CommunalCoordinator implements Coordinator { + final Executor mExecutor; final CommunalStateController mCommunalStateController; final NotificationEntryManager mNotificationEntryManager; final NotificationLockscreenUserManager mNotificationLockscreenUserManager; @Inject - public CommunalCoordinator(NotificationEntryManager notificationEntryManager, + public CommunalCoordinator(@Main Executor executor, + NotificationEntryManager notificationEntryManager, NotificationLockscreenUserManager notificationLockscreenUserManager, CommunalStateController communalStateController) { + mExecutor = executor; mNotificationEntryManager = notificationEntryManager; mNotificationLockscreenUserManager = notificationLockscreenUserManager; mCommunalStateController = communalStateController; @@ -57,8 +63,10 @@ public class CommunalCoordinator implements Coordinator { final CommunalStateController.Callback mStateCallback = new CommunalStateController.Callback() { @Override public void onCommunalViewShowingChanged() { - mFilter.invalidateList(); - mNotificationEntryManager.updateNotifications("Communal mode state changed"); + mExecutor.execute(() -> { + mFilter.invalidateList(); + mNotificationEntryManager.updateNotifications("Communal mode state changed"); + }); } }; diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/CommunalCoordinatorTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/CommunalCoordinatorTest.java index 01e4cce0cc304..f4452bc248b15 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/CommunalCoordinatorTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/CommunalCoordinatorTest.java @@ -17,6 +17,7 @@ package com.android.systemui.statusbar.notification.collection.coordinator; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import android.test.suitebuilder.annotation.SmallTest; @@ -29,6 +30,8 @@ import com.android.systemui.statusbar.notification.collection.NotifPipeline; import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter; import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.Pluggable; +import com.android.systemui.util.concurrency.FakeExecutor; +import com.android.systemui.util.time.FakeSystemClock; import org.junit.Before; import org.junit.Test; @@ -39,6 +42,8 @@ import org.mockito.MockitoAnnotations; @SmallTest public class CommunalCoordinatorTest extends SysuiTestCase { + private final FakeExecutor mExecutor = new FakeExecutor(new FakeSystemClock()); + @Mock CommunalStateController mCommunalStateController; @Mock @@ -57,7 +62,7 @@ public class CommunalCoordinatorTest extends SysuiTestCase { @Before public void setup() { MockitoAnnotations.initMocks(this); - mCoordinator = new CommunalCoordinator(mNotificationEntryManager, + mCoordinator = new CommunalCoordinator(mExecutor, mNotificationEntryManager, mNotificationLockscreenUserManager, mCommunalStateController); } @@ -84,6 +89,12 @@ public class CommunalCoordinatorTest extends SysuiTestCase { // Verify that notifications are filtered out when communal is showing and that the filter // pipeline is notified. stateCallback.onCommunalViewShowingChanged(); + // Make sure callback depends on executor to run. + verify(mFilterListener, never()).onPluggableInvalidated(any()); + verify(mNotificationEntryManager, never()).updateNotifications(any()); + + mExecutor.runAllReady(); + verify(mFilterListener).onPluggableInvalidated(any()); verify(mNotificationEntryManager).updateNotifications(any()); assert (filter.shouldFilterOut(mNotificationEntry, 0));