Merge "Deflake NotificationEntryManagerInflationTest" into rvc-dev am: 31ddf754dc am: b95f96a24f

Change-Id: Ie7afe6f2542dfa211e832d448082bc71df726ada
This commit is contained in:
TreeHugger Robot
2020-03-19 21:53:00 +00:00
committed by Automerger Merge Worker
4 changed files with 84 additions and 49 deletions

View File

@@ -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,
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);
}

View File

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

View File

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

View File

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