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

Commit 7a121090 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 

am: b2b70838

Change-Id: I97a5f603ed5c220736b45a6c96fe3a6477608e84
parents 75e223f5 b2b70838

packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java

+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;
@@ -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();

    }

packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationViewHierarchyManagerTest.java

+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;
@@ -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() { }

    }

}