From 5a0be52d3db2398cfe01d18adf779d9e443edf82 Mon Sep 17 00:00:00 2001
From: Alan Newberger <alann@google.com>
Date: Thu, 9 Apr 2015 10:42:06 -0700
Subject: [PATCH] Fix view recycling in filmstrip

View recycling is pretty broken, with code put in for the old
camera preview in filmstrip being used to avoid recycling in all
cases. This looks like its been broken in a number of releases, but
less so due to a destructive removal of views during updates. I had
fixed that but it then exposed the issue that recycling wasn't occurring
elsewhere. This CL removes views from hierarchy, confirmed no more leaks
when capturing and when swiping through filmstrip.

Bug: 19970885
Change-Id: Ic63b9231bd24db0f44a99567278886eb0998d740
---
 src/com/android/camera/widget/FilmstripView.java | 40 ++++++++++++++----------
 1 file changed, 23 insertions(+), 17 deletions(-)

diff --git a/src/com/android/camera/widget/FilmstripView.java b/src/com/android/camera/widget/FilmstripView.java
index 6d0d4fe04..f6d56c52f 100644
--- a/src/com/android/camera/widget/FilmstripView.java
+++ b/src/com/android/camera/widget/FilmstripView.java
@@ -191,6 +191,10 @@ public class FilmstripView extends ViewGroup {
             mView.setPivotY(0f);
         }
 
+        public FilmstripItem getData() {
+            return mData;
+        }
+
         public void setData(FilmstripItem item) {
             mData = item;
 
@@ -426,20 +430,12 @@ public class FilmstripView extends ViewGroup {
         }
 
         /**
-         * Removes from the hierarchy. Keeps the view in the view hierarchy if
-         * view type is {@code VIEW_TYPE_STICKY} and set to invisible instead.
-         *
-         * @param force {@code true} to remove the view from the hierarchy
-         *                          regardless of the view type.
+         * Removes from the hierarchy.
          */
-        public void removeViewFromHierarchy(boolean force) {
-            if (force) {
-                mFilmstrip.removeView(mView);
-                mData.recycle(mView);
-                mFilmstrip.recycleView(mView, mIndex);
-            } else {
-                setVisibility(View.INVISIBLE);
-            }
+        public void removeViewFromHierarchy() {
+            mFilmstrip.removeView(mView);
+            mData.recycle(mView);
+            mFilmstrip.recycleView(mView, mIndex);
         }
 
         /**
@@ -680,6 +676,7 @@ public class FilmstripView extends ViewGroup {
     }
 
     private void recycleView(View view, int index) {
+        Log.v(TAG, "recycleView");
         final int viewType = (Integer) view.getTag(R.id.mediadata_tag_viewtype);
         if (viewType > 0) {
             Queue<View> recycledViewsForType = recycledViews.get(viewType);
@@ -701,6 +698,7 @@ public class FilmstripView extends ViewGroup {
         if (view != null) {
             view.setVisibility(View.GONE);
         }
+        Log.v(TAG, "getRecycledView, recycled=" + (view != null));
         return view;
     }
 
@@ -892,7 +890,7 @@ public class FilmstripView extends ViewGroup {
             Log.w(TAG, "removeItem() - Trying to remove a null item!");
             return;
         }
-        mViewItems[bufferIndex].removeViewFromHierarchy(false);
+        mViewItems[bufferIndex].removeViewFromHierarchy();
         mViewItems[bufferIndex] = null;
     }
 
@@ -1444,7 +1442,7 @@ public class FilmstripView extends ViewGroup {
         postDelayed(new Runnable() {
             @Override
             public void run() {
-                removedItem.removeViewFromHierarchy(false);
+                removedItem.removeViewFromHierarchy();
             }
         }, GEOMETRY_ADJUST_TIME_MS);
 
@@ -1673,10 +1671,18 @@ public class FilmstripView extends ViewGroup {
             return;
         }
 
+        FilmstripItem oldFilmstripItem = item.getData();
+
         // In case the underlying data item is changed (commonly from
         // SessionItem to PhotoItem for an image requiring processing), set the
         // new FilmstripItem on the ViewItem
-        item.setData(filmstripItem);
+        if (!filmstripItem.equals(oldFilmstripItem)) {
+            oldFilmstripItem.recycle(item.getView());
+            item.setData(filmstripItem);
+            Log.v(TAG, "updateViewItem() - recycling old data item and setting new");
+        } else {
+            Log.v(TAG, "updateViewItem() - updating data with the same item");
+        }
 
         // In case state changed from a new FilmStripItem or the existing one,
         // redraw the View contents. We call getView here as it will refill the
@@ -1799,7 +1805,7 @@ public class FilmstripView extends ViewGroup {
             if (mViewItems[i] == null) {
                 continue;
             }
-            mViewItems[i].removeViewFromHierarchy(false);
+            mViewItems[i].removeViewFromHierarchy();
         }
 
         // Clear out the mViewItems and rebuild with camera in the center.
-- 
2.11.0