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

Commit a0a90f24 authored by Ned Burns's avatar Ned Burns Committed by Android (Google) Code Review
Browse files

Merge "Make ForegroundCoordinator single-threaded"

parents 2277fc41 5db037a6

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/ForegroundCoordinator.java

+46 −35
Original line number Diff line number Diff line
@@ -17,7 +17,6 @@ 
package com.android.systemui.statusbar.notification.collection.coordinator;



import android.app.Notification;

import android.os.Handler;

import android.os.UserHandle;

import android.service.notification.StatusBarNotification;

import android.util.ArraySet;
@@ -30,6 +29,8 @@ import com.android.systemui.statusbar.notification.collection.NotificationEntry; 
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter;

import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener;

import com.android.systemui.statusbar.notification.collection.notifcollection.NotifLifetimeExtender;

import com.android.systemui.util.Assert;

import com.android.systemui.util.concurrency.DelayableExecutor;



import java.util.HashMap;

import java.util.Map;
@@ -47,6 +48,8 @@ import javax.inject.Singleton; 
 *  frameworks/base/packages/SystemUI/src/com/android/systemui/ForegroundServiceController

 *  frameworks/base/packages/SystemUI/src/com/android/systemui/ForegroundServiceNotificationListener

 *  frameworks/base/packages/SystemUI/src/com/android/systemui/ForegroundServiceLifetimeExtender

 *

 *  TODO: AppOps stuff should be spun off into its own coordinator

 */

@Singleton

