DO NOT MERGE leanback: fix fastRelayout for extra views added in preLayout Then in post layout pass, fastRelayout() should invalidate after the item that position is inconsistent with Grid position. Previously this was relying on didStructureChange(), it is now no longer safe to do so when prelayout may add items before first child. Bug: 62727612 also no longer reproduce Bug 36738635 after this change Test: testMoveIntoPrelayoutItems Change-Id: If67f6697b21c132ed5934bdc0fc2790ac4aae32c
diff --git a/v17/leanback/src/android/support/v17/leanback/widget/Grid.java b/v17/leanback/src/android/support/v17/leanback/widget/Grid.java index ebfa813..332a7bc 100644 --- a/v17/leanback/src/android/support/v17/leanback/widget/Grid.java +++ b/v17/leanback/src/android/support/v17/leanback/widget/Grid.java
@@ -244,7 +244,9 @@ /** * Invalidate items after or equal to index. This will remove visible items - * after that and invalidate cache of layout results after that. + * after that and invalidate cache of layout results after that. Note that it's client's + * responsibility to perform removing child action, {@link Provider#removeItem(int)} will not + * be called because the index might be invalidated. */ public void invalidateItemsAfter(int index) { if (index < 0) { @@ -253,9 +255,8 @@ if (mLastVisibleIndex < 0) { return; } - while (mLastVisibleIndex >= index) { - mProvider.removeItem(mLastVisibleIndex); - mLastVisibleIndex--; + if (mLastVisibleIndex >= index) { + mLastVisibleIndex = index - 1; } resetVisibleIndexIfEmpty(); if (getFirstVisibleIndex() < 0) {
diff --git a/v17/leanback/src/android/support/v17/leanback/widget/GridLayoutManager.java b/v17/leanback/src/android/support/v17/leanback/widget/GridLayoutManager.java index fa7f9db..d4e694a 100644 --- a/v17/leanback/src/android/support/v17/leanback/widget/GridLayoutManager.java +++ b/v17/leanback/src/android/support/v17/leanback/widget/GridLayoutManager.java
@@ -1631,7 +1631,9 @@ } if (TRACE) TraceCompat.endSection(); - updateScrollLimits(); + if (!mState.isPreLayout()) { + updateScrollLimits(); + } if (!mInLayout && mPendingMoveSmoothScroller != null) { mPendingMoveSmoothScroller.consumePendingMovesAfterLayout(); } @@ -1656,11 +1658,8 @@ @Override public int getEdge(int index) { - if (mReverseFlowPrimary) { - return getViewMax(findViewByPosition(index - mPositionDeltaInPreLayout)); - } else { - return getViewMin(findViewByPosition(index - mPositionDeltaInPreLayout)); - } + View v = findViewByPosition(index - mPositionDeltaInPreLayout); + return mReverseFlowPrimary ? getViewMax(v) : getViewMin(v); } @Override @@ -1877,15 +1876,21 @@ private void fastRelayout() { boolean invalidateAfter = false; final int childCount = getChildCount(); - int position = -1; - for (int index = 0; index < childCount; index++) { + int position = mGrid.getFirstVisibleIndex(); + int index = 0; + for (; index < childCount; index++, position++) { View view = getChildAt(index); - position = getAdapterPositionByView(view); + // We don't hit fastRelayout() if State.didStructure() is true, but prelayout may add + // extra views and invalidate existing Grid position. Also the prelayout calling + // getViewForPosotion() may retrieve item from cache with FLAG_INVALID. The adapter + // postion will be -1 for this case. Either case, we should invalidate after this item + // and call getViewForPosition() again to rebind. + if (position != getAdapterPositionByView(view)) { + invalidateAfter = true; + break; + } Grid.Location location = mGrid.getLocation(position); - // The last Prelayout calling getViewForPosition() may retrieve item from cache with - // FLAG_INVALID that causes NO_POSITION. Prelayout does not rebind data. Now postlayout - // should invalid after this item and call getViewForPosition() again to rebind. - if (location == null || (position == NO_POSITION)) { + if (location == null) { invalidateAfter = true; break; } @@ -1921,6 +1926,10 @@ } if (invalidateAfter) { final int savedLastPos = mGrid.getLastVisibleIndex(); + for (int i = childCount - 1; i >= index; i--) { + View v = getChildAt(i); + detachAndScrapView(v, mRecycler); + } mGrid.invalidateItemsAfter(position); if (mPruneChild) { // in regular prune child mode, we just append items up to edge limit
diff --git a/v17/leanback/tests/java/android/support/v17/leanback/widget/GridWidgetTest.java b/v17/leanback/tests/java/android/support/v17/leanback/widget/GridWidgetTest.java index 1ba49f9..1a173b4 100644 --- a/v17/leanback/tests/java/android/support/v17/leanback/widget/GridWidgetTest.java +++ b/v17/leanback/tests/java/android/support/v17/leanback/widget/GridWidgetTest.java
@@ -865,6 +865,59 @@ mGridView.getLayoutManager().getChildAt(0))); } + @Test + public void testMoveIntoPrelayoutItems() throws Throwable { + Intent intent = new Intent(); + intent.putExtra(GridActivity.EXTRA_LAYOUT_RESOURCE_ID, R.layout.vertical_linear); + intent.putExtra(GridActivity.EXTRA_NUM_ITEMS, 1000); + intent.putExtra(GridActivity.EXTRA_STAGGERED, false); + mNumRows = 1; + initActivity(intent); + mOrientation = BaseGridView.VERTICAL; + + final int lastItemPos = mGridView.getChildCount() - 1; + assertTrue(mGridView.getChildCount() >= 4); + // notify change of 3 items, so prelayout will layout extra 3 items, then move an item + // into the extra layout range. Post layout's fastRelayout() should handle this properly. + mActivityTestRule.runOnUiThread(new Runnable() { + @Override + public void run() { + mGridView.getAdapter().notifyItemChanged(lastItemPos - 3); + mGridView.getAdapter().notifyItemChanged(lastItemPos - 2); + mGridView.getAdapter().notifyItemChanged(lastItemPos - 1); + mActivity.moveItem(900, lastItemPos + 2, true); + } + }); + waitForItemAnimation(); + } + + @Test + public void testMoveIntoPrelayoutItems2() throws Throwable { + Intent intent = new Intent(); + intent.putExtra(GridActivity.EXTRA_LAYOUT_RESOURCE_ID, R.layout.vertical_linear); + intent.putExtra(GridActivity.EXTRA_NUM_ITEMS, 1000); + intent.putExtra(GridActivity.EXTRA_STAGGERED, false); + mNumRows = 1; + initActivity(intent); + mOrientation = BaseGridView.VERTICAL; + + setSelectedPosition(999); + final int firstItemPos = mGridView.getChildAdapterPosition(mGridView.getChildAt(0)); + assertTrue(mGridView.getChildCount() >= 4); + // notify change of 3 items, so prelayout will layout extra 3 items, then move an item + // into the extra layout range. Post layout's fastRelayout() should handle this properly. + mActivityTestRule.runOnUiThread(new Runnable() { + @Override + public void run() { + mGridView.getAdapter().notifyItemChanged(firstItemPos + 1); + mGridView.getAdapter().notifyItemChanged(firstItemPos + 2); + mGridView.getAdapter().notifyItemChanged(firstItemPos + 3); + mActivity.moveItem(0, firstItemPos - 2, true); + } + }); + waitForItemAnimation(); + } + void preparePredictiveLayout() throws Throwable { Intent intent = new Intent(); intent.putExtra(GridActivity.EXTRA_LAYOUT_RESOURCE_ID,