Remove setInflationCallback

The current use cases for NotificationRowBinderImpl and NotifInflater's
API don't really need a persistent inflation callback stored as opposed
to a one-shot callback. By explicitly putting the callback in the
inflate methods, it also makes it more clear which calls inflate and
expect a callback to be called.

Bug: 145749521
Test: atest SystemUITests
Test: smoke test (add, update, remove notification)
Change-Id: I001413f17b7472ea2ef637be21d9d454ee17015e
This commit is contained in:
Kevin Han
2020-03-10 18:27:50 -07:00
parent 3742322d04
commit bd142939eb
12 changed files with 103 additions and 173 deletions

View File

@@ -88,14 +88,11 @@ import dagger.Lazy;
* @see #getActiveNotificationUnfiltered(String) to check if a key exists
* @see #getPendingNotificationsIterator() for an iterator over the pending notifications
* @see #getPendingOrActiveNotif(String) to find a notification exists for that key in any list
* @see #getPendingAndActiveNotifications() to get the entire set of Notifications that we're
* aware of
* @see #getActiveNotificationsForCurrentUser() to see every notification that the current user owns
*/
public class NotificationEntryManager implements
CommonNotifCollection,
Dumpable,
InflationCallback,
VisualStabilityManager.Callback {
private static final String TAG = "NotificationEntryMgr";
private static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG);
@@ -309,12 +306,7 @@ public class NotificationEntryManager implements
*
* WARNING: this will call back into us. Don't hold any locks.
*/
@Override
public void handleInflationException(NotificationEntry n, Exception e) {
handleInflationException(n.getSbn(), e);
}
public void handleInflationException(StatusBarNotification n, Exception e) {
private void handleInflationException(StatusBarNotification n, Exception e) {
removeNotificationInternal(
n.getKey(), null, null, true /* forceRemove */, false /* removedByUser */,
REASON_ERROR);
@@ -323,30 +315,37 @@ public class NotificationEntryManager implements
}
}
@Override
public void onAsyncInflationFinished(NotificationEntry entry) {
mPendingNotifications.remove(entry.getKey());
// If there was an async task started after the removal, we don't want to add it back to
// the list, otherwise we might get leaks.
if (!entry.isRowRemoved()) {
boolean isNew = getActiveNotificationUnfiltered(entry.getKey()) == null;
mLogger.logNotifInflated(entry.getKey(), isNew);
if (isNew) {
for (NotificationEntryListener listener : mNotificationEntryListeners) {
listener.onEntryInflated(entry);
}
addActiveNotification(entry);
updateNotifications("onAsyncInflationFinished");
for (NotificationEntryListener listener : mNotificationEntryListeners) {
listener.onNotificationAdded(entry);
}
} else {
for (NotificationEntryListener listener : mNotificationEntryListeners) {
listener.onEntryReinflated(entry);
private final InflationCallback mInflationCallback = new InflationCallback() {
@Override
public void handleInflationException(NotificationEntry entry, Exception e) {
NotificationEntryManager.this.handleInflationException(entry.getSbn(), e);
}
@Override
public void onAsyncInflationFinished(NotificationEntry entry) {
mPendingNotifications.remove(entry.getKey());
// If there was an async task started after the removal, we don't want to add it back to
// the list, otherwise we might get leaks.
if (!entry.isRowRemoved()) {
boolean isNew = getActiveNotificationUnfiltered(entry.getKey()) == null;
mLogger.logNotifInflated(entry.getKey(), isNew);
if (isNew) {
for (NotificationEntryListener listener : mNotificationEntryListeners) {
listener.onEntryInflated(entry);
}
addActiveNotification(entry);
updateNotifications("onAsyncInflationFinished");
for (NotificationEntryListener listener : mNotificationEntryListeners) {
listener.onNotificationAdded(entry);
}
} else {
for (NotificationEntryListener listener : mNotificationEntryListeners) {
listener.onEntryReinflated(entry);
}
}
}
}
}
};
private final NotificationHandler mNotifListener = new NotificationHandler() {
@Override
@@ -572,8 +571,10 @@ public class NotificationEntryManager implements
// Construct the expanded view.
if (!mFeatureFlags.isNewNotifPipelineRenderingEnabled()) {
mNotificationRowBinderLazy.get()
.inflateViews(entry, () -> performRemoveNotification(notification,
REASON_CANCEL));
.inflateViews(
entry,
() -> performRemoveNotification(notification, REASON_CANCEL),
mInflationCallback);
}
abortExistingInflation(key, "addNotification");
@@ -630,8 +631,10 @@ public class NotificationEntryManager implements
if (!mFeatureFlags.isNewNotifPipelineRenderingEnabled()) {
mNotificationRowBinderLazy.get()
.inflateViews(entry, () -> performRemoveNotification(notification,
REASON_CANCEL));
.inflateViews(
entry,
() -> performRemoveNotification(notification, REASON_CANCEL),
mInflationCallback);
}
updateNotifications("updateNotificationInternal");

View File

@@ -47,7 +47,6 @@ public class NotifInflaterImpl implements NotifInflater {
private final NotifPipeline mNotifPipeline;
private NotificationRowBinderImpl mNotificationRowBinder;
private InflationCallback mExternalInflationCallback;
@Inject
public NotifInflaterImpl(
@@ -66,17 +65,11 @@ public class NotifInflaterImpl implements NotifInflater {
*/
public void setRowBinder(NotificationRowBinderImpl rowBinder) {
mNotificationRowBinder = rowBinder;
mNotificationRowBinder.setInflationCallback(mInflationCallback);
}
@Override
public void setInflationCallback(InflationCallback callback) {
mExternalInflationCallback = callback;
}
@Override
public void rebindViews(NotificationEntry entry) {
inflateViews(entry);
public void rebindViews(NotificationEntry entry, InflationCallback callback) {
inflateViews(entry, callback);
}
/**
@@ -84,11 +77,14 @@ public class NotifInflaterImpl implements NotifInflater {
* views are bound.
*/
@Override
public void inflateViews(NotificationEntry entry) {
public void inflateViews(NotificationEntry entry, InflationCallback callback) {
try {
requireBinder().inflateViews(entry, getDismissCallback(entry));
requireBinder().inflateViews(
entry,
getDismissCallback(entry),
wrapInflationCallback(callback));
} catch (InflationException e) {
// logged in mInflationCallback.handleInflationException
mNotifErrorManager.setInflationError(entry, e);
}
}
@@ -121,6 +117,26 @@ public class NotifInflaterImpl implements NotifInflater {
};
}
private NotificationContentInflater.InflationCallback wrapInflationCallback(
InflationCallback callback) {
return new NotificationContentInflater.InflationCallback() {
@Override
public void handleInflationException(
NotificationEntry entry,
Exception e) {
mNotifErrorManager.setInflationError(entry, e);
}
@Override
public void onAsyncInflationFinished(NotificationEntry entry) {
mNotifErrorManager.clearInflationError(entry);
if (callback != null) {
callback.onInflationFinished(entry);
}
}
};
}
private NotificationRowBinderImpl requireBinder() {
if (mNotificationRowBinder == null) {
throw new RuntimeException("NotificationRowBinder must be attached before using "
@@ -128,22 +144,4 @@ public class NotifInflaterImpl implements NotifInflater {
}
return mNotificationRowBinder;
}
private final NotificationContentInflater.InflationCallback mInflationCallback =
new NotificationContentInflater.InflationCallback() {
@Override
public void handleInflationException(
NotificationEntry entry,
Exception e) {
mNotifErrorManager.setInflationError(entry, e);
}
@Override
public void onAsyncInflationFinished(NotificationEntry entry) {
mNotifErrorManager.clearInflationError(entry);
if (mExternalInflationCallback != null) {
mExternalInflationCallback.onInflationFinished(entry);
}
}
};
}

View File

@@ -72,7 +72,6 @@ public class PreparationCoordinator implements Coordinator {
) {
mLogger = logger;
mNotifInflater = notifInflater;
mNotifInflater.setInflationCallback(mInflationCallback);
mNotifErrorManager = errorManager;
mNotifErrorManager.addInflationErrorListener(mInflationErrorListener);
mViewBarn = viewBarn;
@@ -218,11 +217,11 @@ public class PreparationCoordinator implements Coordinator {
private void inflateEntry(NotificationEntry entry, String reason) {
abortInflation(entry, reason);
mNotifInflater.inflateViews(entry);
mNotifInflater.inflateViews(entry, mInflationCallback);
}
private void rebind(NotificationEntry entry, String reason) {
mNotifInflater.rebindViews(entry);
mNotifInflater.rebindViews(entry, mInflationCallback);
}
private void abortInflation(NotificationEntry entry, String reason) {

View File

@@ -24,22 +24,20 @@ import com.android.systemui.statusbar.notification.collection.coordinator.Prepar
* main thread. When the inflation is finished, NotifInflater will trigger its InflationCallback.
*/
public interface NotifInflater {
/**
* Callback used when inflation is finished.
*/
void setInflationCallback(InflationCallback callback);
/**
* Called to rebind the entry's views.
*
* @param callback callback called after inflation finishes
*/
void rebindViews(NotificationEntry entry);
void rebindViews(NotificationEntry entry, InflationCallback callback);
/**
* Called to inflate the views of an entry. Views are not considered inflated until all of its
* views are bound. Once all views are inflated, the InflationCallback is triggered.
*
* @param callback callback called after inflation finishes
*/
void inflateViews(NotificationEntry entry);
void inflateViews(NotificationEntry entry, InflationCallback callback);
/**
* Request to stop the inflation of an entry. For example, called when a notification is

View File

@@ -22,6 +22,7 @@ import com.android.systemui.statusbar.NotificationUiAdjustment;
import com.android.systemui.statusbar.notification.InflationException;
import com.android.systemui.statusbar.notification.NotificationEntryManager;
import com.android.systemui.statusbar.notification.collection.NotificationEntry;
import com.android.systemui.statusbar.notification.row.NotificationRowContentBinder;
/**
* Used by the {@link NotificationEntryManager}. When notifications are added or updated, the binder
@@ -37,7 +38,8 @@ public interface NotificationRowBinder {
*/
void inflateViews(
NotificationEntry entry,
Runnable onDismissRunnable)
Runnable onDismissRunnable,
NotificationRowContentBinder.InflationCallback callback)
throws InflationException;
/**

View File

@@ -67,7 +67,6 @@ public class NotificationRowBinderImpl implements NotificationRowBinder {
private NotificationPresenter mPresenter;
private NotificationListContainer mListContainer;
private NotificationRowContentBinder.InflationCallback mInflationCallback;
private BindRowCallback mBindRowCallback;
private NotificationClicker mNotificationClicker;
@@ -108,10 +107,6 @@ public class NotificationRowBinderImpl implements NotificationRowBinder {
mIconManager.attach();
}
public void setInflationCallback(NotificationRowContentBinder.InflationCallback callback) {
mInflationCallback = callback;
}
public void setNotificationClicker(NotificationClicker clicker) {
mNotificationClicker = clicker;
}
@@ -120,7 +115,10 @@ public class NotificationRowBinderImpl implements NotificationRowBinder {
* Inflates the views for the given entry (possibly asynchronously).
*/
@Override
public void inflateViews(NotificationEntry entry, Runnable onDismissRunnable)
public void inflateViews(
NotificationEntry entry,
Runnable onDismissRunnable,
NotificationRowContentBinder.InflationCallback callback)
throws InflationException {
ViewGroup parent = mListContainer.getViewParentForNotification(entry);
@@ -129,7 +127,7 @@ public class NotificationRowBinderImpl implements NotificationRowBinder {
ExpandableNotificationRow row = entry.getRow();
row.reset();
updateRow(entry, row);
inflateContentViews(entry, row);
inflateContentViews(entry, row, callback);
entry.getRowController().setOnDismissRunnable(onDismissRunnable);
} else {
mIconManager.createIcons(entry);
@@ -141,7 +139,6 @@ public class NotificationRowBinderImpl implements NotificationRowBinder {
.expandableNotificationRow(row)
.notificationEntry(entry)
.onDismissRunnable(onDismissRunnable)
.inflationCallback(mInflationCallback)
.rowContentBindStage(mRowContentBindStage)
.onExpandClickListener(mPresenter)
.build();
@@ -151,7 +148,7 @@ public class NotificationRowBinderImpl implements NotificationRowBinder {
entry.setRowController(rowController);
bindRow(entry, row);
updateRow(entry, row);
inflateContentViews(entry, row);
inflateContentViews(entry, row, callback);
});
}
}
@@ -175,6 +172,8 @@ public class NotificationRowBinderImpl implements NotificationRowBinder {
/**
* Updates the views bound to an entry when the entry's ranking changes, either in-place or by
* reinflating them.
*
* TODO: Should this method be in this class?
*/
@Override
public void onNotificationRankingUpdated(
@@ -187,7 +186,7 @@ public class NotificationRowBinderImpl implements NotificationRowBinder {
ExpandableNotificationRow row = entry.getRow();
row.reset();
updateRow(entry, row);
inflateContentViews(entry, row);
inflateContentViews(entry, row, null /* callback */);
} else {
// Once the RowInflaterTask is done, it will pick up the updated entry, so
// no-op here.
@@ -221,7 +220,8 @@ public class NotificationRowBinderImpl implements NotificationRowBinder {
*/
private void inflateContentViews(
NotificationEntry entry,
ExpandableNotificationRow row) {
ExpandableNotificationRow row,
NotificationRowContentBinder.InflationCallback inflationCallback) {
final boolean useIncreasedCollapsedHeight =
mMessagingUtil.isImportantMessaging(entry.getSbn(), entry.getImportance());
final boolean isLowPriority = entry.isAmbient();
@@ -238,7 +238,7 @@ public class NotificationRowBinderImpl implements NotificationRowBinder {
mRowContentBindStage.requestRebind(entry, en -> {
row.setUsesIncreasedCollapsedHeight(useIncreasedCollapsedHeight);
row.setIsLowPriority(isLowPriority);
mInflationCallback.onAsyncInflationFinished(en);
inflationCallback.onAsyncInflationFinished(en);
});
}

View File

@@ -108,8 +108,6 @@ class NotificationsControllerImpl @Inject constructor(
if (featureFlags.isNewNotifPipelineRenderingEnabled) {
// TODO
} else {
notificationRowBinder.setInflationCallback(entryManager)
remoteInputUriController.attach(entryManager)
groupAlertTransferHelper.bind(entryManager, groupManager)
headsUpManager.addListener(groupManager)

View File

@@ -25,7 +25,6 @@ import com.android.systemui.statusbar.notification.collection.NotificationEntry;
import com.android.systemui.statusbar.notification.row.ActivatableNotificationView;
import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow;
import com.android.systemui.statusbar.notification.row.ExpandableNotificationRowController;
import com.android.systemui.statusbar.notification.row.NotificationRowContentBinder;
import com.android.systemui.statusbar.notification.row.RowContentBindStage;
import com.android.systemui.statusbar.phone.StatusBar;
@@ -60,8 +59,6 @@ public interface ExpandableNotificationRowComponent {
@BindsInstance
Builder rowContentBindStage(RowContentBindStage rowContentBindStage);
@BindsInstance
Builder inflationCallback(NotificationRowContentBinder.InflationCallback inflationCallback);
@BindsInstance
Builder onExpandClickListener(ExpandableNotificationRow.OnExpandClickListener presenter);
ExpandableNotificationRowComponent build();
}

View File

@@ -1,63 +0,0 @@
/*
* Copyright (C) 2019 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.android.systemui.statusbar.notification
import com.android.systemui.statusbar.FeatureFlags
import com.android.systemui.statusbar.NotificationPresenter
import com.android.systemui.statusbar.NotificationRemoteInputManager
import com.android.systemui.statusbar.notification.collection.NotificationEntry
import com.android.systemui.statusbar.notification.collection.NotificationRankingManager
import com.android.systemui.statusbar.notification.collection.inflation.NotificationRowBinder
import com.android.systemui.statusbar.phone.NotificationGroupManager
import com.android.systemui.util.leak.LeakDetector
import dagger.Lazy
import java.util.concurrent.CountDownLatch
/**
* Enable some test capabilities for NEM without making everything public on the base class
*/
class TestableNotificationEntryManager(
logger: NotificationEntryManagerLogger,
gm: NotificationGroupManager,
rm: NotificationRankingManager,
ke: KeyguardEnvironment,
ff: FeatureFlags,
rb: Lazy<NotificationRowBinder>,
notificationRemoteInputManagerLazy: Lazy<NotificationRemoteInputManager>,
leakDetector: LeakDetector,
fgsFeatureController: ForegroundServiceDismissalFeatureController
) : NotificationEntryManager(logger, gm, rm, ke, ff, rb, notificationRemoteInputManagerLazy,
leakDetector, fgsFeatureController) {
public var countDownLatch: CountDownLatch = CountDownLatch(1)
override fun onAsyncInflationFinished(entry: NotificationEntry) {
super.onAsyncInflationFinished(entry)
countDownLatch.countDown()
}
fun setUpForTest(
presenter: NotificationPresenter?
) {
super.setUpWithPresenter(presenter)
}
fun setActiveNotificationList(activeList: List<NotificationEntry>) {
mSortedAndFiltered.clear()
mSortedAndFiltered.addAll(activeList)
}
}

View File

@@ -18,6 +18,7 @@ package com.android.systemui.statusbar.notification.collection.coordinator;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
@@ -62,7 +63,6 @@ public class PreparationCoordinatorTest extends SysuiTestCase {
private OnBeforeFinalizeFilterListener mBeforeFilterListener;
private NotifFilter mUninflatedFilter;
private NotifFilter mInflationErrorFilter;
private NotifInflaterImpl.InflationCallback mCallback;
private NotifInflationErrorManager mErrorManager;
private NotificationEntry mEntry;
private Exception mInflationError;
@@ -104,9 +104,6 @@ public class PreparationCoordinatorTest extends SysuiTestCase {
mBeforeFilterListenerCaptor.capture());
mBeforeFilterListener = mBeforeFilterListenerCaptor.getValue();
verify(mNotifInflater).setInflationCallback(mCallbackCaptor.capture());
mCallback = mCallbackCaptor.getValue();
mCollectionListener.onEntryInit(mEntry);
}
@@ -142,7 +139,7 @@ public class PreparationCoordinatorTest extends SysuiTestCase {
mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry));
// THEN we inflate it
verify(mNotifInflater).inflateViews(mEntry);
verify(mNotifInflater).inflateViews(eq(mEntry), any());
// THEN we filter it out until it's done inflating.
assertTrue(mUninflatedFilter.shouldFilterOut(mEntry, 0));
@@ -151,14 +148,17 @@ public class PreparationCoordinatorTest extends SysuiTestCase {
@Test
public void testRebindsInflatedNotificationsOnUpdate() {
// GIVEN an inflated notification
mCallback.onInflationFinished(mEntry);
mCollectionListener.onEntryAdded(mEntry);
mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry));
verify(mNotifInflater).inflateViews(eq(mEntry), mCallbackCaptor.capture());
mCallbackCaptor.getValue().onInflationFinished(mEntry);
// WHEN notification is updated
mCollectionListener.onEntryUpdated(mEntry);
mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry));
// THEN we rebind it
verify(mNotifInflater).rebindViews(mEntry);
verify(mNotifInflater).rebindViews(eq(mEntry), any());
// THEN we do not filter it because it's not the first inflation.
assertFalse(mUninflatedFilter.shouldFilterOut(mEntry, 0));
@@ -166,8 +166,11 @@ public class PreparationCoordinatorTest extends SysuiTestCase {
@Test
public void testDoesntFilterInflatedNotifs() {
// WHEN a notification is inflated
mCallback.onInflationFinished(mEntry);
// GIVEN an inflated notification
mCollectionListener.onEntryAdded(mEntry);
mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry));
verify(mNotifInflater).inflateViews(eq(mEntry), mCallbackCaptor.capture());
mCallbackCaptor.getValue().onInflationFinished(mEntry);
// THEN it isn't filtered from shade list
assertFalse(mUninflatedFilter.shouldFilterOut(mEntry, 0));

View File

@@ -214,9 +214,6 @@ public class NotificationEntryManagerInflationTest extends SysuiTestCase {
when(mExpandableNotificationRowComponentBuilder
.onDismissRunnable(any()))
.thenReturn(mExpandableNotificationRowComponentBuilder);
when(mExpandableNotificationRowComponentBuilder
.inflationCallback(any()))
.thenReturn(mExpandableNotificationRowComponentBuilder);
when(mExpandableNotificationRowComponentBuilder
.rowContentBindStage(any()))
.thenReturn(mExpandableNotificationRowComponentBuilder);
@@ -273,7 +270,6 @@ public class NotificationEntryManagerInflationTest extends SysuiTestCase {
mEntryManager.addNotificationEntryListener(mEntryListener);
mRowBinder.setUpWithPresenter(mPresenter, mListContainer, mBindCallback);
mRowBinder.setInflationCallback(mEntryManager);
mRowBinder.setNotificationClicker(mock(NotificationClicker.class));
Ranking ranking = new Ranking();

View File

@@ -69,7 +69,6 @@ import com.android.systemui.statusbar.notification.NotificationEntryManager;
import com.android.systemui.statusbar.notification.NotificationEntryManagerLogger;
import com.android.systemui.statusbar.notification.NotificationFilter;
import com.android.systemui.statusbar.notification.NotificationSectionsFeatureManager;
import com.android.systemui.statusbar.notification.TestableNotificationEntryManager;
import com.android.systemui.statusbar.notification.VisualStabilityManager;
import com.android.systemui.statusbar.notification.collection.NotifCollection;
import com.android.systemui.statusbar.notification.collection.NotifPipeline;
@@ -138,7 +137,7 @@ public class NotificationStackScrollLayoutTest extends SysuiTestCase {
@Mock private NotificationLockscreenUserManager mLockscreenUserManager;
@Mock private FeatureFlags mFeatureFlags;
private UserChangedListener mUserChangedListener;
private TestableNotificationEntryManager mEntryManager;
private NotificationEntryManager mEntryManager;
private int mOriginalInterruptionModelSetting;
private UiEventLoggerFake mUiEventLoggerFake = new UiEventLoggerFake();
@@ -167,7 +166,7 @@ public class NotificationStackScrollLayoutTest extends SysuiTestCase {
ArgumentCaptor<UserChangedListener> userChangedCaptor = ArgumentCaptor
.forClass(UserChangedListener.class);
mEntryManager = new TestableNotificationEntryManager(
mEntryManager = new NotificationEntryManager(
mock(NotificationEntryManagerLogger.class),
mock(NotificationGroupManager.class),
new NotificationRankingManager(
@@ -187,7 +186,7 @@ public class NotificationStackScrollLayoutTest extends SysuiTestCase {
mock(LeakDetector.class),
mock(ForegroundServiceDismissalFeatureController.class)
);
mEntryManager.setUpForTest(mock(NotificationPresenter.class));
mEntryManager.setUpWithPresenter(mock(NotificationPresenter.class));
when(mFeatureFlags.isNewNotifPipelineRenderingEnabled()).thenReturn(false);
NotificationShelf notificationShelf = mock(NotificationShelf.class);