From bb2fb2ffd61e2d043af50983b9c9460fedb43eb1 Mon Sep 17 00:00:00 2001
From: Shen Lin <shhenlin@gmail.com>
Date: Thu, 20 Oct 2022 10:18:09 +0800
Subject: [PATCH] Ensure search highlight position when scheduled runnable
 starts

Search highlight function includes two steps: Scroll list to target position first, then notifyItemChanged to it.

We use a Handler.postDelay to implement this. However, when scheduled runnable starts, the original target position could have changed due to preference list update, calling recyclerview's methods after that will be easy to cause an exception.

This CL ensures highlight position every time before calling recyclerView update, which also contribute to origin fix of RecyclerView IllegalArgumentException to a certain extent.

Test: atest, also test some search results, and see the correct behavior
Fixes: 246411107

Change-Id: Ifa758ce3718b047138079246cdfce99fdf66d5b2
---
 .../HighlightablePreferenceGroupAdapter.java  | 30 ++++++++++++++++---
 ...ghlightablePreferenceGroupAdapterTest.java |  4 +--
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/src/com/android/settings/widget/HighlightablePreferenceGroupAdapter.java b/src/com/android/settings/widget/HighlightablePreferenceGroupAdapter.java
index 9992ae71916..a64ec89e332 100644
--- a/src/com/android/settings/widget/HighlightablePreferenceGroupAdapter.java
+++ b/src/com/android/settings/widget/HighlightablePreferenceGroupAdapter.java
@@ -141,6 +141,8 @@ public class HighlightablePreferenceGroupAdapter extends PreferenceGroupAdapter
             return;
         }
 
+        // Highlight request accepted
+        mHighlightRequested = true;
         // Collapse app bar after 300 milliseconds.
         if (appBarLayout != null) {
             root.postDelayed(() -> {
@@ -152,17 +154,37 @@ public class HighlightablePreferenceGroupAdapter extends PreferenceGroupAdapter
         recyclerView.setItemAnimator(null);
         // Scroll to correct position after 600 milliseconds.
         root.postDelayed(() -> {
-            mHighlightRequested = true;
-            recyclerView.smoothScrollToPosition(position);
-            mHighlightPosition = position;
+            if (ensureHighlightPosition()) {
+                recyclerView.smoothScrollToPosition(mHighlightPosition);
+            }
         }, DELAY_HIGHLIGHT_DURATION_MILLIS);
 
         // Highlight preference after 900 milliseconds.
         root.postDelayed(() -> {
-            notifyItemChanged(position);
+            if (ensureHighlightPosition()) {
+                notifyItemChanged(mHighlightPosition);
+            }
         }, DELAY_COLLAPSE_DURATION_MILLIS + DELAY_HIGHLIGHT_DURATION_MILLIS);
     }
 
+    /**
+     * Make sure we highlight the real-wanted position in case of preference position already
+     * changed when the delay time comes.
+     */
+    private boolean ensureHighlightPosition() {
+        if (TextUtils.isEmpty(mHighlightKey)) {
+            return false;
+        }
+        final int position = getPreferenceAdapterPosition(mHighlightKey);
+        final boolean allowHighlight = position >= 0;
+        if (allowHighlight && mHighlightPosition != position) {
+            Log.w(TAG, "EnsureHighlight: position has changed since last highlight request");
+            // Make sure RecyclerView always uses latest correct position to avoid exceptions.
+            mHighlightPosition = position;
+        }
+        return allowHighlight;
+    }
+
     public boolean isHighlightRequested() {
         return mHighlightRequested;
     }
diff --git a/tests/robotests/src/com/android/settings/widget/HighlightablePreferenceGroupAdapterTest.java b/tests/robotests/src/com/android/settings/widget/HighlightablePreferenceGroupAdapterTest.java
index e297b788af9..2453a30587c 100644
--- a/tests/robotests/src/com/android/settings/widget/HighlightablePreferenceGroupAdapterTest.java
+++ b/tests/robotests/src/com/android/settings/widget/HighlightablePreferenceGroupAdapterTest.java
@@ -129,7 +129,7 @@ public class HighlightablePreferenceGroupAdapterTest {
     }
 
     @Test
-    public void adjustInitialExpandedChildCount_hasHightlightKey_shouldExpandAllChildren() {
+    public void adjustInitialExpandedChildCount_hasHighlightKey_shouldExpandAllChildren() {
         final Bundle args = new Bundle();
         when(mFragment.getArguments()).thenReturn(args);
         args.putString(SettingsActivity.EXTRA_FRAGMENT_ARG_KEY, "testkey");
@@ -208,7 +208,7 @@ public class HighlightablePreferenceGroupAdapterTest {
     }
 
     @Test
-    public void updateBackground_reuseHightlightedRowForNormalRow_shouldResetBackgroundAndTag() {
+    public void updateBackground_reuseHighlightedRowForNormalRow_shouldResetBackgroundAndTag() {
         ReflectionHelpers.setField(mAdapter, "mHighlightPosition", 10);
         mViewHolder.itemView.setTag(R.id.preference_highlighted, true);