Set up view state before attaching, jump drawables if needed

View attachment calls jumpDrawablesToCurrentState(), so the view state
needs to be set up prior to attachment. For views that are already
attached but are being moved to a new position, manually jump.

Cleans up comments in methods that were modified.

Bug: 29978498
Change-Id: Ica27b2c60ad7ee98b9d1e4912c4f8b8c248af88d
(cherry picked from commit 26489e1688633ee270ff1469d0df38c90bbdf674)

3 files changed, 136 insertions, 128 deletions

diff --git a/core/java/android/widget/AbsListView.java b/core/java/android/widget/AbsListView.java
index b331be72b4cf..5ba1e9d03538 100644
--- a/core/java/android/widget/AbsListView.java
+++ b/core/java/android/widget/AbsListView.java
@@ -2315,22 +2315,24 @@ public abstract class AbsListView extends AdapterView<ListAdapter> implements Te
     }
 
     /**
-     * Get a view and have it show the data associated with the specified
-     * position. This is called when we have already discovered that the view is
-     * not available for reuse in the recycle bin. The only choices left are
+     * Gets a view and have it show the data associated with the specified
+     * position. This is called when we have already discovered that the view
+     * is not available for reuse in the recycle bin. The only choices left are
      * converting an old view or making a new one.
      *
-     * @param position The position to display
-     * @param isScrap Array of at least 1 boolean, the first entry will become true if
-     *                the returned view was taken from the "temporary detached" scrap heap, false if
-     *                otherwise.
+     * @param position the position to display
+     * @param outMetadata an array of at least 1 boolean where the first entry
+     *                    will be set {@code true} if the view is currently
+     *                    attached to the window, {@code false} otherwise (e.g.
+     *                    newly-inflated or remained scrap for multiple layout
+     *                    passes)
      *
      * @return A view displaying the data associated with the specified position
      */
-    View obtainView(int position, boolean[] isScrap) {
+    View obtainView(int position, boolean[] outMetadata) {
         Trace.traceBegin(Trace.TRACE_TAG_VIEW, "obtainView");
 
-        isScrap[0] = false;
+        outMetadata[0] = false;
 
         // Check whether we have a transient state view. Attempt to re-bind the
         // data and discard the view if we fail.
@@ -2349,7 +2351,7 @@ public abstract class AbsListView extends AdapterView<ListAdapter> implements Te
                 }
             }
 
-            isScrap[0] = true;
+            outMetadata[0] = true;
 
             // Finish the temporary detach started in addScrapView().
             transientView.dispatchFinishTemporaryDetach();
@@ -2362,19 +2364,11 @@ public abstract class AbsListView extends AdapterView<ListAdapter> implements Te
             if (child != scrapView) {
                 // Failed to re-bind the data, return scrap to the heap.
                 mRecycler.addScrapView(scrapView, position);
-            } else {
-                if (child.isTemporarilyDetached()) {
-                    isScrap[0] = true;
-
-                    // Finish the temporary detach started in addScrapView().
-                    child.dispatchFinishTemporaryDetach();
-                } else {
-                    // we set isScrap to "true" only if the view is temporarily detached.
-                    // if the view is fully detached, it is as good as a view created by the
-                    // adapter
-                    isScrap[0] = false;
-                }
+            } else if (child.isTemporarilyDetached()) {
+                outMetadata[0] = true;
 
+                // Finish the temporary detach started in addScrapView().
+                child.dispatchFinishTemporaryDetach();
             }
         }
 
