diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationContentInflater.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationContentInflater.java index f2ce1affba46a..719f74fdcde26 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationContentInflater.java +++ b/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 mSmartReplyConstants; private final Lazy mSmartReplyController; private final ConversationNotificationProcessor mConversationProcessor; + private final Executor mBgExecutor; @Inject NotificationContentInflater( @@ -81,12 +84,14 @@ public class NotificationContentInflater implements NotificationRowContentBinder NotificationRemoteInputManager remoteInputManager, Lazy smartReplyConstants, Lazy 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, - entry, row, isNewView, remoteViewClickHandler, callback, privateLayout, - privateLayout.getContractedChild(), privateLayout.getVisibleWrapper( + applyRemoteView(bgExecutor, inflateSynchronously, result, reInflateFlags, flag, + remoteViewCache, entry, row, isNewView, remoteViewClickHandler, callback, + 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, - entry, row, isNewView, remoteViewClickHandler, + applyRemoteView(bgExecutor, inflateSynchronously, result, reInflateFlags, flag, + 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, - entry, row, isNewView, remoteViewClickHandler, + applyRemoteView(bgExecutor, inflateSynchronously, result, reInflateFlags, flag, + 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, - entry, row, isNewView, remoteViewClickHandler, callback, + applyRemoteView(bgExecutor, inflateSynchronously, result, reInflateFlags, flag, + 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, - mRemoteViewCache, mEntry, mRow, mRemoteViewClickHandler, this); + mCancellationSignal = apply( + mBgExecutor, + mInflateSynchronously, + result, + mReInflateFlags, + mRemoteViewCache, + mEntry, + mRow, + mRemoteViewClickHandler, + this); } else { handleError(mError); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationContentInflaterTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationContentInflaterTest.java index 524afacd1ace8..6a65269c34b04 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationContentInflaterTest.java +++ b/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, diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationEntryManagerInflationTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationEntryManagerInflationTest.java index 1a0f29147931a..bdd7a2e064287 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationEntryManagerInflationTest.java +++ b/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 - // row inflation, system notification, remote views, contracted view - waitForMessages(4); + waitForInflation(); // 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 - // remote views, contracted view - waitForMessages(2); + waitForInflation(); // 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 - * occur. + * Wait for inflation to finish. * - * As part of the inflation pipeline, the main thread is forced to deal with several callbacks - * due to the nature of the API used (generally because they're {@link android.os.AsyncTask} - * callbacks). In order, these are - * - * 1) Callback after row inflation. See {@link RowInflaterTask}. - * 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). + * A few things to note + * 1) Row inflation is done via {@link AsyncLayoutInflater} on its own background thread that + * 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 + * so we control when that work is done. The callback is still always on the main thread. */ - 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); } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationTestHelper.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationTestHelper.java index 23a90f2fd84d7..74f80df6442b5 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationTestHelper.java +++ b/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),