From 8d0030d874addb7aca11e0f7e7ff41fdff94409a Mon Sep 17 00:00:00 2001 From: Zaiyue Xue Date: Tue, 9 Aug 2022 16:22:33 +0800 Subject: [PATCH] Fix b/241872474 Battery usage page will crash when selecting the last hour chart bar, going to app detail page, and going back This bug is because we always use mHourlyChartIndex to construct every view model in mHoulyViewModels. However, mHourlyChartIndex could be got from saved instance. So mHourlyChartIndex may be out of bound in some hourly view model which has not many hours data. This fix removes the selectedIndex in BatteryChartViewModel constructor. Suppose the selectedIndex should be set everytime the view model is used. Test: manual Bug: 236101166 Bug: 241872474 Change-Id: I0bb5568ac33fcc23c406fe3af308b8d2706c5542 --- .../BatteryChartPreferenceControllerV2.java | 2 - .../batteryusage/BatteryChartViewModel.java | 34 +++++++-------- ...atteryChartPreferenceControllerV2Test.java | 43 ++++++++----------- .../batteryusage/BatteryChartViewV2Test.java | 12 +++--- 4 files changed, 38 insertions(+), 53 deletions(-) diff --git a/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerV2.java b/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerV2.java index b847f04d048..2bc8c46a992 100644 --- a/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerV2.java +++ b/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerV2.java @@ -284,7 +284,6 @@ public class BatteryChartPreferenceControllerV2 extends AbstractPreferenceContro generateTimestampDayOfWeekTexts( mContext, batteryLevelData.getDailyBatteryLevels().getTimestamps(), /* isAbbreviation= */ true), - mDailyChartIndex, BatteryChartViewModel.AxisLabelPosition.CENTER_OF_TRAPEZOIDS); mHourlyViewModels = new ArrayList<>(); for (BatteryLevelData.PeriodBatteryLevelData hourlyBatteryLevelsPerDay : @@ -293,7 +292,6 @@ public class BatteryChartPreferenceControllerV2 extends AbstractPreferenceContro hourlyBatteryLevelsPerDay.getLevels(), generateTimestampHourTexts( mContext, hourlyBatteryLevelsPerDay.getTimestamps()), - mHourlyChartIndex, BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS)); } refreshUi(); diff --git a/src/com/android/settings/fuelgauge/batteryusage/BatteryChartViewModel.java b/src/com/android/settings/fuelgauge/batteryusage/BatteryChartViewModel.java index 493891f45bc..82a41c4a26e 100644 --- a/src/com/android/settings/fuelgauge/batteryusage/BatteryChartViewModel.java +++ b/src/com/android/settings/fuelgauge/batteryusage/BatteryChartViewModel.java @@ -41,22 +41,18 @@ class BatteryChartViewModel { private final List mLevels; private final List mTexts; private final AxisLabelPosition mAxisLabelPosition; - private int mSelectedIndex; + private int mSelectedIndex = SELECTED_INDEX_ALL; BatteryChartViewModel( - @NonNull List levels, @NonNull List texts, int selectedIndex, + @NonNull List levels, @NonNull List texts, @NonNull AxisLabelPosition axisLabelPosition) { Preconditions.checkArgument( - levels.size() == texts.size() - && levels.size() >= MIN_LEVELS_DATA_SIZE - && selectedIndex >= SELECTED_INDEX_ALL - && selectedIndex < levels.size(), - String.format(Locale.ENGLISH, "Invalid BatteryChartViewModel" - + " levels.size: %d\ntexts.size: %d\nselectedIndex: %d.", - levels.size(), texts.size(), selectedIndex)); + levels.size() == texts.size() && levels.size() >= MIN_LEVELS_DATA_SIZE, + String.format(Locale.ENGLISH, + "Invalid BatteryChartViewModel levels.size: %d, texts.size: %d.", + levels.size(), texts.size())); mLevels = levels; mTexts = texts; - mSelectedIndex = selectedIndex; mAxisLabelPosition = axisLabelPosition; } @@ -72,6 +68,10 @@ class BatteryChartViewModel { return mTexts; } + public AxisLabelPosition axisLabelPosition() { + return mAxisLabelPosition; + } + public int selectedIndex() { return mSelectedIndex; } @@ -80,10 +80,6 @@ class BatteryChartViewModel { mSelectedIndex = index; } - public AxisLabelPosition axisLabelPosition() { - return mAxisLabelPosition; - } - @Override public int hashCode() { return Objects.hash(mLevels, mTexts, mSelectedIndex, mAxisLabelPosition); @@ -99,15 +95,15 @@ class BatteryChartViewModel { final BatteryChartViewModel batteryChartViewModel = (BatteryChartViewModel) other; return Objects.equals(mLevels, batteryChartViewModel.mLevels) && Objects.equals(mTexts, batteryChartViewModel.mTexts) - && mSelectedIndex == batteryChartViewModel.mSelectedIndex - && mAxisLabelPosition == batteryChartViewModel.mAxisLabelPosition; + && mAxisLabelPosition == batteryChartViewModel.mAxisLabelPosition + && mSelectedIndex == batteryChartViewModel.mSelectedIndex; } @Override public String toString() { return String.format(Locale.ENGLISH, - "levels: %s\ntexts: %s\nselectedIndex: %d, axisLabelPosition: %s", - Objects.toString(mLevels), Objects.toString(mTexts), mSelectedIndex, - mAxisLabelPosition); + "levels: %s,\ntexts: %s,\naxisLabelPosition: %s, selectedIndex: %d", + Objects.toString(mLevels), Objects.toString(mTexts), mAxisLabelPosition, + mSelectedIndex); } } diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerV2Test.java b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerV2Test.java index cd985b6ccc5..e738e6534f6 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerV2Test.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerV2Test.java @@ -180,21 +180,21 @@ public final class BatteryChartPreferenceControllerV2Test { verify(mHourlyChartView).setViewModel(new BatteryChartViewModel( List.of(100, 97, 95), List.of("8 am", "10 am", "12 pm"), - BatteryChartViewModel.SELECTED_INDEX_ALL, BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS)); } @Test public void setBatteryChartViewModel_60Hours() { + BatteryChartViewModel expectedDailyViewModel = new BatteryChartViewModel( + List.of(100, 83, 59, 41), + List.of("Sat", "Sun", "Mon", "Mon"), + BatteryChartViewModel.AxisLabelPosition.CENTER_OF_TRAPEZOIDS); + mBatteryChartPreferenceController.setBatteryHistoryMap(createBatteryHistoryMap(60)); verify(mDailyChartView, atLeastOnce()).setVisibility(View.VISIBLE); verify(mHourlyChartView, atLeastOnce()).setVisibility(View.GONE); - verify(mDailyChartView).setViewModel(new BatteryChartViewModel( - List.of(100, 83, 59, 41), - List.of("Sat", "Sun", "Mon", "Mon"), - BatteryChartViewModel.SELECTED_INDEX_ALL, - BatteryChartViewModel.AxisLabelPosition.CENTER_OF_TRAPEZOIDS)); + verify(mDailyChartView).setViewModel(expectedDailyViewModel); reset(mDailyChartView); reset(mHourlyChartView); @@ -203,16 +203,13 @@ public final class BatteryChartPreferenceControllerV2Test { mBatteryChartPreferenceController.refreshUi(); verify(mDailyChartView).setVisibility(View.VISIBLE); verify(mHourlyChartView).setVisibility(View.VISIBLE); - verify(mDailyChartView).setViewModel(new BatteryChartViewModel( - List.of(100, 83, 59, 41), - List.of("Sat", "Sun", "Mon", "Mon"), - 0, - BatteryChartViewModel.AxisLabelPosition.CENTER_OF_TRAPEZOIDS)); + + expectedDailyViewModel.setSelectedIndex(0); + verify(mDailyChartView).setViewModel(expectedDailyViewModel); verify(mHourlyChartView).setViewModel(new BatteryChartViewModel( List.of(100, 97, 95, 93, 91, 89, 87, 85, 83), List.of("8 am", "10 am", "12 pm", "2 pm", "4 pm", "6 pm", "8 pm", "10 pm", "12 am"), - BatteryChartViewModel.SELECTED_INDEX_ALL, BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS)); reset(mDailyChartView); @@ -223,17 +220,15 @@ public final class BatteryChartPreferenceControllerV2Test { mBatteryChartPreferenceController.refreshUi(); verify(mDailyChartView).setVisibility(View.VISIBLE); verify(mHourlyChartView).setVisibility(View.VISIBLE); - verify(mDailyChartView).setViewModel(new BatteryChartViewModel( - List.of(100, 83, 59, 41), - List.of("Sat", "Sun", "Mon", "Mon"), - 1, - BatteryChartViewModel.AxisLabelPosition.CENTER_OF_TRAPEZOIDS)); - verify(mHourlyChartView).setViewModel(new BatteryChartViewModel( + expectedDailyViewModel.setSelectedIndex(1); + verify(mDailyChartView).setViewModel(expectedDailyViewModel); + BatteryChartViewModel expectedHourlyViewModel = new BatteryChartViewModel( List.of(83, 81, 79, 77, 75, 73, 71, 69, 67, 65, 63, 61, 59), List.of("12 am", "2 am", "4 am", "6 am", "8 am", "10 am", "12 pm", "2 pm", "4 pm", "6 pm", "8 pm", "10 pm", "12 am"), - 6, - BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS)); + BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS); + expectedHourlyViewModel.setSelectedIndex(6); + verify(mHourlyChartView).setViewModel(expectedHourlyViewModel); reset(mDailyChartView); reset(mHourlyChartView); @@ -244,16 +239,12 @@ public final class BatteryChartPreferenceControllerV2Test { mBatteryChartPreferenceController.refreshUi(); verify(mDailyChartView).setVisibility(View.VISIBLE); verify(mHourlyChartView).setVisibility(View.VISIBLE); - verify(mDailyChartView).setViewModel(new BatteryChartViewModel( - List.of(100, 83, 59, 41), - List.of("Sat", "Sun", "Mon", "Mon"), - 2, - BatteryChartViewModel.AxisLabelPosition.CENTER_OF_TRAPEZOIDS)); + expectedDailyViewModel.setSelectedIndex(2); + verify(mDailyChartView).setViewModel(expectedDailyViewModel); verify(mHourlyChartView).setViewModel(new BatteryChartViewModel( List.of(59, 57, 55, 53, 51, 49, 47, 45, 43, 41), List.of("12 am", "2 am", "4 am", "6 am", "8 am", "10 am", "12 pm", "2 pm", "4 pm", "6 pm"), - BatteryChartViewModel.SELECTED_INDEX_ALL, BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS)); } diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartViewV2Test.java b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartViewV2Test.java index 174733536bb..10e62d69dfb 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartViewV2Test.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartViewV2Test.java @@ -102,10 +102,11 @@ public final class BatteryChartViewV2Test { @Test public void onClick_invokesCallback() { final int originalSelectedIndex = 2; - mBatteryChartView.setViewModel( - new BatteryChartViewModel(List.of(90, 80, 70, 60), List.of("", "", "", ""), - originalSelectedIndex, - BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS)); + BatteryChartViewModel batteryChartViewModel = new BatteryChartViewModel( + List.of(90, 80, 70, 60), List.of("", "", "", ""), + BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS); + batteryChartViewModel.setSelectedIndex(originalSelectedIndex); + mBatteryChartView.setViewModel(batteryChartViewModel); for (int i = 0; i < mBatteryChartView.mTrapezoidSlots.length; i++) { mBatteryChartView.mTrapezoidSlots[i] = new BatteryChartViewV2.TrapezoidSlot(); mBatteryChartView.mTrapezoidSlots[i].mLeft = i; @@ -192,8 +193,7 @@ public final class BatteryChartViewV2Test { levels.add(index + 1); texts.add(""); } - mBatteryChartView.setViewModel(new BatteryChartViewModel( - levels, texts, BatteryChartViewModel.SELECTED_INDEX_ALL, + mBatteryChartView.setViewModel(new BatteryChartViewModel(levels, texts, BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS)); mBatteryChartView.setClickableForce(true); when(mPowerUsageFeatureProvider.isChartGraphSlotsEnabled(mContext))