Handle unexpected scrolls gracefull.

Bug: 167821507
Test: Added new unit test coverage.
Change-Id: I013a5d3bfc2675caaf9ba51e056abf2c1f9c99d2
(cherry picked from commit d209da0255ea98831da70651ce7d8da682590952)

2 files changed, 54 insertions, 6 deletions

diff --git a/recyclerview/recyclerview-selection/src/androidTest/java/androidx/recyclerview/selection/BandSelectionHelperTest.java b/recyclerview/recyclerview-selection/src/androidTest/java/androidx/recyclerview/selection/BandSelectionHelperTest.java
index 3a9a8869cdd..f2683f028a8 100644
--- a/recyclerview/recyclerview-selection/src/androidTest/java/androidx/recyclerview/selection/BandSelectionHelperTest.java
+++ b/recyclerview/recyclerview-selection/src/androidTest/java/androidx/recyclerview/selection/BandSelectionHelperTest.java
@@ -16,6 +16,7 @@
 
 package androidx.recyclerview.selection;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
@@ -40,6 +41,7 @@ import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 
@@ -121,6 +123,21 @@ public class BandSelectionHelperTest {
     }
 
     @Test
+    public void testOnScroll() {
+        startBandSelect();
+        // Produces an NPE prior to fix for b/167821507.
+        mHostEnv.scrollTo(100, 100);
+        mHostEnv.assertBandShown(true);
+    }
+
+    @Test
+    public void testIgnoresScrollWhenNotStarted() {
+        // Produces an NPE prior to fix for b/167821507.
+        mHostEnv.scrollTo(100, 100);
+        mHostEnv.assertBandShown(false);
+    }
+
+    @Test
     public void testStart_RejectedByPredicate() {
         mBandPredicate.setCanInitiate(false);
         assertFalse(startBandSelect());
@@ -189,6 +206,8 @@ public class BandSelectionHelperTest {
     // GridHost extends BandHost. We satisfy both by implementing GridHost.
     private final class TestHostEnvironment extends GridHost<String> {
 
+        private final List<OnScrollListener> mScrollListeners = new ArrayList<>();
+        private boolean mBandShown;
         private boolean mBandHidden;
 
         @Override
@@ -200,8 +219,13 @@ public class BandSelectionHelperTest {
         }
 
         @Override
-        void showBand(Rect rect) {
-            throw new UnsupportedOperationException();
+        void showBand(@NonNull Rect rect) {
+            mBandShown = true;
+        }
+
+        // Asserts that a call was made to hide the band.
+        void assertBandShown(boolean expected) {
+            assertEquals(expected, mBandShown);
         }
 
         @Override
@@ -214,14 +238,20 @@ public class BandSelectionHelperTest {
             assertTrue(mBandHidden);
         }
 
+        void scrollTo(int x, int y) {
+            for (OnScrollListener l : mScrollListeners) {
+                l.onScrolled(null, x, y);
+            }
+        }
+
         @Override
-        void addOnScrollListener(OnScrollListener listener) {
-            // ignored for testing
+        void addOnScrollListener(@NonNull OnScrollListener listener) {
+            mScrollListeners.add(listener);
         }
 
         @Override
         void removeOnScrollListener(@NonNull OnScrollListener listener) {
-            // ignored for testing
+            mScrollListeners.remove(listener);
         }
 
         @Override
diff --git a/recyclerview/recyclerview-selection/src/main/java/androidx/recyclerview/selection/BandSelectionHelper.java b/recyclerview/recyclerview-selection/src/main/java/androidx/recyclerview/selection/BandSelectionHelper.java
index 591d1274ac9..5a70673308c 100644
--- a/recyclerview/recyclerview-selection/src/main/java/androidx/recyclerview/selection/BandSelectionHelper.java
+++ b/recyclerview/recyclerview-selection/src/main/java/androidx/recyclerview/selection/BandSelectionHelper.java
@@ -210,7 +210,7 @@ class BandSelectionHelper<K> implements OnItemTouchListener, Resettable {
         }
 
         // We shouldn't get any events in this method when band select is not active,
-        // but it turns some guests show up late to the party.
+        // but it turns out some guests show up late to the party.
         // Probably happening when a re-layout is happening to the ReyclerView (ie. Pull-To-Refresh)
         if (!isActive()) {
             return;
@@ -254,6 +254,8 @@ class BandSelectionHelper<K> implements OnItemTouchListener, Resettable {
         mLock.start();
         mFocusDelegate.clearFocus();
         mOrigin = origin;
+        mCurrentPosition = origin;
+
         // NOTE: Pay heed that resizeBand modifies the y coordinates
         // in onScrolled. Not sure if model expects this. If not
         // it should be defending against this.
@@ -323,6 +325,22 @@ class BandSelectionHelper<K> implements OnItemTouchListener, Resettable {
             return;
         }
 
+        // mOrigin and mCurrentPosition should never be null when onScrolled is called,
+        // but "never say never" increasingly looks like a motto to follow.
+        // For this reason we guard those specific cases and provide a clear
+        // error message in the logs.
+        if (mOrigin == null) {
+            Log.e(TAG, "onScrolled called while mOrigin null.");
+            if (DEBUG) throw new IllegalStateException("mOrigin is null.");
+            return;
+        }
+
+        if (mCurrentPosition == null) {
+            Log.e(TAG, "onScrolled called while mCurrentPosition null.");
+            if (DEBUG) throw new IllegalStateException("mCurrentPosition is null.");
+            return;
+        }
+
         // Adjust the y-coordinate of the origin the opposite number of pixels so that the
         // origin remains in the same place relative to the view's items.
         mOrigin.y -= dy;