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

Commit 5fcd050e authored by Hugo Benichi's avatar Hugo Benichi
Browse files

NetworkNotificationManager: correctly handle existing notifications 

This patch corrects a regression added by commit fb2609d3 that did
not take into account the case of multiple notifications shown for a
single network id. Given how network notifications are triggered, it can
happen that NO_INTERNET and SIGN_IN notifications are both triggered for
the same network when captive portal detection is slow.

Contrary to the situation before commit fb2609d3, a notification
priority order is introduced so that SIGN_IN always overrides
NO_INTERNET, and NO_INTERNET is ignored if SIGN_IN is already present.

Bug: 63676954
Bug: 62503737
Test: runtest frameworks-net, added new unit tests
Change-Id: Ib8658601e8d4dc6c41b335ab7dd8caa0cccd9531
parent f44ebf5f

services/core/java/com/android/server/connectivity/NetworkNotificationManager.java

+30 −0
Original line number Diff line number Diff line
@@ -140,6 +140,18 @@ public class NetworkNotificationManager { 
            extraInfo = null;

        }



        // Clear any previous notification with lower priority, otherwise return. http://b/63676954.

        // A new SIGN_IN notification with a new intent should override any existing one.

        final int previousEventId = mNotificationTypeMap.get(id);

        final NotificationType previousNotifyType = NotificationType.getFromId(previousEventId);

        if (priority(previousNotifyType) > priority(notifyType)) {

            Slog.d(TAG, String.format(

                    "ignoring notification %s for network %s with existing notification %s",

                    notifyType, id, previousNotifyType));

            return;

        }

        clearNotification(id);



        if (DBG) {

            Slog.d(TAG, String.format(

                    "showNotification tag=%s event=%s transport=%s extraInfo=%s highPrioriy=%s",
@@ -270,4 +282,22 @@ public class NetworkNotificationManager { 
        NotificationType t = NotificationType.getFromId(eventId);

        return (t != null) ? t.name() : "UNKNOWN";

    }



    private static int priority(NotificationType t) {

        if (t == null) {

            return 0;

        }

        switch (t) {

            case SIGN_IN:

                return 4;

            case NO_INTERNET:

                return 3;

            case NETWORK_SWITCH:

                return 2;

            case LOST_INTERNET:

                return 1;

            default:

                return 0;

        }

    }

}

tests/net/java/com/android/server/connectivity/NetworkNotificationManagerTest.java

+53 −9
Original line number Diff line number Diff line
@@ -16,6 +16,16 @@ 


package com.android.server.connectivity;



import static com.android.server.connectivity.NetworkNotificationManager.NotificationType.*;

import static org.mockito.Mockito.any;

import static org.mockito.Mockito.anyInt;

import static org.mockito.Mockito.eq;

import static org.mockito.Mockito.never;

import static org.mockito.Mockito.reset;

import static org.mockito.Mockito.times;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.when;



import android.app.Notification;

import android.app.NotificationManager;

import android.content.Context;
@@ -37,15 +47,6 @@ import org.mockito.Mock; 
import org.mockito.Mockito;

import org.mockito.MockitoAnnotations;



import static com.android.server.connectivity.NetworkNotificationManager.NotificationType.*;

import static org.mockito.Mockito.any;

import static org.mockito.Mockito.anyInt;

import static org.mockito.Mockito.eq;

import static org.mockito.Mockito.never;

import static org.mockito.Mockito.times;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.when;



public class NetworkNotificationManagerTest extends TestCase {



    static final NetworkCapabilities CELL_CAPABILITIES = new NetworkCapabilities();
@@ -140,4 +141,47 @@ public class NetworkNotificationManagerTest extends TestCase { 


        verify(mNotificationManager, never()).notifyAsUser(any(), anyInt(), any(), any());

    }



    @SmallTest

    public void testDuplicatedNotificationsNoInternetThenSignIn() {

        final int id = 101;

        final String tag = NetworkNotificationManager.tagFor(id);



        // Show first NO_INTERNET

        mManager.showNotification(id, NO_INTERNET, mWifiNai, mCellNai, null, false);

        verify(mNotificationManager, times(1))

                .notifyAsUser(eq(tag), eq(NO_INTERNET.eventId), any(), any());



        // Captive portal detection triggers SIGN_IN a bit later, clearing the previous NO_INTERNET

        mManager.showNotification(id, SIGN_IN, mWifiNai, mCellNai, null, false);

        verify(mNotificationManager, times(1))

                .cancelAsUser(eq(tag), eq(NO_INTERNET.eventId), any());

        verify(mNotificationManager, times(1))

                .notifyAsUser(eq(tag), eq(SIGN_IN.eventId), any(), any());



        // Network disconnects

        mManager.clearNotification(id);

        verify(mNotificationManager, times(1)).cancelAsUser(eq(tag), eq(SIGN_IN.eventId), any());

    }



    @SmallTest

    public void testDuplicatedNotificationsSignInThenNoInternet() {

        final int id = 101;

        final String tag = NetworkNotificationManager.tagFor(id);



        // Show first SIGN_IN

        mManager.showNotification(id, SIGN_IN, mWifiNai, mCellNai, null, false);

        verify(mNotificationManager, times(1))

                .notifyAsUser(eq(tag), eq(SIGN_IN.eventId), any(), any());

        reset(mNotificationManager);



        // NO_INTERNET arrives after, but is ignored.

        mManager.showNotification(id, NO_INTERNET, mWifiNai, mCellNai, null, false);

        verify(mNotificationManager, never()).cancelAsUser(any(), anyInt(), any());

        verify(mNotificationManager, never()).notifyAsUser(any(), anyInt(), any(), any());



        // Network disconnects

        mManager.clearNotification(id);

        verify(mNotificationManager, times(1)).cancelAsUser(eq(tag), eq(SIGN_IN.eventId), any());

    }

}