DO NOT MERGE: Revert "Force FGS notifications to show for a minimum time"
This reverts commit 09843a687bc19f8e360b2f5b0493a1f378bb603b.
Reason for revert: There is possible reentrant behavior when posting the safeToRemove callback after a FGS has been canceled/reposted
Bug: 140948482
Change-Id: I2c5b9135e57a87f412dce8c167147dcb38f6bbae
diff --git a/packages/SystemUI/src/com/android/systemui/ForegroundServiceNotificationListener.java b/packages/SystemUI/src/com/android/systemui/ForegroundServiceNotificationListener.java
index 46e0866..96b62ac 100644
--- a/packages/SystemUI/src/com/android/systemui/ForegroundServiceNotificationListener.java
+++ b/packages/SystemUI/src/com/android/systemui/ForegroundServiceNotificationListener.java
@@ -16,20 +16,14 @@
package com.android.systemui;
-import android.annotation.NonNull;
import android.app.Notification;
import android.app.NotificationManager;
import android.content.Context;
import android.os.Bundle;
-import android.os.Handler;
-import android.os.Looper;
import android.service.notification.StatusBarNotification;
-import android.util.ArraySet;
import android.util.Log;
-import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.statusbar.NotificationVisibility;
-import com.android.systemui.statusbar.NotificationLifetimeExtender;
import com.android.systemui.statusbar.notification.NotificationEntryListener;
import com.android.systemui.statusbar.notification.NotificationEntryManager;
import com.android.systemui.statusbar.notification.collection.NotificationEntry;
@@ -72,9 +66,6 @@
removeNotification(entry.notification);
}
});
-
- notificationEntryManager.addNotificationLifetimeExtender(
- new ForegroundServiceNotificationListener.ForegroundServiceLifetimeExtender());
}
/**
@@ -153,65 +144,4 @@
},
true /* create if not found */);
}
-
- /**
- * Extends the lifetime of foreground notification services such that they show for at least
- * five seconds
- */
- public static class ForegroundServiceLifetimeExtender implements NotificationLifetimeExtender {
-
- private static final String TAG = "FGSLifetimeExtender";
- @VisibleForTesting
- static final int MIN_FGS_TIME_MS = 5000;
-
- private NotificationSafeToRemoveCallback mNotificationSafeToRemoveCallback;
- private ArraySet<NotificationEntry> mManagedEntries = new ArraySet<>();
- private Handler mHandler = new Handler(Looper.getMainLooper());
-
- public ForegroundServiceLifetimeExtender() {
- }
-
- @Override
- public void setCallback(@NonNull NotificationSafeToRemoveCallback callback) {
- mNotificationSafeToRemoveCallback = callback;
- }
-
- @Override
- public boolean shouldExtendLifetime(@NonNull NotificationEntry entry) {
- if ((entry.notification.getNotification().flags
- & Notification.FLAG_FOREGROUND_SERVICE) == 0) {
- return false;
- }
-
- long currentTime = System.currentTimeMillis();
- return currentTime - entry.notification.getPostTime() < MIN_FGS_TIME_MS;
- }
-
- @Override
- public boolean shouldExtendLifetimeForPendingNotification(
- @NonNull NotificationEntry entry) {
- return shouldExtendLifetime(entry);
- }
-
- @Override
- public void setShouldManageLifetime(
- @NonNull NotificationEntry entry, boolean shouldManage) {
- if (!shouldManage) {
- mManagedEntries.remove(entry);
- return;
- }
-
- mManagedEntries.add(entry);
-
- Runnable r = () -> {
- if (mManagedEntries.contains(entry)) {
- if (mNotificationSafeToRemoveCallback != null) {
- mNotificationSafeToRemoveCallback.onSafeToRemove(entry.key);
- }
- mManagedEntries.remove(entry);
- }
- };
- mHandler.postDelayed(r, MIN_FGS_TIME_MS);
- }
- }
}
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLifetimeExtender.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLifetimeExtender.java
index 48e2923..0f295ba 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLifetimeExtender.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLifetimeExtender.java
@@ -27,18 +27,6 @@
boolean shouldExtendLifetime(@NonNull NotificationEntry entry);
/**
- * It's possible that a notification was canceled before it ever became visible. This callback
- * gives lifetime extenders a chance to make sure it shows up. For example if a foreground
- * service is canceled too quickly but we still want to make sure a FGS notification shows.
- * @param pendingEntry the canceled (but pending) entry
- * @return true if the notification lifetime should be extended
- */
- default boolean shouldExtendLifetimeForPendingNotification(
- @NonNull NotificationEntry pendingEntry) {
- return false;
- }
-
- /**
* Sets whether or not the lifetime should be managed by the extender. In practice, if
* shouldManage is true, this is where the extender starts managing the entry internally and is
* now responsible for calling {@link NotificationSafeToRemoveCallback#onSafeToRemove(String)}
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java
index a37367e..f8fef7d 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java
@@ -281,25 +281,11 @@
}
final NotificationEntry entry = mNotificationData.get(key);
+
+ abortExistingInflation(key);
+
boolean lifetimeExtended = false;
- // Notification was canceled before it got inflated
- if (entry == null) {
- NotificationEntry pendingEntry = mPendingNotifications.get(key);
- if (pendingEntry != null) {
- for (NotificationLifetimeExtender extender : mNotificationLifetimeExtenders) {
- if (extender.shouldExtendLifetimeForPendingNotification(pendingEntry)) {
- extendLifetime(pendingEntry, extender);
- lifetimeExtended = true;
- }
- }
- }
- }
-
- if (!lifetimeExtended) {
- abortExistingInflation(key);
- }
-
if (entry != null) {
// If a manager needs to keep the notification around for whatever reason, we
// keep the notification
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationPresenter.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationPresenter.java
index 06921980..a870590 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationPresenter.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationPresenter.java
@@ -224,7 +224,7 @@
mVisualStabilityManager.setUpWithPresenter(this);
mGutsManager.setUpWithPresenter(this,
notifListContainer, mCheckSaveListener, mOnSettingsClickListener);
- // ForegroundServiceNotificationListener adds its listener in its constructor
+ // ForegroundServiceControllerListener adds its listener in its constructor
// but we need to request it here in order for it to be instantiated.
// TODO: figure out how to do this correctly once Dependency.get() is gone.
Dependency.get(ForegroundServiceNotificationListener.class);
diff --git a/packages/SystemUI/tests/src/com/android/systemui/ForegroundServiceNotificationListenerTest.java b/packages/SystemUI/tests/src/com/android/systemui/ForegroundServiceNotificationListenerTest.java
deleted file mode 100644
index 72a457e..0000000
--- a/packages/SystemUI/tests/src/com/android/systemui/ForegroundServiceNotificationListenerTest.java
+++ /dev/null
@@ -1,86 +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;
-
-import static com.android.systemui.ForegroundServiceNotificationListener.ForegroundServiceLifetimeExtender.MIN_FGS_TIME_MS;
-
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-
-import android.app.Notification;
-import android.service.notification.StatusBarNotification;
-
-import androidx.test.filters.SmallTest;
-import androidx.test.runner.AndroidJUnit4;
-
-import com.android.systemui.statusbar.notification.collection.NotificationEntry;
-
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-
-@RunWith(AndroidJUnit4.class)
-@SmallTest
-public class ForegroundServiceNotificationListenerTest extends SysuiTestCase {
- private ForegroundServiceNotificationListener.ForegroundServiceLifetimeExtender mExtender =
- new ForegroundServiceNotificationListener.ForegroundServiceLifetimeExtender();
- private StatusBarNotification mSbn;
- private NotificationEntry mEntry;
- private Notification mNotif;
-
- @Before
- public void setup() {
- mNotif = new Notification.Builder(mContext, "")
- .setSmallIcon(R.drawable.ic_person)
- .setContentTitle("Title")
- .setContentText("Text")
- .build();
-
- mSbn = mock(StatusBarNotification.class);
- when(mSbn.getNotification()).thenReturn(mNotif);
-
- mEntry = new NotificationEntry(mSbn);
- }
-
- /**
- * ForegroundServiceLifetimeExtenderTest
- */
- @Test
- public void testShouldExtendLifetime_should_foreground() {
- // Extend the lifetime of a FGS notification iff it has not been visible
- // for the minimum time
- mNotif.flags |= Notification.FLAG_FOREGROUND_SERVICE;
- when(mSbn.getPostTime()).thenReturn(System.currentTimeMillis());
- assertTrue(mExtender.shouldExtendLifetime(mEntry));
- }
-
- @Test
- public void testShouldExtendLifetime_shouldNot_foreground() {
- mNotif.flags |= Notification.FLAG_FOREGROUND_SERVICE;
- when(mSbn.getPostTime()).thenReturn(System.currentTimeMillis() - MIN_FGS_TIME_MS - 1);
- assertFalse(mExtender.shouldExtendLifetime(mEntry));
- }
-
- @Test
- public void testShouldExtendLifetime_shouldNot_notForeground() {
- mNotif.flags = 0;
- when(mSbn.getPostTime()).thenReturn(System.currentTimeMillis() - MIN_FGS_TIME_MS - 1);
- assertFalse(mExtender.shouldExtendLifetime(mEntry));
- }
-}
diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java
index c0b2646..d4b3138 100644
--- a/services/core/java/com/android/server/notification/NotificationManagerService.java
+++ b/services/core/java/com/android/server/notification/NotificationManagerService.java
@@ -5236,16 +5236,8 @@
}
synchronized (mNotificationLock) {
- // If the notification is currently enqueued, repost this runnable so it has a
- // chance to notify listeners
- if ((findNotificationByListLocked(mEnqueuedNotifications, mPkg, mTag, mId, mUserId))
- != null) {
- mHandler.post(this);
- return;
- }
- // Look for the notification in the posted list, since we already checked enqueued.
- NotificationRecord r =
- findNotificationByListLocked(mNotificationList, mPkg, mTag, mId, mUserId);
+ // Look for the notification, searching both the posted and enqueued lists.
+ NotificationRecord r = findNotificationLocked(mPkg, mTag, mId, mUserId);
if (r != null) {
// The notification was found, check if it should be removed.
diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java
index 5d9e0e2..b5e4934 100644
--- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java
+++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java
@@ -962,20 +962,6 @@
}
@Test
- public void testCancelImmediatelyAfterEnqueueNotifiesListeners_ForegroundServiceFlag()
- throws Exception {
- final StatusBarNotification sbn = generateNotificationRecord(null).sbn;
- sbn.getNotification().flags =
- Notification.FLAG_ONGOING_EVENT | FLAG_FOREGROUND_SERVICE;
- mBinderService.enqueueNotificationWithTag(PKG, PKG, "tag",
- sbn.getId(), sbn.getNotification(), sbn.getUserId());
- mBinderService.cancelNotificationWithTag(PKG, "tag", sbn.getId(), sbn.getUserId());
- waitForIdle();
- verify(mListeners, times(1)).notifyPostedLocked(any(), any());
- verify(mListeners, times(1)).notifyRemovedLocked(any(), anyInt(), any());
- }
-
- @Test
public void testUserInitiatedClearAll_noLeak() throws Exception {
final NotificationRecord n = generateNotificationRecord(
mTestNotificationChannel, 1, "group", true);