From 2a68270c76def0b635bd1583e9c1817d09dff969 Mon Sep 17 00:00:00 2001
From: Steve Elliott <steell@google.com>
Date: Tue, 15 Feb 2022 00:18:13 -0500
Subject: [PATCH] Defer MessagingGroup#recycle until bind completes

There is a "race condition" where a MessagingGroup is recycled while the
contents of that group are being used elsewhere in the
ConversationLayout, namely in the "image message container" used to show
the most recent image message in the collapsed state.

This state isn't cleared until later in the bind() process, which
unfortunately depends on the groups being updated. For that reason, we
defer all synchronous calls to recycle() until the end of bind(), which
will ensure that the old groups are still around as long as necessary.

Fixes: 212689920
Test: manual
Change-Id: Idef815d54690544615512bd2bd1006f172403e18
---
 .../internal/widget/ConversationLayout.java   | 21 ++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/core/java/com/android/internal/widget/ConversationLayout.java b/core/java/com/android/internal/widget/ConversationLayout.java
index 78bb53d335393..5fa4a652e1766 100644
--- a/core/java/com/android/internal/widget/ConversationLayout.java
+++ b/core/java/com/android/internal/widget/ConversationLayout.java
@@ -150,6 +150,7 @@ public class ConversationLayout extends FrameLayout
     private Icon mShortcutIcon;
     private View mAppNameDivider;
     private TouchDelegateComposite mTouchDelegate = new TouchDelegateComposite(this);
+    private ArrayList<MessagingGroup> mToRecycle = new ArrayList<>();
 
     public ConversationLayout(@NonNull Context context) {
         super(context);
@@ -472,6 +473,12 @@ public class ConversationLayout extends FrameLayout
         updateTitleAndNamesDisplay();
 
         updateConversationLayout();
+
+        // Recycle everything at the end of the update, now that we know it's no longer needed.
+        for (MessagingGroup group : mToRecycle) {
+            group.recycle();
+        }
+        mToRecycle.clear();
     }
 
     /**
@@ -745,18 +752,18 @@ public class ConversationLayout extends FrameLayout
             MessagingGroup group = oldGroups.get(i);
             if (!mGroups.contains(group)) {
                 List<MessagingMessage> messages = group.getMessages();
-                Runnable endRunnable = () -> {
-                    mMessagingLinearLayout.removeTransientView(group);
-                    group.recycle();
-                };
-
                 boolean wasShown = group.isShown();
                 mMessagingLinearLayout.removeView(group);
                 if (wasShown && !MessagingLinearLayout.isGone(group)) {
                     mMessagingLinearLayout.addTransientView(group, 0);
-                    group.removeGroupAnimated(endRunnable);
+                    group.removeGroupAnimated(() -> {
+                        mMessagingLinearLayout.removeTransientView(group);
+                        group.recycle();
+                    });
                 } else {
-                    endRunnable.run();
+                    // Defer recycling until after the update is done, since we may still need the
+                    // old group around to perform other updates.
+                    mToRecycle.add(group);
                 }
                 mMessages.removeAll(messages);
                 mHistoricMessages.removeAll(messages);