diff options
4 files changed, 98 insertions, 4 deletions
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java index b70b45b9db84..792594e7a5e4 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; @@ -43,6 +46,7 @@ import java.util.List; import java.util.Stack; import javax.inject.Inject; +import javax.inject.Named; import javax.inject.Singleton; import dagger.Lazy; @@ -58,6 +62,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<>(); @@ -86,9 +92,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, @@ -97,6 +107,7 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle Lazy<ShadeController> shadeController, BubbleData bubbleData, DynamicPrivacyController privacyController) { + mHandler = mainHandler; mLockscreenUserManager = notificationLockscreenUserManager; mGroupManager = groupManager; mVisualStabilityManager = visualStabilityManager; @@ -435,6 +446,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 c476d802c4e5..5103e8e89209 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; @@ -78,13 +82,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); @@ -97,7 +107,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(DynamicPrivacyController.class)); Dependency.get(InitController.class).executePostInitTasks(); @@ -212,9 +222,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) {} @@ -263,7 +324,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) { @@ -298,5 +363,7 @@ public class NotificationViewHierarchyManagerTest extends SysuiTestCase { return false; } + @Override + public void onNotificationViewUpdateFinished() { } } } diff --git a/services/core/java/com/android/server/wm/ActivityStackSupervisor.java b/services/core/java/com/android/server/wm/ActivityStackSupervisor.java index c992a69c2ecb..19916bc617f4 100644 --- a/services/core/java/com/android/server/wm/ActivityStackSupervisor.java +++ b/services/core/java/com/android/server/wm/ActivityStackSupervisor.java @@ -415,7 +415,7 @@ public class ActivityStackSupervisor implements RecentTasks.Callbacks { void sendErrorResult(String message) { try { - if (callerApp.hasThread()) { + if (callerApp != null && callerApp.hasThread()) { callerApp.getThread().scheduleCrash(message); } } catch (RemoteException e) { diff --git a/services/core/java/com/android/server/wm/DragState.java b/services/core/java/com/android/server/wm/DragState.java index 6127303141f4..553b0ffa6999 100644 --- a/services/core/java/com/android/server/wm/DragState.java +++ b/services/core/java/com/android/server/wm/DragState.java @@ -176,6 +176,8 @@ class DragState { mTransaction.transferTouchFocus(mTransferTouchFromToken, h.token); mTransferTouchFromToken = null; + // syncInputWindows here to ensure the input channel isn't removed before the transfer. + mTransaction.syncInputWindows(); mTransaction.apply(); } |