Merge "Deflake NotificationEntryManagerInflationTest" into rvc-dev

packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationContentInflater.java

@@ -36,6 +36,7 @@ import android.widget.RemoteViews;
 
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.widget.ImageMessageConsumer;
+import com.android.systemui.dagger.qualifiers.Background;
 import com.android.systemui.statusbar.InflationTask;
 import com.android.systemui.statusbar.NotificationRemoteInputManager;
 import com.android.systemui.statusbar.SmartReplyController;
@@ -52,6 +53,7 @@ import com.android.systemui.statusbar.policy.SmartReplyConstants;
 import com.android.systemui.util.Assert;
 
 import java.util.HashMap;
+import java.util.concurrent.Executor;
 
 import javax.inject.Inject;
 import javax.inject.Singleton;
@@ -74,6 +76,7 @@ public class NotificationContentInflater implements NotificationRowContentBinder
     private final Lazy<SmartReplyConstants> mSmartReplyConstants;
     private final Lazy<SmartReplyController> mSmartReplyController;
     private final ConversationNotificationProcessor mConversationProcessor;
+    private final Executor mBgExecutor;
 
     @Inject
     NotificationContentInflater(
@@ -81,12 +84,14 @@ public class NotificationContentInflater implements NotificationRowContentBinder
             NotificationRemoteInputManager remoteInputManager,
             Lazy<SmartReplyConstants> smartReplyConstants,
             Lazy<SmartReplyController> smartReplyController,
-            ConversationNotificationProcessor conversationProcessor) {
+            ConversationNotificationProcessor conversationProcessor,
+            @Background Executor bgExecutor) {
         mRemoteViewCache = remoteViewCache;
         mRemoteInputManager = remoteInputManager;
         mSmartReplyConstants = smartReplyConstants;
         mSmartReplyController = smartReplyController;
         mConversationProcessor = conversationProcessor;
+        mBgExecutor = bgExecutor;
     }
 
     @Override
@@ -114,6 +119,7 @@ public class NotificationContentInflater implements NotificationRowContentBinder
         }
 
         AsyncInflationTask task = new AsyncInflationTask(
+                mBgExecutor,
                 mInflateSynchronously,
                 contentToBind,
                 mRemoteViewCache,
@@ -131,7 +137,7 @@ public class NotificationContentInflater implements NotificationRowContentBinder
         if (mInflateSynchronously) {
             task.onPostExecute(task.doInBackground());
         } else {
-            task.execute();
+            task.executeOnExecutor(mBgExecutor);
         }
     }
 
@@ -157,6 +163,7 @@ public class NotificationContentInflater implements NotificationRowContentBinder
                 row.getExistingSmartRepliesAndActions());
 
         apply(
+                mBgExecutor,
                 inflateSynchronously,
                 result,
                 reInflateFlags,
@@ -280,6 +287,7 @@ public class NotificationContentInflater implements NotificationRowContentBinder
     }
 
     private static CancellationSignal apply(
+            Executor bgExecutor,
             boolean inflateSynchronously,
             InflationProgress result,
             @InflationFlag int reInflateFlags,
@@ -308,9 +316,10 @@ public class NotificationContentInflater implements NotificationRowContentBinder
                     return result.newContentView;
                 }
             };
-            applyRemoteView(inflateSynchronously, result, reInflateFlags, flag, remoteViewCache,
+            applyRemoteView(bgExecutor, inflateSynchronously, result, reInflateFlags, flag,
-                    entry, row, isNewView, remoteViewClickHandler, callback, privateLayout,
+                    remoteViewCache, entry, row, isNewView, remoteViewClickHandler, callback,
-                    privateLayout.getContractedChild(), privateLayout.getVisibleWrapper(
+                    privateLayout,  privateLayout.getContractedChild(),
+                    privateLayout.getVisibleWrapper(
                             NotificationContentView.VISIBLE_TYPE_CONTRACTED),
                     runningInflations, applyCallback);
         }
@@ -332,8 +341,8 @@ public class NotificationContentInflater implements NotificationRowContentBinder
                         return result.newExpandedView;
                     }
                 };
-                applyRemoteView(inflateSynchronously, result, reInflateFlags, flag, remoteViewCache,
+                applyRemoteView(bgExecutor, inflateSynchronously, result, reInflateFlags, flag,
-                        entry, row, isNewView, remoteViewClickHandler,
+                        remoteViewCache, entry, row, isNewView, remoteViewClickHandler,
                         callback, privateLayout, privateLayout.getExpandedChild(),
                         privateLayout.getVisibleWrapper(
                                 NotificationContentView.VISIBLE_TYPE_EXPANDED), runningInflations,
@@ -358,8 +367,8 @@ public class NotificationContentInflater implements NotificationRowContentBinder
                         return result.newHeadsUpView;
                     }
                 };
