Merge "Add inflation error manager"

This commit is contained in:
TreeHugger Robot
2020-02-12 01:49:18 +00:00
committed by Android (Google) Code Review
9 changed files with 269 additions and 72 deletions

View File

@@ -151,10 +151,6 @@ public class ListDumper {
.append(" ");
}
if (notifEntry.hasInflationError()) {
rksb.append("(!)hasInflationError ");
}
if (notifEntry.getDismissState() != NOT_DISMISSED) {
rksb.append("dismissState=")
.append(notifEntry.getDismissState())

View File

@@ -18,9 +18,7 @@ package com.android.systemui.statusbar.notification.collection;
import static android.service.notification.NotificationStats.DISMISS_SENTIMENT_NEUTRAL;
import android.os.RemoteException;
import android.service.notification.NotificationStats;
import android.service.notification.StatusBarNotification;
import com.android.internal.statusbar.IStatusBarService;
import com.android.internal.statusbar.NotificationVisibility;
@@ -29,6 +27,7 @@ import com.android.systemui.statusbar.notification.collection.inflation.NotifInf
import com.android.systemui.statusbar.notification.collection.inflation.NotificationRowBinderImpl;
import com.android.systemui.statusbar.notification.collection.notifcollection.DismissedByUserStats;
import com.android.systemui.statusbar.notification.logging.NotificationLogger;
import com.android.systemui.statusbar.notification.row.NotifInflationErrorManager;
import com.android.systemui.statusbar.notification.row.NotificationContentInflater;
import javax.inject.Inject;
@@ -44,6 +43,7 @@ public class NotifInflaterImpl implements NotifInflater {
private final IStatusBarService mStatusBarService;
private final NotifCollection mNotifCollection;
private final NotifInflationErrorManager mNotifErrorManager;
private NotificationRowBinderImpl mNotificationRowBinder;
private InflationCallback mExternalInflationCallback;
@@ -51,9 +51,11 @@ public class NotifInflaterImpl implements NotifInflater {
@Inject
public NotifInflaterImpl(
IStatusBarService statusBarService,
NotifCollection notifCollection) {
NotifCollection notifCollection,
NotifInflationErrorManager errorManager) {
mStatusBarService = statusBarService;
mNotifCollection = notifCollection;
mNotifErrorManager = errorManager;
}
/**
@@ -81,7 +83,6 @@ public class NotifInflaterImpl implements NotifInflater {
@Override
public void inflateViews(NotificationEntry entry) {
try {
entry.setHasInflationError(false);
requireBinder().inflateViews(entry, getDismissCallback(entry));
} catch (InflationException e) {
// logged in mInflationCallback.handleInflationException
@@ -131,25 +132,12 @@ public class NotifInflaterImpl implements NotifInflater {
public void handleInflationException(
NotificationEntry entry,
Exception e) {
entry.setHasInflationError(true);
try {
final StatusBarNotification sbn = entry.getSbn();
// report notification inflation errors back up
// to notification delegates
mStatusBarService.onNotificationError(
sbn.getPackageName(),
sbn.getTag(),
sbn.getId(),
sbn.getUid(),
sbn.getInitialPid(),
e.getMessage(),
sbn.getUserId());
} catch (RemoteException ex) {
}
mNotifErrorManager.setInflationError(entry, e);
}
@Override
public void onAsyncInflationFinished(NotificationEntry entry) {
mNotifErrorManager.clearInflationError(entry);
if (mExternalInflationCallback != null) {
mExternalInflationCallback.onInflationFinished(entry);
}

View File

@@ -122,9 +122,6 @@ public final class NotificationEntry extends ListEntry {
*/
@CancellationReason int mCancellationReason = REASON_NOT_CANCELED;
/** @see #hasInflationError() */
private boolean mHasInflationError;
/** @see #getDismissState() */
@NonNull private DismissState mDismissState = DismissState.NOT_DISMISSED;
@@ -273,23 +270,6 @@ public final class NotificationEntry extends ListEntry {
* Bookkeeping getters and setters
*/
/**
* Whether this notification had an error when attempting to inflate. This is only used in
* the NewNotifPipeline
*/
public boolean hasInflationError() {
return mHasInflationError;
}
/**
* Set whether the notification has an error while inflating.
*
* TODO: Move this into an inflation error manager class.
*/
public void setHasInflationError(boolean hasError) {
mHasInflationError = hasError;
}
/**
* Set if the user has dismissed this notif but we haven't yet heard back from system server to
* confirm the dismissal.

View File

@@ -16,12 +16,18 @@
package com.android.systemui.statusbar.notification.collection.coordinator;
import android.os.RemoteException;
import android.service.notification.StatusBarNotification;
import com.android.internal.statusbar.IStatusBarService;
import com.android.systemui.statusbar.notification.collection.NotifInflaterImpl;
import com.android.systemui.statusbar.notification.collection.NotifPipeline;
import com.android.systemui.statusbar.notification.collection.NotificationEntry;
import com.android.systemui.statusbar.notification.collection.ShadeListBuilder;
import com.android.systemui.statusbar.notification.collection.inflation.NotifInflater;
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter;
import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener;
import com.android.systemui.statusbar.notification.row.NotifInflationErrorManager;
import java.util.ArrayList;
import java.util.List;
@@ -34,7 +40,7 @@ import javax.inject.Singleton;
* Aborts inflation when a notification is removed.
*
* If a notification is not done inflating, this coordinator will filter the notification out
* from the NotifListBuilder.
* from the {@link ShadeListBuilder}.
*/
@Singleton
public class PreparationCoordinator implements Coordinator {
@@ -42,15 +48,22 @@ public class PreparationCoordinator implements Coordinator {
private final PreparationCoordinatorLogger mLogger;
private final NotifInflater mNotifInflater;
private final NotifInflationErrorManager mNotifErrorManager;
private final List<NotificationEntry> mPendingNotifications = new ArrayList<>();
private final IStatusBarService mStatusBarService;
@Inject
public PreparationCoordinator(
PreparationCoordinatorLogger logger,
NotifInflaterImpl notifInflater) {
NotifInflaterImpl notifInflater,
NotifInflationErrorManager errorManager,
IStatusBarService service) {
mLogger = logger;
mNotifInflater = notifInflater;
mNotifInflater.setInflationCallback(mInflationCallback);
mNotifErrorManager = errorManager;
mNotifErrorManager.addInflationErrorListener(mInflationErrorListener);
mStatusBarService = service;
}
@Override
@@ -84,8 +97,7 @@ public class PreparationCoordinator implements Coordinator {
*/
@Override
public boolean shouldFilterOut(NotificationEntry entry, long now) {
if (entry.hasInflationError()) {
mPendingNotifications.remove(entry);
if (mNotifErrorManager.hasInflationError(entry)) {
return true;
}
return false;
@@ -112,6 +124,34 @@ public class PreparationCoordinator implements Coordinator {
}
};
private final NotifInflationErrorManager.NotifInflationErrorListener mInflationErrorListener =
new NotifInflationErrorManager.NotifInflationErrorListener() {
@Override
public void onNotifInflationError(NotificationEntry entry, Exception e) {
mPendingNotifications.remove(entry);
try {
final StatusBarNotification sbn = entry.getSbn();
// report notification inflation errors back up
// to notification delegates
mStatusBarService.onNotificationError(
sbn.getPackageName(),
sbn.getTag(),
sbn.getId(),
sbn.getUid(),
sbn.getInitialPid(),
e.getMessage(),
sbn.getUserId());
} catch (RemoteException ex) {
}
mNotifInflationErrorFilter.invalidateList();
}
@Override
public void onNotifInflationErrorCleared(NotificationEntry entry) {
mNotifInflationErrorFilter.invalidateList();
}
};
private void inflateEntry(NotificationEntry entry, String reason) {
abortInflation(entry, reason);
mPendingNotifications.add(entry);

View File

@@ -0,0 +1,100 @@
/*
* Copyright (C) 2020 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.row;
import androidx.annotation.NonNull;
import androidx.collection.ArraySet;
import com.android.systemui.statusbar.notification.collection.NotificationEntry;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;
import javax.inject.Inject;
import javax.inject.Singleton;
/**
* A manager handling the error state of a notification when it encounters an exception while
* inflating. We don't want to show these notifications to the user but may want to keep them
* around for logging purposes.
*/
@Singleton
public class NotifInflationErrorManager {
Set<NotificationEntry> mErroredNotifs = new ArraySet<>();
List<NotifInflationErrorListener> mListeners = new ArrayList<>();
@Inject
public NotifInflationErrorManager() { }
/**
* Mark the notification as errored out due to encountering an exception while inflating.
*
* @param e the exception encountered while inflating
*/
public void setInflationError(NotificationEntry entry, Exception e) {
mErroredNotifs.add(entry);
for (int i = 0; i < mListeners.size(); i++) {
mListeners.get(i).onNotifInflationError(entry, e);
}
}
/**
* Notification inflated successfully and is no longer errored out.
*/
public void clearInflationError(NotificationEntry entry) {
if (mErroredNotifs.contains(entry)) {
mErroredNotifs.remove(entry);
for (int i = 0; i < mListeners.size(); i++) {
mListeners.get(i).onNotifInflationErrorCleared(entry);
}
}
}
/**
* Whether or not the notification encountered an exception while inflating.
*/
public boolean hasInflationError(@NonNull NotificationEntry entry) {
return mErroredNotifs.contains(entry);
}
/**
* Add listener for changes in inflation error state.
*/
public void addInflationErrorListener(NotifInflationErrorListener listener) {
mListeners.add(listener);
}
/**
* Listener for changes in notification inflation error state.
*/
public interface NotifInflationErrorListener {
/**
* Called when notification encounters an inflation exception.
*
* @param e the exception encountered while inflating
*/
void onNotifInflationError(NotificationEntry entry, Exception e);
/**
* Called when notification inflation error is cleared.
*/
default void onNotifInflationErrorCleared(NotificationEntry entry) {}
}
}

View File

@@ -18,12 +18,8 @@ package com.android.systemui.statusbar.notification.row;
import static com.android.systemui.statusbar.notification.row.NotificationRowContentBinder.FLAG_CONTENT_VIEW_ALL;
import android.os.RemoteException;
import android.service.notification.StatusBarNotification;
import androidx.annotation.NonNull;
import com.android.internal.statusbar.IStatusBarService;
import com.android.systemui.statusbar.notification.collection.NotificationEntry;
import com.android.systemui.statusbar.notification.row.NotificationRowContentBinder.BindParams;
import com.android.systemui.statusbar.notification.row.NotificationRowContentBinder.InflationCallback;
@@ -41,14 +37,14 @@ import javax.inject.Singleton;
@Singleton
public class RowContentBindStage extends BindStage<RowContentBindParams> {
private final NotificationRowContentBinder mBinder;
private final IStatusBarService mStatusBarService;
private final NotifInflationErrorManager mNotifInflationErrorManager;
@Inject
RowContentBindStage(
NotificationRowContentBinder binder,
IStatusBarService statusBarService) {
NotifInflationErrorManager errorManager) {
mBinder = binder;
mStatusBarService = statusBarService;
mNotifInflationErrorManager = errorManager;
}
@Override
@@ -78,24 +74,12 @@ public class RowContentBindStage extends BindStage<RowContentBindParams> {
InflationCallback inflationCallback = new InflationCallback() {
@Override
public void handleInflationException(NotificationEntry entry, Exception e) {
entry.setHasInflationError(true);
try {
final StatusBarNotification sbn = entry.getSbn();
mStatusBarService.onNotificationError(
sbn.getPackageName(),
sbn.getTag(),
sbn.getId(),
sbn.getUid(),
sbn.getInitialPid(),
e.getMessage(),
sbn.getUserId());
} catch (RemoteException ex) {
}
mNotifInflationErrorManager.setInflationError(entry, e);
}
@Override
public void onAsyncInflationFinished(NotificationEntry entry) {
entry.setHasInflationError(false);
mNotifInflationErrorManager.clearInflationError(entry);
getStageParams(entry).clearDirtyContentViews();
callback.onStageFinished(entry);
}

View File

@@ -0,0 +1,111 @@
/*
* Copyright (C) 2020 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.collection.coordinator;
import static junit.framework.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import android.os.RemoteException;
import android.testing.AndroidTestingRunner;
import android.testing.TestableLooper;
import androidx.test.filters.SmallTest;
import com.android.internal.statusbar.IStatusBarService;
import com.android.systemui.SysuiTestCase;
import com.android.systemui.statusbar.notification.collection.NotifInflaterImpl;
import com.android.systemui.statusbar.notification.collection.NotifPipeline;
import com.android.systemui.statusbar.notification.collection.NotificationEntry;
import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder;
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter;
import com.android.systemui.statusbar.notification.row.NotifInflationErrorManager;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import java.util.List;
@SmallTest
@RunWith(AndroidTestingRunner.class)
@TestableLooper.RunWithLooper
public class PreparationCoordinatorTest extends SysuiTestCase {
private static final String TEST_MESSAGE = "TEST_MESSAGE";
private PreparationCoordinator mCoordinator;
private NotifFilter mInflationErrorFilter;
private NotifInflationErrorManager mErrorManager;
private NotificationEntry mEntry;
private Exception mInflationError;
@Mock
private NotifPipeline mNotifPipeline;
@Mock private IStatusBarService mService;
@Before
public void setUp() {
MockitoAnnotations.initMocks(this);
mEntry = new NotificationEntryBuilder().build();
mInflationError = new Exception(TEST_MESSAGE);
mErrorManager = new NotifInflationErrorManager();
mCoordinator = new PreparationCoordinator(
mock(PreparationCoordinatorLogger.class),
mock(NotifInflaterImpl.class),
mErrorManager,
mService);
ArgumentCaptor<NotifFilter> filterCaptor = ArgumentCaptor.forClass(NotifFilter.class);
mCoordinator.attach(mNotifPipeline);
verify(mNotifPipeline, times(2)).addPreRenderFilter(filterCaptor.capture());
List<NotifFilter> filters = filterCaptor.getAllValues();
mInflationErrorFilter = filters.get(0);
}
@Test
public void testErrorLogsToService() throws RemoteException {
// WHEN an entry has an inflation error.
mErrorManager.setInflationError(mEntry, mInflationError);
// THEN we log to status bar service.
verify(mService).onNotificationError(
eq(mEntry.getSbn().getPackageName()),
eq(mEntry.getSbn().getTag()),
eq(mEntry.getSbn().getId()),
eq(mEntry.getSbn().getUid()),
eq(mEntry.getSbn().getInitialPid()),
eq(mInflationError.getMessage()),
eq(mEntry.getSbn().getUserId()));
}
@Test
public void testFiltersOutErroredNotifications() {
// WHEN an entry has an inflation error.
mErrorManager.setInflationError(mEntry, mInflationError);
// THEN we filter it from the notification list.
assertTrue(mInflationErrorFilter.shouldFilterOut(mEntry, 0));
}
}

View File

@@ -41,7 +41,6 @@ import android.text.TextUtils;
import android.view.LayoutInflater;
import android.widget.RemoteViews;
import com.android.internal.statusbar.IStatusBarService;
import com.android.systemui.TestableDependency;
import com.android.systemui.bubbles.BubbleController;
import com.android.systemui.bubbles.BubblesTestActivity;
@@ -112,7 +111,7 @@ public class NotificationTestHelper {
mock(NotifRemoteViewCache.class),
mock(NotificationRemoteInputManager.class));
contentBinder.setInflateSynchronously(true);
mBindStage = new RowContentBindStage(contentBinder, mock(IStatusBarService.class));
mBindStage = new RowContentBindStage(contentBinder, mock(NotifInflationErrorManager.class));
CommonNotifCollection collection = mock(CommonNotifCollection.class);

View File

@@ -34,7 +34,6 @@ import android.testing.TestableLooper;
import androidx.test.filters.SmallTest;
import com.android.internal.statusbar.IStatusBarService;
import com.android.systemui.SysuiTestCase;
import com.android.systemui.statusbar.notification.collection.NotificationEntry;
import com.android.systemui.statusbar.notification.row.NotificationRowContentBinder.BindParams;
@@ -62,7 +61,7 @@ public class RowContentBindStageTest extends SysuiTestCase {
MockitoAnnotations.initMocks(this);
mRowContentBindStage = new RowContentBindStage(mBinder,
mock(IStatusBarService.class));
mock(NotifInflationErrorManager.class));
mRowContentBindStage.createStageParams(mEntry);
}