Fix Android Picker ArrayOutOfBoundsException during Picker.Item update from a long list (#25276)

Summary:
axe-fb reported this side effect from my previous commit in https://github.com/facebook/react-native/pull/24793#issuecomment-502202082
After revisited the implementation of Android Spinner, it seems the formal way to update existing adapter is mutating it, i.e. `arrayAdapter.clear()` & `arrayAdapter.addAll()` to update a Spinner Adapter.
`setAdapter()` will reset everything including `mDataChanged`.
A race condition may happens between rendering a long picker list and reseting adapter.
Here is a code snippet: https://snack.expo.io/kudochien/80f810
To reproduce the issue, please select large item (e.g. 500) first and click the button right hand side.
Please not to verify this on Expo directly in the meantime, because Expo with RN 0.59 does not include my previous commit.

## Changelog

[Android] [Fixed] - Fix Picker ArrayOutOfBoundsException during Picker.Item update from a long list
Pull Request resolved: https://github.com/facebook/react-native/pull/25276

Test Plan:
1. Check the test case https://snack.expo.io/kudochien/80f810 will have exception or not.
2. Regression of https://snack.expo.io/Sy1JClEag from https://github.com/facebook/react-native/issues/13351
3. Regression of https://snack.expo.io/kudochien/android-picker-issue from https://github.com/facebook/react-native/issues/22821
4. RNTester Picker example

Reviewed By: mdvacca

Differential Revision: D15857426

Pulled By: axe-fb

fbshipit-source-id: 8ef902447fdd1b8aeab50ad061545cd14c735e51

ReactAndroid/src/main/java/com/facebook/react/views/picker/ReactPicker.java

@@ -8,24 +8,27 @@
 package com.facebook.react.views.picker;
 
 import android.content.Context;
-import androidx.appcompat.widget.AppCompatSpinner;
 import android.util.AttributeSet;
 import android.view.View;
 import android.widget.AdapterView;
 import android.widget.Spinner;
-import android.widget.SpinnerAdapter;
+
+import androidx.appcompat.widget.AppCompatSpinner;
 
 import com.facebook.react.common.annotations.VisibleForTesting;
 
+import java.util.List;
+
 import javax.annotation.Nullable;
 
 public class ReactPicker extends AppCompatSpinner {
 
   private int mMode = Spinner.MODE_DIALOG;
-  private @Nullable Integer mPrimaryColor;
   private @Nullable OnSelectListener mOnSelectListener;
-  private @Nullable SpinnerAdapter mStagedAdapter;
+  private @Nullable List<ReactPickerItem> mItems;
+  private @Nullable List<ReactPickerItem> mStagedItems;
   private @Nullable Integer mStagedSelection;
+  private @Nullable Integer mStagedPrimaryTextColor;
 
   private final OnItemSelectedListener mItemSelectedListener = new OnItemSelectedListener() {
     @Override
@@ -113,8 +116,8 @@ public class ReactPicker extends AppCompatSpinner {
     return mOnSelectListener;
   }
 
-  /* package */ void setStagedAdapter(final SpinnerAdapter adapter) {
-   mStagedAdapter = adapter;
+  /* package */ void setStagedItems(final @Nullable List<ReactPickerItem> items) {
+   mStagedItems = items;
   }
 
   /**
@@ -125,6 +128,10 @@ public class ReactPicker extends AppCompatSpinner {
     mStagedSelection = selection;
   }
 
+  /* package */ void setStagedPrimaryTextColor(@Nullable Integer primaryColor) {
+    mStagedPrimaryTextColor = primaryColor;
+  }
+
   /**
    * Used to commit staged data into ReactPicker view.
    * During this period, we will disable {@link OnSelectListener#onItemSelected(int)} temporarily,
@@ -133,14 +140,19 @@ public class ReactPicker extends AppCompatSpinner {
   /* package */ void commitStagedData() {
     setOnItemSelectedListener(null);
 
+    ReactPickerAdapter adapter = (ReactPickerAdapter) getAdapter();
     final int origSelection = getSelectedItemPosition();
-    if (mStagedAdapter != null && mStagedAdapter != getAdapter()) {
-      setAdapter(mStagedAdapter);
-      // After setAdapter(), Spinner will reset selection and cause unnecessary onValueChange event.
-      // Explicitly setup selection again to prevent this.
-      // Ref: https://android.googlesource.com/platform/frameworks/base/+/master/core/java/android/widget/AbsSpinner.java#123
-      setSelection(origSelection, false);
-      mStagedAdapter = null;
+    if (mStagedItems != null && mStagedItems != mItems) {
+      mItems = mStagedItems;
+      mStagedItems = null;
+      if (adapter == null) {
+        adapter = new ReactPickerAdapter(getContext(), mItems);
+        setAdapter(adapter);
+      } else {
+        adapter.clear();
+        adapter.addAll(mItems);
+        adapter.notifyDataSetChanged();
+      }
     }
 
     if (mStagedSelection != null && mStagedSelection != origSelection) {
@@ -148,17 +160,16 @@ public class ReactPicker extends AppCompatSpinner {
       mStagedSelection = null;
     }
 
+    if (mStagedPrimaryTextColor != null &&
+        adapter != null &&
+        mStagedPrimaryTextColor != adapter.getPrimaryTextColor()) {
+      adapter.setPrimaryTextColor(mStagedPrimaryTextColor);
+      mStagedPrimaryTextColor = null;
+    }
+
     setOnItemSelectedListener(mItemSelectedListener);
   }
 
-  public @Nullable Integer getPrimaryColor() {
-    return mPrimaryColor;
-  }
-
-  public void setPrimaryColor(@Nullable Integer primaryColor) {
-    mPrimaryColor = primaryColor;
-  }
-
   @VisibleForTesting
   public int getMode() {
     return mMode;

ReactAndroid/src/main/java/com/facebook/react/views/picker/ReactPickerAdapter.java

@@ -0,0 +1,68 @@
+package com.facebook.react.views.picker;
+
+import android.content.Context;
+import android.view.LayoutInflater;
+import android.view.View;
+import android.view.ViewGroup;
+import android.widget.ArrayAdapter;
+import android.widget.TextView;
+
+import com.facebook.infer.annotation.Assertions;
+
+import java.util.List;
+
+import javax.annotation.Nullable;
+
+/* package */
+class ReactPickerAdapter extends ArrayAdapter<ReactPickerItem> {
+
+  private final LayoutInflater mInflater;
+  private @Nullable
+  Integer mPrimaryTextColor;
+
+  public ReactPickerAdapter(Context context, List<ReactPickerItem> data) {
+    super(context, 0, data);
+
+    mInflater = (LayoutInflater) Assertions.assertNotNull(
+      context.getSystemService(Context.LAYOUT_INFLATER_SERVICE));
+  }
+
+  @Override
+  public View getView(int position, View convertView, ViewGroup parent) {
+    return getView(position, convertView, parent, false);
+  }
+
+  @Override
+  public View getDropDownView(int position, View convertView, ViewGroup parent) {
+    return getView(position, convertView, parent, true);
+  }
+
+  private View getView(int position, View convertView, ViewGroup parent, boolean isDropdown) {
+    ReactPickerItem item = getItem(position);
+    if (convertView == null) {
+      int layoutResId = isDropdown
+        ? android.R.layout.simple_spinner_dropdown_item
+        : android.R.layout.simple_spinner_item;
+      convertView = mInflater.inflate(layoutResId, parent, false);
+    }
+
+    TextView textView = (TextView) convertView;
+    textView.setText(item.label);
+    if (!isDropdown && mPrimaryTextColor != null) {
+      textView.setTextColor(mPrimaryTextColor);
+    } else if (item.color != null) {
+      textView.setTextColor(item.color);
+    }
+
+    return convertView;
+  }
+
+  public @Nullable Integer getPrimaryTextColor() {
+    return mPrimaryTextColor;
+  }
+
+  public void setPrimaryTextColor(@Nullable Integer primaryTextColor) {
+    mPrimaryTextColor = primaryTextColor;
+    notifyDataSetChanged();
+  }
+}

ReactAndroid/src/main/java/com/facebook/react/views/picker/ReactPickerItem.java

@@ -0,0 +1,39 @@
+package com.facebook.react.views.picker;
+
+import com.facebook.react.bridge.ReadableArray;
+import com.facebook.react.bridge.ReadableMap;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import javax.annotation.Nullable;
+
+/* package */
+class ReactPickerItem {
+  public final String label;
+  @Nullable
+  public final Integer color;
+
+  public ReactPickerItem(final ReadableMap jsMapData) {
+    label = jsMapData.getString("label");
+
+    if (jsMapData.hasKey("color") && !jsMapData.isNull("color")) {
+      color = jsMapData.getInt("color");
+    } else {
+      color = null;
+    }
+  }
+
+  @Nullable
+  public static List<ReactPickerItem> createFromJsArrayMap(final ReadableArray jsArrayMap) {
+    if (jsArrayMap == null) {
+      return null;
+    }
+
+    final List<ReactPickerItem> items = new ArrayList<>(jsArrayMap.size());
+    for (int i = 0; i < jsArrayMap.size(); ++i) {
+      items.add(new ReactPickerItem(jsArrayMap.getMap(i)));
+    }
+    return items;
+  }
+}

ReactAndroid/src/main/java/com/facebook/react/views/picker/ReactPickerManager.java

@@ -7,16 +7,9 @@
 
 package com.facebook.react.views.picker;
 
-import android.content.Context;
-import android.view.LayoutInflater;
-import android.view.View;
-import android.view.ViewGroup;
-import android.widget.ArrayAdapter;
 import android.widget.Spinner;
-import android.widget.TextView;
-import com.facebook.infer.annotation.Assertions;
+
 import com.facebook.react.bridge.ReadableArray;
-import com.facebook.react.bridge.ReadableMap;
 import com.facebook.react.uimanager.SimpleViewManager;
 import com.facebook.react.uimanager.ThemedReactContext;
 import com.facebook.react.uimanager.UIManagerModule;
@@ -24,6 +17,9 @@ import com.facebook.react.uimanager.ViewProps;
 import com.facebook.react.uimanager.annotations.ReactProp;
 import com.facebook.react.uimanager.events.EventDispatcher;
 import com.facebook.react.views.picker.events.PickerItemSelectEvent;
+
+import java.util.List;
+
 import javax.annotation.Nullable;
 
 /**
@@ -37,26 +33,13 @@ public abstract class ReactPickerManager extends SimpleViewManager<ReactPicker>
 
   @ReactProp(name = "items")
   public void setItems(ReactPicker view, @Nullable ReadableArray items) {
-    if (items != null) {
-      ReadableMap[] data = new ReadableMap[items.size()];
-      for (int i = 0; i < items.size(); i++) {
-        data[i] = items.getMap(i);
-      }
-      ReactPickerAdapter adapter = new ReactPickerAdapter(view.getContext(), data);
-      adapter.setPrimaryTextColor(view.getPrimaryColor());
-      view.setStagedAdapter(adapter);
-    } else {
-      view.setStagedAdapter(null);
-    }
+    final List<ReactPickerItem> pickerItems = ReactPickerItem.createFromJsArrayMap(items);
+    view.setStagedItems(pickerItems);
   }
 
   @ReactProp(name = ViewProps.COLOR, customType = "Color")
   public void setColor(ReactPicker view, @Nullable Integer color) {
-    view.setPrimaryColor(color);
-    ReactPickerAdapter adapter = (ReactPickerAdapter) view.getAdapter();
-    if (adapter != null) {
-      adapter.setPrimaryTextColor(color);
-    }
+    view.setStagedPrimaryTextColor(color);
   }
 
   @ReactProp(name = "prompt")
@@ -90,55 +73,6 @@ public abstract class ReactPickerManager extends SimpleViewManager<ReactPicker>
                     reactContext.getNativeModule(UIManagerModule.class).getEventDispatcher()));
   }
 
-  private static class ReactPickerAdapter extends ArrayAdapter<ReadableMap> {
-
-    private final LayoutInflater mInflater;
-    private @Nullable Integer mPrimaryTextColor;
-
-    public ReactPickerAdapter(Context context, ReadableMap[] data) {
-      super(context, 0, data);
-
-      mInflater = (LayoutInflater) Assertions.assertNotNull(
-          context.getSystemService(Context.LAYOUT_INFLATER_SERVICE));
-    }
-
-    @Override
-    public View getView(int position, View convertView, ViewGroup parent) {
-      return getView(position, convertView, parent, false);
-    }
-
-    @Override
-    public View getDropDownView(int position, View convertView, ViewGroup parent) {
-      return getView(position, convertView, parent, true);
-    }
-
-    private View getView(int position, View convertView, ViewGroup parent, boolean isDropdown) {
-      ReadableMap item = getItem(position);
-
-      if (convertView == null) {
-        int layoutResId = isDropdown
-            ? android.R.layout.simple_spinner_dropdown_item
-            : android.R.layout.simple_spinner_item;
-        convertView = mInflater.inflate(layoutResId, parent, false);
-      }
-
-      TextView textView = (TextView) convertView;
-      textView.setText(item.getString("label"));
-      if (!isDropdown && mPrimaryTextColor != null) {
-        textView.setTextColor(mPrimaryTextColor);
-      } else if (item.hasKey("color") && !item.isNull("color")) {
-        textView.setTextColor(item.getInt("color"));
-      }
-
-      return convertView;
-    }
-
-    public void setPrimaryTextColor(@Nullable Integer primaryTextColor) {
-      mPrimaryTextColor = primaryTextColor;
-      notifyDataSetChanged();
-    }
-  }
-
   private static class PickerEventEmitter implements ReactPicker.OnSelectListener {
 
     private final ReactPicker mReactPicker;