GridLayoutManager: fix bug smoothscroller gets interrupted
Vertical scroll in Videos app is janky.
During VerticalGridView scroll pass it appends a row and layout
the row, the row layout might request focus on a child, that causes
a onRequestChildFocus() call back into the top level VerticalGridView.
In that case high level verticalGridView should not start another
scrollToView() animation. onRequestChildFocus() is supposed to handle
user's interaction only.
The bug is difficult to test, both RV.smoothScrollBy() and LinearSmoothScroller
uses the same scroll state, calling RV.smoothScrollBy() does not
stop linearSmoothScroller. Added a override class to intercept calls
on RV.smoothScrollBy().
Bug: 19846906
Change-Id: Ica857f443e41f14710b2bf177eafba30403817c7
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 b0849e9..ad2211c 100644
--- a/v17/leanback/src/android/support/v17/leanback/widget/GridLayoutManager.java
+++ b/v17/leanback/src/android/support/v17/leanback/widget/GridLayoutManager.java
@@ -418,7 +418,8 @@
private RecyclerView.State mState;
private RecyclerView.Recycler mRecycler;
- private boolean mInLayout = false;
+ private boolean mInLayout;
+ private boolean mInScroll;
private boolean mInFastRelayout;
/**
* During full layout pass, when GridView had focus: onLayoutChildren will
@@ -1752,6 +1753,7 @@
return 0;
}
saveContext(recycler, state);
+ mInScroll = true;
int result;
if (mOrientation == HORIZONTAL) {
result = scrollDirectionPrimary(dx);
@@ -1759,6 +1761,7 @@
result = scrollDirectionSecondary(dx);
}
leaveContext();
+ mInScroll = false;
return result;
}
@@ -1768,6 +1771,7 @@
if (!mLayoutEnabled || !hasDoneFirstLayout()) {
return 0;
}
+ mInScroll = true;
saveContext(recycler, state);
int result;
if (mOrientation == VERTICAL) {
@@ -1776,6 +1780,7 @@
result = scrollDirectionSecondary(dy);
}
leaveContext();
+ mInScroll = false;
return result;
}
@@ -2144,7 +2149,7 @@
// scroll to a view whose item has been removed.
return true;
}
- if (!mInLayout && !mInSelection) {
+ if (!mInLayout && !mInSelection && !mInScroll) {
scrollToView(child, true);
}
return true;
diff --git a/v17/tests/res/layout/horizontal_grid.xml b/v17/tests/res/layout/horizontal_grid.xml
index 5119f79..6c4eaf1 100644
--- a/v17/tests/res/layout/horizontal_grid.xml
+++ b/v17/tests/res/layout/horizontal_grid.xml
@@ -4,7 +4,7 @@
android:layout_width="match_parent"
android:layout_height="match_parent"
>
- <android.support.v17.leanback.widget.HorizontalGridView
+ <android.support.v17.leanback.widget.HorizontalGridViewEx
android:id="@+id/gridview"
android:layout_width="match_parent"
android:layout_height="match_parent"
diff --git a/v17/tests/res/layout/horizontal_grid_testredundantappendremove2.xml b/v17/tests/res/layout/horizontal_grid_testredundantappendremove2.xml
index b539f3c..fdfc0ea 100644
--- a/v17/tests/res/layout/horizontal_grid_testredundantappendremove2.xml
+++ b/v17/tests/res/layout/horizontal_grid_testredundantappendremove2.xml
@@ -4,7 +4,7 @@
android:layout_width="match_parent"
android:layout_height="match_parent"
>
- <android.support.v17.leanback.widget.HorizontalGridView
+ <android.support.v17.leanback.widget.HorizontalGridViewEx
android:id="@+id/gridview"
android:layout_width="960dp"
android:layout_height="492dp"
diff --git a/v17/tests/res/layout/vertical_grid.xml b/v17/tests/res/layout/vertical_grid.xml
index 85e1d46..f4c0065 100644
--- a/v17/tests/res/layout/vertical_grid.xml
+++ b/v17/tests/res/layout/vertical_grid.xml
@@ -4,7 +4,7 @@
android:layout_width="match_parent"
android:layout_height="match_parent"
>
- <android.support.v17.leanback.widget.VerticalGridView
+ <android.support.v17.leanback.widget.VerticalGridViewEx
android:id="@+id/gridview"
android:layout_width="match_parent"
android:layout_height="match_parent"
diff --git a/v17/tests/res/layout/vertical_grid_testredundantappendremove.xml b/v17/tests/res/layout/vertical_grid_testredundantappendremove.xml
index 4ce56c5..bf056f8 100644
--- a/v17/tests/res/layout/vertical_grid_testredundantappendremove.xml
+++ b/v17/tests/res/layout/vertical_grid_testredundantappendremove.xml
@@ -4,7 +4,7 @@
android:layout_width="match_parent"
android:layout_height="match_parent"
>
- <android.support.v17.leanback.widget.VerticalGridView
+ <android.support.v17.leanback.widget.VerticalGridViewEx
android:id="@+id/gridview"
android:layout_width="960dp"
android:layout_height="492dp"
diff --git a/v17/tests/res/layout/vertical_linear.xml b/v17/tests/res/layout/vertical_linear.xml
index b2c74df..0a1d00c 100644
--- a/v17/tests/res/layout/vertical_linear.xml
+++ b/v17/tests/res/layout/vertical_linear.xml
@@ -4,7 +4,7 @@
android:layout_width="match_parent"
android:layout_height="match_parent"
>
- <android.support.v17.leanback.widget.VerticalGridView
+ <android.support.v17.leanback.widget.VerticalGridViewEx
android:id="@+id/gridview"
android:layout_width="match_parent"
android:layout_height="match_parent"
diff --git a/v17/tests/src/android/support/v17/leanback/widget/GridActivity.java b/v17/tests/src/android/support/v17/leanback/widget/GridActivity.java
index ba6cd39..fdef479 100644
--- a/v17/tests/src/android/support/v17/leanback/widget/GridActivity.java
+++ b/v17/tests/src/android/support/v17/leanback/widget/GridActivity.java
@@ -21,7 +21,6 @@
import android.support.v17.leanback.widget.BaseGridView;
import android.support.v17.leanback.widget.OnChildSelectedListener;
import android.app.Activity;
-import android.content.Context;
import android.content.Intent;
import android.graphics.Color;
import android.os.Bundle;
@@ -38,6 +37,7 @@
* @hide from javadoc
*/
public class GridActivity extends Activity {
+
private static final String TAG = "GridActivity";
public static final String EXTRA_LAYOUT_RESOURCE_ID = "layoutResourceId";
@@ -46,11 +46,13 @@
public static final String EXTRA_ITEMS_FOCUSABLE = "itemsFocusable";
public static final String EXTRA_STAGGERED = "staggered";
public static final String EXTRA_REQUEST_LAYOUT_ONFOCUS = "requestLayoutOnFocus";
+ public static final String EXTRA_REQUEST_FOCUS_ONLAYOUT = "requstFocusOnLayout";
public static final String SELECT_ACTION = "android.test.leanback.widget.SELECT";
static final int DEFAULT_NUM_ITEMS = 100;
static final boolean DEFAULT_STAGGERED = true;
static final boolean DEFAULT_REQUEST_LAYOUT_ONFOCUS = false;
+ static final boolean DEFAULT_REQUEST_FOCUS_ONLAYOUT = false;
private static final boolean DEBUG = false;
@@ -59,6 +61,7 @@
int mNumItems;
boolean mStaggered;
boolean mRequestLayoutOnFocus;
+ boolean mRequestFocusOnLayout;
int[] mGridViewLayoutSize;
BaseGridView mGridView;
@@ -92,6 +95,8 @@
mStaggered = intent.getBooleanExtra(EXTRA_STAGGERED, DEFAULT_STAGGERED);
mRequestLayoutOnFocus = intent.getBooleanExtra(EXTRA_REQUEST_LAYOUT_ONFOCUS,
DEFAULT_REQUEST_LAYOUT_ONFOCUS);
+ mRequestFocusOnLayout = intent.getBooleanExtra(EXTRA_REQUEST_FOCUS_ONLAYOUT,
+ DEFAULT_REQUEST_FOCUS_ONLAYOUT);
mItemLengths = intent.getIntArrayExtra(EXTRA_ITEMS);
mItemFocusables = intent.getBooleanArrayExtra(EXTRA_ITEMS_FOCUSABLE);
@@ -200,7 +205,18 @@
@Override
public RecyclerView.ViewHolder onCreateViewHolder(ViewGroup parent, int viewType) {
if (DEBUG) Log.v(TAG, "createViewHolder " + viewType);
- TextView textView = new TextView(parent.getContext());
+ TextView textView = new TextView(parent.getContext()) {
+ @Override
+ protected void onLayout(boolean change, int left, int top, int right, int bottom) {
+ super.onLayout(change, left, top, right, bottom);
+ if (mRequestFocusOnLayout) {
+ if (hasFocus()) {
+ clearFocus();
+ requestFocus();
+ }
+ }
+ }
+ };
textView.setTextColor(Color.BLACK);
textView.setOnFocusChangeListener(mItemFocusChangeListener);
return new ViewHolder(textView);
diff --git a/v17/tests/src/android/support/v17/leanback/widget/GridWidgetTest.java b/v17/tests/src/android/support/v17/leanback/widget/GridWidgetTest.java
index 178d59a..80fc63f 100644
--- a/v17/tests/src/android/support/v17/leanback/widget/GridWidgetTest.java
+++ b/v17/tests/src/android/support/v17/leanback/widget/GridWidgetTest.java
@@ -1089,4 +1089,48 @@
}
+ public void testSmoothscrollerInterrupted() throws Throwable {
+ mInstrumentation = getInstrumentation();
+ Intent intent = new Intent(mInstrumentation.getContext(), GridActivity.class);
+ intent.putExtra(GridActivity.EXTRA_LAYOUT_RESOURCE_ID,
+ R.layout.vertical_linear);
+ intent.putExtra(GridActivity.EXTRA_REQUEST_FOCUS_ONLAYOUT, true);
+ int[] items = new int[100];
+ for (int i = 0; i < items.length; i++) {
+ items[i] = 680;
+ }
+ intent.putExtra(GridActivity.EXTRA_ITEMS, items);
+ intent.putExtra(GridActivity.EXTRA_STAGGERED, false);
+ mOrientation = BaseGridView.VERTICAL;
+ mNumRows = 1;
+
+ initActivity(intent);
+
+ mGridView.setSelectedPositionSmooth(0);
+ waitForScrollIdle(mVerifyLayout);
+ assertTrue(mGridView.getChildAt(0).hasFocus());
+
+ // Pressing lots of key to make sure smooth scroller is running
+ for (int i = 0; i < 20; i++) {
+ sendKeys(KeyEvent.KEYCODE_DPAD_DOWN);
+ }
+ Thread.sleep(100);
+ int total = 0;
+ while (mGridView.getLayoutManager().isSmoothScrolling() ||
+ mGridView.getScrollState() != BaseGridView.SCROLL_STATE_IDLE) {
+ if ((total += 10) >= WAIT_FOR_SCROLL_IDLE_TIMEOUT_MS) {
+ throw new RuntimeException("waitForScrollIdle Timeout");
+ }
+ try {
+ // Repeatedly pressing to make sure pending keys does not drop to zero.
+ Thread.sleep(10);
+ sendKeys(KeyEvent.KEYCODE_DPAD_DOWN);
+ } catch (InterruptedException ex) {
+ break;
+ }
+ }
+
+ assertTrue("LinearSmoothScroller would not use many RV.smoothScrollBy() calls",
+ ((VerticalGridViewEx) mGridView).mSmoothScrollByCalled < 10);
+ }
}
diff --git a/v17/tests/src/android/support/v17/leanback/widget/HorizontalGridViewEx.java b/v17/tests/src/android/support/v17/leanback/widget/HorizontalGridViewEx.java
new file mode 100644
index 0000000..2c49283
--- /dev/null
+++ b/v17/tests/src/android/support/v17/leanback/widget/HorizontalGridViewEx.java
@@ -0,0 +1,27 @@
+package android.support.v17.leanback.widget;
+
+import android.content.Context;
+import android.util.AttributeSet;
+
+class HorizontalGridViewEx extends HorizontalGridView {
+
+ public int mSmoothScrollByCalled;
+
+ public HorizontalGridViewEx(Context context) {
+ super(context);
+ }
+
+ public HorizontalGridViewEx(Context context, AttributeSet attrs) {
+ super(context, attrs);
+ }
+
+ public HorizontalGridViewEx(Context context, AttributeSet attrs, int defStyle) {
+ super(context, attrs, defStyle);
+ }
+
+ @Override
+ public void smoothScrollBy(int dx, int dy) {
+ mSmoothScrollByCalled++;
+ super.smoothScrollBy(dx, dy);
+ }
+}
\ No newline at end of file
diff --git a/v17/tests/src/android/support/v17/leanback/widget/VerticalGridViewEx.java b/v17/tests/src/android/support/v17/leanback/widget/VerticalGridViewEx.java
new file mode 100644
index 0000000..e262d54
--- /dev/null
+++ b/v17/tests/src/android/support/v17/leanback/widget/VerticalGridViewEx.java
@@ -0,0 +1,27 @@
+package android.support.v17.leanback.widget;
+
+import android.content.Context;
+import android.util.AttributeSet;
+
+class VerticalGridViewEx extends VerticalGridView {
+
+ public int mSmoothScrollByCalled;
+
+ public VerticalGridViewEx(Context context) {
+ super(context);
+ }
+
+ public VerticalGridViewEx(Context context, AttributeSet attrs) {
+ super(context, attrs);
+ }
+
+ public VerticalGridViewEx(Context context, AttributeSet attrs, int defStyle) {
+ super(context, attrs, defStyle);
+ }
+
+ @Override
+ public void smoothScrollBy(int dx, int dy) {
+ mSmoothScrollByCalled++;
+ super.smoothScrollBy(dx, dy);
+ }
+}
\ No newline at end of file