Merge "Delay possible re-entrant call to updateNotificationViews()" into qt-dev
am: bce828f157
Change-Id: I1510f3e1d6a7c1a1149e917dfd55b3a634e91aec
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java
index 787cc97..aeb8574 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.Stack;
import javax.inject.Inject;
+import javax.inject.Named;
import javax.inject.Singleton;
import dagger.Lazy;
@@ -59,6 +63,8 @@
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 @@
// 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 @@
BubbleData bubbleData,
KeyguardBypassController bypassController,
DynamicPrivacyController privacyController) {
+ mHandler = mainHandler;
mLockscreenUserManager = notificationLockscreenUserManager;
mBypassController = bypassController;
mGroupManager = groupManager;
@@ -438,6 +449,20 @@
@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 010b85e..58fb53a 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 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 @@
@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 @@
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 @@
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 @@
}
@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 @@
return false;
}
+ @Override
+ public void onNotificationViewUpdateFinished() { }
}
}