public class ForegroundCoordinator implements Coordinator {
@@ -54,7 +57,7 @@ public class ForegroundCoordinator implements Coordinator { 


    private final ForegroundServiceController mForegroundServiceController;

    private final AppOpsController mAppOpsController;

    private final Handler mMainHandler;

    private final DelayableExecutor mMainExecutor;



    private NotifPipeline mNotifPipeline;
@@ -62,10 +65,10 @@ public class ForegroundCoordinator implements Coordinator { 
    public ForegroundCoordinator(

            ForegroundServiceController foregroundServiceController,

            AppOpsController appOpsController,

            @Main Handler mainHandler) {

            @Main DelayableExecutor mainExecutor) {

        mForegroundServiceController = foregroundServiceController;

        mAppOpsController = appOpsController;

        mMainHandler = mainHandler;

        mMainExecutor = mainExecutor;

    }



    @Override
@@ -78,11 +81,11 @@ public class ForegroundCoordinator implements Coordinator { 
        // listen for new notifications to add appOps

        mNotifPipeline.addCollectionListener(mNotifCollectionListener);



        // when appOps change, update any relevant notifications to update appOps for

        mAppOpsController.addCallback(ForegroundServiceController.APP_OPS, this::onAppOpsChanged);



        // filter out foreground service notifications that aren't necessary anymore

        mNotifPipeline.addPreGroupFilter(mNotifFilter);



        // when appOps change, update any relevant notifications to update appOps for

        mAppOpsController.addCallback(ForegroundServiceController.APP_OPS, this::onAppOpsChanged);

    }



    /**
@@ -93,7 +96,8 @@ public class ForegroundCoordinator implements Coordinator { 
        public boolean shouldFilterOut(NotificationEntry entry, long now) {

            StatusBarNotification sbn = entry.getSbn();

            if (mForegroundServiceController.isDisclosureNotification(sbn)

                    && !mForegroundServiceController.isDisclosureNeededForUser(sbn.getUserId())) {

                    && !mForegroundServiceController.isDisclosureNeededForUser(

                            sbn.getUser().getIdentifier())) {

                return true;

            }
@@ -102,7 +106,7 @@ public class ForegroundCoordinator implements Coordinator { 
                        Notification.EXTRA_FOREGROUND_APPS);

                if (apps != null && apps.length >= 1) {

                    if (!mForegroundServiceController.isSystemAlertWarningNeeded(

                            sbn.getUserId(), apps[0])) {

                            sbn.getUser().getIdentifier(), apps[0])) {

                        return true;

                    }

                }
@@ -119,7 +123,7 @@ public class ForegroundCoordinator implements Coordinator { 
            new NotifLifetimeExtender() {

        private static final int MIN_FGS_TIME_MS = 5000;

        private OnEndLifetimeExtensionCallback mEndCallback;

        private Map<String, Runnable> mEndRunnables = new HashMap<>();

        private Map<NotificationEntry, Runnable> mEndRunnables = new HashMap<>();



        @Override

        public String getName() {
@@ -142,16 +146,18 @@ public class ForegroundCoordinator implements Coordinator { 
            final boolean extendLife = currTime - entry.getSbn().getPostTime() < MIN_FGS_TIME_MS;



            if (extendLife) {

                if (!mEndRunnables.containsKey(entry.getKey())) {

                    final Runnable runnable = new Runnable() {

                        @Override

                        public void run() {

                            mEndCallback.onEndLifetimeExtension(mForegroundLifetimeExtender, entry);

                        }

                if (!mEndRunnables.containsKey(entry)) {

                    final Runnable endExtensionRunnable = () -> {

                        mEndRunnables.remove(entry);

                        mEndCallback.onEndLifetimeExtension(

                                mForegroundLifetimeExtender,

                                entry);

                    };

                    mEndRunnables.put(entry.getKey(), runnable);

                    mMainHandler.postDelayed(runnable,



                    final Runnable cancelRunnable = mMainExecutor.executeDelayed(

                            endExtensionRunnable,

                            MIN_FGS_TIME_MS - (currTime - entry.getSbn().getPostTime()));

                    mEndRunnables.put(entry, cancelRunnable);

                }

            }
@@ -160,9 +166,9 @@ public class ForegroundCoordinator implements Coordinator { 


        @Override

        public void cancelLifetimeExtension(NotificationEntry entry) {

            if (mEndRunnables.containsKey(entry.getKey())) {

                Runnable endRunnable = mEndRunnables.remove(entry.getKey());

                mMainHandler.removeCallbacks(endRunnable);

            Runnable cancelRunnable = mEndRunnables.remove(entry);

            if (cancelRunnable != null) {

                cancelRunnable.run();

            }

        }

    };
@@ -184,25 +190,32 @@ public class ForegroundCoordinator implements Coordinator { 
        private void tagForeground(NotificationEntry entry) {

            final StatusBarNotification sbn = entry.getSbn();

            // note: requires that the ForegroundServiceController is updating their appOps first

            ArraySet<Integer> activeOps = mForegroundServiceController.getAppOps(sbn.getUserId(),

            ArraySet<Integer> activeOps =

                    mForegroundServiceController.getAppOps(

                            sbn.getUser().getIdentifier(),

                            sbn.getPackageName());

            if (activeOps != null) {

                synchronized (entry.mActiveAppOps) {

                entry.mActiveAppOps.clear();

                entry.mActiveAppOps.addAll(activeOps);

            }

        }

        }

    };



    private void onAppOpsChanged(int code, int uid, String packageName, boolean active) {

        mMainExecutor.execute(() -> handleAppOpsChanged(code, uid, packageName, active));

    }



    /**

     * Update the appOp for the posted notification associated with the current foreground service

     *

     * @param code code for appOp to add/remove

     * @param uid of user the notification is sent to

     * @param packageName package that created the notification

     * @param active whether the appOpCode is active or not

     */

    private void onAppOpsChanged(int code, int uid, String packageName, boolean active) {

    private void handleAppOpsChanged(int code, int uid, String packageName, boolean active) {

        Assert.isMainThread();



        int userId = UserHandle.getUserId(uid);



        // Update appOp if there's an associated posted notification:
@@ -214,15 +227,13 @@ public class ForegroundCoordinator implements Coordinator { 
                    && uid == entry.getSbn().getUid()

                    && packageName.equals(entry.getSbn().getPackageName())) {

                boolean changed;

                synchronized (entry.mActiveAppOps) {

                if (active) {

                    changed = entry.mActiveAppOps.add(code);

                } else {

                    changed = entry.mActiveAppOps.remove(code);

                }

                }

                if (changed) {

                    mMainHandler.post(mNotifFilter::invalidateList);

                    mNotifFilter.invalidateList();

                }

            }

        }

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/ForegroundCoordinatorTest.java

+96 −4
Original line number Diff line number Diff line
@@ -16,20 +16,23 @@ 


package com.android.systemui.statusbar.notification.collection.coordinator;



import static junit.framework.Assert.assertEquals;

import static junit.framework.Assert.assertFalse;

import static junit.framework.Assert.assertTrue;



import static org.mockito.ArgumentMatchers.any;

import static org.mockito.Mockito.times;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.when;



import android.app.Notification;

import android.os.Bundle;

import android.os.Handler;

import android.os.UserHandle;

import android.service.notification.NotificationListenerService;

import android.service.notification.StatusBarNotification;

import android.testing.AndroidTestingRunner;

import android.testing.TestableLooper;

import android.util.ArraySet;



import androidx.test.filters.SmallTest;
@@ -41,36 +44,53 @@ import com.android.systemui.statusbar.notification.collection.NotificationEntry; 
import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder;

import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter;

import com.android.systemui.statusbar.notification.collection.notifcollection.NotifLifetimeExtender;

import com.android.systemui.util.Assert;

import com.android.systemui.util.concurrency.FakeExecutor;

import com.android.systemui.util.time.FakeSystemClock;



import org.junit.Before;

import org.junit.Test;

import org.junit.runner.RunWith;

import org.mockito.ArgumentCaptor;

import org.mockito.Captor;

import org.mockito.Mock;

import org.mockito.MockitoAnnotations;



import java.util.List;



@SmallTest

@RunWith(AndroidTestingRunner.class)

@TestableLooper.RunWithLooper

public class ForegroundCoordinatorTest extends SysuiTestCase {

    private static final String TEST_PKG = "test_pkg";

    private static final int NOTIF_USER_ID = 0;



    @Mock private Handler mMainHandler;

    @Mock private ForegroundServiceController mForegroundServiceController;

    @Mock private AppOpsController mAppOpsController;

    @Mock private NotifPipeline mNotifPipeline;



    @Captor private ArgumentCaptor<AppOpsController.Callback> mAppOpsCaptor;



    private NotificationEntry mEntry;

    private Notification mNotification;

    private ForegroundCoordinator mForegroundCoordinator;

    private NotifFilter mForegroundFilter;

    private AppOpsController.Callback mAppOpsCallback;

    private NotifLifetimeExtender mForegroundNotifLifetimeExtender;



    private FakeSystemClock mClock = new FakeSystemClock();

    private FakeExecutor mExecutor = new FakeExecutor(mClock);



    @Before

    public void setup() {

        MockitoAnnotations.initMocks(this);

        mForegroundCoordinator = new ForegroundCoordinator(

                mForegroundServiceController, mAppOpsController, mMainHandler);

        Assert.sMainLooper = TestableLooper.get(this).getLooper();



        mForegroundCoordinator =

                new ForegroundCoordinator(

                        mForegroundServiceController,

                        mAppOpsController,

                        mExecutor);



        mNotification = new Notification();

        mEntry = new NotificationEntryBuilder()
@@ -86,9 +106,11 @@ public class ForegroundCoordinatorTest extends SysuiTestCase { 
        verify(mNotifPipeline, times(1)).addPreGroupFilter(filterCaptor.capture());

        verify(mNotifPipeline, times(1)).addNotificationLifetimeExtender(

                lifetimeExtenderCaptor.capture());

        verify(mAppOpsController).addCallback(any(int[].class), mAppOpsCaptor.capture());



        mForegroundFilter = filterCaptor.getValue();

        mForegroundNotifLifetimeExtender = lifetimeExtenderCaptor.getValue();

        mAppOpsCallback = mAppOpsCaptor.getValue();

    }



    @Test
@@ -183,4 +205,74 @@ public class ForegroundCoordinatorTest extends SysuiTestCase { 
        assertFalse(mForegroundNotifLifetimeExtender

                .shouldExtendLifetime(mEntry, NotificationListenerService.REASON_CLICK));

    }



    @Test

    public void testAppOpsAreApplied() {

        // GIVEN Three current notifications, two with the same key but from different users

        NotificationEntry entry1 = new NotificationEntryBuilder()

                .setUser(new UserHandle(NOTIF_USER_ID))

                .setPkg(TEST_PKG)

                .setId(1)

                .build();

        NotificationEntry entry2 = new NotificationEntryBuilder()

                .setUser(new UserHandle(NOTIF_USER_ID))

                .setPkg(TEST_PKG)

                .setId(2)

                .build();

        NotificationEntry entry2Other = new NotificationEntryBuilder()

                .setUser(new UserHandle(NOTIF_USER_ID + 1))

                .setPkg(TEST_PKG)

                .setId(2)

                .build();

        when(mNotifPipeline.getActiveNotifs()).thenReturn(List.of(entry1, entry2, entry2Other));



        // GIVEN that entry2 is currently associated with a foreground service

        when(mForegroundServiceController.getStandardLayoutKey(0, TEST_PKG))

                .thenReturn(entry2.getKey());



        // WHEN a new app ops code comes in

        mAppOpsCallback.onActiveStateChanged(47, NOTIF_USER_ID, TEST_PKG, true);

        mExecutor.runAllReady();



        // THEN entry2's app ops are updated, but no one else's are

        assertEquals(

                new ArraySet<>(),

                entry1.mActiveAppOps);

        assertEquals(

                new ArraySet<>(List.of(47)),

                entry2.mActiveAppOps);

        assertEquals(

                new ArraySet<>(),

                entry2Other.mActiveAppOps);

    }



    @Test

    public void testAppOpsAreRemoved() {

        // GIVEN One notification which is associated with app ops

        NotificationEntry entry = new NotificationEntryBuilder()

                .setUser(new UserHandle(NOTIF_USER_ID))

                .setPkg(TEST_PKG)

                .setId(2)

                .build();

        when(mNotifPipeline.getActiveNotifs()).thenReturn(List.of(entry));

        when(mForegroundServiceController.getStandardLayoutKey(0, TEST_PKG))

                .thenReturn(entry.getKey());



        // GIVEN that the notification's app ops are already [47, 33]

        mAppOpsCallback.onActiveStateChanged(47, NOTIF_USER_ID, TEST_PKG, true);

        mAppOpsCallback.onActiveStateChanged(33, NOTIF_USER_ID, TEST_PKG, true);

        mExecutor.runAllReady();

        assertEquals(

                new ArraySet<>(List.of(47, 33)),

                entry.mActiveAppOps);



        // WHEN one of the app ops is removed

        mAppOpsCallback.onActiveStateChanged(47, NOTIF_USER_ID, TEST_PKG, false);

        mExecutor.runAllReady();



        // THEN the entry's active app ops are updated as well

        assertEquals(

                new ArraySet<>(List.of(33)),

                entry.mActiveAppOps);

    }

}