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

Commit d1869f47 authored by Matías Hernández's avatar Matías Hernández
Browse files

Throttle broadcasts of ACTION_EFFECTS_SUPPRESSOR_CHANGED

1) Don't broadcast at all if the set of suppressed effects has changed but not the set of suppressors themselves (e.g. if an NLS changes the hints it provides).
2) Instead of sending it out immediately, wait for 250 ms after the last change (and restart this wait if it changes again).

Bug: 371776935
Test: atest NotificationManagerServiceTest
Flag: com.android.server.notification.nm_binder_perf_throttle_effects_suppressor_broadcast
Change-Id: I48645b08dd80b49729396c85db37525d16949d37
parent 354e3282
Loading
Loading
Loading
Loading
+43 −6
Original line number Diff line number Diff line
@@ -632,6 +632,8 @@ public class NotificationManagerService extends SystemService {
    // Minium number of sparse groups for a package before autogrouping them
    private static final int AUTOGROUP_SPARSE_GROUPS_AT_COUNT = 3;
    private static final Duration ZEN_BROADCAST_DELAY = Duration.ofMillis(250);
    private IActivityManager mAm;
    private ActivityTaskManagerInternal mAtm;
    private ActivityManager mActivityManager;
@@ -3178,6 +3180,24 @@ public class NotificationManagerService extends SystemService {
        sendRegisteredOnlyBroadcast(new Intent(action));
    }
    /**
     * Schedules a broadcast to be sent to runtime receivers and DND-policy-access packages. The
     * broadcast will be sent after {@link #ZEN_BROADCAST_DELAY}, unless a new broadcast is
     * scheduled in the interim, in which case the previous one is dropped and the waiting period
     * is <em>restarted</em>.
     *
     * <p>Note that this uses <em>equality of the {@link Intent#getAction}</em> as the criteria for
     * deduplicating pending broadcasts, ignoring the extras and anything else. This is intentional
     * so that e.g. rapidly changing some value A -> B -> C will only produce a broadcast for C
     * (instead of every time because the extras are different).
     */
    private void sendZenBroadcastWithDelay(Intent intent) {
        String token = "zen_broadcast:" + intent.getAction();
        mHandler.removeCallbacksAndEqualMessages(token);
        mHandler.postDelayed(() -> sendRegisteredOnlyBroadcast(intent), token,
                ZEN_BROADCAST_DELAY.toMillis());
    }
    private void sendRegisteredOnlyBroadcast(Intent baseIntent) {
        int[] userIds = mUmInternal.getProfileIds(mAmi.getCurrentUserId(), true);
        if (Flags.nmBinderPerfReduceZenBroadcasts()) {
@@ -3371,15 +3391,26 @@ public class NotificationManagerService extends SystemService {
    @GuardedBy("mNotificationLock")
    private void updateEffectsSuppressorLocked() {
        final long oldSuppressedEffects = mZenModeHelper.getSuppressedEffects();
        final long updatedSuppressedEffects = calculateSuppressedEffects();
        if (updatedSuppressedEffects == mZenModeHelper.getSuppressedEffects()) return;
        if (updatedSuppressedEffects == oldSuppressedEffects) return;
        final List<ComponentName> suppressors = getSuppressors();
        ZenLog.traceEffectsSuppressorChanged(
                mEffectsSuppressors, suppressors, updatedSuppressedEffects);
        mEffectsSuppressors = suppressors;
                mEffectsSuppressors, suppressors, oldSuppressedEffects, updatedSuppressedEffects);
        mZenModeHelper.setSuppressedEffects(updatedSuppressedEffects);
        if (Flags.nmBinderPerfThrottleEffectsSuppressorBroadcast()) {
            if (!suppressors.equals(mEffectsSuppressors)) {
                mEffectsSuppressors = suppressors;
                sendZenBroadcastWithDelay(
                        new Intent(NotificationManager.ACTION_EFFECTS_SUPPRESSOR_CHANGED));
            }
        } else {
            mEffectsSuppressors = suppressors;
            sendRegisteredOnlyBroadcast(NotificationManager.ACTION_EFFECTS_SUPPRESSOR_CHANGED);
        }
    }
    private void exitIdle() {
        if (mDeviceIdleManager != null) {
@@ -3500,13 +3531,19 @@ public class NotificationManagerService extends SystemService {
    }
    private ArrayList<ComponentName> getSuppressors() {
        ArrayList<ComponentName> names = new ArrayList<ComponentName>();
        ArrayList<ComponentName> names = new ArrayList<>();
        for (int i = mListenersDisablingEffects.size() - 1; i >= 0; --i) {
            ArraySet<ComponentName> serviceInfoList = mListenersDisablingEffects.valueAt(i);
            for (ComponentName info : serviceInfoList) {
                if (Flags.nmBinderPerfThrottleEffectsSuppressorBroadcast()) {
                    if (!names.contains(info)) {
                        names.add(info);
                    }
                } else {
                    names.add(info);
                }
            }
        }
        return names;
+3 −2
Original line number Diff line number Diff line
@@ -140,8 +140,9 @@ public class ZenLog {
    }

    public static void traceEffectsSuppressorChanged(List<ComponentName> oldSuppressors,
            List<ComponentName> newSuppressors, long suppressedEffects) {
        append(TYPE_SUPPRESSOR_CHANGED, "suppressed effects:" + suppressedEffects + ","
            List<ComponentName> newSuppressors, long oldSuppressedEffects, long suppressedEffects) {
        append(TYPE_SUPPRESSOR_CHANGED, "suppressed effects:"
                + oldSuppressedEffects + "->" + suppressedEffects + ","
                + componentListToString(oldSuppressors) + "->"
                + componentListToString(newSuppressors));
    }
+10 −0
Original line number Diff line number Diff line
@@ -181,6 +181,16 @@ flag {
  }
}

flag {
  name: "nm_binder_perf_throttle_effects_suppressor_broadcast"
  namespace: "systemui"
  description: "Delay sending the ACTION_EFFECTS_SUPPRESSOR_CHANGED broadcast if it changes too often"
  bug: "371776935"
  metadata {
    purpose: PURPOSE_BUGFIX
  }
}

flag {
  name: "fix_calling_uid_from_cps"
  namespace: "systemui"
+74 −5
Original line number Diff line number Diff line
@@ -51,6 +51,7 @@ import static android.app.NotificationChannel.RECS_ID;
import static android.app.NotificationChannel.SOCIAL_MEDIA_ID;
import static android.app.NotificationChannel.USER_LOCKED_ALLOW_BUBBLE;
import static android.app.NotificationManager.ACTION_AUTOMATIC_ZEN_RULE_STATUS_CHANGED;
import static android.app.NotificationManager.ACTION_EFFECTS_SUPPRESSOR_CHANGED;
import static android.app.NotificationManager.ACTION_INTERRUPTION_FILTER_CHANGED;
import static android.app.NotificationManager.AUTOMATIC_RULE_STATUS_ACTIVATED;
import static android.app.NotificationManager.BUBBLE_PREFERENCE_ALL;
@@ -123,7 +124,9 @@ import static android.service.notification.Flags.FLAG_REDACT_SENSITIVE_NOTIFICAT
import static android.service.notification.NotificationListenerService.FLAG_FILTER_TYPE_ALERTING;
import static android.service.notification.NotificationListenerService.FLAG_FILTER_TYPE_CONVERSATIONS;
import static android.service.notification.NotificationListenerService.FLAG_FILTER_TYPE_ONGOING;
import static android.service.notification.NotificationListenerService.HINT_HOST_DISABLE_CALL_EFFECTS;
import static android.service.notification.NotificationListenerService.HINT_HOST_DISABLE_EFFECTS;
import static android.service.notification.NotificationListenerService.HINT_HOST_DISABLE_NOTIFICATION_EFFECTS;
import static android.service.notification.NotificationListenerService.REASON_APP_CANCEL;
import static android.service.notification.NotificationListenerService.REASON_CANCEL;
import static android.service.notification.NotificationListenerService.REASON_LOCKDOWN;
@@ -163,6 +166,7 @@ import static junit.framework.Assert.fail;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertThrows;
import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.Matchers.anyBoolean;
import static org.mockito.Matchers.anyLong;
@@ -361,7 +365,6 @@ import org.junit.rules.TestRule;
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.mockito.ArgumentMatcher;
import org.mockito.ArgumentMatchers;
import org.mockito.InOrder;
import org.mockito.Mock;
import org.mockito.Mockito;
@@ -369,6 +372,9 @@ import org.mockito.MockitoAnnotations;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import platform.test.runner.parameterized.ParameterizedAndroidJunit4;
import platform.test.runner.parameterized.Parameters;
import java.io.BufferedInputStream;
import java.io.BufferedOutputStream;
import java.io.ByteArrayInputStream;
@@ -384,9 +390,6 @@ import java.util.Map;
import java.util.concurrent.CountDownLatch;
import java.util.function.Consumer;
import platform.test.runner.parameterized.ParameterizedAndroidJunit4;
import platform.test.runner.parameterized.Parameters;
@SmallTest
@RunWith(ParameterizedAndroidJunit4.class)
@RunWithLooper
@@ -11404,8 +11407,14 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
        verify(mContext).sendBroadcastAsUser(eqIntent(expected), eq(UserHandle.of(mUserId)));
    }
    private static Intent isIntentWithAction(String wantedAction) {
        return argThat(
                intent -> intent != null && wantedAction.equals(intent.getAction())
        );
    }
    private static Intent eqIntent(Intent wanted) {
        return ArgumentMatchers.argThat(
        return argThat(
                new ArgumentMatcher<Intent>() {
                    @Override
                    public boolean matches(Intent argument) {
@@ -17505,6 +17514,66 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
        assertThat(mBinderService.getEffectsSuppressor()).isEqualTo(mListener.component);
    }
    @Test
    @EnableFlags({Flags.FLAG_NM_BINDER_PERF_THROTTLE_EFFECTS_SUPPRESSOR_BROADCAST,
            Flags.FLAG_NM_BINDER_PERF_REDUCE_ZEN_BROADCASTS})
    public void requestHintsFromListener_changingEffectsButNotSuppressor_noBroadcast()
            throws Exception {
        // Note that NM_BINDER_PERF_REDUCE_ZEN_BROADCASTS is not strictly necessary; however each
        // path will do slightly different calls so we force one of them to simplify the test.
        when(mUmInternal.getProfileIds(anyInt(), anyBoolean())).thenReturn(new int[]{mUserId});
        when(mListeners.checkServiceTokenLocked(any())).thenReturn(mListener);
        INotificationListener token = mock(INotificationListener.class);
        mService.isSystemUid = true;
        mBinderService.requestHintsFromListener(token, HINT_HOST_DISABLE_CALL_EFFECTS);
        mTestableLooper.moveTimeForward(500); // more than ZEN_BROADCAST_DELAY
        waitForIdle();
        verify(mContext, times(1)).sendBroadcastMultiplePermissions(
                isIntentWithAction(ACTION_EFFECTS_SUPPRESSOR_CHANGED), any(), any(), any());
        // Same suppressor suppresses something else.
        mBinderService.requestHintsFromListener(token, HINT_HOST_DISABLE_NOTIFICATION_EFFECTS);
        mTestableLooper.moveTimeForward(500); // more than ZEN_BROADCAST_DELAY
        waitForIdle();
        // Still 1 total calls (the previous one).
        verify(mContext, times(1)).sendBroadcastMultiplePermissions(
                isIntentWithAction(ACTION_EFFECTS_SUPPRESSOR_CHANGED), any(), any(), any());
    }
    @Test
    @EnableFlags({Flags.FLAG_NM_BINDER_PERF_THROTTLE_EFFECTS_SUPPRESSOR_BROADCAST,
            Flags.FLAG_NM_BINDER_PERF_REDUCE_ZEN_BROADCASTS})
    public void requestHintsFromListener_changingSuppressor_throttlesBroadcast() throws Exception {
        // Note that NM_BINDER_PERF_REDUCE_ZEN_BROADCASTS is not strictly necessary; however each
        // path will do slightly different calls so we force one of them to simplify the test.
        when(mUmInternal.getProfileIds(anyInt(), anyBoolean())).thenReturn(new int[]{mUserId});
        when(mListeners.checkServiceTokenLocked(any())).thenReturn(mListener);
        INotificationListener token = mock(INotificationListener.class);
        mService.isSystemUid = true;
        // Several updates in quick succession.
        mBinderService.requestHintsFromListener(token, HINT_HOST_DISABLE_CALL_EFFECTS);
        mBinderService.clearRequestedListenerHints(token);
        mBinderService.requestHintsFromListener(token, HINT_HOST_DISABLE_NOTIFICATION_EFFECTS);
        mBinderService.clearRequestedListenerHints(token);
        mBinderService.requestHintsFromListener(token, HINT_HOST_DISABLE_CALL_EFFECTS);
        mBinderService.clearRequestedListenerHints(token);
        mBinderService.requestHintsFromListener(token, HINT_HOST_DISABLE_NOTIFICATION_EFFECTS);
        // No broadcasts yet!
        verify(mContext, never()).sendBroadcastMultiplePermissions(any(), any(), any(), any());
        mTestableLooper.moveTimeForward(500); // more than ZEN_BROADCAST_DELAY
        waitForIdle();
        // Only one broadcast after idle time.
        verify(mContext, times(1)).sendBroadcastMultiplePermissions(
                isIntentWithAction(ACTION_EFFECTS_SUPPRESSOR_CHANGED), any(), any(), any());
    }
    @Test
    @EnableFlags(android.service.notification.Flags.FLAG_NOTIFICATION_CLASSIFICATION)
    public void testApplyAdjustment_keyType_validType() throws Exception {