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,