diff --git a/core/java/android/widget/GridView.java b/core/java/android/widget/GridView.java
index 6d7313da0043..b95aa52c11e9 100644
--- a/core/java/android/widget/GridView.java
+++ b/core/java/android/widget/GridView.java
@@ -1407,72 +1407,73 @@ public class GridView extends AbsListView {
 
 
     /**
-     * Obtain the view and add it to our list of children. The view can be made
-     * fresh, converted from an unused view, or used as is if it was in the
-     * recycle bin.
+     * Obtains the view and adds it to our list of children. The view can be
+     * made fresh, converted from an unused view, or used as is if it was in
+     * the recycle bin.
      *
-     * @param position Logical position in the list
-     * @param y Top or bottom edge of the view to add
-     * @param flow if true, align top edge to y. If false, align bottom edge to
-     *        y.
-     * @param childrenLeft Left edge where children should be positioned
-     * @param selected Is this position selected?
-     * @param where to add new item in the list
+     * @param position logical position in the list
+     * @param y top or bottom edge of the view to add
+     * @param flow {@code true} to align top edge to y, {@code false} to align
+     *             bottom edge to y
+     * @param childrenLeft left edge where children should be positioned
+     * @param selected {@code true} if the position is selected, {@code false}
+     *                 otherwise
+     * @param where position at which to add new item in the list
      * @return View that was added
      */
     private View makeAndAddView(int position, int y, boolean flow, int childrenLeft,
             boolean selected, int where) {
-        View child;
-
         if (!mDataChanged) {
             // Try to use an existing view for this position
-            child = mRecycler.getActiveView(position);
-            if (child != null) {
+            final View activeView = mRecycler.getActiveView(position);
+            if (activeView != null) {
                 // Found it -- we're using an existing child
                 // This just needs to be positioned
-                setupChild(child, position, y, flow, childrenLeft, selected, true, where);
-                return child;
+                setupChild(activeView, position, y, flow, childrenLeft, selected, true, where);
+                return activeView;
             }
         }
 
         // Make a new view for this position, or convert an unused view if
-        // possible
-        child = obtainView(position, mIsScrap);
+        // possible.
+        final View child = obtainView(position, mIsScrap);
 
-        // This needs to be positioned and measured
+        // This needs to be positioned and measured.
         setupChild(child, position, y, flow, childrenLeft, selected, mIsScrap[0], where);
 
         return child;
     }
 
     /**
-     * Add a view as a child and make sure it is measured (if necessary) and
+     * Adds a view as a child and make sure it is measured (if necessary) and
      * positioned properly.
      *
-     * @param child The view to add
-     * @param position The position of the view
-     * @param y The y position relative to which this view will be positioned
-     * @param flow if true, align top edge to y. If false, align bottom edge
-     *        to y.
-     * @param childrenLeft Left edge where children should be positioned
-     * @param selected Is this position selected?
-     * @param recycled Has this view been pulled from the recycle bin? If so it
-     *        does not need to be remeasured.
-     * @param where Where to add the item in the list
+     * @param child the view to add
+     * @param position the position of this child
+     * @param y the y position relative to which this view will be positioned
+     * @param flowDown {@code true} to align top edge to y, {@code false} to
+     *                 align bottom edge to y
+     * @param childrenLeft left edge where children should be positioned
+     * @param selected {@code true} if the position is selected, {@code false}
+     *                 otherwise
+     * @param isAttachedToWindow {@code true} if the view is already attached
+     *                           to the window, e.g. whether it was reused, or
+     *                           {@code false} otherwise
+     * @param where position at which to add new item in the list
      *
      */
-    private void setupChild(View child, int position, int y, boolean flow, int childrenLeft,
-            boolean selected, boolean recycled, int where) {
+    private void setupChild(View child, int position, int y, boolean flowDown, int childrenLeft,
+            boolean selected, boolean isAttachedToWindow, int where) {
         Trace.traceBegin(Trace.TRACE_TAG_VIEW, "setupGridItem");
 
         boolean isSelected = selected && shouldShowSelector();
         final boolean updateChildSelected = isSelected != child.isSelected();
         final int mode = mTouchMode;
-        final boolean isPressed = mode > TOUCH_MODE_DOWN && mode < TOUCH_MODE_SCROLL &&
-                mMotionPosition == position;
+        final boolean isPressed = mode > TOUCH_MODE_DOWN && mode < TOUCH_MODE_SCROLL
+                && mMotionPosition == position;
         final boolean updateChildPressed = isPressed != child.isPressed();
-        
-        boolean needToMeasure = !recycled || updateChildSelected || child.isLayoutRequested();
+        final boolean needToMeasure = !isAttachedToWindow || updateChildSelected
+                || child.isLayoutRequested();
 
         // Respect layout params that are already in the view. Otherwise make
         // some up...
@@ -1483,13 +1484,9 @@ public class GridView extends AbsListView {
         p.viewType = mAdapter.getItemViewType(position);
         p.isEnabled = mAdapter.isEnabled(position);
 
-        if (recycled && !p.forceAdd) {
-            attachViewToParent(child, where, p);
-        } else {
-            p.forceAdd = false;
-            addViewInLayout(child, where, p, true);
-        }
-
+        // Set up view state before attaching the view, since we may need to
+        // rely on the jumpDrawablesToCurrentState() call that occurs as part
+        // of view attachment.
         if (updateChildSelected) {
             child.setSelected(isSelected);
             if (isSelected) {
@@ -1510,6 +1507,21 @@ public class GridView extends AbsListView {
             }
         }
 
+        if (isAttachedToWindow && !p.forceAdd) {
+            attachViewToParent(child, where, p);
+
+            // If the view isn't attached, or if it's attached but for a different
+            // position, then jump the drawables.
+            if (!isAttachedToWindow
+                    || (((AbsListView.LayoutParams) child.getLayoutParams()).scrappedFromPosition)
+                            != position) {
+                child.jumpDrawablesToCurrentState();
+            }
+        } else {
+            p.forceAdd = false;
+            addViewInLayout(child, where, p, true);
+        }
+
         if (needToMeasure) {
             int childHeightSpec = ViewGroup.getChildMeasureSpec(
                     MeasureSpec.makeMeasureSpec(0, MeasureSpec.UNSPECIFIED), 0, p.height);
@@ -1525,7 +1537,7 @@ public class GridView extends AbsListView {
         final int h = child.getMeasuredHeight();
 
         int childLeft;
-        final int childTop = flow ? y : y - h;
+        final int childTop = flowDown ? y : y - h;
 
         final int layoutDirection = getLayoutDirection();
         final int absoluteGravity = Gravity.getAbsoluteGravity(mGravity, layoutDirection);
@@ -1553,15 +1565,10 @@ public class GridView extends AbsListView {
             child.offsetTopAndBottom(childTop - child.getTop());
         }
 
-        if (mCachingStarted) {
+        if (mCachingStarted && !child.isDrawingCacheEnabled()) {
             child.setDrawingCacheEnabled(true);
         }
 
-        if (recycled && (((AbsListView.LayoutParams)child.getLayoutParams()).scrappedFromPosition)
-                != position) {
-            child.jumpDrawablesToCurrentState();
-        }
-
         Trace.traceEnd(Trace.TRACE_TAG_VIEW);
     }
 
diff --git a/core/java/android/widget/ListView.java b/core/java/android/widget/ListView.java
index 0e04e3003d7c..b8b7c553a1db 100644
--- a/core/java/android/widget/ListView.java
+++ b/core/java/android/widget/ListView.java
@@ -1939,72 +1939,73 @@ public class ListView extends AbsListView {
     }
 
     /**
-     * Obtain the view and add it to our list of children. The view can be made
-     * fresh, converted from an unused view, or used as is if it was in the
-     * recycle bin.
+     * Obtains the view and adds it to our list of children. The view can be
+     * made fresh, converted from an unused view, or used as is if it was in
+     * the recycle bin.
      *
-     * @param position Logical position in the list
-     * @param y Top or bottom edge of the view to add
-     * @param flow If flow is true, align top edge to y. If false, align bottom
-     *        edge to y.
-     * @param childrenLeft Left edge where children should be positioned
-     * @param selected Is this position selected?
-     * @return View that was added
+     * @param position logical position in the list
+     * @param y top or bottom edge of the view to add
+     * @param flow {@code true} to align top edge to y, {@code false} to align
+     *             bottom edge to y
+     * @param childrenLeft left edge where children should be positioned
+     * @param selected {@code true} if the position is selected, {@code false}
+     *                 otherwise
+     * @return the view that was added
      */
     private View makeAndAddView(int position, int y, boolean flow, int childrenLeft,
             boolean selected) {
-        View child;
-
-
         if (!mDataChanged) {
-            // Try to use an existing view for this position
-            child = mRecycler.getActiveView(position);
-            if (child != null) {
-                // Found it -- we're using an existing child
-                // This just needs to be positioned
-                setupChild(child, position, y, flow, childrenLeft, selected, true);
-
-                return child;
+            // Try to use an existing view for this position.
+            final View activeView = mRecycler.getActiveView(position);
+            if (activeView != null) {
+                // Found it. We're reusing an existing child, so it just needs
+                // to be positioned like a scrap view.
+                setupChild(activeView, position, y, flow, childrenLeft, selected, true);
+                return activeView;
             }
         }
 
-        // Make a new view for this position, or convert an unused view if possible
-        child = obtainView(position, mIsScrap);
+        // Make a new view for this position, or convert an unused view if
+        // possible.
+        final View child = obtainView(position, mIsScrap);
 
-        // This needs to be positioned and measured
+        // This needs to be positioned and measured.
         setupChild(child, position, y, flow, childrenLeft, selected, mIsScrap[0]);
 
         return child;
     }
 
     /**
-     * Add a view as a child and make sure it is measured (if necessary) and
+     * Adds a view as a child and make sure it is measured (if necessary) and
      * positioned properly.
      *
-     * @param child The view to add
-     * @param position The position of this child
-     * @param y The y position relative to which this view will be positioned
-     * @param flowDown If true, align top edge to y. If false, align bottom
-     *        edge to y.
-     * @param childrenLeft Left edge where children should be positioned
-     * @param selected Is this position selected?
-     * @param recycled Has this view been pulled from the recycle bin? If so it
-     *        does not need to be remeasured.
+     * @param child the view to add
+     * @param position the position of this child
+     * @param y the y position relative to which this view will be positioned
+     * @param flowDown {@code true} to align top edge to y, {@code false} to
+     *                 align bottom edge to y
+     * @param childrenLeft left edge where children should be positioned
+     * @param selected {@code true} if the position is selected, {@code false}
+     *                 otherwise
+     * @param isAttachedToWindow {@code true} if the view is already attached
+     *                           to the window, e.g. whether it was reused, or
+     *                           {@code false} otherwise
      */
     private void setupChild(View child, int position, int y, boolean flowDown, int childrenLeft,
-            boolean selected, boolean recycled) {
+            boolean selected, boolean isAttachedToWindow) {
         Trace.traceBegin(Trace.TRACE_TAG_VIEW, "setupListItem");
 
         final boolean isSelected = selected && shouldShowSelector();
         final boolean updateChildSelected = isSelected != child.isSelected();
         final int mode = mTouchMode;
-        final boolean isPressed = mode > TOUCH_MODE_DOWN && mode < TOUCH_MODE_SCROLL &&
-                mMotionPosition == position;
+        final boolean isPressed = mode > TOUCH_MODE_DOWN && mode < TOUCH_MODE_SCROLL
+                && mMotionPosition == position;
         final boolean updateChildPressed = isPressed != child.isPressed();
-        final boolean needToMeasure = !recycled || updateChildSelected || child.isLayoutRequested();
+        final boolean needToMeasure = !isAttachedToWindow || updateChildSelected
+                || child.isLayoutRequested();
 
-        // Respect layout params that are already in the view. Otherwise make some up...
-        // noinspection unchecked
+        // Respect layout params that are already in the view. Otherwise make
+        // some up...
         AbsListView.LayoutParams p = (AbsListView.LayoutParams) child.getLayoutParams();
         if (p == null) {
             p = (AbsListView.LayoutParams) generateDefaultLayoutParams();
@@ -2012,17 +2013,9 @@ public class ListView extends AbsListView {
         p.viewType = mAdapter.getItemViewType(position);
         p.isEnabled = mAdapter.isEnabled(position);
 
-        if ((recycled && !p.forceAdd) || (p.recycledHeaderFooter
-                && p.viewType == AdapterView.ITEM_VIEW_TYPE_HEADER_OR_FOOTER)) {
-            attachViewToParent(child, flowDown ? -1 : 0, p);
-        } else {
-            p.forceAdd = false;
-            if (p.viewType == AdapterView.ITEM_VIEW_TYPE_HEADER_OR_FOOTER) {
-                p.recycledHeaderFooter = true;
-            }
-            addViewInLayout(child, flowDown ? -1 : 0, p, true);
-        }
-
+        // Set up view state before attaching the view, since we may need to
+        // rely on the jumpDrawablesToCurrentState() call that occurs as part
+        // of view attachment.
         if (updateChildSelected) {
             child.setSelected(isSelected);
         }
@@ -2040,6 +2033,25 @@ public class ListView extends AbsListView {
             }
         }
 
+        if ((isAttachedToWindow && !p.forceAdd) || (p.recycledHeaderFooter
+                && p.viewType == AdapterView.ITEM_VIEW_TYPE_HEADER_OR_FOOTER)) {
+            attachViewToParent(child, flowDown ? -1 : 0, p);
+
+            // If the view was previously attached for a different position,
+            // then manually jump the drawables.
+            if (isAttachedToWindow
+                    && (((AbsListView.LayoutParams) child.getLayoutParams()).scrappedFromPosition)
+                            != position) {
+                child.jumpDrawablesToCurrentState();
+            }
+        } else {
+            p.forceAdd = false;
+            if (p.viewType == AdapterView.ITEM_VIEW_TYPE_HEADER_OR_FOOTER) {
+                p.recycledHeaderFooter = true;
+            }
+            addViewInLayout(child, flowDown ? -1 : 0, p, true);
+        }
+
         if (needToMeasure) {
             final int childWidthSpec = ViewGroup.getChildMeasureSpec(mWidthMeasureSpec,
                     mListPadding.left + mListPadding.right, p.width);
@@ -2073,11 +2085,6 @@ public class ListView extends AbsListView {
             child.setDrawingCacheEnabled(true);
         }
 
-        if (recycled && (((AbsListView.LayoutParams)child.getLayoutParams()).scrappedFromPosition)
-                != position) {
-            child.jumpDrawablesToCurrentState();
-        }
-
         Trace.traceEnd(Trace.TRACE_TAG_VIEW);
     }