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

Commit 3d6b3964 authored by Ned Burns's avatar Ned Burns
Browse files

Make mPendingNotifications truly private (mk II) 

(Added a nullcheck to callback in removeNotificationInternal)

Previously, NotificationEntryManager was exposing a reference to its
internal HashMap, mPendingNotifications, to
NotificationGroupAlertTransferHelper. NGATH only needed the reference to
iterate over the pending notifications, so now NEM exposes a new
getPendingNotificationsIterator() method that just exposes a read-only
iterator.

Also removes any references to NGATH in NEM. Initializes NGATH
independently from NEM.

Test: updated unit tests
Change-Id: I2bb345097b34fe18f008834c8f95fdb01f421436
parent b1f5aea8

packages/SystemUI/src/com/android/systemui/statusbar/notification/AlertTransferListener.java

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

 * Copyright (C) 2018 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.statusbar.notification;



/**

 * Listener interface for when NotificationEntryManager needs to tell

 * NotificationGroupAlertTransferHelper things. Will eventually grow to be a general-purpose

 * listening interface for the NotificationEntryManager.

 */

public interface AlertTransferListener {

    /**

     * Called when a new notification is posted. At this point, the notification is "pending": its

     * views haven't been inflated yet and most of the system pretends like it doesn't exist yet.

     */

    void onPendingEntryAdded(NotificationData.Entry entry);



    /**

     * Called when an existing notification's views are reinflated (usually due to an update being

     * posted to that notification).

     */

    void onEntryReinflated(NotificationData.Entry entry);



    /**

     * Called when a notification has been removed (either because the user swiped it away or

     * because the developer retracted it).

     */

    void onEntryRemoved(NotificationData.Entry entry);

}

packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java

+23 −13
Original line number Diff line number Diff line
@@ -62,7 +62,6 @@ import com.android.systemui.Dependency; 
import com.android.systemui.Dumpable;

import com.android.systemui.EventLogTags;

import com.android.systemui.ForegroundServiceController;

import com.android.systemui.InitController;

import com.android.systemui.R;

import com.android.systemui.UiOffloadThread;

import com.android.systemui.bubbles.BubbleController;
@@ -84,7 +83,6 @@ import com.android.systemui.statusbar.notification.row.NotificationInflater; 
import com.android.systemui.statusbar.notification.row.NotificationInflater.InflationFlag;

import com.android.systemui.statusbar.notification.row.RowInflaterTask;

import com.android.systemui.statusbar.notification.stack.NotificationListContainer;

import com.android.systemui.statusbar.phone.NotificationGroupAlertTransferHelper;

import com.android.systemui.statusbar.phone.NotificationGroupManager;

import com.android.systemui.statusbar.phone.ShadeController;

import com.android.systemui.statusbar.phone.StatusBar;
@@ -121,8 +119,6 @@ public class NotificationEntryManager implements Dumpable, NotificationInflater. 


    private final NotificationGroupManager mGroupManager =

            Dependency.get(NotificationGroupManager.class);

    private final NotificationGroupAlertTransferHelper mGroupAlertTransferHelper =

            Dependency.get(NotificationGroupAlertTransferHelper.class);

    private final NotificationGutsManager mGutsManager =

            Dependency.get(NotificationGutsManager.class);

    private final MetricsLogger mMetricsLogger = Dependency.get(MetricsLogger.class);
