Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 09f16abd authored by Ned Burns's avatar Ned Burns Committed by android-build-merger
Browse files

Merge "Delay possible re-entrant call to updateNotificationViews()" into qt-dev

am: bce828f1

Change-Id: Ia8cf73996aa49d286880ddd5c9f23b915346ad6a
parents ed912b87 bce828f1
Loading
Loading
Loading
Loading
+25 −0
Original line number Diff line number Diff line
@@ -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();
    }

+70 −3
Original line number Diff line number Diff line
@@ -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() { }
    }
}