-                applyRemoteView(inflateSynchronously, result, reInflateFlags, flag, remoteViewCache,
+                applyRemoteView(bgExecutor, inflateSynchronously, result, reInflateFlags, flag,
-                        entry, row, isNewView, remoteViewClickHandler,
+                        remoteViewCache, entry, row, isNewView, remoteViewClickHandler,
                         callback, privateLayout, privateLayout.getHeadsUpChild(),
                         privateLayout.getVisibleWrapper(
                                 VISIBLE_TYPE_HEADSUP), runningInflations,
@@ -383,8 +392,8 @@ public class NotificationContentInflater implements NotificationRowContentBinder
                     return result.newPublicView;
                 }
             };
-            applyRemoteView(inflateSynchronously, result, reInflateFlags, flag, remoteViewCache,
+            applyRemoteView(bgExecutor, inflateSynchronously, result, reInflateFlags, flag,
-                    entry, row, isNewView, remoteViewClickHandler, callback,
+                    remoteViewCache, entry, row, isNewView, remoteViewClickHandler, callback,
                     publicLayout, publicLayout.getContractedChild(),
                     publicLayout.getVisibleWrapper(NotificationContentView.VISIBLE_TYPE_CONTRACTED),
                     runningInflations, applyCallback);
@@ -401,6 +410,7 @@ public class NotificationContentInflater implements NotificationRowContentBinder
 
     @VisibleForTesting
     static void applyRemoteView(
+            Executor bgExecutor,
             boolean inflateSynchronously,
             final InflationProgress result,
             final @InflationFlag int reInflateFlags,
@@ -495,14 +505,14 @@ public class NotificationContentInflater implements NotificationRowContentBinder
             cancellationSignal = newContentView.applyAsync(
                     result.packageContext,
                     parentLayout,
-                    null,
+                    bgExecutor,
                     listener,
                     remoteViewClickHandler);
         } else {
             cancellationSignal = newContentView.reapplyAsync(
                     result.packageContext,
                     existingView,
-                    null,
+                    bgExecutor,
                     listener,
                     remoteViewClickHandler);
         }
@@ -672,6 +682,7 @@ public class NotificationContentInflater implements NotificationRowContentBinder
         private final NotifRemoteViewCache mRemoteViewCache;
         private final SmartReplyConstants mSmartReplyConstants;
         private final SmartReplyController mSmartReplyController;
+        private final Executor mBgExecutor;
         private ExpandableNotificationRow mRow;
         private Exception mError;
         private RemoteViews.OnClickHandler mRemoteViewClickHandler;
@@ -679,6 +690,7 @@ public class NotificationContentInflater implements NotificationRowContentBinder
         private final ConversationNotificationProcessor mConversationProcessor;
 
         private AsyncInflationTask(
+                Executor bgExecutor,
                 boolean inflateSynchronously,
                 @InflationFlag int reInflateFlags,
                 NotifRemoteViewCache cache,
@@ -697,6 +709,7 @@ public class NotificationContentInflater implements NotificationRowContentBinder
             mRow = row;
             mSmartReplyConstants = smartReplyConstants;
             mSmartReplyController = smartReplyController;
+            mBgExecutor = bgExecutor;
             mInflateSynchronously = inflateSynchronously;
             mReInflateFlags = reInflateFlags;
             mRemoteViewCache = cache;
@@ -755,8 +768,16 @@ public class NotificationContentInflater implements NotificationRowContentBinder
         @Override
         protected void onPostExecute(InflationProgress result) {
             if (mError == null) {
-                mCancellationSignal = apply(mInflateSynchronously, result, mReInflateFlags,
+                mCancellationSignal = apply(
-                        mRemoteViewCache, mEntry, mRow, mRemoteViewClickHandler, this);
+                        mBgExecutor,
+                        mInflateSynchronously,
+                        result,
+                        mReInflateFlags,
+                        mRemoteViewCache,
+                        mEntry,
+                        mRow,
+                        mRemoteViewClickHandler,
+                        this);
             } else {
                 handleError(mError);
             }

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationContentInflaterTest.java

@@ -35,6 +35,7 @@ import static org.mockito.Mockito.when;
 
 import android.app.Notification;
 import android.content.Context;
+import android.os.AsyncTask;
 import android.os.CancellationSignal;
 import android.os.Handler;
 import android.os.Looper;
@@ -104,7 +105,8 @@ public class NotificationContentInflaterTest extends SysuiTestCase {
                 mock(NotificationRemoteInputManager.class),
                 () -> smartReplyConstants,
                 () -> smartReplyController,
-                mConversationNotificationProcessor);
+                mConversationNotificationProcessor,
+                mock(Executor.class));
     }
 
     @Test
@@ -191,6 +193,7 @@ public class NotificationContentInflaterTest extends SysuiTestCase {
         result.packageContext = mContext;
         CountDownLatch countDownLatch = new CountDownLatch(1);
         NotificationContentInflater.applyRemoteView(
+                AsyncTask.SERIAL_EXECUTOR,
                 false /* inflateSynchronously */,
                 result,
                 FLAG_CONTENT_VIEW_EXPANDED,

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationEntryManagerInflationTest.java

@@ -36,6 +36,7 @@ import android.service.notification.StatusBarNotification;
 import android.testing.AndroidTestingRunner;
 import android.testing.TestableLooper;
 
+import androidx.asynclayoutinflater.view.AsyncLayoutInflater;
 import androidx.test.filters.SmallTest;
 
 import com.android.internal.util.NotificationMessagingUtil;
@@ -68,7 +69,6 @@ import com.android.systemui.statusbar.notification.icon.IconManager;
 import com.android.systemui.statusbar.notification.interruption.NotificationInterruptStateProvider;
 import com.android.systemui.statusbar.notification.logging.NotificationLogger;
 import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier;
-import com.android.systemui.statusbar.notification.row.NotificationRowContentBinder.InflationFlag;
 import com.android.systemui.statusbar.notification.row.dagger.ExpandableNotificationRowComponent;
 import com.android.systemui.statusbar.notification.row.dagger.NotificationRowComponent;
 import com.android.systemui.statusbar.notification.stack.NotificationListContainer;
@@ -76,12 +76,12 @@ import com.android.systemui.statusbar.phone.KeyguardBypassController;
 import com.android.systemui.statusbar.phone.NotificationGroupManager;
 import com.android.systemui.statusbar.policy.HeadsUpManager;
 import com.android.systemui.statusbar.policy.SmartReplyConstants;
+import com.android.systemui.util.concurrency.FakeExecutor;
 import com.android.systemui.util.leak.LeakDetector;
 import com.android.systemui.util.time.FakeSystemClock;
 
 import org.junit.After;
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
@@ -90,11 +90,12 @@ import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
 import org.mockito.stubbing.Answer;
 
+import java.util.concurrent.CountDownLatch;
+
 /**
  * Functional tests for notification inflation from {@link NotificationEntryManager}.
  */
 @SmallTest
-@Ignore("Flaking")
 @RunWith(AndroidTestingRunner.class)
 @TestableLooper.RunWithLooper(setAsMainLooper = true)
 public class NotificationEntryManagerInflationTest extends SysuiTestCase {
@@ -136,6 +137,7 @@ public class NotificationEntryManagerInflationTest extends SysuiTestCase {
     private NotificationEntryManager mEntryManager;
     private NotificationRowBinderImpl mRowBinder;
     private Handler mHandler;
+    private FakeExecutor mBgExecutor;
 
     @Before
     public void setUp() {
@@ -180,12 +182,14 @@ public class NotificationEntryManagerInflationTest extends SysuiTestCase {
         NotifBindPipeline pipeline = new NotifBindPipeline(
                 mEntryManager,
                 mock(NotifBindPipelineLogger.class));
+        mBgExecutor = new FakeExecutor(new FakeSystemClock());
         NotificationContentInflater binder = new NotificationContentInflater(
                 cache,
                 mRemoteInputManager,
                 () -> mock(SmartReplyConstants.class),
                 () -> mock(SmartReplyController.class),
-                mock(ConversationNotificationProcessor.class));
+                mock(ConversationNotificationProcessor.class),
+                mBgExecutor);
         RowContentBindStage stage = new RowContentBindStage(
                 binder,
                 mock(NotifInflationErrorManager.class),
@@ -310,9 +314,7 @@ public class NotificationEntryManagerInflationTest extends SysuiTestCase {
         verify(mEntryListener).onPendingEntryAdded(entryCaptor.capture());
         NotificationEntry entry = entryCaptor.getValue();
 
-        // Wait for inflation
+        waitForInflation();
-        // row inflation, system notification, remote views, contracted view
-        waitForMessages(4);
 
         // THEN the notification has its row inflated
         assertNotNull(entry.getRow());
@@ -339,7 +341,7 @@ public class NotificationEntryManagerInflationTest extends SysuiTestCase {
                 NotificationEntry.class);
         verify(mEntryListener).onPendingEntryAdded(entryCaptor.capture());
         NotificationEntry entry = entryCaptor.getValue();
-        waitForMessages(4);
+        waitForInflation();
 
         Mockito.reset(mEntryListener);
         Mockito.reset(mPresenter);
@@ -347,9 +349,7 @@ public class NotificationEntryManagerInflationTest extends SysuiTestCase {
         // WHEN the notification is updated
         mEntryManager.updateNotification(mSbn, mRankingMap);
 
-        // Wait for inflation
+        waitForInflation();
-        // remote views, contracted view
-        waitForMessages(2);
 
         // THEN the notification has its row and inflated
         assertNotNull(entry.getRow());
@@ -363,31 +363,40 @@ public class NotificationEntryManagerInflationTest extends SysuiTestCase {
     }
 
     /**
-     * Wait for a certain number of messages to finish before continuing, timing out if they never
+     * Wait for inflation to finish.
-     * occur.
      *
-     * As part of the inflation pipeline, the main thread is forced to deal with several callbacks
+     * A few things to note
-     * due to the nature of the API used (generally because they're {@link android.os.AsyncTask}
+     * 1) Row inflation is done via {@link AsyncLayoutInflater} on its own background thread that
-     * callbacks). In order, these are
+     * calls back to main thread which is why we wait on main thread.
-     *
+     * 2) Row *content* inflation is done on the {@link FakeExecutor} we pass in in this test class
-     * 1) Callback after row inflation. See {@link RowInflaterTask}.
+     * so we control when that work is done. The callback is still always on the main thread.
-     * 2) Callback checking if row is system notification. See
-     *    {@link ExpandableNotificationRow#setEntry}
-     * 3) Callback after remote views are created. See
-     *    {@link NotificationContentInflater.AsyncInflationTask}.
-     * 4-6) Callback after each content view is inflated/rebound from remote view. See
-     *      {@link NotificationContentInflater#applyRemoteView} and {@link InflationFlag}.
-     *
-     * Depending on the test, only some of these will be necessary. For example, generally, not
-     * every content view is inflated or the row may not be inflated if one already exists.
-     *
-     * Currently, the burden is on the developer to figure these out until we have a much more
-     * test-friendly way of executing inflation logic (i.e. pass in an executor).
      */
-    private void waitForMessages(int numMessages) {
+    private void waitForInflation() {
         mHandler.postDelayed(TIMEOUT_RUNNABLE, TIMEOUT_TIME);
-        TestableLooper.get(this).processMessages(numMessages);
+        final CountDownLatch latch = new CountDownLatch(1);
+        NotificationEntryListener inflationListener = new NotificationEntryListener() {
+            @Override
+            public void onEntryInflated(NotificationEntry entry) {
+                latch.countDown();
+            }
+
+            @Override
+            public void onEntryReinflated(NotificationEntry entry) {
+                latch.countDown();
+            }
+
+            @Override
+            public void onInflationError(StatusBarNotification notification, Exception exception) {
+                latch.countDown();
+            }
+        };
+        mEntryManager.addNotificationEntryListener(inflationListener);
+        while (latch.getCount() != 0) {
+            mBgExecutor.runAllReady();
+            TestableLooper.get(this).processMessages(1);
+        }
         mHandler.removeCallbacks(TIMEOUT_RUNNABLE);
+        mEntryManager.removeNotificationEntryListener(inflationListener);
     }
 
 }

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationTestHelper.java

@@ -71,6 +71,7 @@ import com.android.systemui.tests.R;
 import org.mockito.ArgumentCaptor;
 
 import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.Executor;
 import java.util.concurrent.TimeUnit;
 
 /**
@@ -121,7 +122,8 @@ public class NotificationTestHelper {
                 mock(NotificationRemoteInputManager.class),
                 () -> mock(SmartReplyConstants.class),
                 () -> mock(SmartReplyController.class),
-                mock(ConversationNotificationProcessor.class));
+                mock(ConversationNotificationProcessor.class),
+                mock(Executor.class));
         contentBinder.setInflateSynchronously(true);
         mBindStage = new RowContentBindStage(contentBinder,
                 mock(NotifInflationErrorManager.class),