From cec677aca48c3c3b85e7320d05440b30c24aef1f Mon Sep 17 00:00:00 2001 From: Evan Laird Date: Fri, 8 May 2020 11:56:18 -0400 Subject: [PATCH 1/4] DO NOT MERGE: Create a shim for StatusBarManager click methods `StatusBarManager#onNotificationClick` and `StatusBarManager#onNotificationActionClick` are signals we send to system server about notification clicks. This CL adds a shim so that we can have an in-process callback about the exact same events. This CL also adds NotificationInteractionTracker, which basically just merges the NotificationClickNotifier callbacks with the notification collection and will be able to answer the question "has the user interacted with this notification" Lastly, this modifies the logic in ForegroundServiceLifetimeExtender which now checks the interaction flag for notifications. So if a user tapped on a notification action (for instance) which _then_ triggers a notification cancel, it will succeed. It _does not_ as of yet release the notification from lifetime extension upon interaction. So if a notification is canceled and then interacted with, it will still live the full amount of time. Test: atest SystemUITests Bug: 144324894 Bug: 119041698 Change-Id: I42201d6e7b7ffe9ad4f19c774b638a36a51750ef (cherry picked from commit 9b2a480ceb42bb7a79a1ff532dac42b0f407a88f) --- .../src/com/android/systemui/Dependency.java | 3 + .../android/systemui/DependencyBinder.java | 7 + .../ForegroundServiceLifetimeExtender.java | 20 +- ...ForegroundServiceNotificationListener.java | 7 +- .../com/android/systemui/SystemUIFactory.java | 6 +- .../statusbar/NotificationClickNotifier.kt | 90 ++++++++ .../NotificationInteractionTracker.kt | 45 ++++ ...NotificationLockscreenUserManagerImpl.java | 18 +- .../NotificationRemoteInputManager.java | 11 +- .../statusbar/SmartReplyController.java | 13 +- .../NotificationEntryListener.java | 4 +- .../NotificationEntryManager.java | 6 +- .../collection/NotificationEntry.java | 71 ++++-- .../systemui/statusbar/phone/StatusBar.java | 5 +- .../StatusBarNotificationActivityStarter.java | 15 +- .../util/concurrency/DelayableExecutor.java | 66 ++++++ .../systemui/util/time/SystemClock.java | 44 ++++ .../systemui/util/time/SystemClockImpl.java | 51 ++++ .../ForegroundServiceControllerTest.java | 4 +- ...roundServiceNotificationListenerTest.java} | 45 +++- ...NotificationLockscreenUserManagerTest.java | 3 +- .../NotificationRemoteInputManagerTest.java | 9 +- .../statusbar/SmartReplyControllerTest.java | 6 +- .../collection/NotificationDataTest.java | 6 +- .../collection/NotificationEntryTest.java | 2 + ...tusBarNotificationActivityStarterTest.java | 15 +- .../util/concurrency/FakeExecutor.java | 217 ++++++++++++++++++ .../systemui/util/time/FakeSystemClock.java | 125 ++++++++++ 28 files changed, 826 insertions(+), 88 deletions(-) create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/NotificationClickNotifier.kt create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/NotificationInteractionTracker.kt create mode 100644 packages/SystemUI/src/com/android/systemui/util/concurrency/DelayableExecutor.java create mode 100644 packages/SystemUI/src/com/android/systemui/util/time/SystemClock.java create mode 100644 packages/SystemUI/src/com/android/systemui/util/time/SystemClockImpl.java rename packages/SystemUI/tests/src/com/android/systemui/{ForegroundServiceLifetimeExtenderTest.java => ForegroundServiceNotificationListenerTest.java} (62%) create mode 100644 packages/SystemUI/tests/src/com/android/systemui/util/concurrency/FakeExecutor.java create mode 100644 packages/SystemUI/tests/src/com/android/systemui/util/time/FakeSystemClock.java diff --git a/packages/SystemUI/src/com/android/systemui/Dependency.java b/packages/SystemUI/src/com/android/systemui/Dependency.java index 9f4a4e08bbfde..689ccf7ffc36d 100644 --- a/packages/SystemUI/src/com/android/systemui/Dependency.java +++ b/packages/SystemUI/src/com/android/systemui/Dependency.java @@ -55,6 +55,7 @@ import com.android.systemui.shared.system.DevicePolicyManagerWrapper; import com.android.systemui.shared.system.PackageManagerWrapper; import com.android.systemui.statusbar.AmbientPulseManager; import com.android.systemui.statusbar.NavigationBarController; +import com.android.systemui.statusbar.NotificationClickNotifier; import com.android.systemui.statusbar.NotificationListener; import com.android.systemui.statusbar.NotificationLockscreenUserManager; import com.android.systemui.statusbar.NotificationMediaManager; @@ -302,6 +303,7 @@ public class Dependency extends SystemUI { @Inject Lazy mChannelEditorDialogController; @Inject Lazy mINotificationManager; @Inject Lazy mFalsingManager; + @Inject Lazy mClickNotifier; @Inject public Dependency() { @@ -479,6 +481,7 @@ public class Dependency extends SystemUI { mProviders.put(ChannelEditorDialogController.class, mChannelEditorDialogController::get); mProviders.put(INotificationManager.class, mINotificationManager::get); mProviders.put(FalsingManager.class, mFalsingManager::get); + mProviders.put(NotificationClickNotifier.class, mClickNotifier::get); // TODO(b/118592525): to support multi-display , we start to add something which is // per-display, while others may be global. I think it's time to add diff --git a/packages/SystemUI/src/com/android/systemui/DependencyBinder.java b/packages/SystemUI/src/com/android/systemui/DependencyBinder.java index 057d70ccdc0df..79d8b17e6d6aa 100644 --- a/packages/SystemUI/src/com/android/systemui/DependencyBinder.java +++ b/packages/SystemUI/src/com/android/systemui/DependencyBinder.java @@ -70,6 +70,8 @@ import com.android.systemui.statusbar.policy.ZenModeController; import com.android.systemui.statusbar.policy.ZenModeControllerImpl; import com.android.systemui.tuner.TunerService; import com.android.systemui.tuner.TunerServiceImpl; +import com.android.systemui.util.time.SystemClock; +import com.android.systemui.util.time.SystemClockImpl; import com.android.systemui.volume.VolumeDialogControllerImpl; import dagger.Binds; @@ -241,4 +243,9 @@ public abstract class DependencyBinder { */ @Binds public abstract FalsingManager provideFalsingmanager(FalsingManagerProxy falsingManagerImpl); + + /** + */ + @Binds + public abstract SystemClock provideSystemClock(SystemClockImpl systemClock); } diff --git a/packages/SystemUI/src/com/android/systemui/ForegroundServiceLifetimeExtender.java b/packages/SystemUI/src/com/android/systemui/ForegroundServiceLifetimeExtender.java index 05acdd080aa53..7db6642276f8c 100644 --- a/packages/SystemUI/src/com/android/systemui/ForegroundServiceLifetimeExtender.java +++ b/packages/SystemUI/src/com/android/systemui/ForegroundServiceLifetimeExtender.java @@ -23,8 +23,12 @@ import android.os.Looper; import android.util.ArraySet; import com.android.internal.annotations.VisibleForTesting; +import com.android.systemui.statusbar.NotificationInteractionTracker; import com.android.systemui.statusbar.NotificationLifetimeExtender; import com.android.systemui.statusbar.notification.collection.NotificationEntry; +import com.android.systemui.util.time.SystemClock; + +import javax.inject.Inject; /** * Extends the lifetime of foreground notification services such that they show for at least @@ -39,8 +43,15 @@ public class ForegroundServiceLifetimeExtender implements NotificationLifetimeEx private NotificationSafeToRemoveCallback mNotificationSafeToRemoveCallback; private ArraySet mManagedEntries = new ArraySet<>(); private Handler mHandler = new Handler(Looper.getMainLooper()); + private final SystemClock mSystemClock; + private final NotificationInteractionTracker mInteractionTracker; - public ForegroundServiceLifetimeExtender() { + @Inject + public ForegroundServiceLifetimeExtender( + NotificationInteractionTracker interactionTracker, + SystemClock systemClock) { + mSystemClock = systemClock; + mInteractionTracker = interactionTracker; } @Override @@ -55,8 +66,9 @@ public class ForegroundServiceLifetimeExtender implements NotificationLifetimeEx return false; } - long currentTime = System.currentTimeMillis(); - return currentTime - entry.notification.getPostTime() < MIN_FGS_TIME_MS; + boolean hasInteracted = mInteractionTracker.hasUserInteractedWith(entry.key); + long aliveTime = mSystemClock.uptimeMillis() - entry.getCreationTime(); + return aliveTime < MIN_FGS_TIME_MS && !hasInteracted; } @Override @@ -84,7 +96,7 @@ public class ForegroundServiceLifetimeExtender implements NotificationLifetimeEx } }; long delayAmt = MIN_FGS_TIME_MS - - (System.currentTimeMillis() - entry.notification.getPostTime()); + - (mSystemClock.uptimeMillis() - entry.getCreationTime()); mHandler.postDelayed(r, delayAmt); } } diff --git a/packages/SystemUI/src/com/android/systemui/ForegroundServiceNotificationListener.java b/packages/SystemUI/src/com/android/systemui/ForegroundServiceNotificationListener.java index 0162deb55143f..1e1eaf3e2cb96 100644 --- a/packages/SystemUI/src/com/android/systemui/ForegroundServiceNotificationListener.java +++ b/packages/SystemUI/src/com/android/systemui/ForegroundServiceNotificationListener.java @@ -44,7 +44,8 @@ public class ForegroundServiceNotificationListener { @Inject public ForegroundServiceNotificationListener(Context context, ForegroundServiceController foregroundServiceController, - NotificationEntryManager notificationEntryManager) { + NotificationEntryManager notificationEntryManager, + ForegroundServiceLifetimeExtender fgsLifetimeExtender) { mContext = context; mForegroundServiceController = foregroundServiceController; notificationEntryManager.addNotificationEntryListener(new NotificationEntryListener() { @@ -66,9 +67,7 @@ public class ForegroundServiceNotificationListener { removeNotification(entry.notification); } }); - - notificationEntryManager.addNotificationLifetimeExtender( - new ForegroundServiceLifetimeExtender()); + notificationEntryManager.addNotificationLifetimeExtender(fgsLifetimeExtender); } /** diff --git a/packages/SystemUI/src/com/android/systemui/SystemUIFactory.java b/packages/SystemUI/src/com/android/systemui/SystemUIFactory.java index d815d95f23d57..9900a93971f85 100644 --- a/packages/SystemUI/src/com/android/systemui/SystemUIFactory.java +++ b/packages/SystemUI/src/com/android/systemui/SystemUIFactory.java @@ -41,6 +41,7 @@ import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.power.EnhancedEstimates; import com.android.systemui.power.EnhancedEstimatesImpl; import com.android.systemui.statusbar.KeyguardIndicationController; +import com.android.systemui.statusbar.NotificationClickNotifier; import com.android.systemui.statusbar.NotificationListener; import com.android.systemui.statusbar.NotificationLockscreenUserManager; import com.android.systemui.statusbar.NotificationLockscreenUserManagerImpl; @@ -167,8 +168,9 @@ public class SystemUIFactory { @Singleton @Provides public NotificationLockscreenUserManager provideNotificationLockscreenUserManager( - Context context) { - return new NotificationLockscreenUserManagerImpl(context); + Context context, + NotificationClickNotifier clickNotifier) { + return new NotificationLockscreenUserManagerImpl(context, clickNotifier); } @Singleton diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationClickNotifier.kt b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationClickNotifier.kt new file mode 100644 index 0000000000000..0d3948853cda9 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationClickNotifier.kt @@ -0,0 +1,90 @@ +package com.android.systemui.statusbar + +import android.app.Notification +import android.os.Handler +import android.os.RemoteException + +import com.android.internal.statusbar.IStatusBarService +import com.android.internal.statusbar.NotificationVisibility +import com.android.systemui.Dependency +import com.android.systemui.util.Assert + +import javax.inject.Inject +import javax.inject.Named +import javax.inject.Singleton + +/** + * Class to shim calls to IStatusBarManager#onNotificationClick/#onNotificationActionClick that + * allow an in-process notification to go out (e.g., for tracking interactions) as well as + * sending the messages along to system server. + * + * NOTE: this class eats exceptions from system server, as no current client of these APIs cares + * about errors + */ +@Singleton +public class NotificationClickNotifier @Inject constructor( + val barService: IStatusBarService, + @Named(Dependency.MAIN_HANDLER_NAME) val mainHandler: Handler +) { + val listeners = mutableListOf() + + fun addNotificationInteractionListener(listener: NotificationInteractionListener) { + Assert.isMainThread() + listeners.add(listener) + } + + fun removeNotificationInteractionListener(listener: NotificationInteractionListener) { + Assert.isMainThread() + listeners.remove(listener) + } + + private fun notifyListenersAboutInteraction(key: String) { + for (l in listeners) { + l.onNotificationInteraction(key) + } + } + + fun onNotificationActionClick( + key: String, + actionIndex: Int, + action: Notification.Action, + visibility: NotificationVisibility, + generatedByAssistant: Boolean + ) { + try { + barService.onNotificationActionClick( + key, actionIndex, action, visibility, generatedByAssistant) + } catch (e: RemoteException) { + // nothing + } + + mainHandler.post { + notifyListenersAboutInteraction(key) + } + } + + fun onNotificationClick( + key: String, + visibility: NotificationVisibility + ) { + try { + barService.onNotificationClick(key, visibility) + } catch (e: RemoteException) { + // nothing + } + + mainHandler.post { + notifyListenersAboutInteraction(key) + } + } +} + +/** + * Interface for listeners to get notified when a notification is interacted with via a click or + * interaction with remote input or actions + */ +interface NotificationInteractionListener { + fun onNotificationInteraction(key: String) +} + +private const val TAG = "NotificationClickNotifier" diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationInteractionTracker.kt b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationInteractionTracker.kt new file mode 100644 index 0000000000000..40a3ed64f2c2e --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationInteractionTracker.kt @@ -0,0 +1,45 @@ +package com.android.systemui.statusbar + +import com.android.internal.statusbar.NotificationVisibility +import com.android.systemui.statusbar.notification.NotificationEntryManager +import com.android.systemui.statusbar.notification.NotificationEntryListener +import com.android.systemui.statusbar.notification.collection.NotificationEntry +import javax.inject.Inject +import javax.inject.Singleton + +/** + * Class to track user interaction with notifications. It's a glorified map of key : bool that can + * merge multiple "user interacted with notification" signals into a single place. + */ +@Singleton +class NotificationInteractionTracker @Inject constructor( + private val clicker: NotificationClickNotifier, + private val entryManager: NotificationEntryManager +) : NotificationEntryListener, NotificationInteractionListener { + private val interactions = mutableMapOf() + + init { + clicker.addNotificationInteractionListener(this) + entryManager.addNotificationEntryListener(this) + } + + fun hasUserInteractedWith(key: String): Boolean = key in interactions + + override fun onNotificationAdded(entry: NotificationEntry) { + interactions[entry.key] = false + } + + override fun onEntryRemoved( + entry: NotificationEntry, + visibility: NotificationVisibility?, + removedByUser: Boolean + ) { + interactions.remove(entry.key) + } + + override fun onNotificationInteraction(key: String) { + interactions[key] = true + } +} + +private const val TAG = "NotificationInteractionTracker" diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerImpl.java index e08a5ae07bd89..67218fc309765 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerImpl.java @@ -28,8 +28,6 @@ import android.content.IntentFilter; import android.content.IntentSender; import android.content.pm.UserInfo; import android.database.ContentObserver; -import android.os.RemoteException; -import android.os.ServiceManager; import android.os.UserHandle; import android.os.UserManager; import android.provider.Settings; @@ -37,7 +35,6 @@ import android.util.Log; import android.util.SparseArray; import android.util.SparseBooleanArray; -import com.android.internal.statusbar.IStatusBarService; import com.android.internal.statusbar.NotificationVisibility; import com.android.internal.widget.LockPatternUtils; import com.android.keyguard.KeyguardUpdateMonitor; @@ -80,8 +77,8 @@ public class NotificationLockscreenUserManagerImpl implements private final SparseBooleanArray mUsersAllowingPrivateNotifications = new SparseBooleanArray(); private final SparseBooleanArray mUsersAllowingNotifications = new SparseBooleanArray(); private final UserManager mUserManager; - private final IStatusBarService mBarService; private final List mListeners = new ArrayList<>(); + private final NotificationClickNotifier mClickNotifier; private boolean mShowLockscreenNotifications; private boolean mAllowLockscreenRemoteInput; @@ -146,11 +143,7 @@ public class NotificationLockscreenUserManagerImpl implements getEntryManager().getNotificationData().get(notificationKey)); final NotificationVisibility nv = NotificationVisibility.obtain(notificationKey, rank, count, true, location); - try { - mBarService.onNotificationClick(notificationKey, nv); - } catch (RemoteException e) { - /* ignore */ - } + mClickNotifier.onNotificationClick(notificationKey, nv); } } } @@ -171,15 +164,16 @@ public class NotificationLockscreenUserManagerImpl implements return mEntryManager; } - public NotificationLockscreenUserManagerImpl(Context context) { + public NotificationLockscreenUserManagerImpl( + Context context, + NotificationClickNotifier clickNotifier) { mContext = context; mDevicePolicyManager = (DevicePolicyManager) mContext.getSystemService( Context.DEVICE_POLICY_SERVICE); mUserManager = (UserManager) mContext.getSystemService(Context.USER_SERVICE); mCurrentUserId = ActivityManager.getCurrentUser(); - mBarService = IStatusBarService.Stub.asInterface( - ServiceManager.getService(Context.STATUS_BAR_SERVICE)); Dependency.get(StatusBarStateController.class).addCallback(this); + mClickNotifier = clickNotifier; mLockPatternUtils = new LockPatternUtils(context); mKeyguardManager = context.getSystemService(KeyguardManager.class); } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationRemoteInputManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationRemoteInputManager.java index 1440803f1524e..3898ef754ed38 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationRemoteInputManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationRemoteInputManager.java @@ -121,6 +121,7 @@ public class NotificationRemoteInputManager implements Dumpable { protected final Context mContext; private final UserManager mUserManager; private final KeyguardManager mKeyguardManager; + private final NotificationClickNotifier mClickNotifier; protected RemoteInputController mRemoteInputController; protected NotificationLifetimeExtender.NotificationSafeToRemoveCallback @@ -203,11 +204,7 @@ public class NotificationRemoteInputManager implements Dumpable { mEntryManager.getNotificationData().get(key)); final NotificationVisibility nv = NotificationVisibility.obtain(key, rank, count, true, location); - try { - mBarService.onNotificationActionClick(key, buttonIndex, action, nv, false); - } catch (RemoteException e) { - // Ignore - } + mClickNotifier.onNotificationActionClick(key, buttonIndex, action, nv, false); } private StatusBarNotification getNotificationForParent(ViewParent parent) { @@ -259,7 +256,8 @@ public class NotificationRemoteInputManager implements Dumpable { SmartReplyController smartReplyController, NotificationEntryManager notificationEntryManager, Lazy shadeController, - @Named(MAIN_HANDLER_NAME) Handler mainHandler) { + @Named(MAIN_HANDLER_NAME) Handler mainHandler, + NotificationClickNotifier clickNotifier) { mContext = context; mLockscreenUserManager = lockscreenUserManager; mSmartReplyController = smartReplyController; @@ -271,6 +269,7 @@ public class NotificationRemoteInputManager implements Dumpable { mUserManager = (UserManager) mContext.getSystemService(Context.USER_SERVICE); addLifetimeExtenders(); mKeyguardManager = context.getSystemService(KeyguardManager.class); + mClickNotifier = clickNotifier; notificationEntryManager.addNotificationEntryListener(new NotificationEntryListener() { @Override diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/SmartReplyController.java b/packages/SystemUI/src/com/android/systemui/statusbar/SmartReplyController.java index 736b9ebea5c36..2a1f864dfd671 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/SmartReplyController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/SmartReplyController.java @@ -38,14 +38,17 @@ import javax.inject.Singleton; public class SmartReplyController { private final IStatusBarService mBarService; private final NotificationEntryManager mEntryManager; + private final NotificationClickNotifier mClickNotifier; private Set mSendingKeys = new ArraySet<>(); private Callback mCallback; @Inject public SmartReplyController(NotificationEntryManager entryManager, - IStatusBarService statusBarService) { + IStatusBarService statusBarService, + NotificationClickNotifier clickNotifier) { mBarService = statusBarService; mEntryManager = entryManager; + mClickNotifier = clickNotifier; } public void setCallback(Callback callback) { @@ -79,12 +82,8 @@ public class SmartReplyController { NotificationLogger.getNotificationLocation(entry); final NotificationVisibility nv = NotificationVisibility.obtain( entry.key, rank, count, true, location); - try { - mBarService.onNotificationActionClick( - entry.key, actionIndex, action, nv, generatedByAssistant); - } catch (RemoteException e) { - // Nothing to do, system going down - } + mClickNotifier.onNotificationActionClick( + entry.key, actionIndex, action, nv, generatedByAssistant); } /** diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryListener.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryListener.java index 1aa6bc9ae5f99..848f1a0d8fdd5 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryListener.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryListener.java @@ -20,6 +20,8 @@ import android.service.notification.NotificationListenerService; import android.service.notification.NotificationListenerService.RankingMap; import android.service.notification.StatusBarNotification; +import androidx.annotation.NonNull; + import com.android.internal.statusbar.NotificationVisibility; import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.statusbar.notification.row.NotificationContentInflater.InflationFlag; @@ -96,7 +98,7 @@ public interface NotificationEntryListener { * @param removedByUser true if the notification was removed by a user action */ default void onEntryRemoved( - NotificationEntry entry, + @NonNull NotificationEntry entry, @Nullable NotificationVisibility visibility, boolean removedByUser) { } 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 cfc1a5f2ef3d1..e50c02b666046 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java @@ -21,6 +21,7 @@ import static android.service.notification.NotificationListenerService.REASON_ER import android.annotation.Nullable; import android.app.Notification; import android.content.Context; +import android.os.SystemClock; import android.service.notification.NotificationListenerService; import android.service.notification.StatusBarNotification; import android.util.ArrayMap; @@ -385,7 +386,10 @@ public class NotificationEntryManager implements NotificationListenerService.Ranking ranking = new NotificationListenerService.Ranking(); rankingMap.getRanking(key, ranking); - NotificationEntry entry = new NotificationEntry(notification, ranking); + NotificationEntry entry = new NotificationEntry( + notification, + ranking, + SystemClock.uptimeMillis()); Dependency.get(LeakDetector.class).trackInstance(entry); // Construct the expanded view. diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java index d157f06c03e96..10e37591eebef 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java @@ -28,6 +28,7 @@ import static android.app.NotificationManager.Policy.SUPPRESSED_EFFECT_NOTIFICAT import static android.app.NotificationManager.Policy.SUPPRESSED_EFFECT_PEEK; import static android.app.NotificationManager.Policy.SUPPRESSED_EFFECT_STATUS_BAR; +import android.annotation.CurrentTimeMillisLong; import android.annotation.NonNull; import android.app.Notification; import android.app.NotificationChannel; @@ -96,6 +97,7 @@ public final class NotificationEntry { public StatusBarIconView icon; public StatusBarIconView expandedIcon; public StatusBarIconView centeredIcon; + private long mCreationTime; private boolean interruption; public boolean autoRedacted; // whether the redacted notification was generated by us public int targetSdk; @@ -148,6 +150,46 @@ public final class NotificationEntry { */ private boolean hasSentReply; + + /** + * @param sbn the StatusBarNotification from system server + * @param creationTime SystemClock.uptimeMillis of when we were created + */ + public NotificationEntry( + @NonNull StatusBarNotification sbn, + long creationTime) { + this(sbn, null, creationTime); + } + + public NotificationEntry( + @NonNull StatusBarNotification sbn, + @Nullable NotificationListenerService.Ranking ranking, + long creationTime + ) { + + mCreationTime = creationTime; + this.key = sbn.getKey(); + this.notification = sbn; + + if (ranking != null) { + populateFromRanking(ranking); + } + } + + /** + * This method exists _only_ for tests that don't know how to pass in a creation time, and + * before a NotificationEntry builder was introduced for testing. + * + * It will always set SystemClock.uptimeMillis() as the creation time + * + * @param sbn the StatusBarNotification from system server + * + * @VisibleForTesting + */ + public NotificationEntry(@NonNull StatusBarNotification sbn) { + this(sbn, null, SystemClock.uptimeMillis()); + } + /** * Whether this notification has been approved globally, at the app level, and at the channel * level for bubbling. @@ -167,6 +209,21 @@ public final class NotificationEntry { */ private boolean mUserDismissedBubble; + /** + * A timestamp of SystemClock.uptimeMillis() of when this entry was first created, regardless + * of any changes to the data presented. It is set once on creation and will never change, and + * allows us to know exactly how long this notification has been alive for in our listener + * service. It is entirely unrelated to the information inside of the notification. + * + * This is different to Notification#when because it persists throughout updates, whereas + * system server treats every single call to notify() as a new notification and we handle + * updates to NotificationEntry locally. + */ + @CurrentTimeMillisLong + public long getCreationTime() { + return mCreationTime; + } + /** * Whether this notification is shown to the user as a high priority notification: visible on * the lock screen/status bar and in the top section in the shade. @@ -175,20 +232,6 @@ public final class NotificationEntry { private boolean mIsTopBucket; - public NotificationEntry(StatusBarNotification n) { - this(n, null); - } - - public NotificationEntry( - StatusBarNotification n, - @Nullable NotificationListenerService.Ranking ranking) { - this.key = n.getKey(); - this.notification = n; - if (ranking != null) { - populateFromRanking(ranking); - } - } - public void populateFromRanking(@NonNull NotificationListenerService.Ranking ranking) { channel = ranking.getChannel(); lastAudiblyAlertedMs = ranking.getLastAudiblyAlertedMillis(); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBar.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBar.java index 2c305dff3246a..9d0bb707832d8 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBar.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBar.java @@ -181,6 +181,7 @@ import com.android.systemui.statusbar.GestureRecorder; import com.android.systemui.statusbar.KeyboardShortcuts; import com.android.systemui.statusbar.KeyguardIndicationController; import com.android.systemui.statusbar.NavigationBarController; +import com.android.systemui.statusbar.NotificationClickNotifier; import com.android.systemui.statusbar.NotificationListener; import com.android.systemui.statusbar.NotificationLockscreenUserManager; import com.android.systemui.statusbar.NotificationMediaManager; @@ -595,6 +596,7 @@ public class StatusBar extends SystemUI implements DemoMode, }; private ActivityIntentHelper mActivityIntentHelper; private ShadeController mShadeController; + private NotificationClickNotifier mClickNotifier; @Override public void onActiveStateChanged(int code, int uid, String packageName, boolean active) { @@ -612,6 +614,7 @@ public class StatusBar extends SystemUI implements DemoMode, @Override public void start() { + mClickNotifier = Dependency.get(NotificationClickNotifier.class); mGroupManager = Dependency.get(NotificationGroupManager.class); mGroupAlertTransferHelper = Dependency.get(NotificationGroupAlertTransferHelper.class); mVisualStabilityManager = Dependency.get(VisualStabilityManager.class); @@ -1073,7 +1076,7 @@ public class StatusBar extends SystemUI implements DemoMode, mNotificationActivityStarter = new StatusBarNotificationActivityStarter(mContext, mCommandQueue, mAssistManager, mNotificationPanel, mPresenter, mEntryManager, mHeadsUpManager, activityStarter, mActivityLaunchAnimator, - mBarService, mStatusBarStateController, mKeyguardManager, mDreamManager, + mClickNotifier, mStatusBarStateController, mKeyguardManager, mDreamManager, mRemoteInputManager, mStatusBarRemoteInputCallback, mGroupManager, mLockscreenUserManager, mShadeController, mKeyguardMonitor, mNotificationInterruptionStateProvider, mMetricsLogger, diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarter.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarter.java index e00d439dc1c70..8609c4fc60701 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarter.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarter.java @@ -42,7 +42,6 @@ import android.util.Log; import android.view.RemoteAnimationAdapter; import com.android.internal.logging.MetricsLogger; -import com.android.internal.statusbar.IStatusBarService; import com.android.internal.statusbar.NotificationVisibility; import com.android.internal.widget.LockPatternUtils; import com.android.systemui.ActivityIntentHelper; @@ -54,6 +53,7 @@ import com.android.systemui.bubbles.BubbleController; import com.android.systemui.plugins.ActivityStarter; import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.statusbar.CommandQueue; +import com.android.systemui.statusbar.NotificationClickNotifier; import com.android.systemui.statusbar.NotificationLockscreenUserManager; import com.android.systemui.statusbar.NotificationPresenter; import com.android.systemui.statusbar.NotificationRemoteInputManager; @@ -97,7 +97,7 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit private final HeadsUpManagerPhone mHeadsUpManager; private final KeyguardManager mKeyguardManager; private final ActivityLaunchAnimator mActivityLaunchAnimator; - private final IStatusBarService mBarService; + private final NotificationClickNotifier mClickNotifier; private final CommandQueue mCommandQueue; private final IDreamManager mDreamManager; private final Handler mMainThreadHandler; @@ -116,7 +116,7 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit HeadsUpManagerPhone headsUpManager, ActivityStarter activityStarter, ActivityLaunchAnimator activityLaunchAnimator, - IStatusBarService statusBarService, + NotificationClickNotifier clickNotifier, StatusBarStateController statusBarStateController, KeyguardManager keyguardManager, IDreamManager dreamManager, @@ -138,7 +138,7 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit mPresenter = presenter; mHeadsUpManager = headsUpManager; mActivityLaunchAnimator = activityLaunchAnimator; - mBarService = statusBarService; + mClickNotifier = clickNotifier; mCommandQueue = commandQueue; mKeyguardManager = keyguardManager; mDreamManager = dreamManager; @@ -334,11 +334,8 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit mEntryManager.getNotificationData().get(notificationKey)); final NotificationVisibility nv = NotificationVisibility.obtain(notificationKey, rank, count, true, location); - try { - mBarService.onNotificationClick(notificationKey, nv); - } catch (RemoteException ex) { - // system process is dead if we're here. - } + mClickNotifier.onNotificationClick(notificationKey, nv); + if (!isBubble) { if (parentToCancelFinal != null) { removeNotification(parentToCancelFinal); diff --git a/packages/SystemUI/src/com/android/systemui/util/concurrency/DelayableExecutor.java b/packages/SystemUI/src/com/android/systemui/util/concurrency/DelayableExecutor.java new file mode 100644 index 0000000000000..c594621552e38 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/util/concurrency/DelayableExecutor.java @@ -0,0 +1,66 @@ +/* + * 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.util.concurrency; + +import java.util.concurrent.Executor; +import java.util.concurrent.TimeUnit; + +/** + * A sub-class of {@link Executor} that allows Runnables to be delayed and/or cancelled. + */ +public interface DelayableExecutor extends Executor { + /** + * Execute supplied Runnable on the Executors thread after a specified delay. + * + * See {@link android.os.Handler#postDelayed(Runnable, long)}. + * + * @return A Runnable that, when run, removes the supplied argument from the Executor queue. + */ + default Runnable executeDelayed(Runnable r, long delayMillis) { + return executeDelayed(r, delayMillis, TimeUnit.MILLISECONDS); + } + + /** + * Execute supplied Runnable on the Executors thread after a specified delay. + * + * See {@link android.os.Handler#postDelayed(Runnable, long)}. + * + * @return A Runnable that, when run, removes the supplied argument from the Executor queue.. + */ + Runnable executeDelayed(Runnable r, long delay, TimeUnit unit); + + /** + * Execute supplied Runnable on the Executors thread at a specified uptime. + * + * See {@link android.os.Handler#postAtTime(Runnable, long)}. + * + * @return A Runnable that, when run, removes the supplied argument from the Executor queue. + */ + default Runnable executeAtTime(Runnable r, long uptime) { + return executeAtTime(r, uptime, TimeUnit.MILLISECONDS); + } + + /** + * Execute supplied Runnable on the Executors thread at a specified uptime. + * + * See {@link android.os.Handler#postAtTime(Runnable, long)}. + * + * @return A Runnable that, when run, removes the supplied argument from the Executor queue. + */ + Runnable executeAtTime(Runnable r, long uptimeMillis, TimeUnit unit); +} + diff --git a/packages/SystemUI/src/com/android/systemui/util/time/SystemClock.java b/packages/SystemUI/src/com/android/systemui/util/time/SystemClock.java new file mode 100644 index 0000000000000..26c77b00cae85 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/util/time/SystemClock.java @@ -0,0 +1,44 @@ +/* + * 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.util.time; + +/** + * Testable wrapper around {@link android.os.SystemClock}. + * + * Dagger can inject this wrapper into your classes. The implementation just proxies calls to the + * real SystemClock. + * + * In tests, pass an instance of FakeSystemClock, which allows you to control the values returned by + * the various getters below. + */ +public interface SystemClock { + /** @see android.os.SystemClock#uptimeMillis() */ + long uptimeMillis(); + + /** @see android.os.SystemClock#elapsedRealtime() */ + long elapsedRealtime(); + + /** @see android.os.SystemClock#elapsedRealtimeNanos() */ + long elapsedRealtimeNanos(); + + /** @see android.os.SystemClock#currentThreadTimeMillis() */ + long currentThreadTimeMillis(); + + /** @see System#currentTimeMillis() */ + long currentTimeMillis(); +} + diff --git a/packages/SystemUI/src/com/android/systemui/util/time/SystemClockImpl.java b/packages/SystemUI/src/com/android/systemui/util/time/SystemClockImpl.java new file mode 100644 index 0000000000000..501da990d0efc --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/util/time/SystemClockImpl.java @@ -0,0 +1,51 @@ +/* + * 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.util.time; + +import javax.inject.Inject; + +/** Default implementation of {@link SystemClock}. */ +public class SystemClockImpl implements SystemClock { + @Inject + public SystemClockImpl() {} + + @Override + public long uptimeMillis() { + return android.os.SystemClock.uptimeMillis(); + } + + @Override + public long elapsedRealtime() { + return android.os.SystemClock.elapsedRealtime(); + } + + @Override + public long elapsedRealtimeNanos() { + return android.os.SystemClock.elapsedRealtimeNanos(); + } + + @Override + public long currentThreadTimeMillis() { + return android.os.SystemClock.currentThreadTimeMillis(); + } + + @Override + public long currentTimeMillis() { + return System.currentTimeMillis(); + } +} + diff --git a/packages/SystemUI/tests/src/com/android/systemui/ForegroundServiceControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/ForegroundServiceControllerTest.java index 8a6ee12d70689..c1f94a5cefd33 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/ForegroundServiceControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/ForegroundServiceControllerTest.java @@ -41,6 +41,7 @@ import com.android.systemui.statusbar.notification.NotificationEntryListener; import com.android.systemui.statusbar.notification.NotificationEntryManager; import com.android.systemui.statusbar.notification.collection.NotificationEntry; + import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -61,7 +62,8 @@ public class ForegroundServiceControllerTest extends SysuiTestCase { mFsc = new ForegroundServiceController(); NotificationEntryManager notificationEntryManager = mock(NotificationEntryManager.class); mListener = new ForegroundServiceNotificationListener( - mContext, mFsc, notificationEntryManager); + mContext, mFsc, notificationEntryManager, + mock(ForegroundServiceLifetimeExtender.class)); ArgumentCaptor entryListenerCaptor = ArgumentCaptor.forClass(NotificationEntryListener.class); verify(notificationEntryManager).addNotificationEntryListener( diff --git a/packages/SystemUI/tests/src/com/android/systemui/ForegroundServiceLifetimeExtenderTest.java b/packages/SystemUI/tests/src/com/android/systemui/ForegroundServiceNotificationListenerTest.java similarity index 62% rename from packages/SystemUI/tests/src/com/android/systemui/ForegroundServiceLifetimeExtenderTest.java rename to packages/SystemUI/tests/src/com/android/systemui/ForegroundServiceNotificationListenerTest.java index b1dabdda2241b..6dbe1b34c1128 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/ForegroundServiceLifetimeExtenderTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/ForegroundServiceNotificationListenerTest.java @@ -20,63 +20,84 @@ import static com.android.systemui.ForegroundServiceLifetimeExtender.MIN_FGS_TIM 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.ActivityManager; import android.app.Notification; +import android.os.UserHandle; import android.service.notification.StatusBarNotification; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; +import com.android.systemui.statusbar.NotificationInteractionTracker; import com.android.systemui.statusbar.notification.collection.NotificationEntry; +import com.android.systemui.util.time.FakeSystemClock; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; @RunWith(AndroidJUnit4.class) @SmallTest -public class ForegroundServiceLifetimeExtenderTest extends SysuiTestCase { - private ForegroundServiceLifetimeExtender mExtender = new ForegroundServiceLifetimeExtender(); - private StatusBarNotification mSbn; +public class ForegroundServiceNotificationListenerTest extends SysuiTestCase { + private static final String TEST_PACKAGE_NAME = "test"; + private static final int TEST_UID = 0; + + private ForegroundServiceLifetimeExtender mExtender; private NotificationEntry mEntry; + private StatusBarNotification mSbn; private Notification mNotif; + private final FakeSystemClock mClock = new FakeSystemClock(); + + @Mock + private NotificationInteractionTracker mInteractionTracker; @Before public void setup() { + MockitoAnnotations.initMocks(this); + mExtender = new ForegroundServiceLifetimeExtender(mInteractionTracker, mClock); + 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); + mSbn = new StatusBarNotification(TEST_PACKAGE_NAME, TEST_PACKAGE_NAME, 0, null, TEST_UID, + 0, mNotif, new UserHandle(ActivityManager.getCurrentUser()), null, 0); + mEntry = new NotificationEntry(mSbn, mClock.uptimeMillis()); } + /** + * 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()); + + // No time has elapsed, keep showing 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); + + // Entry was created at mClock.uptimeMillis(), advance it MIN_FGS_TIME_MS + 1 + mClock.advanceTime(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); + + // Entry was created at mClock.uptimeMillis(), advance it MIN_FGS_TIME_MS + 1 + mClock.advanceTime(MIN_FGS_TIME_MS + 1); assertFalse(mExtender.shouldExtendLifetime(mEntry)); } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerTest.java index 57dd8c94c7900..c66114b29ee1e 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerTest.java @@ -71,6 +71,7 @@ public class NotificationLockscreenUserManagerTest extends SysuiTestCase { @Mock private NotificationData mNotificationData; @Mock private DeviceProvisionedController mDeviceProvisionedController; @Mock private StatusBarKeyguardViewManager mKeyguardViewManager; + @Mock private NotificationClickNotifier mClickNotifier; private int mCurrentUserId; private TestNotificationLockscreenUserManager mLockscreenUserManager; @@ -185,7 +186,7 @@ public class NotificationLockscreenUserManagerTest extends SysuiTestCase { private class TestNotificationLockscreenUserManager extends NotificationLockscreenUserManagerImpl { public TestNotificationLockscreenUserManager(Context context) { - super(context); + super(context, mClickNotifier); } public BroadcastReceiver getBaseBroadcastReceiverForTest() { diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationRemoteInputManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationRemoteInputManagerTest.java index b81e048214634..fd91e4823a399 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationRemoteInputManagerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationRemoteInputManagerTest.java @@ -54,6 +54,7 @@ public class NotificationRemoteInputManagerTest extends SysuiTestCase { @Mock private SmartReplyController mSmartReplyController; @Mock private NotificationListenerService.RankingMap mRanking; @Mock private ExpandableNotificationRow mRow; + @Mock private NotificationClickNotifier mClickNotifier; // Dependency mocks: @Mock private NotificationEntryManager mEntryManager; @@ -73,7 +74,8 @@ public class NotificationRemoteInputManagerTest extends SysuiTestCase { mRemoteInputManager = new TestableNotificationRemoteInputManager(mContext, mLockscreenUserManager, mSmartReplyController, mEntryManager, () -> mock(ShadeController.class), - Handler.createAsync(Looper.myLooper())); + Handler.createAsync(Looper.myLooper()), + mClickNotifier); mSbn = new StatusBarNotification(TEST_PACKAGE_NAME, TEST_PACKAGE_NAME, 0, null, TEST_UID, 0, new Notification(), UserHandle.CURRENT, null, 0); mEntry = new NotificationEntry(mSbn); @@ -202,9 +204,10 @@ public class NotificationRemoteInputManagerTest extends SysuiTestCase { SmartReplyController smartReplyController, NotificationEntryManager notificationEntryManager, Lazy shadeController, - Handler mainHandler) { + Handler mainHandler, + NotificationClickNotifier clickNotifier) { super(context, lockscreenUserManager, smartReplyController, notificationEntryManager, - shadeController, mainHandler); + shadeController, mainHandler, clickNotifier); } public void setUpWithPresenterForTest(Callback callback, diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/SmartReplyControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/SmartReplyControllerTest.java index 81e373a8be272..8f1b6d3bb7f54 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/SmartReplyControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/SmartReplyControllerTest.java @@ -70,6 +70,7 @@ public class SmartReplyControllerTest extends SysuiTestCase { @Mock private StatusBarNotification mSbn; @Mock private NotificationEntryManager mNotificationEntryManager; @Mock private IStatusBarService mIStatusBarService; + @Mock private NotificationClickNotifier mClickNotifier; @Before public void setUp() { @@ -78,14 +79,15 @@ public class SmartReplyControllerTest extends SysuiTestCase { mNotificationEntryManager); mSmartReplyController = new SmartReplyController(mNotificationEntryManager, - mIStatusBarService); + mIStatusBarService, mClickNotifier); mDependency.injectTestDependency(SmartReplyController.class, mSmartReplyController); mRemoteInputManager = new NotificationRemoteInputManager(mContext, mock(NotificationLockscreenUserManager.class), mSmartReplyController, mNotificationEntryManager, () -> mock(ShadeController.class), - Handler.createAsync(Looper.myLooper())); + Handler.createAsync(Looper.myLooper()), + mClickNotifier); mRemoteInputManager.setUpWithCallback(mCallback, mDelegate); mNotification = new Notification.Builder(mContext, "") .setSmallIcon(R.drawable.ic_person) diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationDataTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationDataTest.java index f629757e4c683..76713e4015b79 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationDataTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationDataTest.java @@ -54,6 +54,7 @@ import android.graphics.drawable.Icon; import android.media.session.MediaSession; import android.os.Bundle; import android.os.Process; +import android.os.SystemClock; import android.service.notification.NotificationListenerService; import android.service.notification.NotificationListenerService.Ranking; import android.service.notification.SnoozeCriterion; @@ -339,7 +340,10 @@ public class NotificationDataTest extends SysuiTestCase { when(ranking.getSnoozeCriteria()).thenReturn(snoozeCriterions); NotificationEntry entry = - new NotificationEntry(mMockStatusBarNotification, ranking); + new NotificationEntry( + mMockStatusBarNotification, + ranking, + SystemClock.uptimeMillis()); assertEquals(systemGeneratedSmartActions, entry.systemGeneratedSmartActions); assertEquals(NOTIFICATION_CHANNEL, entry.channel); diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationEntryTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationEntryTest.java index cca9f2834e93e..258ddf3c98a50 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationEntryTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationEntryTest.java @@ -30,6 +30,7 @@ import android.testing.TestableLooper; import androidx.test.filters.SmallTest; import com.android.systemui.SysuiTestCase; +import com.android.systemui.util.time.FakeSystemClock; import org.junit.Before; import org.junit.Test; @@ -48,6 +49,7 @@ public class NotificationEntryTest extends SysuiTestCase { private NotificationEntry mEntry; private Bundle mExtras; + private final FakeSystemClock mClock = new FakeSystemClock(); @Before public void setUp() { diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarterTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarterTest.java index 06d76ebcff285..8ec6a65d4c106 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarterTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarterTest.java @@ -48,7 +48,6 @@ import android.testing.TestableLooper; import androidx.test.filters.SmallTest; import com.android.internal.logging.MetricsLogger; -import com.android.internal.statusbar.IStatusBarService; import com.android.internal.statusbar.NotificationVisibility; import com.android.internal.widget.LockPatternUtils; import com.android.systemui.ActivityIntentHelper; @@ -58,6 +57,7 @@ import com.android.systemui.bubbles.BubbleController; import com.android.systemui.plugins.ActivityStarter; import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.statusbar.CommandQueue; +import com.android.systemui.statusbar.NotificationClickNotifier; import com.android.systemui.statusbar.NotificationLockscreenUserManager; import com.android.systemui.statusbar.NotificationPresenter; import com.android.systemui.statusbar.NotificationRemoteInputManager; @@ -94,7 +94,7 @@ public class StatusBarNotificationActivityStarterTest extends SysuiTestCase { @Mock private ActivityStarter mActivityStarter; @Mock - private IStatusBarService mStatusBarService; + private NotificationClickNotifier mClickNotifier; @Mock private StatusBarStateController mStatusBarStateController; @Mock @@ -165,7 +165,8 @@ public class StatusBarNotificationActivityStarterTest extends SysuiTestCase { mNotificationActivityStarter = new StatusBarNotificationActivityStarter(getContext(), mock(CommandQueue.class), mAssistManager, mock(NotificationPanelView.class), mock(NotificationPresenter.class), mEntryManager, mock(HeadsUpManagerPhone.class), - mActivityStarter, mock(ActivityLaunchAnimator.class), mStatusBarService, + mActivityStarter, mock(ActivityLaunchAnimator.class), + mClickNotifier, mock(StatusBarStateController.class), mock(KeyguardManager.class), mock(IDreamManager.class), mRemoteInputManager, mock(StatusBarRemoteInputCallback.class), mock(NotificationGroupManager.class), @@ -222,7 +223,7 @@ public class StatusBarNotificationActivityStarterTest extends SysuiTestCase { verify(mAssistManager).hideAssist(); - verify(mStatusBarService).onNotificationClick( + verify(mClickNotifier).onNotificationClick( eq(sbn.getKey()), any(NotificationVisibility.class)); // Notification is removed due to FLAG_AUTO_CANCEL @@ -248,7 +249,7 @@ public class StatusBarNotificationActivityStarterTest extends SysuiTestCase { verify(mAssistManager).hideAssist(); - verify(mStatusBarService).onNotificationClick( + verify(mClickNotifier).onNotificationClick( eq(sbn.getKey()), any(NotificationVisibility.class)); // The content intent should NOT be sent on click. @@ -278,7 +279,7 @@ public class StatusBarNotificationActivityStarterTest extends SysuiTestCase { verify(mAssistManager).hideAssist(); - verify(mStatusBarService).onNotificationClick( + verify(mClickNotifier).onNotificationClick( eq(sbn.getKey()), any(NotificationVisibility.class)); // The content intent should NOT be sent on click. @@ -308,7 +309,7 @@ public class StatusBarNotificationActivityStarterTest extends SysuiTestCase { verify(mAssistManager).hideAssist(); - verify(mStatusBarService).onNotificationClick( + verify(mClickNotifier).onNotificationClick( eq(sbn.getKey()), any(NotificationVisibility.class)); // The content intent should NOT be sent on click. diff --git a/packages/SystemUI/tests/src/com/android/systemui/util/concurrency/FakeExecutor.java b/packages/SystemUI/tests/src/com/android/systemui/util/concurrency/FakeExecutor.java new file mode 100644 index 0000000000000..42e5a5eab6bf2 --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/util/concurrency/FakeExecutor.java @@ -0,0 +1,217 @@ +/* + * 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.util.concurrency; + +import com.android.systemui.util.time.FakeSystemClock; + +import java.util.Collections; +import java.util.PriorityQueue; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +public class FakeExecutor implements DelayableExecutor { + private final FakeSystemClock mClock; + private PriorityQueue mQueuedRunnables = new PriorityQueue<>(); + private boolean mIgnoreClockUpdates; + + /** + * Initializes a fake executor. + * + * @param clock FakeSystemClock allowing control over delayed runnables. It is strongly + * recommended that this clock have its auto-increment setting set to false to + * prevent unexpected advancement of the time. + */ + public FakeExecutor(FakeSystemClock clock) { + mClock = clock; + mClock.addListener(() -> { + if (!mIgnoreClockUpdates) { + runAllReady(); + } + }); + } + + /** + * Runs a single runnable if it's scheduled to run according to the internal clock. + * + * If constructed to advance the clock automatically, this will advance the clock enough to + * run the next pending item. + * + * This method does not advance the clock past the item that was run. + * + * @return Returns true if an item was run. + */ + public boolean runNextReady() { + if (!mQueuedRunnables.isEmpty() && mQueuedRunnables.peek().mWhen <= mClock.uptimeMillis()) { + mQueuedRunnables.poll().mRunnable.run(); + return true; + } + + return false; + } + + /** + * Runs all Runnables that are scheduled to run according to the internal clock. + * + * If constructed to advance the clock automatically, this will advance the clock enough to + * run all the pending items. This method does not advance the clock past items that were + * run. It is equivalent to calling {@link #runNextReady()} in a loop. + * + * @return Returns the number of items that ran. + */ + public int runAllReady() { + int num = 0; + while (runNextReady()) { + num++; + } + + return num; + } + + /** + * Advances the internal clock to the next item to run. + * + * The clock will only move forward. If the next item is set to run in the past or there is no + * next item, the clock does not change. + * + * Note that this will cause one or more items to actually run. + * + * @return The delta in uptimeMillis that the clock advanced, or 0 if the clock did not advance. + */ + public long advanceClockToNext() { + if (mQueuedRunnables.isEmpty()) { + return 0; + } + + long startTime = mClock.uptimeMillis(); + long nextTime = mQueuedRunnables.peek().mWhen; + if (nextTime <= startTime) { + return 0; + } + updateClock(nextTime); + + return nextTime - startTime; + } + + + /** + * Advances the internal clock to the last item to run. + * + * The clock will only move forward. If the last item is set to run in the past or there is no + * next item, the clock does not change. + * + * @return The delta in uptimeMillis that the clock advanced, or 0 if the clock did not advance. + */ + public long advanceClockToLast() { + if (mQueuedRunnables.isEmpty()) { + return 0; + } + + long startTime = mClock.uptimeMillis(); + long nextTime = Collections.max(mQueuedRunnables).mWhen; + if (nextTime <= startTime) { + return 0; + } + + updateClock(nextTime); + + return nextTime - startTime; + } + + /** + * Returns the number of un-executed runnables waiting to run. + */ + public int numPending() { + return mQueuedRunnables.size(); + } + + @Override + public Runnable executeDelayed(Runnable r, long delay, TimeUnit unit) { + if (delay < 0) { + delay = 0; + } + return executeAtTime(r, mClock.uptimeMillis() + unit.toMillis(delay)); + } + + @Override + public Runnable executeAtTime(Runnable r, long uptime, TimeUnit unit) { + long uptimeMillis = unit.toMillis(uptime); + + QueuedRunnable container = new QueuedRunnable(r, uptimeMillis); + + mQueuedRunnables.offer(container); + + return () -> mQueuedRunnables.remove(container); + } + + @Override + public void execute(Runnable command) { + executeDelayed(command, 0); + } + + /** + * Run all Executors in a loop until they all report they have no ready work to do. + * + * Useful if you have Executors the post work to other Executors, and you simply want to + * run them all until they stop posting work. + */ + public static void exhaustExecutors(FakeExecutor ...executors) { + boolean didAnything; + do { + didAnything = false; + for (FakeExecutor executor : executors) { + didAnything = didAnything || executor.runAllReady() != 0; + } + } while (didAnything); + } + + private void updateClock(long nextTime) { + mIgnoreClockUpdates = true; + mClock.setUptimeMillis(nextTime); + mIgnoreClockUpdates = false; + } + + private static class QueuedRunnable implements Comparable { + private static AtomicInteger sCounter = new AtomicInteger(); + + Runnable mRunnable; + long mWhen; + private int mCounter; + + private QueuedRunnable(Runnable r, long when) { + mRunnable = r; + mWhen = when; + + // PrioirityQueue orders items arbitrarily when equal. We want to ensure that + // otherwise-equal elements are ordered according to their insertion order. Because this + // class only is constructed right before insertion, we use a static counter to track + // insertion order of otherwise equal elements. + mCounter = sCounter.incrementAndGet(); + } + + @Override + public int compareTo(QueuedRunnable other) { + long diff = mWhen - other.mWhen; + + if (diff == 0) { + return mCounter - other.mCounter; + } + + return diff > 0 ? 1 : -1; + } + } +} + diff --git a/packages/SystemUI/tests/src/com/android/systemui/util/time/FakeSystemClock.java b/packages/SystemUI/tests/src/com/android/systemui/util/time/FakeSystemClock.java new file mode 100644 index 0000000000000..181636f4e7c81 --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/util/time/FakeSystemClock.java @@ -0,0 +1,125 @@ +/* + * 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.util.time; + +import com.android.systemui.util.concurrency.FakeExecutor; + +import java.util.ArrayList; +import java.util.List; + +/** + * A fake {@link SystemClock} for use with {@link FakeExecutor}. + * + * Attempts to simulate the behavior of a real system clock. Time can be moved forward but not + * backwards. uptimeMillis, elapsedRealtime, and currentThreadTimeMillis are all kept in sync. + * + * Unless otherwise specified, uptimeMillis and elapsedRealtime will advance the same amount with + * every call to {@link #advanceTime}. Thread time always lags by 50% of the uptime + * advancement to simulate time loss due to scheduling. + */ +public class FakeSystemClock implements SystemClock { + private long mUptimeMillis = 10000; + private long mElapsedRealtime = 10000; + private long mCurrentThreadTimeMillis = 10000; + private long mCurrentTimeMillis = 1555555500000L; + private final List mListeners = new ArrayList<>(); + + @Override + public long uptimeMillis() { + return mUptimeMillis; + } + + @Override + public long elapsedRealtime() { + return mElapsedRealtime; + } + + @Override + public long elapsedRealtimeNanos() { + return mElapsedRealtime * 1000000 + 447; + } + + @Override + public long currentThreadTimeMillis() { + return mCurrentThreadTimeMillis; + } + + @Override + public long currentTimeMillis() { + return mCurrentTimeMillis; + } + + public void setUptimeMillis(long uptime) { + advanceTime(uptime - mUptimeMillis); + } + + public void setCurrentTimeMillis(long millis) { + mCurrentTimeMillis = millis; + } + + /** + * Advances the time tracked by the fake clock and notifies any listeners that the time has + * changed (for example, an attached {@link FakeExecutor} may fire its pending runnables). + * + * All tracked times increment by [millis], with the exception of currentThreadTimeMillis, + * which advances by [millis] * 0.5 + */ + public void advanceTime(long millis) { + advanceTime(millis, 0); + } + + /** + * Advances the time tracked by the fake clock and notifies any listeners that the time has + * changed (for example, an attached {@link FakeExecutor} may fire its pending runnables). + * + * The tracked times change as follows: + * - uptimeMillis increments by [awakeMillis] + * - currentThreadTimeMillis increments by [awakeMillis] * 0.5 + * - elapsedRealtime increments by [awakeMillis] + [sleepMillis] + * - currentTimeMillis increments by [awakeMillis] + [sleepMillis] + */ + public void advanceTime(long awakeMillis, long sleepMillis) { + if (awakeMillis < 0 || sleepMillis < 0) { + throw new IllegalArgumentException("Time cannot go backwards"); + } + + if (awakeMillis > 0 || sleepMillis > 0) { + mUptimeMillis += awakeMillis; + mElapsedRealtime += awakeMillis + sleepMillis; + mCurrentTimeMillis += awakeMillis + sleepMillis; + + mCurrentThreadTimeMillis += Math.ceil(awakeMillis * 0.5); + + for (ClockTickListener listener : mListeners) { + listener.onClockTick(); + } + } + } + + public void addListener(ClockTickListener listener) { + mListeners.add(listener); + } + + public void removeListener(ClockTickListener listener) { + mListeners.remove(listener); + } + + public interface ClockTickListener { + void onClockTick(); + } +} + From 98d6853f1a3aba3a0abfe7080524da7e1b737873 Mon Sep 17 00:00:00 2001 From: Evan Laird Date: Wed, 27 May 2020 23:46:32 -0400 Subject: [PATCH 2/4] DO NOT MERGE: Allow interrupting notifications to bypass lifetime extension Notifications which have interruped the UI (usually a HUN) can safely bypass FGS lifetime extension because the system has done the best it can to show the user this notification. This valve is important in particular for things like a dialer which might want to interrupt a user but need to do so again on the same channel, for instance when getting multiple phone calls quickly in succession. Bug: 155594347 Bug: 119041698 Test: atest ForegroundServiceNotificationListenerTest Change-Id: Id80fba3191cc133d1e73ca04015f9cbed62fc086 --- .../systemui/ForegroundServiceLifetimeExtender.java | 6 ++++++ .../ForegroundServiceNotificationListenerTest.java | 12 ++++++++++++ 2 files changed, 18 insertions(+) diff --git a/packages/SystemUI/src/com/android/systemui/ForegroundServiceLifetimeExtender.java b/packages/SystemUI/src/com/android/systemui/ForegroundServiceLifetimeExtender.java index 7db6642276f8c..32620d332498c 100644 --- a/packages/SystemUI/src/com/android/systemui/ForegroundServiceLifetimeExtender.java +++ b/packages/SystemUI/src/com/android/systemui/ForegroundServiceLifetimeExtender.java @@ -66,6 +66,12 @@ public class ForegroundServiceLifetimeExtender implements NotificationLifetimeEx return false; } + // Entry has triggered a HUN or some other interruption, therefore it has been seen and the + // interrupter might be retaining it anyway. + if (entry.hasInterrupted()) { + return false; + } + boolean hasInteracted = mInteractionTracker.hasUserInteractedWith(entry.key); long aliveTime = mSystemClock.uptimeMillis() - entry.getCreationTime(); return aliveTime < MIN_FGS_TIME_MS && !hasInteracted; diff --git a/packages/SystemUI/tests/src/com/android/systemui/ForegroundServiceNotificationListenerTest.java b/packages/SystemUI/tests/src/com/android/systemui/ForegroundServiceNotificationListenerTest.java index 6dbe1b34c1128..1956e99683b18 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/ForegroundServiceNotificationListenerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/ForegroundServiceNotificationListenerTest.java @@ -100,4 +100,16 @@ public class ForegroundServiceNotificationListenerTest extends SysuiTestCase { mClock.advanceTime(MIN_FGS_TIME_MS + 1); assertFalse(mExtender.shouldExtendLifetime(mEntry)); } + + @Test + public void testShouldExtendLifetime_shouldNot_interruped() { + // GIVEN a notification that would trigger lifetime extension + mNotif.flags |= Notification.FLAG_FOREGROUND_SERVICE; + + // GIVEN the notification has alerted + mEntry.setInterruption(); + + // THEN the notification does not need to have its lifetime extended by this extender + assertFalse(mExtender.shouldExtendLifetime(mEntry)); + } } From 6dc09419945bd898607d33a05dc3aca31f6a2a3b Mon Sep 17 00:00:00 2001 From: Evan Laird Date: Tue, 29 Sep 2020 01:09:12 -0400 Subject: [PATCH 3/4] DO NOT MERGE: Associate notif cancels with notif posts CancelNotificationRunnables just spin on the work handler of NotificationManagerService, hoping that they get executed at the correct moment after a PostNotificationRunnable and before the next EnqueueNotificationRunnable completes. Otherwise, you end up in a bad state where the cancel either is canceling notifications before they get a chance to post, or missing its only chance to cancel the notification (for instance, ActivityManagerService is the only caller that can cancel FGS notifications). This change attempts to execute a CancelNotificationRunnable at the moment its run() method is called, otherwise it associates the runnable with the latest enqueued notificaiton record which has yet to post. It then associates PostNotificationRunnable with the delayed cancel list, executing any missed cancel operations immediately upon finishing the PostNotificationRunnable. Test: atest SystemUITests NotificationManagerServiceTest; manual Bug: 162652224 Bug: 119041698 Change-Id: I88d3c5f4fd910a83974c2f84ae3e8a9498d18133 --- .../notification/InjectableSystemClock.java | 44 +++ .../InjectableSystemClockImpl.java | 51 ++++ .../NotificationManagerService.java | 258 ++++++++++++++---- .../notification/NotificationRecord.java | 4 + .../notification/BuzzBeepBlinkTest.java | 3 +- .../server/notification/FakeSystemClock.java | 111 ++++++++ .../NotificationManagerServiceTest.java | 186 ++++++++++++- .../server/notification/RoleObserverTest.java | 8 +- 8 files changed, 593 insertions(+), 72 deletions(-) create mode 100644 services/core/java/com/android/server/notification/InjectableSystemClock.java create mode 100644 services/core/java/com/android/server/notification/InjectableSystemClockImpl.java create mode 100644 services/tests/uiservicestests/src/com/android/server/notification/FakeSystemClock.java diff --git a/services/core/java/com/android/server/notification/InjectableSystemClock.java b/services/core/java/com/android/server/notification/InjectableSystemClock.java new file mode 100644 index 0000000000000..4d993d19e57bc --- /dev/null +++ b/services/core/java/com/android/server/notification/InjectableSystemClock.java @@ -0,0 +1,44 @@ +/* + * 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.server.notification; + +/** + * Testable wrapper around {@link android.os.SystemClock}. + * + * The default implementation at InjectableSystemClockImpl just proxies calls to the real + * SystemClock + * + * In tests, pass an instance of FakeSystemClock, which allows you to control the values returned by + * the various getters below. + */ +public interface InjectableSystemClock { + /** @see android.os.SystemClock#uptimeMillis() */ + long uptimeMillis(); + + /** @see android.os.SystemClock#elapsedRealtime() */ + long elapsedRealtime(); + + /** @see android.os.SystemClock#elapsedRealtimeNanos() */ + long elapsedRealtimeNanos(); + + /** @see android.os.SystemClock#currentThreadTimeMillis() */ + long currentThreadTimeMillis(); + + /** @see System#currentTimeMillis() */ + long currentTimeMillis(); +} + diff --git a/services/core/java/com/android/server/notification/InjectableSystemClockImpl.java b/services/core/java/com/android/server/notification/InjectableSystemClockImpl.java new file mode 100644 index 0000000000000..43d756f46176a --- /dev/null +++ b/services/core/java/com/android/server/notification/InjectableSystemClockImpl.java @@ -0,0 +1,51 @@ +/* + * 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.server.notification; + +/** + * Default implementation of {@link InjectableSystemClock}. + * + * @hide + */ +public class InjectableSystemClockImpl implements InjectableSystemClock { + public InjectableSystemClockImpl() {} + + @Override + public long uptimeMillis() { + return android.os.SystemClock.uptimeMillis(); + } + + @Override + public long elapsedRealtime() { + return android.os.SystemClock.elapsedRealtime(); + } + + @Override + public long elapsedRealtimeNanos() { + return android.os.SystemClock.elapsedRealtimeNanos(); + } + + @Override + public long currentThreadTimeMillis() { + return android.os.SystemClock.currentThreadTimeMillis(); + } + + @Override + public long currentTimeMillis() { + return System.currentTimeMillis(); + } +} diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index dbd7573f475f8..5ba554ec3d8a1 100644 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -167,7 +167,6 @@ import android.os.RemoteException; import android.os.ResultReceiver; import android.os.ServiceManager; import android.os.ShellCallback; -import android.os.SystemClock; import android.os.SystemProperties; import android.os.UserHandle; import android.os.UserManager; @@ -417,6 +416,11 @@ public class NotificationManagerService extends SystemService { final ArrayMap> mAutobundledSummaries = new ArrayMap<>(); final ArrayList mToastQueue = new ArrayList<>(); final ArrayMap mSummaryByGroupKey = new ArrayMap<>(); + // Keep track of `CancelNotificationRunnable`s which have been delayed due to awaiting + // enqueued notifications to post + @GuardedBy("mNotificationLock") + final ArrayMap> mDelayedCancelations = + new ArrayMap<>(); // The last key in this list owns the hardware. ArrayList mLights = new ArrayList<>(); @@ -465,6 +469,7 @@ public class NotificationManagerService extends SystemService { private MetricsLogger mMetricsLogger; private TriPredicate mAllowedManagedServicePackages; + private final InjectableSystemClock mSystemClock; private static class Archive { final int mBufferSize; @@ -789,7 +794,7 @@ public class NotificationManagerService extends SystemService { Slog.w(TAG, "No notification with key: " + key); return; } - final long now = System.currentTimeMillis(); + final long now = mSystemClock.currentTimeMillis(); MetricsLogger.action(r.getItemLogMaker() .setType(MetricsEvent.TYPE_ACTION) .addTaggedData(MetricsEvent.NOTIFICATION_SHADE_INDEX, nv.rank) @@ -819,7 +824,7 @@ public class NotificationManagerService extends SystemService { Slog.w(TAG, "No notification with key: " + key); return; } - final long now = System.currentTimeMillis(); + final long now = mSystemClock.currentTimeMillis(); MetricsLogger.action(r.getLogMaker(now) .setCategory(MetricsEvent.NOTIFICATION_ITEM_ACTION) .setType(MetricsEvent.TYPE_ACTION) @@ -1441,7 +1446,15 @@ public class NotificationManagerService extends SystemService { } public NotificationManagerService(Context context) { + this(context, new InjectableSystemClockImpl()); + } + + @VisibleForTesting + public NotificationManagerService( + Context context, + InjectableSystemClock systemClock) { super(context); + mSystemClock = systemClock; Notification.processWhitelistToken = WHITELIST_TOKEN; } @@ -1739,6 +1752,11 @@ public class NotificationManagerService extends SystemService { com.android.internal.R.array.config_priorityOnlyDndExemptPackages)); } + @VisibleForTesting + protected Handler getWorkHandler() { + return mHandler; + } + @Override public void onStart() { SnoozeHelper snoozeHelper = new SnoozeHelper(getContext(), new SnoozeHelper.Callback() { @@ -4317,7 +4335,7 @@ public class NotificationManagerService extends SystemService { GroupHelper.AUTOGROUP_KEY, adjustedSbn.getUid(), adjustedSbn.getInitialPid(), summaryNotification, adjustedSbn.getUser(), GroupHelper.AUTOGROUP_KEY, - System.currentTimeMillis()); + mSystemClock.currentTimeMillis()); summaryRecord = new NotificationRecord(getContext(), summarySbn, notificationRecord.getChannel()); summaryRecord.setIsAppImportanceLocked( @@ -4542,6 +4560,22 @@ public class NotificationManagerService extends SystemService { mSnoozeHelper.dump(pw, filter); } + + // Log delayed notification cancels + pw.println(); + pw.println(" Delayed notification cancels:"); + if (mDelayedCancelations.isEmpty()) { + pw.println(" None"); + } else { + Set delayedKeys = mDelayedCancelations.keySet(); + for (NotificationRecord record : delayedKeys) { + ArrayList queuedCancels = + mDelayedCancelations.get(record); + pw.println(" (" + queuedCancels.size() + ") cancels enqueued for" + + record.getKey()); + } + } + pw.println(); } if (!zenOnly) { @@ -4731,7 +4765,7 @@ public class NotificationManagerService extends SystemService { final StatusBarNotification n = new StatusBarNotification( pkg, opPkg, id, tag, notificationUid, callingPid, notification, - user, null, System.currentTimeMillis()); + user, null, mSystemClock.currentTimeMillis()); final NotificationRecord r = new NotificationRecord(getContext(), n, channel); r.setIsAppImportanceLocked(mPreferencesHelper.getIsAppImportanceLocked(pkg, callingUid)); @@ -5025,7 +5059,7 @@ public class NotificationManagerService extends SystemService { final float appEnqueueRate = mUsageStats.getAppEnqueueRate(pkg); if (appEnqueueRate > mMaxPackageEnqueueRate) { mUsageStats.registerOverRateQuota(pkg); - final long now = SystemClock.elapsedRealtime(); + final long now = mSystemClock.elapsedRealtime(); if ((now - mLastOverRateLogTime) > MIN_PACKAGE_OVERRATE_LOG_INTERVAL) { Slog.e(TAG, "Package enqueue rate is " + appEnqueueRate + ". Shedding " + r.sbn.getKey() + ". package=" + pkg); @@ -5204,6 +5238,7 @@ public class NotificationManagerService extends SystemService { private final int mRank; private final int mCount; private final ManagedServiceInfo mListener; + private final long mWhen; CancelNotificationRunnable(final int callingUid, final int callingPid, final String pkg, final String tag, final int id, @@ -5223,6 +5258,47 @@ public class NotificationManagerService extends SystemService { this.mRank = rank; this.mCount = count; this.mListener = listener; + this.mWhen = mSystemClock.currentTimeMillis(); + } + + // Move the work to this function so it can be called from PostNotificationRunnable + private void doNotificationCancelLocked() { + // Look for the notification in the posted list, since we already checked enqueued. + String listenerName = mListener == null ? null : mListener.component.toShortString(); + NotificationRecord r = + findNotificationByListLocked(mNotificationList, mPkg, mTag, mId, mUserId); + if (r != null) { + // The notification was found, check if it should be removed. + + // Ideally we'd do this in the caller of this method. However, that would + // require the caller to also find the notification. + if (mReason == REASON_CLICK) { + mUsageStats.registerClickedByUser(r); + } + + if ((r.getNotification().flags & mMustHaveFlags) != mMustHaveFlags) { + return; + } + if ((r.getNotification().flags & mMustNotHaveFlags) != 0) { + return; + } + + // Cancel the notification. + boolean wasPosted = removePreviousFromNotificationListsLocked(r, mWhen); + cancelNotificationLocked( + r, mSendDelete, mReason, mRank, mCount, wasPosted, listenerName); + cancelGroupChildrenLocked(r, mCallingUid, mCallingPid, listenerName, + mSendDelete, null); + updateLightsLocked(); + } else { + // No notification was found, assume that it is snoozed and cancel it. + if (mReason != REASON_SNOOZED) { + final boolean wasSnoozed = mSnoozeHelper.cancel(mUserId, mPkg, mTag, mId); + if (wasSnoozed) { + handleSavePolicyFile(); + } + } + } } @Override @@ -5234,48 +5310,28 @@ public class NotificationManagerService extends SystemService { } 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); + // Check to see if there is a notification in the enqueued list that hasn't had a + // chance to post yet. + List enqueued = findEnqueuedNotificationsForCriteria( + mPkg, mTag, mId, mUserId); + if (enqueued.size() > 0) { + // We have found notifications that were enqueued before this cancel, but not + // yet posted. Attach this cancel to the last enqueue (the most recent), and + // we will be executed in that notification's PostNotificationRunnable + NotificationRecord enqueuedToAttach = enqueued.get(enqueued.size() - 1); + + ArrayList delayed = + mDelayedCancelations.get(enqueuedToAttach); + if (delayed == null) { + delayed = new ArrayList<>(); + } + + delayed.add(this); + mDelayedCancelations.put(enqueuedToAttach, delayed); return; } - // Look for the notification in the posted list, since we already checked enqueued. - NotificationRecord r = - findNotificationByListLocked(mNotificationList, mPkg, mTag, mId, mUserId); - if (r != null) { - // The notification was found, check if it should be removed. - // Ideally we'd do this in the caller of this method. However, that would - // require the caller to also find the notification. - if (mReason == REASON_CLICK) { - mUsageStats.registerClickedByUser(r); - } - - if ((r.getNotification().flags & mMustHaveFlags) != mMustHaveFlags) { - return; - } - if ((r.getNotification().flags & mMustNotHaveFlags) != 0) { - return; - } - - // Cancel the notification. - boolean wasPosted = removeFromNotificationListsLocked(r); - cancelNotificationLocked( - r, mSendDelete, mReason, mRank, mCount, wasPosted, listenerName); - cancelGroupChildrenLocked(r, mCallingUid, mCallingPid, listenerName, - mSendDelete, null); - updateLightsLocked(); - } else { - // No notification was found, assume that it is snoozed and cancel it. - if (mReason != REASON_SNOOZED) { - final boolean wasSnoozed = mSnoozeHelper.cancel(mUserId, mPkg, mTag, mId); - if (wasSnoozed) { - handleSavePolicyFile(); - } - } - } + doNotificationCancelLocked(); } } } @@ -5335,18 +5391,29 @@ public class NotificationManagerService extends SystemService { enqueueStatus); } - // tell the assistant service about the notification - if (mAssistants.isEnabled()) { - mAssistants.onNotificationEnqueuedLocked(r); - mHandler.postDelayed(new PostNotificationRunnable(r.getKey()), - DELAY_FOR_ASSISTANT_TIME); - } else { - mHandler.post(new PostNotificationRunnable(r.getKey())); - } + postPostNotificationRunnableMaybeDelayedLocked( + r, new PostNotificationRunnable(r.getKey())); } } } + /** + * Mainly needed as a hook for tests which require setting up enqueued-but-not-posted + * notification records + */ + @GuardedBy("mNotificationLock") + protected void postPostNotificationRunnableMaybeDelayedLocked( + NotificationRecord r, + PostNotificationRunnable runnable) { + // tell the assistant service about the notification + if (mAssistants.isEnabled()) { + mAssistants.onNotificationEnqueuedLocked(r); + mHandler.postDelayed(runnable, DELAY_FOR_ASSISTANT_TIME); + } else { + mHandler.post(runnable); + } + } + @GuardedBy("mNotificationLock") private boolean isPackageSuspendedLocked(NotificationRecord r) { final String pkg = r.sbn.getPackageName(); @@ -5460,13 +5527,23 @@ public class NotificationManagerService extends SystemService { maybeRecordInterruptionLocked(r); } finally { int N = mEnqueuedNotifications.size(); + NotificationRecord enqueued = null; for (int i = 0; i < N; i++) { - final NotificationRecord enqueued = mEnqueuedNotifications.get(i); + enqueued = mEnqueuedNotifications.get(i); if (Objects.equals(key, enqueued.getKey())) { mEnqueuedNotifications.remove(i); break; } } + + // If the enqueued notification record had a cancel attached after it, execute + // it right now + if (enqueued != null && mDelayedCancelations.get(enqueued) != null) { + for (CancelNotificationRunnable r : mDelayedCancelations.get(enqueued)) { + r.doNotificationCancelLocked(); + } + mDelayedCancelations.remove(enqueued); + } } } } @@ -5670,7 +5747,8 @@ public class NotificationManagerService extends SystemService { .putExtra(EXTRA_KEY, record.getKey()), PendingIntent.FLAG_UPDATE_CURRENT); mAlarmManager.setExactAndAllowWhileIdle(AlarmManager.ELAPSED_REALTIME_WAKEUP, - SystemClock.elapsedRealtime() + record.getNotification().getTimeoutAfter(), pi); + mSystemClock.elapsedRealtime() + record.getNotification().getTimeoutAfter(), + pi); } } @@ -6159,7 +6237,7 @@ public class NotificationManagerService extends SystemService { changed = indexBefore != indexAfter || interceptBefore != interceptAfter || visibilityBefore != visibilityAfter; if (interceptBefore && !interceptAfter - && record.isNewEnoughForAlerting(System.currentTimeMillis())) { + && record.isNewEnoughForAlerting(mSystemClock.currentTimeMillis())) { buzzBeepBlinkLocked(record); } } @@ -6435,6 +6513,34 @@ public class NotificationManagerService extends SystemService { return wasPosted; } + /** + * Similar to the above method, removes all NotificationRecords with the same key as the given + * NotificationRecord, but skips any records which are newer than the given one. + */ + private boolean removePreviousFromNotificationListsLocked(NotificationRecord r, + long removeBefore) { + // Remove notification records that occurred before the given record from both lists, + // specifically allowing newer ones to respect ordering + boolean wasPosted = false; + List matching = + findNotificationsByListLocked(mNotificationList, r.getKey()); + for (NotificationRecord record : matching) { + // We don't need to check against update time for posted notifs + mNotificationList.remove(record); + mNotificationsByKey.remove(record.sbn.getKey()); + wasPosted = true; + } + + matching = findNotificationsByListLocked(mEnqueuedNotifications, r.getKey()); + for (NotificationRecord record : matching) { + if (record.getUpdateTimeMs() <= removeBefore) { + mNotificationList.remove(record); + } + } + + return wasPosted; + } + @GuardedBy("mNotificationLock") private void cancelNotificationLocked(NotificationRecord r, boolean sendDelete, int reason, boolean wasPosted, String listenerName) { @@ -6546,7 +6652,7 @@ public class NotificationManagerService extends SystemService { // Save it for users of getHistoricalNotifications() mArchive.record(r.sbn); - final long now = System.currentTimeMillis(); + final long now = mSystemClock.currentTimeMillis(); final LogMaker logMaker = r.getItemLogMaker() .setType(MetricsEvent.TYPE_DISMISS) .setSubtype(reason); @@ -7042,6 +7148,44 @@ public class NotificationManagerService extends SystemService { return null; } + @GuardedBy("mNotificationLock") + private List findNotificationsByListLocked( + ArrayList list, + String key) { + List matching = new ArrayList<>(); + final int n = list.size(); + for (int i = 0; i < n; i++) { + NotificationRecord r = list.get(i); + if (key.equals(r.getKey())) { + matching.add(r); + } + } + return matching; + } + + /** + * There may be multiple records that match your criteria. For instance if there have been + * multiple notifications posted which are enqueued for the same pkg, tag, id, userId. This + * method will find all of them in the given list + * @return + */ + @GuardedBy("mNotificationLock") + private List findEnqueuedNotificationsForCriteria( + String pkg, String tag, int id, int userId) { + final ArrayList records = new ArrayList<>(); + final int n = mEnqueuedNotifications.size(); + for (int i = 0; i < n; i++) { + NotificationRecord r = mEnqueuedNotifications.get(i); + if (notificationMatchesUserId(r, userId) + && r.sbn.getId() == id + && TextUtils.equals(r.sbn.getTag(), tag) + && r.sbn.getPackageName().equals(pkg)) { + records.add(r); + } + } + return records; + } + @GuardedBy("mNotificationLock") int indexOfNotificationLocked(String key) { final int N = mNotificationList.size(); diff --git a/services/core/java/com/android/server/notification/NotificationRecord.java b/services/core/java/com/android/server/notification/NotificationRecord.java index c2e559a8a96bf..93e5457355e4a 100644 --- a/services/core/java/com/android/server/notification/NotificationRecord.java +++ b/services/core/java/com/android/server/notification/NotificationRecord.java @@ -905,6 +905,10 @@ public final class NotificationRecord { return (int) (now - mInterruptionTimeMs); } + public long getUpdateTimeMs() { + return mUpdateTimeMs; + } + /** * Set the visibility of the notification. */ diff --git a/services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java b/services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java index 6061d51f3d792..16b5ba9d4fb5e 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java @@ -96,6 +96,7 @@ public class BuzzBeepBlinkTest extends UiServiceTestCase { NotificationUsageStats mUsageStats; @Mock IAccessibilityManager mAccessibilityService; + private InjectableSystemClock mSystemClock = new FakeSystemClock(); private NotificationManagerService mService; private String mPkg = "com.android.server.notification"; @@ -145,7 +146,7 @@ public class BuzzBeepBlinkTest extends UiServiceTestCase { verify(mAccessibilityService).addClient(any(IAccessibilityManagerClient.class), anyInt()); assertTrue(accessibilityManager.isEnabled()); - mService = spy(new NotificationManagerService(getContext())); + mService = spy(new NotificationManagerService(getContext(), mSystemClock)); mService.setAudioManager(mAudioManager); mService.setVibrator(mVibrator); mService.setSystemReady(true); diff --git a/services/tests/uiservicestests/src/com/android/server/notification/FakeSystemClock.java b/services/tests/uiservicestests/src/com/android/server/notification/FakeSystemClock.java new file mode 100644 index 0000000000000..c960f1766612a --- /dev/null +++ b/services/tests/uiservicestests/src/com/android/server/notification/FakeSystemClock.java @@ -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.server.notification; + +import java.util.ArrayList; +import java.util.List; + +/** + * A fake {@link InjectableSystemClock} + * + * Attempts to simulate the behavior of a real system clock. Time can be moved forward but not + * backwards. uptimeMillis, elapsedRealtime, and currentThreadTimeMillis are all kept in sync. + * + * Unless otherwise specified, uptimeMillis and elapsedRealtime will advance the same amount with + * every call to {@link #advanceTime(long)}. Thread time always lags by 50% of the uptime + * advancement to simulate time loss due to scheduling. + * + * @hide + */ +public class FakeSystemClock implements InjectableSystemClock { + private long mUptimeMillis = 10000; + private long mElapsedRealtime = 10000; + private long mCurrentThreadTimeMillis = 10000; + + private long mCurrentTimeMillis = 1555555500000L; + + private final List mListeners = new ArrayList<>(); + @Override + public long uptimeMillis() { + return mUptimeMillis; + } + + @Override + public long elapsedRealtime() { + return mElapsedRealtime; + } + + @Override + public long elapsedRealtimeNanos() { + return mElapsedRealtime * 1000000 + 447; + } + + @Override + public long currentThreadTimeMillis() { + return mCurrentThreadTimeMillis; + } + + @Override + public long currentTimeMillis() { + return mCurrentTimeMillis; + } + + public void setUptimeMillis(long uptime) { + advanceTime(uptime - mUptimeMillis); + } + + public void setCurrentTimeMillis(long millis) { + mCurrentTimeMillis = millis; + } + + public void advanceTime(long uptime) { + advanceTime(uptime, 0); + } + + public void advanceTime(long uptime, long sleepTime) { + if (uptime < 0 || sleepTime < 0) { + throw new IllegalArgumentException("Time cannot go backwards."); + } + + if (uptime > 0 || sleepTime > 0) { + mUptimeMillis += uptime; + mElapsedRealtime += uptime + sleepTime; + mCurrentTimeMillis += uptime + sleepTime; + + mCurrentThreadTimeMillis += Math.ceil(uptime * 0.5); + + for (ClockTickListener listener : mListeners) { + listener.onClockTick(); + } + } + } + + public void addListener(ClockTickListener listener) { + mListeners.add(listener); + } + + public void removeListener(ClockTickListener listener) { + mListeners.remove(listener); + } + + public interface ClockTickListener { + void onClockTick(); + } + + private static final long START_TIME = 10000; +} + 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 504e531976809..d33aaacb63800 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -166,6 +166,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.function.Consumer; @SmallTest @@ -238,17 +239,22 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mNotificationAssistantAccessGrantedCallback; @Mock UserManager mUm; + private final FakeSystemClock mSystemClock = new FakeSystemClock(); // Use a Testable subclass so we can simulate calls from the system without failing. private static class TestableNotificationManagerService extends NotificationManagerService { int countSystemChecks = 0; boolean isSystemUid = true; int countLogSmartSuggestionsVisible = 0; + // If true, don't enqueue the PostNotificationRunnables, just trap them + boolean trapEnqueuedNotifications = false; + final ArrayList trappedRunnables = + new ArrayList<>(); @Nullable NotificationAssistantAccessGrantedCallback mNotificationAssistantAccessGrantedCallback; - TestableNotificationManagerService(Context context) { - super(context); + TestableNotificationManagerService(Context context, InjectableSystemClock systemClock) { + super(context, systemClock); } @Override @@ -294,6 +300,23 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { super.setNotificationAssistantAccessGrantedForUserInternal(assistant, userId, granted); } + @Override + protected void postPostNotificationRunnableMaybeDelayedLocked(NotificationRecord record, + PostNotificationRunnable runnable) { + if (trapEnqueuedNotifications) { + trappedRunnables.add(runnable); + return; + } + + super.postPostNotificationRunnableMaybeDelayedLocked(record, runnable); + } + + void drainTrappedRunnableQueue() { + for (Runnable r : trappedRunnables) { + getWorkHandler().post(r); + } + } + private void setNotificationAssistantAccessGrantedCallback( @Nullable NotificationAssistantAccessGrantedCallback callback) { this.mNotificationAssistantAccessGrantedCallback = callback; @@ -335,7 +358,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { doNothing().when(mContext).sendBroadcastAsUser(any(), any(), any()); - mService = new TestableNotificationManagerService(mContext); + mService = new TestableNotificationManagerService(mContext, mSystemClock); // Use this testable looper. mTestableLooper = TestableLooper.get(this); @@ -448,7 +471,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { .setGroupSummary(isSummary); StatusBarNotification sbn = new StatusBarNotification(PKG, PKG, id, "tag", mUid, 0, - nb.build(), new UserHandle(mUid), null, 0); + nb.build(), new UserHandle(mUid), null, mSystemClock.currentTimeMillis()); return new NotificationRecord(mContext, sbn, channel); } @@ -476,7 +499,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { nb.setBubbleMetadata(getBasicBubbleMetadataBuilder().build()); } StatusBarNotification sbn = new StatusBarNotification(PKG, PKG, 1, "tag", mUid, 0, - nb.build(), new UserHandle(mUid), null, 0); + nb.build(), new UserHandle(mUid), null, mSystemClock.currentTimeMillis()); return new NotificationRecord(mContext, sbn, channel); } @@ -916,6 +939,83 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { assertEquals(0, mService.getNotificationRecordCount()); } + @Test + public void testPostCancelPostNotifiesListeners() throws Exception { + // WHEN a notification is posted + final StatusBarNotification sbn = generateNotificationRecord(null).sbn; + mBinderService.enqueueNotificationWithTag(PKG, PKG, "tag", sbn.getId(), + sbn.getNotification(), sbn.getUserId()); + mSystemClock.advanceTime(1); + // THEN it is canceled + mBinderService.cancelNotificationWithTag(PKG, "tag", sbn.getId(), sbn.getUserId()); + mSystemClock.advanceTime(1); + // THEN it is posted again (before the cancel has a chance to finish) + mBinderService.enqueueNotificationWithTag(PKG, PKG, "tag", sbn.getId(), + sbn.getNotification(), sbn.getUserId()); + // THEN the later enqueue isn't swallowed by the cancel. I.e., ordering is respected + waitForIdle(); + + // The final enqueue made it to the listener instead of being canceled + StatusBarNotification[] notifs = + mBinderService.getActiveNotifications(PKG); + assertEquals(1, notifs.length); + assertEquals(1, mService.getNotificationRecordCount()); + } + + @Test + public void testChangeSystemTimeAfterPost_thenCancel_noFgs() throws Exception { + // GIVEN time X + mSystemClock.setCurrentTimeMillis(10000); + + // GIVEN a notification is posted + final StatusBarNotification sbn = generateNotificationRecord(null).sbn; + mBinderService.enqueueNotificationWithTag(PKG, PKG, "tag", sbn.getId(), + sbn.getNotification(), sbn.getUserId()); + mSystemClock.advanceTime(1); + waitForIdle(); + + // THEN the system time is changed to an earlier time + mSystemClock.setCurrentTimeMillis(5000); + + // THEN a cancel is requested + mBinderService.cancelNotificationWithTag(PKG, "tag", sbn.getId(), sbn.getUserId()); + waitForIdle(); + + // It should work + StatusBarNotification[] notifs = + mBinderService.getActiveNotifications(PKG); + assertEquals(0, notifs.length); + assertEquals(0, mService.getNotificationRecordCount()); + } + + @Test + public void testChangeSystemTimeAfterPost_thenCancel_fgs() throws Exception { + // GIVEN time X + mSystemClock.setCurrentTimeMillis(10000); + + // GIVEN a notification is posted + 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()); + mSystemClock.advanceTime(1); + waitForIdle(); + + // THEN the system time is changed to an earlier time + mSystemClock.setCurrentTimeMillis(5000); + + // THEN a cancel is requested + mBinderService.cancelNotificationWithTag(PKG, "tag", sbn.getId(), sbn.getUserId()); + waitForIdle(); + + // It should work + StatusBarNotification[] notifs = + mBinderService.getActiveNotifications(PKG); + assertEquals(0, notifs.length); + assertEquals(0, mService.getNotificationRecordCount()); + } + @Test public void testCancelNotificationWhilePostedAndEnqueued() throws Exception { mBinderService.enqueueNotificationWithTag(PKG, PKG, "tag", 0, @@ -934,6 +1034,56 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { assertEquals(NotificationStats.DISMISSAL_OTHER, captor.getValue().getDismissalSurface()); } + @Test + public void testDelayCancelWhenEnqueuedHasNotPosted() throws Exception { + // Don't allow PostNotificationRunnables to execute so we can set up problematic state + mService.trapEnqueuedNotifications = true; + // GIVEN an enqueued notification + mBinderService.enqueueNotificationWithTag(PKG, PKG, + "testDelayCancelWhenEnqueuedHasNotPosted", 0, + generateNotificationRecord(null).getNotification(), 0); + mSystemClock.advanceTime(1); + // WHEN a cancel is requested before it has posted + mBinderService.cancelNotificationWithTag(PKG, + "testDelayCancelWhenEnqueuedHasNotPosted", 0, 0); + + waitForIdle(); + + // THEN the cancel notification runnable is captured and associated with that record + ArrayMap> delayed = + mService.mDelayedCancelations; + Set keySet = delayed.keySet(); + assertEquals(1, keySet.size()); + } + + @Test + public void testDelayedCancelsExecuteAfterPost() throws Exception { + // Don't allow PostNotificationRunnables to execute so we can set up problematic state + mService.trapEnqueuedNotifications = true; + // GIVEN an enqueued notification + mBinderService.enqueueNotificationWithTag(PKG, PKG, + "testDelayCancelWhenEnqueuedHasNotPosted", 0, + generateNotificationRecord(null).getNotification(), 0); + mSystemClock.advanceTime(1); + // WHEN a cancel is requested before it has posted + mBinderService.cancelNotificationWithTag(PKG, + "testDelayCancelWhenEnqueuedHasNotPosted", 0, 0); + + waitForIdle(); + + // We're now in a state with an a notification awaiting PostNotificationRunnable to execute + // WHEN the PostNotificationRunnable is allowed to execute + mService.drainTrappedRunnableQueue(); + waitForIdle(); + + // THEN the cancel executes and the notification is removed + StatusBarNotification[] notifs = + mBinderService.getActiveNotifications(PKG); + assertEquals(0, notifs.length); + assertEquals(0, mService.getNotificationRecordCount()); + } + @Test public void testCancelNotificationsFromListenerImmediatelyAfterEnqueue() throws Exception { NotificationRecord r = generateNotificationRecord(null); @@ -1973,7 +2123,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { @Test public void testHasCompanionDevice_noService() { - mService = new TestableNotificationManagerService(mContext); + mService = new TestableNotificationManagerService(mContext, mSystemClock); assertFalse(mService.hasCompanionDevice(mListener)); } @@ -4833,8 +4983,16 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { @Test public void testCancelAllNotificationsFromListener_ignoresBubbles() throws Exception { - final NotificationRecord nrNormal = generateNotificationRecord(mTestNotificationChannel); - final NotificationRecord nrBubble = generateNotificationRecord(mTestNotificationChannel); + final NotificationRecord nrNormal = generateNotificationRecord( + mTestNotificationChannel /* channel */, + 1 /* id */, + null /* groupKey */, + false /* isSummary */); + final NotificationRecord nrBubble = generateNotificationRecord( + mTestNotificationChannel /* channel */, + 2 /* id */, + null /* groupKey */, + false /* isSummary */); nrBubble.sbn.getNotification().flags |= FLAG_BUBBLE; mService.addNotification(nrNormal); @@ -4850,8 +5008,16 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { @Test public void testCancelNotificationsFromListener_ignoresBubbles() throws Exception { - final NotificationRecord nrNormal = generateNotificationRecord(mTestNotificationChannel); - final NotificationRecord nrBubble = generateNotificationRecord(mTestNotificationChannel); + final NotificationRecord nrNormal = generateNotificationRecord( + mTestNotificationChannel /* channel */, + 1 /* id */, + null /* groupKey */, + false /* isSummary */); + final NotificationRecord nrBubble = generateNotificationRecord( + mTestNotificationChannel /* channel */, + 2 /* id */, + null /* groupKey */, + false /* isSummary */); nrBubble.sbn.getNotification().flags |= FLAG_BUBBLE; mService.addNotification(nrNormal); diff --git a/services/tests/uiservicestests/src/com/android/server/notification/RoleObserverTest.java b/services/tests/uiservicestests/src/com/android/server/notification/RoleObserverTest.java index f37ff1177fe91..b397930e508ff 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/RoleObserverTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/RoleObserverTest.java @@ -18,7 +18,6 @@ package com.android.server.notification; import static android.app.role.RoleManager.ROLE_DIALER; import static android.app.role.RoleManager.ROLE_EMERGENCY; -import static android.app.role.RoleManager.ROLE_SMS; import static android.content.pm.PackageManager.MATCH_ALL; import static junit.framework.Assert.assertFalse; @@ -94,11 +93,12 @@ public class RoleObserverTest extends UiServiceTestCase { private RoleManager mRoleManager; private List mUsers; + private InjectableSystemClock mSystemClock = new FakeSystemClock(); private static class TestableNotificationManagerService extends NotificationManagerService { - TestableNotificationManagerService(Context context) { - super(context); + TestableNotificationManagerService(Context context, InjectableSystemClock systemClock) { + super(context, systemClock); } @Override @@ -125,7 +125,7 @@ public class RoleObserverTest extends UiServiceTestCase { mUsers.add(new UserInfo(10, "second", 0)); when(mUm.getUsers()).thenReturn(mUsers); - mService = new TestableNotificationManagerService(mContext); + mService = new TestableNotificationManagerService(mContext, mSystemClock); mRoleObserver = mService.new RoleObserver(mRoleManager, mPm, mExecutor); try { From 0e2acaf0a58e7c8a2abe916e18f7c7b281803ccd Mon Sep 17 00:00:00 2001 From: Evan Laird Date: Tue, 26 May 2020 17:47:30 -0400 Subject: [PATCH 4/4] DO NOT MERGE: Fix interaction tracking logic Tried to put a clever kotlin-ism there, but then the interaction tracker was returning `true` for every notification because it only checked if the key existed Test: manual Bug: 144324894 Bug: 119041698 Change-Id: Ie2f489acca973c0aebbd8e7d8fc7fbef2bac793f --- .../systemui/statusbar/NotificationInteractionTracker.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationInteractionTracker.kt b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationInteractionTracker.kt index 40a3ed64f2c2e..d140e342960eb 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationInteractionTracker.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationInteractionTracker.kt @@ -23,7 +23,9 @@ class NotificationInteractionTracker @Inject constructor( entryManager.addNotificationEntryListener(this) } - fun hasUserInteractedWith(key: String): Boolean = key in interactions + fun hasUserInteractedWith(key: String): Boolean { + return interactions[key] ?: false + } override fun onNotificationAdded(entry: NotificationEntry) { interactions[entry.key] = false