@@ -164,6 +160,7 @@ public class NotificationEntryManager implements Dumpable, NotificationInflater. 
            = new ArrayList<>();

    private ExpandableNotificationRow.OnAppOpsClickListener mOnAppOpsClickListener;

    private NotificationViewHierarchyManager.StatusBarStateListener mStatusBarStateListener;

    @Nullable private AlertTransferListener mAlertTransferListener;



    private final class NotificationClicker implements View.OnClickListener {
@@ -265,13 +262,11 @@ public class NotificationEntryManager implements Dumpable, NotificationInflater. 
        mMessagingUtil = new NotificationMessagingUtil(context);

        mBubbleController.setDismissListener(this /* bubbleEventListener */);

        mNotificationData = new NotificationData();

        Dependency.get(InitController.class).addPostInitTask(this::onPostInit);

        mDeferredNotificationViewUpdateHandler = new Handler();

    }



    private void onPostInit() {

        mGroupAlertTransferHelper.setPendingEntries(mPendingNotifications);

        mGroupManager.addOnGroupChangeListener(mGroupAlertTransferHelper);

    public void setAlertTransferListener(AlertTransferListener listener) {

        mAlertTransferListener = listener;

    }



    /**
@@ -607,7 +602,9 @@ public class NotificationEntryManager implements Dumpable, NotificationInflater. 
                    mVisualStabilityManager.onLowPriorityUpdated(entry);

                    mPresenter.updateNotificationViews();

                }

                mGroupAlertTransferHelper.onInflationFinished(entry);

                if (mAlertTransferListener != null) {

                    mAlertTransferListener.onEntryReinflated(entry);

                }

            }

        }

        entry.setLowPriorityStateUpdated(false);
@@ -620,8 +617,12 @@ public class NotificationEntryManager implements Dumpable, NotificationInflater. 


    private void removeNotificationInternal(String key,

            @Nullable NotificationListenerService.RankingMap ranking, boolean forceRemove) {

        final NotificationData.Entry entry = mNotificationData.get(key);



        abortExistingInflation(key);

        mGroupAlertTransferHelper.cleanUpPendingAlertInfo(key);

        if (mAlertTransferListener != null && entry != null) {

            mAlertTransferListener.onEntryRemoved(entry);

        }



        // Attempt to remove notifications from their alert managers (heads up, ambient pulse).

        // Though the remove itself may fail, it lets the manager know to remove as soon as
@@ -640,8 +641,6 @@ public class NotificationEntryManager implements Dumpable, NotificationInflater. 
            mAmbientPulseManager.removeNotification(key, false /* ignoreEarliestRemovalTime */);

        }



        NotificationData.Entry entry = mNotificationData.get(key);



        if (entry == null) {

            mCallback.onNotificationRemoved(key, null /* old */);

            return;
@@ -866,7 +865,9 @@ public class NotificationEntryManager implements Dumpable, NotificationInflater. 
                mNotificationData.getImportance(key));



        mPendingNotifications.put(key, shadeEntry);

        mGroupAlertTransferHelper.onPendingEntryAdded(shadeEntry);

        if (mAlertTransferListener != null) {

            mAlertTransferListener.onPendingEntryAdded(shadeEntry);

        }

    }



    @VisibleForTesting
@@ -1252,6 +1253,15 @@ public class NotificationEntryManager implements Dumpable, NotificationInflater. 
        }

    }



    /**

     * @return An iterator for all "pending" notifications. Pending notifications are newly-posted

     * notifications whose views have not yet been inflated. In general, the system pretends like

     * these don't exist, although there are a couple exceptions.

     */

    public Iterable<NotificationData.Entry> getPendingNotificationsIterator() {

        return mPendingNotifications.values();

    }



    /**

     * Callback for NotificationEntryManager.

     */

packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationGroupAlertTransferHelper.java

+91 −90
Original line number Diff line number Diff line
@@ -29,7 +29,9 @@ import com.android.systemui.statusbar.AmbientPulseManager.OnAmbientChangedListen 
import com.android.systemui.statusbar.InflationTask;

import com.android.systemui.statusbar.StatusBarStateController;

import com.android.systemui.statusbar.StatusBarStateController.StateListener;

import com.android.systemui.statusbar.notification.AlertTransferListener;

import com.android.systemui.statusbar.notification.NotificationData.Entry;

import com.android.systemui.statusbar.notification.NotificationEntryManager;

import com.android.systemui.statusbar.notification.row.NotificationInflater.AsyncInflationTask;

import com.android.systemui.statusbar.notification.row.NotificationInflater.InflationFlag;

import com.android.systemui.statusbar.phone.NotificationGroupManager.NotificationGroup;
@@ -38,8 +40,6 @@ import com.android.systemui.statusbar.policy.HeadsUpManager; 
import com.android.systemui.statusbar.policy.OnHeadsUpChangedListener;



import java.util.ArrayList;

import java.util.Collection;

import java.util.HashMap;

import java.util.Objects;



/**
@@ -47,8 +47,8 @@ import java.util.Objects; 
 * {@link HeadsUpManager}, {@link AmbientPulseManager}. In particular, this class deals with keeping

 * the correct notification in a group alerting based off the group suppression.

 */

