leanback: fix crash in onMeasure when preLayout is true
In some conditions e.g. items are removed before first visible child,
the position in prelayout needs to apply offset. Uses the value
mPositionDeltaInPreLayout in onMeasure() pass.
Also onMeasure should not call getViewForPosition for a child that
is currently in RV.
Historically, leanback LayoutManager implemented custom onMeasure
before RecyclerView introduce "autoMeasure" feature, eventually (in
next major release) leanback should switch to autoMeasure and remove
the custom onMeasure().
Bug: 63897029
Test: testPredictiveOnMeasureWrapContent
Change-Id: I92c70fe90dcc156fd5999bbb6358a1e84f82babb
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 45d69ef..f2dae95 100644
--- a/v17/leanback/src/android/support/v17/leanback/widget/GridLayoutManager.java
+++ b/v17/leanback/src/android/support/v17/leanback/widget/GridLayoutManager.java
@@ -1269,8 +1269,7 @@
if (TRACE) TraceCompat.beginSection("processRowSizeSecondary");
CircularIntArray[] rows = mGrid == null ? null : mGrid.getItemPositionsInRows();
boolean changed = false;
- int scrapChildWidth = -1;
- int scrapChildHeight = -1;
+ int scrapeChildSize = -1;
for (int rowIndex = 0; rowIndex < mNumRows; rowIndex++) {
CircularIntArray row = rows == null ? null : rows[rowIndex];
@@ -1281,7 +1280,7 @@
final int rowIndexStart = row.get(rowItemPairIndex);
final int rowIndexEnd = row.get(rowItemPairIndex + 1);
for (int i = rowIndexStart; i <= rowIndexEnd; i++) {
- final View view = findViewByPosition(i);
+ final View view = findViewByPosition(i - mPositionDeltaInPreLayout);
if (view == null) {
continue;
}
@@ -1299,27 +1298,49 @@
final int itemCount = mState.getItemCount();
if (!mBaseGridView.hasFixedSize() && measure && rowSize < 0 && itemCount > 0) {
- if (scrapChildWidth < 0 && scrapChildHeight < 0) {
- int position;
- if (mFocusPosition == NO_POSITION) {
+ if (scrapeChildSize < 0) {
+ // measure a child that is close to mFocusPosition but not currently visible
+ int position = mFocusPosition;
+ if (position < 0) {
position = 0;
- } else if (mFocusPosition >= itemCount) {
+ } else if (position >= itemCount) {
position = itemCount - 1;
- } else {
- position = mFocusPosition;
}
- measureScrapChild(position,
- MeasureSpec.makeMeasureSpec(0, MeasureSpec.UNSPECIFIED),
- MeasureSpec.makeMeasureSpec(0, MeasureSpec.UNSPECIFIED),
- mMeasuredDimension);
- scrapChildWidth = mMeasuredDimension[0];
- scrapChildHeight = mMeasuredDimension[1];
- if (DEBUG) {
- Log.v(TAG, "measured scrap child: " + scrapChildWidth + " "
- + scrapChildHeight);
+ if (getChildCount() > 0) {
+ int firstPos = mBaseGridView.getChildViewHolder(
+ getChildAt(0)).getLayoutPosition();
+ int lastPos = mBaseGridView.getChildViewHolder(
+ getChildAt(getChildCount() - 1)).getLayoutPosition();
+ // if mFocusPosition is between first and last, choose either
+ // first - 1 or last + 1
+ if (position >= firstPos && position <= lastPos) {
+ position = (position - firstPos <= lastPos - position)
+ ? (firstPos - 1) : (lastPos + 1);
+ // try the other value if the position is invalid. if both values are
+ // invalid, skip measureScrapChild below.
+ if (position < 0 && lastPos < itemCount - 1) {
+ position = lastPos + 1;
+ } else if (position >= itemCount && firstPos > 0) {
+ position = firstPos - 1;
+ }
+ }
+ }
+ if (position >= 0 && position < itemCount) {
+ measureScrapChild(position,
+ MeasureSpec.makeMeasureSpec(0, MeasureSpec.UNSPECIFIED),
+ MeasureSpec.makeMeasureSpec(0, MeasureSpec.UNSPECIFIED),
+ mMeasuredDimension);
+ scrapeChildSize = mOrientation == HORIZONTAL ? mMeasuredDimension[0] :
+ mMeasuredDimension[1];
+ if (DEBUG) {
+ Log.v(TAG, "measured scrap child: " + mMeasuredDimension[0] + " "
+ + mMeasuredDimension[1]);
+ }
}
}
- rowSize = mOrientation == HORIZONTAL ? scrapChildHeight : scrapChildWidth;
+ if (scrapeChildSize >= 0) {
+ rowSize = scrapeChildSize;
+ }
}
if (rowSize < 0) {
rowSize = 0;
@@ -1404,7 +1425,10 @@
mRowSizeSecondary = new int[mNumRows];
}
- // Measure all current children and update cached row heights
+ if (mState.isPreLayout()) {
+ updatePositionDeltaInPreLayout();
+ }
+ // Measure all current children and update cached row height or column width
processRowSizeSecondary(true);
switch (modeSecondary) {
@@ -2057,6 +2081,22 @@
mPositionToRowInPostLayout.clear();
}
+ // in prelayout, first child's getViewPosition can be smaller than old adapter position
+ // if there were items removed before first visible index. For example:
+ // visible items are 3, 4, 5, 6, deleting 1, 2, 3 from adapter; the view position in
+ // prelayout are not 3(deleted), 4, 5, 6. Instead it's 1(deleted), 2, 3, 4.
+ // So there is a delta (2 in this case) between last cached position and prelayout position.
+ void updatePositionDeltaInPreLayout() {
+ if (getChildCount() > 0) {
+ View view = getChildAt(0);
+ LayoutParams lp = (LayoutParams) view.getLayoutParams();
+ mPositionDeltaInPreLayout = mGrid.getFirstVisibleIndex()
+ - lp.getViewLayoutPosition();
+ } else {
+ mPositionDeltaInPreLayout = 0;
+ }
+ }
+
// Lays out items based on the current scroll position
@Override
public void onLayoutChildren(RecyclerView.Recycler recycler, RecyclerView.State state) {
@@ -2094,6 +2134,7 @@
saveContext(recycler, state);
if (state.isPreLayout()) {
+ updatePositionDeltaInPreLayout();
int childCount = getChildCount();
if (mGrid != null && childCount > 0) {
int minChangedEdge = Integer.MAX_VALUE;
@@ -2105,12 +2146,6 @@
for (int i = 0; i < childCount; i++) {
View view = getChildAt(i);
LayoutParams lp = (LayoutParams) view.getLayoutParams();
- if (i == 0) {
- // first child's layout position can be smaller than index if there were
- // items removed before first visible index.
- mPositionDeltaInPreLayout = mGrid.getFirstVisibleIndex()
- - lp.getViewLayoutPosition();
- }
int newAdapterPosition = mBaseGridView.getChildAdapterPosition(view);
// if either of following happening
// 1. item itself has changed or layout parameter changed
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 1f6bdb2..61974cc 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
@@ -920,8 +920,7 @@
void preparePredictiveLayout() throws Throwable {
Intent intent = new Intent();
- intent.putExtra(GridActivity.EXTRA_LAYOUT_RESOURCE_ID,
- R.layout.horizontal_linear);
+ intent.putExtra(GridActivity.EXTRA_LAYOUT_RESOURCE_ID, R.layout.horizontal_linear);
intent.putExtra(GridActivity.EXTRA_NUM_ITEMS, 100);
initActivity(intent);
mOrientation = BaseGridView.HORIZONTAL;
@@ -1016,6 +1015,50 @@
}
@Test
+ public void testPredictiveOnMeasureWrapContent() throws Throwable {
+ Intent intent = new Intent();
+ intent.putExtra(GridActivity.EXTRA_LAYOUT_RESOURCE_ID,
+ R.layout.horizontal_linear_wrap_content);
+ int count = 50;
+ intent.putExtra(GridActivity.EXTRA_NUM_ITEMS, count);
+ initActivity(intent);
+ mOrientation = BaseGridView.HORIZONTAL;
+ mNumRows = 1;
+
+ waitForScrollIdle(mVerifyLayout);
+ mActivityTestRule.runOnUiThread(new Runnable() {
+ @Override
+ public void run() {
+ mGridView.setHasFixedSize(false);
+ }
+ });
+
+ for (int i = 0; i < 30; i++) {
+ final int oldCount = count;
+ final int newCount = i;
+ mActivityTestRule.runOnUiThread(new Runnable() {
+ @Override
+ public void run() {
+ if (oldCount > 0) {
+ mActivity.removeItems(0, oldCount);
+ }
+ if (newCount > 0) {
+ int[] newItems = new int[newCount];
+ for (int i = 0; i < newCount; i++) {
+ newItems[i] = 400;
+ }
+ mActivity.addItems(0, newItems);
+ }
+ }
+ });
+ waitForItemAnimationStart();
+ waitForItemAnimation();
+ count = newCount;
+ }
+
+ }
+
+ @Test
public void testPredictiveLayoutRemove4() throws Throwable {
Intent intent = new Intent();
intent.putExtra(GridActivity.EXTRA_LAYOUT_RESOURCE_ID,
diff --git a/v17/leanback/tests/res/layout/horizontal_linear_wrap_content.xml b/v17/leanback/tests/res/layout/horizontal_linear_wrap_content.xml
new file mode 100644
index 0000000..c0e2715
--- /dev/null
+++ b/v17/leanback/tests/res/layout/horizontal_linear_wrap_content.xml
@@ -0,0 +1,39 @@
+<!--
+ ~ Copyright (C) 2017 The Android Open Source Project
+ ~
+ ~ Licensed under the Apache License, Version 2.0 (the "License");
+ ~ you may not use this file except in compliance with the License.
+ ~ You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing, software
+ ~ distributed under the License is distributed on an "AS IS" BASIS,
+ ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ ~ See the License for the specific language governing permissions and
+ ~ limitations under the License.
+ -->
+
+<LinearLayout
+ xmlns:android="http://schemas.android.com/apk/res/android"
+ xmlns:lb="http://schemas.android.com/apk/res-auto"
+ android:orientation="vertical"
+ android:layout_width="wrap_content"
+ android:layout_height="wrap_content"
+ >
+ <android.support.v17.leanback.widget.HorizontalGridViewEx
+ android:id="@+id/gridview"
+ android:layout_width="match_parent"
+ android:layout_height="wrap_content"
+ android:clipToPadding="false"
+ android:focusable="true"
+ android:focusableInTouchMode="true"
+ android:background="#00ffff"
+ android:horizontalSpacing="12dip"
+ android:verticalSpacing="24dip"
+ lb:rowHeight="wrap_content"
+ android:paddingBottom="12dip"
+ android:paddingLeft="12dip"
+ android:paddingRight="12dip"
+ android:paddingTop="12dip" />
+</LinearLayout>