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

Commit 2e89e8d8 authored by Chris Wren's avatar Chris Wren
Browse files

clone the visibility objects for the handler thread

The main thread was recycling the objects before the hander could
pack up the binder call.

Change-Id: I4289bdcc5b940a0a8209fdd5d3df47972de0fa4b
Fixes: 72953296
Test: atest com.android.notification.functional.NotificationInteractionTests#testNotificationShadeMetrics
parent 6b8014f5
Loading
Loading
Loading
Loading
+27 −4
Original line number Original line Diff line number Diff line
@@ -176,10 +176,9 @@ public class NotificationLogger {
        if (newlyVisible.isEmpty() && noLongerVisible.isEmpty()) {
        if (newlyVisible.isEmpty() && noLongerVisible.isEmpty()) {
            return;
            return;
        }
        }
        NotificationVisibility[] newlyVisibleAr =
        final NotificationVisibility[] newlyVisibleAr = cloneVisibilitiesAsArr(newlyVisible);
                newlyVisible.toArray(new NotificationVisibility[newlyVisible.size()]);
        final NotificationVisibility[] noLongerVisibleAr = cloneVisibilitiesAsArr(noLongerVisible);
        NotificationVisibility[] noLongerVisibleAr =

                noLongerVisible.toArray(new NotificationVisibility[noLongerVisible.size()]);
        mUiOffloadThread.submit(() -> {
        mUiOffloadThread.submit(() -> {
            try {
            try {
                mBarService.onNotificationVisibilityChanged(newlyVisibleAr, noLongerVisibleAr);
                mBarService.onNotificationVisibilityChanged(newlyVisibleAr, noLongerVisibleAr);
@@ -202,6 +201,8 @@ public class NotificationLogger {
                    Log.d(TAG, "failed setNotificationsShown: ", e);
                    Log.d(TAG, "failed setNotificationsShown: ", e);
                }
                }
            }
            }
            recycleAllVisibilityObjects(newlyVisibleAr);
            recycleAllVisibilityObjects(noLongerVisibleAr);
        });
        });
    }
    }


@@ -213,6 +214,28 @@ public class NotificationLogger {
        array.clear();
        array.clear();
    }
    }


    private void recycleAllVisibilityObjects(NotificationVisibility[] array) {
        final int N = array.length;
        for (int i = 0 ; i < N; i++) {
            if (array[i] != null) {
                array[i].recycle();
            }
        }
    }

    private NotificationVisibility[] cloneVisibilitiesAsArr(Collection<NotificationVisibility> c) {

        final NotificationVisibility[] array = new NotificationVisibility[c.size()];
        int i = 0;
        for(NotificationVisibility nv: c) {
            if (nv != null) {
                array[i] = nv.clone();
            }
            i++;
        }
        return array;
    }

    @VisibleForTesting
    @VisibleForTesting
    public Runnable getVisibilityReporter() {
    public Runnable getVisibilityReporter() {
        return mVisibilityReporter;
        return mVisibilityReporter;
+29 −6
Original line number Original line Diff line number Diff line
@@ -16,7 +16,9 @@


package com.android.systemui.statusbar;
package com.android.systemui.statusbar;


import static org.junit.Assert.assertArrayEquals;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verify;
@@ -25,6 +27,7 @@ import static org.mockito.Mockito.when;
import android.app.Notification;
import android.app.Notification;
import android.os.Handler;
import android.os.Handler;
import android.os.Looper;
import android.os.Looper;
import android.os.RemoteException;
import android.os.UserHandle;
import android.os.UserHandle;
import android.service.notification.StatusBarNotification;
import android.service.notification.StatusBarNotification;
import android.support.test.filters.SmallTest;
import android.support.test.filters.SmallTest;
@@ -43,6 +46,9 @@ import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
import org.mockito.MockitoAnnotations;
import org.mockito.stubbing.Answer;

import java.util.concurrent.ConcurrentLinkedQueue;


@SmallTest
@SmallTest
@RunWith(AndroidTestingRunner.class)
@RunWith(AndroidTestingRunner.class)
@@ -64,9 +70,10 @@ public class NotificationLoggerTest extends SysuiTestCase {
    private NotificationData.Entry mEntry;
    private NotificationData.Entry mEntry;
    private StatusBarNotification mSbn;
    private StatusBarNotification mSbn;
    private TestableNotificationLogger mLogger;
    private TestableNotificationLogger mLogger;
    private ConcurrentLinkedQueue<AssertionError> mErrorQueue = new ConcurrentLinkedQueue<>();


    @Before
    @Before
    public void setUp() {
    public void setUp() throws RemoteException {
        MockitoAnnotations.initMocks(this);
        MockitoAnnotations.initMocks(this);
        mDependency.injectTestDependency(NotificationEntryManager.class, mEntryManager);
        mDependency.injectTestDependency(NotificationEntryManager.class, mEntryManager);
        mDependency.injectTestDependency(NotificationListener.class, mListener);
        mDependency.injectTestDependency(NotificationListener.class, mListener);
@@ -84,17 +91,33 @@ public class NotificationLoggerTest extends SysuiTestCase {


    @Test
    @Test
    public void testOnChildLocationsChangedReportsVisibilityChanged() throws Exception {
    public void testOnChildLocationsChangedReportsVisibilityChanged() throws Exception {
        NotificationVisibility[] newlyVisibleKeys = {
                NotificationVisibility.obtain(mEntry.key, 0, 1, true)
        };
        NotificationVisibility[] noLongerVisibleKeys = {};
        doAnswer((Answer) invocation -> {
                    try {
                        assertArrayEquals(newlyVisibleKeys,
                                (NotificationVisibility[]) invocation.getArguments()[0]);
                        assertArrayEquals(noLongerVisibleKeys,
                                (NotificationVisibility[]) invocation.getArguments()[1]);
                    } catch (AssertionError error) {
                        mErrorQueue.offer(error);
                    }
                    return null;
                }
        ).when(mBarService).onNotificationVisibilityChanged(any(NotificationVisibility[].class),
                any(NotificationVisibility[].class));

        when(mListContainer.isInVisibleLocation(any())).thenReturn(true);
        when(mListContainer.isInVisibleLocation(any())).thenReturn(true);
        when(mNotificationData.getActiveNotifications()).thenReturn(Lists.newArrayList(mEntry));
        when(mNotificationData.getActiveNotifications()).thenReturn(Lists.newArrayList(mEntry));
        mLogger.getChildLocationsChangedListenerForTest().onChildLocationsChanged();
        mLogger.getChildLocationsChangedListenerForTest().onChildLocationsChanged();
        TestableLooper.get(this).processAllMessages();
        TestableLooper.get(this).processAllMessages();
        waitForUiOffloadThread();
        waitForUiOffloadThread();


        NotificationVisibility[] newlyVisibleKeys = {
        if(!mErrorQueue.isEmpty()) {
                NotificationVisibility.obtain(mEntry.key, 0, 1, true)
            throw mErrorQueue.poll();
        };
        }
        NotificationVisibility[] noLongerVisibleKeys = {};
        verify(mBarService).onNotificationVisibilityChanged(newlyVisibleKeys, noLongerVisibleKeys);


        // |mEntry| won't change visibility, so it shouldn't be reported again:
        // |mEntry| won't change visibility, so it shouldn't be reported again:
        Mockito.reset(mBarService);
        Mockito.reset(mBarService);
+1 −1

File changed.

Contains only whitespace changes.