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
diff --git a/core/java/com/android/internal/statusbar/NotificationVisibility.java b/core/java/com/android/internal/statusbar/NotificationVisibility.java
index 7fe440c..ea0344d 100644
--- a/core/java/com/android/internal/statusbar/NotificationVisibility.java
+++ b/core/java/com/android/internal/statusbar/NotificationVisibility.java
@@ -51,7 +51,7 @@
@Override
public String toString() {
return "NotificationVisibility(id=" + id
- + "key=" + key
+ + " key=" + key
+ " rank=" + rank
+ " count=" + count
+ (visible?" visible":"")
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLogger.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLogger.java
index 01ec461..8e8e718 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLogger.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLogger.java
@@ -176,10 +176,9 @@
if (newlyVisible.isEmpty() && noLongerVisible.isEmpty()) {
return;
}
- NotificationVisibility[] newlyVisibleAr =
- newlyVisible.toArray(new NotificationVisibility[newlyVisible.size()]);
- NotificationVisibility[] noLongerVisibleAr =
- noLongerVisible.toArray(new NotificationVisibility[noLongerVisible.size()]);
+ final NotificationVisibility[] newlyVisibleAr = cloneVisibilitiesAsArr(newlyVisible);
+ final NotificationVisibility[] noLongerVisibleAr = cloneVisibilitiesAsArr(noLongerVisible);
+
mUiOffloadThread.submit(() -> {
try {
mBarService.onNotificationVisibilityChanged(newlyVisibleAr, noLongerVisibleAr);
@@ -202,6 +201,8 @@
Log.d(TAG, "failed setNotificationsShown: ", e);
}
}
+ recycleAllVisibilityObjects(newlyVisibleAr);
+ recycleAllVisibilityObjects(noLongerVisibleAr);
});
}
@@ -213,6 +214,28 @@
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
public Runnable getVisibilityReporter() {
return mVisibilityReporter;
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationLoggerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationLoggerTest.java
index bbb03d7..42bf290 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationLoggerTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationLoggerTest.java
@@ -16,7 +16,9 @@
package com.android.systemui.statusbar;
+import static org.junit.Assert.assertArrayEquals;
import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
@@ -25,6 +27,7 @@
import android.app.Notification;
import android.os.Handler;
import android.os.Looper;
+import android.os.RemoteException;
import android.os.UserHandle;
import android.service.notification.StatusBarNotification;
import android.support.test.filters.SmallTest;
@@ -43,6 +46,9 @@
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
+import org.mockito.stubbing.Answer;
+
+import java.util.concurrent.ConcurrentLinkedQueue;
@SmallTest
@RunWith(AndroidTestingRunner.class)
@@ -64,9 +70,10 @@
private NotificationData.Entry mEntry;
private StatusBarNotification mSbn;
private TestableNotificationLogger mLogger;
+ private ConcurrentLinkedQueue<AssertionError> mErrorQueue = new ConcurrentLinkedQueue<>();
@Before
- public void setUp() {
+ public void setUp() throws RemoteException {
MockitoAnnotations.initMocks(this);
mDependency.injectTestDependency(NotificationEntryManager.class, mEntryManager);
mDependency.injectTestDependency(NotificationListener.class, mListener);
@@ -84,17 +91,33 @@
@Test
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(mNotificationData.getActiveNotifications()).thenReturn(Lists.newArrayList(mEntry));
mLogger.getChildLocationsChangedListenerForTest().onChildLocationsChanged();
TestableLooper.get(this).processAllMessages();
waitForUiOffloadThread();
- NotificationVisibility[] newlyVisibleKeys = {
- NotificationVisibility.obtain(mEntry.key, 0, 1, true)
- };
- NotificationVisibility[] noLongerVisibleKeys = {};
- verify(mBarService).onNotificationVisibilityChanged(newlyVisibleKeys, noLongerVisibleKeys);
+ if(!mErrorQueue.isEmpty()) {
+ throw mErrorQueue.poll();
+ }
// |mEntry| won't change visibility, so it shouldn't be reported again:
Mockito.reset(mBarService);