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

3 files changed, 57 insertions, 11 deletions

diff --git a/core/java/com/android/internal/statusbar/NotificationVisibility.java b/core/java/com/android/internal/statusbar/NotificationVisibility.java
index 7fe440cf0b89..ea0344d1dadf 100644
--- a/core/java/com/android/internal/statusbar/NotificationVisibility.java
+++ b/core/java/com/android/internal/statusbar/NotificationVisibility.java
@@ -51,7 +51,7 @@ public class NotificationVisibility implements Parcelable {
     @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 01ec46151b38..8e8e7180a8ab 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 @@ public class NotificationLogger {
         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 @@ public class NotificationLogger {
                     Log.d(TAG, "failed setNotificationsShown: ", e);
                 }
             }
+            recycleAllVisibilityObjects(newlyVisibleAr);
+            recycleAllVisibilityObjects(noLongerVisibleAr);
         });
     }
 
@@ -213,6 +214,28 @@ public class NotificationLogger {
         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 bbb03d78bd03..42bf290c70fd 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 static org.mockito.Mockito.when;
 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.junit.runner.RunWith;
 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 @@ public class NotificationLoggerTest extends SysuiTestCase {
     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 @@ public class NotificationLoggerTest extends SysuiTestCase {
 
     @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);