public class NotificationGroupAlertTransferHelper implements OnGroupChangeListener,

        OnHeadsUpChangedListener, OnAmbientChangedListener, StateListener {

public class NotificationGroupAlertTransferHelper implements OnHeadsUpChangedListener,

        OnAmbientChangedListener, StateListener {



    private static final long ALERT_TRANSFER_TIMEOUT = 300;
@@ -69,15 +69,7 @@ public class NotificationGroupAlertTransferHelper implements OnGroupChangeListen 
    private final NotificationGroupManager mGroupManager =

            Dependency.get(NotificationGroupManager.class);



    // TODO(b/119637830): It would be good if GroupManager already had all pending notifications as

    // normal children (i.e. add notifications to GroupManager before inflation) so that we don't

    // have to have this dependency. We'd also have to worry less about the suppression not being up

    // to date.

    /**

     * Notifications that are currently inflating for the first time. Used to remove an incorrectly

     * alerting notification faster.

     */

    private HashMap<String, Entry> mPendingNotifications;

    private NotificationEntryManager mEntryManager;



    private boolean mIsDozing;
@@ -85,6 +77,23 @@ public class NotificationGroupAlertTransferHelper implements OnGroupChangeListen 
        Dependency.get(StatusBarStateController.class).addCallback(this);

    }



    /** Causes the TransferHelper to register itself as a listener to the appropriate classes. */

    public void bind(NotificationEntryManager entryManager,

            NotificationGroupManager groupManager) {

        if (mEntryManager != null) {

            throw new IllegalStateException("Already bound.");

        }



        // TODO(b/119637830): It would be good if GroupManager already had all pending notifications

        // as normal children (i.e. add notifications to GroupManager before inflation) so that we

        // don't have to have this dependency. We'd also have to worry less about the suppression

        // not being up to date.

        mEntryManager = entryManager;



        mEntryManager.setAlertTransferListener(mAlertTransferListener);

        groupManager.addOnGroupChangeListener(mOnGroupChangeListener);

    }



    /**

     * Whether or not a notification has transferred its alert state to the notification and

     * the notification should alert after inflating.
@@ -97,25 +106,10 @@ public class NotificationGroupAlertTransferHelper implements OnGroupChangeListen 
        return alertInfo != null && alertInfo.isStillValid();

    }



    /**

     * Removes any alerts pending on this entry. Note that this will not stop any inflation tasks

     * started by a transfer, so this should only be used as clean-up for when inflation is stopped

     * and the pending alert no longer needs to happen.

     *

     * @param key notification key that may have info that needs to be cleaned up

     */

    public void cleanUpPendingAlertInfo(@NonNull String key) {

        mPendingAlerts.remove(key);

    }



    public void setHeadsUpManager(HeadsUpManager headsUpManager) {

        mHeadsUpManager = headsUpManager;

    }



    public void setPendingEntries(HashMap<String, Entry> pendingNotifications) {

        mPendingNotifications = pendingNotifications;

    }



    @Override

    public void onStateChanged(int newState) {}
@@ -130,6 +124,7 @@ public class NotificationGroupAlertTransferHelper implements OnGroupChangeListen 
        mIsDozing = isDozing;

    }



    private final OnGroupChangeListener mOnGroupChangeListener = new OnGroupChangeListener() {

        @Override

        public void onGroupCreated(NotificationGroup group, String groupKey) {

            mGroupAlertEntries.put(groupKey, new GroupAlertEntry(group));
@@ -167,6 +162,7 @@ public class NotificationGroupAlertTransferHelper implements OnGroupChangeListen 
                }

            }

        }

    };



    @Override

    public void onAmbientStateChanged(Entry entry, boolean isAmbient) {
@@ -185,37 +181,42 @@ public class NotificationGroupAlertTransferHelper implements OnGroupChangeListen 
        }

    }



    /**

     * Called when the entry's reinflation has finished. If there is an alert pending, we then

     * show the alert.

     *

     * @param entry entry whose inflation has finished

     */

    public void onInflationFinished(@NonNull Entry entry) {

    private final AlertTransferListener mAlertTransferListener = new AlertTransferListener() {

        // Called when a new notification has been posted but is not inflated yet. We use this to

        // see as early as we can if we need to abort a transfer.

        @Override

        public void onPendingEntryAdded(Entry entry) {

            String groupKey = mGroupManager.getGroupKey(entry.notification);

            GroupAlertEntry groupAlertEntry = mGroupAlertEntries.get(groupKey);

            if (groupAlertEntry != null) {

                checkShouldTransferBack(groupAlertEntry);

            }

        }



        // Called when the entry's reinflation has finished. If there is an alert pending, we

        // then show the alert.

        @Override

        public void onEntryReinflated(Entry entry) {

            PendingAlertInfo alertInfo = mPendingAlerts.remove(entry.key);

            if (alertInfo != null) {

                if (alertInfo.isStillValid()) {

                    alertNotificationWhenPossible(entry, getActiveAlertManager());

                } else {

                    // The transfer is no longer valid. Free the content.

                entry.getRow().freeContentViewWhenSafe(alertInfo.mAlertManager.getContentFlag());

                    entry.getRow().freeContentViewWhenSafe(

                            alertInfo.mAlertManager.getContentFlag());

                }

            }

        }



    /**

     * Called when a new notification has been posted but is not inflated yet. We use this to see

     * as early as we can if we need to abort a transfer.

     *

     * @param entry entry that has been added

     */

    public void onPendingEntryAdded(@NonNull Entry entry) {

        String groupKey = mGroupManager.getGroupKey(entry.notification);

        GroupAlertEntry groupAlertEntry = mGroupAlertEntries.get(groupKey);

        if (groupAlertEntry != null) {

            checkShouldTransferBack(groupAlertEntry);

        }

        @Override

        public void onEntryRemoved(Entry entry) {

            // Removes any alerts pending on this entry. Note that this will not stop any inflation

            // tasks started by a transfer, so this should only be used as clean-up for when

            // inflation is stopped and the pending alert no longer needs to happen.

            mPendingAlerts.remove(entry.key);

        }

    };



    /**

     * Gets the number of new notifications pending inflation that will be added to the group
@@ -225,11 +226,11 @@ public class NotificationGroupAlertTransferHelper implements OnGroupChangeListen 
     * @return the number of new notifications that will be added to the group

     */

    private int getPendingChildrenNotAlerting(@NonNull NotificationGroup group) {

        if (mPendingNotifications == null) {

        if (mEntryManager == null) {

            return 0;

        }

        int number = 0;

        Collection<Entry> values = mPendingNotifications.values();

        Iterable<Entry> values = mEntryManager.getPendingNotificationsIterator();

        for (Entry entry : values) {

            if (isPendingNotificationInGroup(entry, group) && onlySummaryAlerts(entry)) {

                number++;
@@ -245,10 +246,10 @@ public class NotificationGroupAlertTransferHelper implements OnGroupChangeListen 
     * @return true if a pending notification will add to this group

     */

    private boolean pendingInflationsWillAddChildren(@NonNull NotificationGroup group) {

        if (mPendingNotifications == null) {

        if (mEntryManager == null) {

            return false;

        }

        Collection<Entry> values = mPendingNotifications.values();

        Iterable<Entry> values = mEntryManager.getPendingNotificationsIterator();

        for (Entry entry : values) {

            if (isPendingNotificationInGroup(entry, group)) {

                return true;

packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBar.java

+2 −0
Original line number Diff line number Diff line
@@ -628,6 +628,8 @@ public class StatusBar extends SystemUI implements DemoMode, 
        mBubbleController = Dependency.get(BubbleController.class);

        mBubbleController.setExpandListener(mBubbleExpandListener);



        mGroupAlertTransferHelper.bind(mEntryManager, mGroupManager);



        mColorExtractor.addOnColorsChangedListener(this);

        mStatusBarStateController.addCallback(this, StatusBarStateController.RANK_STATUS_BAR);

packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationGroupAlertTransferHelperTest.java

+21 −10
Original line number Diff line number Diff line
@@ -32,14 +32,19 @@ import android.testing.TestableLooper; 


import com.android.systemui.SysuiTestCase;

import com.android.systemui.statusbar.AmbientPulseManager;

import com.android.systemui.statusbar.notification.AlertTransferListener;

import com.android.systemui.statusbar.notification.NotificationData;

import com.android.systemui.statusbar.notification.NotificationData.Entry;

import com.android.systemui.statusbar.notification.NotificationEntryManager;

import com.android.systemui.statusbar.policy.HeadsUpManager;



import org.junit.Before;

import org.junit.Rule;

import org.junit.Test;

import org.junit.runner.RunWith;

import org.mockito.ArgumentCaptor;

import org.mockito.Captor;

import org.mockito.Mock;

import org.mockito.junit.MockitoJUnit;

import org.mockito.junit.MockitoRule;
@@ -49,13 +54,15 @@ import java.util.HashMap; 
@RunWith(AndroidTestingRunner.class)

@TestableLooper.RunWithLooper

public class NotificationGroupAlertTransferHelperTest extends SysuiTestCase {

    @Rule

    public MockitoRule rule = MockitoJUnit.rule();

    @Rule public MockitoRule rule = MockitoJUnit.rule();



    private NotificationGroupAlertTransferHelper mGroupAlertTransferHelper;

    private NotificationGroupManager mGroupManager;

    private AmbientPulseManager mAmbientPulseManager;

    private HeadsUpManager mHeadsUpManager;

    @Mock private NotificationEntryManager mNotificationEntryManager;

    @Captor private ArgumentCaptor<AlertTransferListener> mListenerCaptor;

    private AlertTransferListener mAlertTransferListener;

    private final HashMap<String, Entry> mPendingEntries = new HashMap<>();

    private final NotificationGroupTestHelper mGroupTestHelper =

            new NotificationGroupTestHelper(mContext);
@@ -67,15 +74,19 @@ public class NotificationGroupAlertTransferHelperTest extends SysuiTestCase { 
        mDependency.injectTestDependency(AmbientPulseManager.class, mAmbientPulseManager);

        mHeadsUpManager = new HeadsUpManager(mContext) {};



        when(mNotificationEntryManager.getPendingNotificationsIterator())

                .thenReturn(mPendingEntries.values());



        mGroupManager = new NotificationGroupManager();

        mDependency.injectTestDependency(NotificationGroupManager.class, mGroupManager);

        mGroupManager.setHeadsUpManager(mHeadsUpManager);



        mGroupAlertTransferHelper = new NotificationGroupAlertTransferHelper();

        mGroupAlertTransferHelper.setHeadsUpManager(mHeadsUpManager);

        mGroupAlertTransferHelper.setPendingEntries(mPendingEntries);



        mGroupManager.addOnGroupChangeListener(mGroupAlertTransferHelper);

        mGroupAlertTransferHelper.bind(mNotificationEntryManager, mGroupManager);

        verify(mNotificationEntryManager).setAlertTransferListener(mListenerCaptor.capture());

        mAlertTransferListener = mListenerCaptor.getValue();

        mHeadsUpManager.addListener(mGroupAlertTransferHelper);

        mAmbientPulseManager.addListener(mGroupAlertTransferHelper);

    }
@@ -110,7 +121,7 @@ public class NotificationGroupAlertTransferHelperTest extends SysuiTestCase { 


        // Add second child notification so that summary is no longer suppressed.

        mPendingEntries.put(childEntry2.key, childEntry2);

        mGroupAlertTransferHelper.onPendingEntryAdded(childEntry2);

        mAlertTransferListener.onPendingEntryAdded(childEntry2);

        mGroupManager.onEntryAdded(childEntry2);



        // The alert state should transfer back to the summary as there is now more than one
@@ -137,7 +148,7 @@ public class NotificationGroupAlertTransferHelperTest extends SysuiTestCase { 


        // Add second child notification so that summary is no longer suppressed.

        mPendingEntries.put(childEntry2.key, childEntry2);

        mGroupAlertTransferHelper.onPendingEntryAdded(childEntry2);

        mAlertTransferListener.onPendingEntryAdded(childEntry2);

        mGroupManager.onEntryAdded(childEntry2);



        // Dozing changed so no reason to re-alert summary.
@@ -175,7 +186,7 @@ public class NotificationGroupAlertTransferHelperTest extends SysuiTestCase { 


        when(childEntry.getRow().isInflationFlagSet(mHeadsUpManager.getContentFlag()))

            .thenReturn(true);

        mGroupAlertTransferHelper.onInflationFinished(childEntry);

        mAlertTransferListener.onEntryReinflated(childEntry);



        // Alert is immediately removed from summary, and we show child as its content is inflated.

        assertFalse(mHeadsUpManager.isAlerting(summaryEntry.key));
@@ -199,13 +210,13 @@ public class NotificationGroupAlertTransferHelperTest extends SysuiTestCase { 


        // Add second child notification so that summary is no longer suppressed.

        mPendingEntries.put(childEntry2.key, childEntry2);

        mGroupAlertTransferHelper.onPendingEntryAdded(childEntry2);

        mAlertTransferListener.onPendingEntryAdded(childEntry2);

        mGroupManager.onEntryAdded(childEntry2);



        // Child entry finishes its inflation.

        when(childEntry.getRow().isInflationFlagSet(mHeadsUpManager.getContentFlag()))

            .thenReturn(true);

        mGroupAlertTransferHelper.onInflationFinished(childEntry);

        mAlertTransferListener.onEntryReinflated(childEntry);



        verify(childEntry.getRow(), times(1)).freeContentViewWhenSafe(mHeadsUpManager

            .getContentFlag());
@@ -225,7 +236,7 @@ public class NotificationGroupAlertTransferHelperTest extends SysuiTestCase { 
        mGroupManager.onEntryAdded(summaryEntry);

        mGroupManager.onEntryAdded(childEntry);



        mGroupAlertTransferHelper.cleanUpPendingAlertInfo(childEntry.key);

        mAlertTransferListener.onEntryRemoved(childEntry);



        assertFalse(mGroupAlertTransferHelper.isAlertTransferPending(childEntry));

    }