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

Commit d4a69f70 authored by Ned Burns's avatar Ned Burns
Browse files

Add main thread and reentrant asserts to chase down crashes 

We're seeing crashes due to view hierarchy violations that shouldn't be
possible. Adding some guards to make sure we aren't running into
off-thread hierarchy manipulation or re-entrant calls to the update
code.

Test: manual
Bug: 135018709
Change-Id: I4b1f2bd7e3a6f80384486d59b9f56fc3713537cf
parent 9ea1842c

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

+31 −2
Original line number Diff line number Diff line
@@ -35,7 +35,7 @@ import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow 
import com.android.systemui.statusbar.notification.stack.NotificationListContainer;

import com.android.systemui.statusbar.phone.NotificationGroupManager;

import com.android.systemui.statusbar.phone.ShadeController;

import com.android.systemui.statusbar.phone.UnlockMethodCache;

import com.android.systemui.util.Assert;



import java.util.ArrayList;

import java.util.HashMap;
@@ -84,6 +84,9 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle 
    private NotificationPresenter mPresenter;

    private NotificationListContainer mListContainer;



    // Used to help track down re-entrant calls to our update methods, which will cause bugs.

    private boolean mPerformingUpdate;



    @Inject

    public NotificationViewHierarchyManager(Context context,

            NotificationLockscreenUserManager notificationLockscreenUserManager,
@@ -119,6 +122,9 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle 
     */

    //TODO: Rewrite this to focus on Entries, or some other data object instead of views

    public void updateNotificationViews() {

        Assert.isMainThread();

        beginUpdate();



        ArrayList<NotificationEntry> activeNotifications = mEntryManager.getNotificationData()

                .getActiveNotifications();

        ArrayList<ExpandableNotificationRow> toShow = new ArrayList<>(activeNotifications.size());
@@ -244,9 +250,11 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle 
        // clear the map again for the next usage

        mTmpChildOrderMap.clear();



        updateRowStates();

        updateRowStatesInternal();



        mListContainer.onNotificationViewUpdateFinished();



        endUpdate();

    }



    private void addNotificationChildrenAndSort() {
@@ -330,6 +338,13 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle 
     * Updates expanded, dimmed and locked states of notification rows.

     */

    public void updateRowStates() {

        Assert.isMainThread();

        beginUpdate();

        updateRowStatesInternal();

        endUpdate();

    }



    private void updateRowStatesInternal() {

        Trace.beginSection("NotificationViewHierarchyManager#updateRowStates");

        final int N = mListContainer.getContainerChildCount();
@@ -422,4 +437,18 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle 
    public void onDynamicPrivacyChanged() {

        updateNotificationViews();

    }



    private void beginUpdate() {

        if (mPerformingUpdate) {

            throw new IllegalStateException("Re-entrant code during update.");

        }

        mPerformingUpdate = true;

    }



    private void endUpdate() {

        if (!mPerformingUpdate) {

            throw new IllegalStateException("Manager state has become desynced.");

        }

        mPerformingUpdate = false;

    }

}

packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java

+9 −0
Original line number Diff line number Diff line
@@ -142,6 +142,7 @@ import com.android.systemui.statusbar.policy.ConfigurationController.Configurati 
import com.android.systemui.statusbar.policy.HeadsUpUtil;

import com.android.systemui.statusbar.policy.ScrollAdapter;

import com.android.systemui.tuner.TunerService;

import com.android.systemui.util.Assert;



import java.io.FileDescriptor;

import java.io.PrintWriter;
@@ -2850,6 +2851,7 @@ public class NotificationStackScrollLayout extends ViewGroup implements ScrollAd 


    @ShadeViewRefactor(RefactorComponent.SHADE_VIEW)

    public void setChildTransferInProgress(boolean childTransferInProgress) {

        Assert.isMainThread();

        mChildTransferInProgress = childTransferInProgress;

    }
@@ -3293,6 +3295,11 @@ public class NotificationStackScrollLayout extends ViewGroup implements ScrollAd 
    @Override

    @ShadeViewRefactor(RefactorComponent.STATE_RESOLVER)

    public void changeViewPosition(ExpandableView child, int newIndex) {

        Assert.isMainThread();

        if (mChangePositionInProgress) {

            throw new IllegalStateException("Reentrant call to changeViewPosition");

        }



        int currentIndex = indexOfChild(child);



        if (currentIndex == -1) {
@@ -5053,12 +5060,14 @@ public class NotificationStackScrollLayout extends ViewGroup implements ScrollAd 
    @Override

    @ShadeViewRefactor(RefactorComponent.SHADE_VIEW)

    public void removeContainerView(View v) {

        Assert.isMainThread();

        removeView(v);

    }



    @Override

    @ShadeViewRefactor(RefactorComponent.SHADE_VIEW)

    public void addContainerView(View v) {

        Assert.isMainThread();

        addView(v);

    }

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java

+6 −2
Original line number Diff line number Diff line
@@ -38,11 +38,12 @@ import static org.mockito.Mockito.when; 


import android.metrics.LogMaker;

import android.provider.Settings;

import android.testing.AndroidTestingRunner;

import android.testing.TestableLooper;

import android.view.View;



import androidx.test.annotation.UiThreadTest;

import androidx.test.filters.SmallTest;

import androidx.test.runner.AndroidJUnit4;



import com.android.internal.logging.MetricsLogger;

import com.android.internal.logging.nano.MetricsProto;
@@ -95,7 +96,8 @@ import java.util.ArrayList; 
 * Tests for {@link NotificationStackScrollLayout}.

 */

@SmallTest

@RunWith(AndroidJUnit4.class)

@RunWith(AndroidTestingRunner.class)

@TestableLooper.RunWithLooper

public class NotificationStackScrollLayoutTest extends SysuiTestCase {



    private NotificationStackScrollLayout mStackScroller;  // Normally test this
@@ -122,6 +124,8 @@ public class NotificationStackScrollLayoutTest extends SysuiTestCase { 
    @Before

    @UiThreadTest

    public void setUp() throws Exception {

        com.android.systemui.util.Assert.sMainLooper = TestableLooper.get(this).getLooper();



        mOriginalInterruptionModelSetting = Settings.Secure.getInt(mContext.getContentResolver(),

                NOTIFICATION_NEW_INTERRUPTION_MODEL, 0);

        Settings.Secure.putInt(mContext.getContentResolver(),