Merge "Add main thread and reentrant asserts to chase down crashes" into qt-dev

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

@@ -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

@@ -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

@@ -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(),