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

Commit a6d5bb47 authored by Kevin Han's avatar Kevin Han
Browse files

Deflake NotificationEntryManagerInflationTest 

Deflake NotificationEntryManagerInflationTest by moving back
to the countdown latch approach and providing our own executor
to AsyncInflationTask.

It was infeasible to depend on the exact number of main thread messages
as other main thread messages from other tests could interfere. Thus.
we move back to the countdown latch for when NotificationEntryManager
finishes inflation. This makes us dependent on the listener API for
determining when inflation is finished, but it's unlikely we can do
better unless we can inject an executor into RowInflaterTask.

In addition, using AsyncTask's SERIAL_EXECUTOR on hwasan builds seemed
to be much slower than normal. This lead to other tests' AysncTask
work delaying the work in this test and leading to the timeout
happening before inflation finished. By providing a test executor
instead and synchronously controlling its execution, we avoid this
issue.

Bug: 150618180
Test: atest --iterations 100 NotificationEntryManagerInflationTest
Test: atest SystemUITests
Test: A forrest run coming soon!
Change-Id: I87c6e2d216c8f26aaf340d311f618c9dccaba8af
parent 035a0710

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

+36 −15
Original line number Diff line number Diff line
@@ -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);

            }

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

+4 −1
Original line number Diff line number Diff line
@@ -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

+41 −32
Original line number Diff line number Diff line
@@ -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.

     *

     * 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}.

     * Wait for inflation to finish.

     *

     * 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);

    }



}

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

+3 −1
Original line number Diff line number Diff line
@@ -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),