diff options
2 files changed, 95 insertions, 3 deletions
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java index 787cc971e6a1..aeb85748fd1a 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java @@ -16,8 +16,11 @@ package com.android.systemui.statusbar; +import static com.android.systemui.Dependency.MAIN_HANDLER_NAME; + import android.content.Context; import android.content.res.Resources; +import android.os.Handler; import android.os.Trace; import android.os.UserHandle; import android.util.Log; @@ -44,6 +47,7 @@ import java.util.List; import java.util.Stack; import javax.inject.Inject; +import javax.inject.Named; import javax.inject.Singleton; import dagger.Lazy; @@ -59,6 +63,8 @@ import dagger.Lazy; public class NotificationViewHierarchyManager implements DynamicPrivacyController.Listener { private static final String TAG = "NotificationViewHierarchyManager"; + private final Handler mHandler; + //TODO: change this top <Entry, List<Entry>>? private final HashMap<ExpandableNotificationRow, List<ExpandableNotificationRow>> mTmpChildOrderMap = new HashMap<>(); @@ -88,9 +94,13 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle // Used to help track down re-entrant calls to our update methods, which will cause bugs. private boolean mPerformingUpdate; + // Hack to get around re-entrant call in onDynamicPrivacyChanged() until we can track down + // the problem. + private boolean mIsHandleDynamicPrivacyChangeScheduled; @Inject public NotificationViewHierarchyManager(Context context, + @Named(MAIN_HANDLER_NAME) Handler mainHandler, NotificationLockscreenUserManager notificationLockscreenUserManager, NotificationGroupManager groupManager, VisualStabilityManager visualStabilityManager, @@ -100,6 +110,7 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle BubbleData bubbleData, KeyguardBypassController bypassController, DynamicPrivacyController privacyController) { + mHandler = mainHandler; mLockscreenUserManager = notificationLockscreenUserManager; mBypassController = bypassController; mGroupManager = groupManager; @@ -438,6 +449,20 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle @Override public void onDynamicPrivacyChanged() { + if (mPerformingUpdate) { + Log.w(TAG, "onDynamicPrivacyChanged made a re-entrant call"); + } + // This listener can be called from updateNotificationViews() via a convoluted listener + // chain, so we post here to prevent a re-entrant call. See b/136186188 + // TODO: Refactor away the need for this + if (!mIsHandleDynamicPrivacyChangeScheduled) { + mIsHandleDynamicPrivacyChangeScheduled = true; + mHandler.post(this::onHandleDynamicPrivacyChanged); + } + } + + private void onHandleDynamicPrivacyChanged() { + mIsHandleDynamicPrivacyChangeScheduled = false; updateNotificationViews(); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationViewHierarchyManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationViewHierarchyManagerTest.java index 010b85edacdd..58fb53aae7bb 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationViewHierarchyManagerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationViewHierarchyManagerTest.java @@ -20,12 +20,16 @@ import static junit.framework.Assert.assertTrue; import static org.junit.Assert.assertEquals; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.Mockito.clearInvocations; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import android.os.Handler; import android.testing.AndroidTestingRunner; import android.testing.TestableLooper; import android.view.View; @@ -79,13 +83,19 @@ public class NotificationViewHierarchyManagerTest extends SysuiTestCase { @Mock private VisualStabilityManager mVisualStabilityManager; @Mock private ShadeController mShadeController; + private TestableLooper mTestableLooper; + private Handler mHandler; private NotificationViewHierarchyManager mViewHierarchyManager; private NotificationTestHelper mHelper; + private boolean mMadeReentrantCall = false; @Before public void setUp() { MockitoAnnotations.initMocks(this); - Assert.sMainLooper = TestableLooper.get(this).getLooper(); + mTestableLooper = TestableLooper.get(this); + Assert.sMainLooper = mTestableLooper.getLooper(); + mHandler = Handler.createAsync(mTestableLooper.getLooper()); + mDependency.injectTestDependency(NotificationEntryManager.class, mEntryManager); mDependency.injectTestDependency(NotificationLockscreenUserManager.class, mLockscreenUserManager); @@ -98,7 +108,7 @@ public class NotificationViewHierarchyManagerTest extends SysuiTestCase { when(mEntryManager.getNotificationData()).thenReturn(mNotificationData); mViewHierarchyManager = new NotificationViewHierarchyManager(mContext, - mLockscreenUserManager, mGroupManager, mVisualStabilityManager, + mHandler, mLockscreenUserManager, mGroupManager, mVisualStabilityManager, mock(StatusBarStateControllerImpl.class), mEntryManager, () -> mShadeController, new BubbleData(mContext), mock(KeyguardBypassController.class), @@ -215,9 +225,60 @@ public class NotificationViewHierarchyManagerTest extends SysuiTestCase { verify(entry0.getRow(), times(1)).showAppOpsIcons(any()); } + @Test + public void testReentrantCallsToOnDynamicPrivacyChangedPostForLater() { + // GIVEN a ListContainer that will make a re-entrant call to updateNotificationViews() + mMadeReentrantCall = false; + doAnswer((invocation) -> { + if (!mMadeReentrantCall) { + mMadeReentrantCall = true; + mViewHierarchyManager.onDynamicPrivacyChanged(); + } + return null; + }).when(mListContainer).setMaxDisplayedNotifications(anyInt()); + + // WHEN we call updateNotificationViews() + mViewHierarchyManager.updateNotificationViews(); + + // THEN onNotificationViewUpdateFinished() is only called once + verify(mListContainer).onNotificationViewUpdateFinished(); + + // WHEN we drain the looper + mTestableLooper.processAllMessages(); + + // THEN updateNotificationViews() is called a second time (for the reentrant call) + verify(mListContainer, times(2)).onNotificationViewUpdateFinished(); + } + + @Test + public void testMultipleReentrantCallsToOnDynamicPrivacyChangedOnlyPostOnce() { + // GIVEN a ListContainer that will make many re-entrant calls to updateNotificationViews() + mMadeReentrantCall = false; + doAnswer((invocation) -> { + if (!mMadeReentrantCall) { + mMadeReentrantCall = true; + mViewHierarchyManager.onDynamicPrivacyChanged(); + mViewHierarchyManager.onDynamicPrivacyChanged(); + mViewHierarchyManager.onDynamicPrivacyChanged(); + mViewHierarchyManager.onDynamicPrivacyChanged(); + } + return null; + }).when(mListContainer).setMaxDisplayedNotifications(anyInt()); + + // WHEN we call updateNotificationViews() and drain the looper + mViewHierarchyManager.updateNotificationViews(); + verify(mListContainer).onNotificationViewUpdateFinished(); + clearInvocations(mListContainer); + mTestableLooper.processAllMessages(); + + // THEN updateNotificationViews() is called only one more time + verify(mListContainer).onNotificationViewUpdateFinished(); + } + private class FakeListContainer implements NotificationListContainer { final LinearLayout mLayout = new LinearLayout(mContext); final List<View> mRows = Lists.newArrayList(); + private boolean mMakeReentrantCallDuringSetMaxDisplayedNotifications; @Override public void setChildTransferInProgress(boolean childTransferInProgress) {} @@ -266,7 +327,11 @@ public class NotificationViewHierarchyManagerTest extends SysuiTestCase { } @Override - public void setMaxDisplayedNotifications(int maxNotifications) {} + public void setMaxDisplayedNotifications(int maxNotifications) { + if (mMakeReentrantCallDuringSetMaxDisplayedNotifications) { + mViewHierarchyManager.onDynamicPrivacyChanged(); + } + } @Override public ViewGroup getViewParentForNotification(NotificationEntry entry) { @@ -301,5 +366,7 @@ public class NotificationViewHierarchyManagerTest extends SysuiTestCase { return false; } + @Override + public void onNotificationViewUpdateFinished() { } } } |