diff --git a/src/com/android/settings/datetime/timezone/BaseTimeZoneAdapter.java b/src/com/android/settings/datetime/timezone/BaseTimeZoneAdapter.java index effa9485a2f..5a614a36888 100644 --- a/src/com/android/settings/datetime/timezone/BaseTimeZoneAdapter.java +++ b/src/com/android/settings/datetime/timezone/BaseTimeZoneAdapter.java @@ -18,6 +18,7 @@ package com.android.settings.datetime.timezone; import android.icu.text.BreakIterator; import android.support.annotation.NonNull; +import android.support.annotation.VisibleForTesting; import android.support.annotation.WorkerThread; import android.support.v7.widget.RecyclerView; import android.text.TextUtils; @@ -42,14 +43,14 @@ public class BaseTimeZoneAdapter extends RecyclerView.Adapter { private final List mOriginalItems; - private final OnListItemClickListener mOnListItemClickListener; + private final OnListItemClickListener mOnListItemClickListener; private final Locale mLocale; private final boolean mShowItemSummary; private List mItems; private ArrayFilter mFilter; - public BaseTimeZoneAdapter(List items, OnListItemClickListener + public BaseTimeZoneAdapter(List items, OnListItemClickListener onListItemClickListener, Locale locale, boolean showItemSummary) { mOriginalItems = items; mItems = items; @@ -69,14 +70,8 @@ public class BaseTimeZoneAdapter @Override public void onBindViewHolder(@NonNull ItemViewHolder holder, int position) { - final AdapterItem item = mItems.get(position); - holder.mSummaryFrame.setVisibility( - mShowItemSummary ? View.VISIBLE : View.GONE); - holder.mTitleView.setText(item.getTitle()); - holder.mIconTextView.setText(item.getIconText()); - holder.mSummaryView.setText(item.getSummary()); - holder.mTimeView.setText(item.getCurrentTime()); - holder.setPosition(position); + holder.setAdapterItem(mItems.get(position)); + holder.mSummaryFrame.setVisibility(mShowItemSummary ? View.VISIBLE : View.GONE); } @Override @@ -89,8 +84,7 @@ public class BaseTimeZoneAdapter return mItems.size(); } - public @NonNull - Filter getFilter() { + public @NonNull ArrayFilter getFilter() { if (mFilter == null) { mFilter = new ArrayFilter(); } @@ -110,18 +104,18 @@ public class BaseTimeZoneAdapter String[] getSearchKeys(); } - public static class ItemViewHolder extends RecyclerView.ViewHolder - implements View.OnClickListener{ + public static class ItemViewHolder + extends RecyclerView.ViewHolder implements View.OnClickListener { - final OnListItemClickListener mOnListItemClickListener; + final OnListItemClickListener mOnListItemClickListener; final View mSummaryFrame; final TextView mTitleView; final TextView mIconTextView; final TextView mSummaryView; final TextView mTimeView; - private int mPosition; + private T mItem; - public ItemViewHolder(View itemView, OnListItemClickListener onListItemClickListener) { + public ItemViewHolder(View itemView, OnListItemClickListener onListItemClickListener) { super(itemView); itemView.setOnClickListener(this); mSummaryFrame = itemView.findViewById(R.id.summary_frame); @@ -132,13 +126,17 @@ public class BaseTimeZoneAdapter mOnListItemClickListener = onListItemClickListener; } - public void setPosition(int position) { - mPosition = position; + public void setAdapterItem(T item) { + mItem = item; + mTitleView.setText(item.getTitle()); + mIconTextView.setText(item.getIconText()); + mSummaryView.setText(item.getSummary()); + mTimeView.setText(item.getCurrentTime()); } @Override public void onClick(View v) { - mOnListItemClickListener.onListItemClick(mPosition); + mOnListItemClickListener.onListItemClick(mItem); } } @@ -151,7 +149,8 @@ public class BaseTimeZoneAdapter * require additional pre-processing. Potentially, a trie structure can be used to match * prefixes of the search keys. */ - private class ArrayFilter extends Filter { + @VisibleForTesting + public class ArrayFilter extends Filter { private BreakIterator mBreakIterator = BreakIterator.getWordInstance(mLocale); @@ -197,8 +196,9 @@ public class BaseTimeZoneAdapter return results; } + @VisibleForTesting @Override - protected void publishResults(CharSequence constraint, FilterResults results) { + public void publishResults(CharSequence constraint, FilterResults results) { mItems = (List) results.values; notifyDataSetChanged(); } diff --git a/src/com/android/settings/datetime/timezone/BaseTimeZoneInfoPicker.java b/src/com/android/settings/datetime/timezone/BaseTimeZoneInfoPicker.java index c1a3f8bb341..d7345428330 100644 --- a/src/com/android/settings/datetime/timezone/BaseTimeZoneInfoPicker.java +++ b/src/com/android/settings/datetime/timezone/BaseTimeZoneInfoPicker.java @@ -31,7 +31,6 @@ import java.util.ArrayList; import java.util.Date; import java.util.List; import java.util.Locale; -import java.util.Objects; /** * Render a list of {@class TimeZoneInfo} into the list view in {@class BaseTimeZonePicker} @@ -52,8 +51,8 @@ public abstract class BaseTimeZoneInfoPicker extends BaseTimeZonePicker { return mAdapter; } - private void onListItemClick(int position) { - final TimeZoneInfo timeZoneInfo = mAdapter.getItem(position).mTimeZoneInfo; + private void onListItemClick(TimeZoneInfoItem item) { + final TimeZoneInfo timeZoneInfo = item.mTimeZoneInfo; getActivity().setResult(Activity.RESULT_OK, prepareResultData(timeZoneInfo)); getActivity().finish(); } @@ -67,7 +66,7 @@ public abstract class BaseTimeZoneInfoPicker extends BaseTimeZonePicker { protected static class ZoneAdapter extends BaseTimeZoneAdapter { public ZoneAdapter(Context context, List timeZones, - OnListItemClickListener onListItemClickListener, Locale locale) { + OnListItemClickListener onListItemClickListener, Locale locale) { super(createTimeZoneInfoItems(context, timeZones, locale), onListItemClickListener, locale, true /* showItemSummary */); } diff --git a/src/com/android/settings/datetime/timezone/BaseTimeZonePicker.java b/src/com/android/settings/datetime/timezone/BaseTimeZonePicker.java index 032e2d296ea..5150045e0a5 100644 --- a/src/com/android/settings/datetime/timezone/BaseTimeZonePicker.java +++ b/src/com/android/settings/datetime/timezone/BaseTimeZonePicker.java @@ -17,6 +17,7 @@ package com.android.settings.datetime.timezone; import android.os.Bundle; +import android.support.annotation.VisibleForTesting; import android.support.v7.widget.LinearLayoutManager; import android.support.v7.widget.RecyclerView; import android.view.LayoutInflater; @@ -161,8 +162,8 @@ public abstract class BaseTimeZonePicker extends InstrumentedFragment return false; } - public interface OnListItemClickListener { - void onListItemClick(int position); + public interface OnListItemClickListener { + void onListItemClick(T item); } } diff --git a/src/com/android/settings/datetime/timezone/RegionSearchPicker.java b/src/com/android/settings/datetime/timezone/RegionSearchPicker.java index bb87e85d0b5..ca4e0bcddc5 100644 --- a/src/com/android/settings/datetime/timezone/RegionSearchPicker.java +++ b/src/com/android/settings/datetime/timezone/RegionSearchPicker.java @@ -18,15 +18,16 @@ package com.android.settings.datetime.timezone; import android.app.Activity; import android.content.Intent; -import android.graphics.Paint; import android.icu.text.Collator; import android.icu.text.LocaleDisplayNames; import android.os.Bundle; +import android.support.annotation.VisibleForTesting; import android.util.Log; import com.android.internal.logging.nano.MetricsProto; import com.android.settings.R; import com.android.settings.core.SubSettingLauncher; +import com.android.settings.datetime.timezone.BaseTimeZoneAdapter.AdapterItem; import com.android.settings.datetime.timezone.model.FilteredCountryTimeZones; import com.android.settings.datetime.timezone.model.TimeZoneData; @@ -63,8 +64,8 @@ public class RegionSearchPicker extends BaseTimeZonePicker { return mAdapter; } - private void onListItemClick(int position) { - final String regionId = mAdapter.getItem(position).getId(); + private void onListItemClick(RegionItem item) { + final String regionId = item.getId(); final FilteredCountryTimeZones countryTimeZones = mTimeZoneData.lookupCountryTimeZones( regionId); final Activity activity = getActivity(); @@ -119,7 +120,8 @@ public class RegionSearchPicker extends BaseTimeZonePicker { return new ArrayList<>(items); } - private static class RegionItem implements BaseTimeZoneAdapter.AdapterItem { + @VisibleForTesting + static class RegionItem implements AdapterItem { private final String mId; private final String mName; diff --git a/tests/robotests/src/com/android/settings/datetime/timezone/RegionSearchPickerTest.java b/tests/robotests/src/com/android/settings/datetime/timezone/RegionSearchPickerTest.java index b2c7f035c36..8da9cbf1c9b 100644 --- a/tests/robotests/src/com/android/settings/datetime/timezone/RegionSearchPickerTest.java +++ b/tests/robotests/src/com/android/settings/datetime/timezone/RegionSearchPickerTest.java @@ -16,7 +16,20 @@ package com.android.settings.datetime.timezone; +import static com.google.common.truth.Truth.assertThat; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import android.app.Activity; +import android.app.Fragment; +import android.widget.Filter; +import android.widget.LinearLayout; + import com.android.settings.datetime.timezone.BaseTimeZoneAdapter.AdapterItem; +import com.android.settings.datetime.timezone.BaseTimeZoneAdapter.ItemViewHolder; +import com.android.settings.datetime.timezone.RegionSearchPicker.RegionItem; import com.android.settings.datetime.timezone.model.TimeZoneData; import com.android.settings.testutils.SettingsRobolectricTestRunner; @@ -24,17 +37,23 @@ import libcore.util.CountryZonesFinder; import org.junit.Test; import org.junit.runner.RunWith; +import org.robolectric.Robolectric; +import org.robolectric.RuntimeEnvironment; +import org.robolectric.annotation.Config; +import org.robolectric.annotation.Implementation; +import org.robolectric.annotation.Implements; + import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Locale; -import static com.google.common.truth.Truth.assertThat; -import static org.junit.Assert.assertEquals; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - @RunWith(SettingsRobolectricTestRunner.class) +@Config(shadows = { + RegionSearchPickerTest.ShadowBaseTimeZonePicker.class, + RegionSearchPickerTest.ShadowFragment.class, + } +) public class RegionSearchPickerTest { @Test @@ -44,16 +63,90 @@ public class RegionSearchPickerTest { CountryZonesFinder finder = mock(CountryZonesFinder.class); when(finder.lookupAllCountryIsoCodes()).thenReturn(regionList); - RegionSearchPicker picker = new RegionSearchPicker() { - @Override - protected Locale getLocale() { - return Locale.US; - } - }; + RegionSearchPicker picker = new RegionSearchPicker(); BaseTimeZoneAdapter adapter = picker.createAdapter(new TimeZoneData(finder)); assertEquals(1, adapter.getItemCount()); AdapterItem item = adapter.getItem(0); assertEquals("United States", item.getTitle().toString()); assertThat(Arrays.asList(item.getSearchKeys())).contains("United States"); } + + // Test RegionSearchPicker does not crash due to the wrong assumption that no view is clicked + // before all views are updated and after internal data structure is updated for text filtering. + // This test mocks the text filtering event and emit click event immediately + // http://b/75322108 + @Test + public void clickItemView_duringRegionSearch_shouldNotCrash() { + List regionList = new ArrayList(); + regionList.add("US"); + CountryZonesFinder finder = mock(CountryZonesFinder.class); + when(finder.lookupAllCountryIsoCodes()).thenReturn(regionList); + + // Prepare the picker and adapter + RegionSearchPicker picker = new RegionSearchPicker(); + BaseTimeZoneAdapter adapter = picker.createAdapter(new TimeZoneData(finder)); + // Prepare and bind a new ItemViewHolder with United States + ItemViewHolder viewHolder = adapter.onCreateViewHolder( + new LinearLayout(RuntimeEnvironment.application), 0); + adapter.onBindViewHolder(viewHolder, 0); + assertEquals(1, adapter.getItemCount()); + + // Pretend to search for a unknown region and no result is found. + FilterWrapper filterWrapper = new FilterWrapper(adapter.getFilter()); + filterWrapper.publishEmptyResult("Unknown region 1"); + + // Assert that the adapter should have been updated with no item + assertEquals(0, adapter.getItemCount()); + viewHolder.itemView.performClick(); // This should not crash + } + + // FilterResults is a protected inner class. Use FilterWrapper to create an empty FilterResults + // instance. + private static class FilterWrapper extends Filter { + + private final BaseTimeZoneAdapter.ArrayFilter mFilter; + + private FilterWrapper(BaseTimeZoneAdapter.ArrayFilter filter) { + mFilter = filter; + } + + @Override + protected FilterResults performFiltering(CharSequence charSequence) { + return null; + } + + private void publishEmptyResult(CharSequence charSequence) { + FilterResults filterResults = new FilterResults(); + filterResults.count = 0; + filterResults.values = new ArrayList<>(); + publishResults(charSequence, filterResults); + } + + @Override + protected void publishResults(CharSequence charSequence, FilterResults filterResults) { + mFilter.publishResults(charSequence, filterResults); + } + } + + // Robolectric can't start android.app.Fragment with support library v4 resources. Pretend + // the fragment has started, and provide the objects in context here. + @Implements(BaseTimeZonePicker.class) + public static class ShadowBaseTimeZonePicker extends ShadowFragment { + + @Implementation + protected Locale getLocale() { + return Locale.US; + } + } + + @Implements(Fragment.class) + public static class ShadowFragment { + + private Activity mActivity = Robolectric.setupActivity(Activity.class); + + @Implementation + public final Activity getActivity() { + return mActivity; + } + } }