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

Commit 6dc09419 authored by Evan Laird's avatar Evan Laird
Browse files

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
parent 98d6853f

services/core/java/com/android/server/notification/InjectableSystemClock.java

0 → 100644
+44 −0
Original line number Diff line number Diff line 
/*

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

}

services/core/java/com/android/server/notification/InjectableSystemClockImpl.java

0 → 100644
+51 −0
Original line number Diff line number Diff line 
/*

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

    }

}

services/core/java/com/android/server/notification/NotificationManagerService.java

+201 −57
Original line number Diff line number Diff line
@@ -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<Integer, ArrayMap<String, String>> mAutobundledSummaries = new ArrayMap<>();

    final ArrayList<ToastRecord> mToastQueue = new ArrayList<>();

    final ArrayMap<String, NotificationRecord> mSummaryByGroupKey = new ArrayMap<>();

    // Keep track of `CancelNotificationRunnable`s which have been delayed due to awaiting

    // enqueued notifications to post

    @GuardedBy("mNotificationLock")

    final ArrayMap<NotificationRecord, ArrayList<CancelNotificationRunnable>> mDelayedCancelations =

            new ArrayMap<>();



    // The last key in this list owns the hardware.

    ArrayList<String> mLights = new ArrayList<>();
@@ -465,6 +469,7 @@ public class NotificationManagerService extends SystemService { 


    private MetricsLogger mMetricsLogger;

    private TriPredicate<String, Integer, String> 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<NotificationRecord> delayedKeys = mDelayedCancelations.keySet();

                    for (NotificationRecord record : delayedKeys) {

                        ArrayList<CancelNotificationRunnable> 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,25 +5258,13 @@ public class NotificationManagerService extends SystemService { 
            this.mRank = rank;

            this.mCount = count;

            this.mListener = listener;

            this.mWhen = mSystemClock.currentTimeMillis();

        }



        @Override

        public void run() {

            String listenerName = mListener == null ? null : mListener.component.toShortString();

            if (DBG) {

                EventLogTags.writeNotificationCancel(mCallingUid, mCallingPid, mPkg, mId, mTag,

                        mUserId, mMustHaveFlags, mMustNotHaveFlags, mReason, listenerName);

            }



            synchronized (mNotificationLock) {

                // If the notification is currently enqueued, repost this runnable so it has a

                // chance to notify listeners

                if ((findNotificationByListLocked(mEnqueuedNotifications, mPkg, mTag, mId, mUserId))

                        != null) {

                    mHandler.post(this);

                    return;

                }

        // 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) {
@@ -5261,7 +5284,7 @@ public class NotificationManagerService extends SystemService { 
                }



                // Cancel the notification.

                    boolean wasPosted = removeFromNotificationListsLocked(r);

                boolean wasPosted = removePreviousFromNotificationListsLocked(r, mWhen);

                cancelNotificationLocked(

                        r, mSendDelete, mReason, mRank, mCount, wasPosted, listenerName);

                cancelGroupChildrenLocked(r, mCallingUid, mCallingPid, listenerName,
@@ -5277,6 +5300,39 @@ public class NotificationManagerService extends SystemService { 
                }

            }

        }



        @Override

        public void run() {

            String listenerName = mListener == null ? null : mListener.component.toShortString();

            if (DBG) {

                EventLogTags.writeNotificationCancel(mCallingUid, mCallingPid, mPkg, mId, mTag,

                        mUserId, mMustHaveFlags, mMustNotHaveFlags, mReason, listenerName);

            }



            synchronized (mNotificationLock) {

                // Check to see if there is a notification in the enqueued list that hasn't had a

                // chance to post yet.

                List<NotificationRecord> 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<CancelNotificationRunnable> delayed =

                            mDelayedCancelations.get(enqueuedToAttach);

                    if (delayed == null) {

                        delayed = new ArrayList<>();

                    }



                    delayed.add(this);

                    mDelayedCancelations.put(enqueuedToAttach, delayed);

                    return;

                }



                doNotificationCancelLocked();

            }

        }

    }
@@ -5335,15 +5391,26 @@ public class NotificationManagerService extends SystemService { 
                            enqueueStatus);

                }



                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(new PostNotificationRunnable(r.getKey()),

                            DELAY_FOR_ASSISTANT_TIME);

            mHandler.postDelayed(runnable, DELAY_FOR_ASSISTANT_TIME);

        } else {

                    mHandler.post(new PostNotificationRunnable(r.getKey()));

                }

            }

            mHandler.post(runnable);

        }

    }
@@ -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<NotificationRecord> 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<NotificationRecord> findNotificationsByListLocked(

            ArrayList<NotificationRecord> list,

            String key) {

        List<NotificationRecord> 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<NotificationRecord> findEnqueuedNotificationsForCriteria(

            String pkg, String tag, int id, int userId) {

        final ArrayList<NotificationRecord> 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();

services/core/java/com/android/server/notification/NotificationRecord.java

+4 −0
Original line number Diff line number Diff line
@@ -905,6 +905,10 @@ public final class NotificationRecord { 
        return (int) (now - mInterruptionTimeMs);

    }



    public long getUpdateTimeMs() {

        return mUpdateTimeMs;

    }



    /**

     * Set the visibility of the notification.

     */

services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java

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