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
This commit is contained in:
@@ -284,7 +284,6 @@ public class BatteryChartPreferenceControllerV2 extends AbstractPreferenceContro
|
|||||||
generateTimestampDayOfWeekTexts(
|
generateTimestampDayOfWeekTexts(
|
||||||
mContext, batteryLevelData.getDailyBatteryLevels().getTimestamps(),
|
mContext, batteryLevelData.getDailyBatteryLevels().getTimestamps(),
|
||||||
/* isAbbreviation= */ true),
|
/* isAbbreviation= */ true),
|
||||||
mDailyChartIndex,
|
|
||||||
BatteryChartViewModel.AxisLabelPosition.CENTER_OF_TRAPEZOIDS);
|
BatteryChartViewModel.AxisLabelPosition.CENTER_OF_TRAPEZOIDS);
|
||||||
mHourlyViewModels = new ArrayList<>();
|
mHourlyViewModels = new ArrayList<>();
|
||||||
for (BatteryLevelData.PeriodBatteryLevelData hourlyBatteryLevelsPerDay :
|
for (BatteryLevelData.PeriodBatteryLevelData hourlyBatteryLevelsPerDay :
|
||||||
@@ -293,7 +292,6 @@ public class BatteryChartPreferenceControllerV2 extends AbstractPreferenceContro
|
|||||||
hourlyBatteryLevelsPerDay.getLevels(),
|
hourlyBatteryLevelsPerDay.getLevels(),
|
||||||
generateTimestampHourTexts(
|
generateTimestampHourTexts(
|
||||||
mContext, hourlyBatteryLevelsPerDay.getTimestamps()),
|
mContext, hourlyBatteryLevelsPerDay.getTimestamps()),
|
||||||
mHourlyChartIndex,
|
|
||||||
BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS));
|
BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS));
|
||||||
}
|
}
|
||||||
refreshUi();
|
refreshUi();
|
||||||
|
@@ -41,22 +41,18 @@ class BatteryChartViewModel {
|
|||||||
private final List<Integer> mLevels;
|
private final List<Integer> mLevels;
|
||||||
private final List<String> mTexts;
|
private final List<String> mTexts;
|
||||||
private final AxisLabelPosition mAxisLabelPosition;
|
private final AxisLabelPosition mAxisLabelPosition;
|
||||||
private int mSelectedIndex;
|
private int mSelectedIndex = SELECTED_INDEX_ALL;
|
||||||
|
|
||||||
BatteryChartViewModel(
|
BatteryChartViewModel(
|
||||||
@NonNull List<Integer> levels, @NonNull List<String> texts, int selectedIndex,
|
@NonNull List<Integer> levels, @NonNull List<String> texts,
|
||||||
@NonNull AxisLabelPosition axisLabelPosition) {
|
@NonNull AxisLabelPosition axisLabelPosition) {
|
||||||
Preconditions.checkArgument(
|
Preconditions.checkArgument(
|
||||||
levels.size() == texts.size()
|
levels.size() == texts.size() && levels.size() >= MIN_LEVELS_DATA_SIZE,
|
||||||
&& levels.size() >= MIN_LEVELS_DATA_SIZE
|
String.format(Locale.ENGLISH,
|
||||||
&& selectedIndex >= SELECTED_INDEX_ALL
|
"Invalid BatteryChartViewModel levels.size: %d, texts.size: %d.",
|
||||||
&& selectedIndex < levels.size(),
|
levels.size(), texts.size()));
|
||||||
String.format(Locale.ENGLISH, "Invalid BatteryChartViewModel"
|
|
||||||
+ " levels.size: %d\ntexts.size: %d\nselectedIndex: %d.",
|
|
||||||
levels.size(), texts.size(), selectedIndex));
|
|
||||||
mLevels = levels;
|
mLevels = levels;
|
||||||
mTexts = texts;
|
mTexts = texts;
|
||||||
mSelectedIndex = selectedIndex;
|
|
||||||
mAxisLabelPosition = axisLabelPosition;
|
mAxisLabelPosition = axisLabelPosition;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -72,6 +68,10 @@ class BatteryChartViewModel {
|
|||||||
return mTexts;
|
return mTexts;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public AxisLabelPosition axisLabelPosition() {
|
||||||
|
return mAxisLabelPosition;
|
||||||
|
}
|
||||||
|
|
||||||
public int selectedIndex() {
|
public int selectedIndex() {
|
||||||
return mSelectedIndex;
|
return mSelectedIndex;
|
||||||
}
|
}
|
||||||
@@ -80,10 +80,6 @@ class BatteryChartViewModel {
|
|||||||
mSelectedIndex = index;
|
mSelectedIndex = index;
|
||||||
}
|
}
|
||||||
|
|
||||||
public AxisLabelPosition axisLabelPosition() {
|
|
||||||
return mAxisLabelPosition;
|
|
||||||
}
|
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public int hashCode() {
|
public int hashCode() {
|
||||||
return Objects.hash(mLevels, mTexts, mSelectedIndex, mAxisLabelPosition);
|
return Objects.hash(mLevels, mTexts, mSelectedIndex, mAxisLabelPosition);
|
||||||
@@ -99,15 +95,15 @@ class BatteryChartViewModel {
|
|||||||
final BatteryChartViewModel batteryChartViewModel = (BatteryChartViewModel) other;
|
final BatteryChartViewModel batteryChartViewModel = (BatteryChartViewModel) other;
|
||||||
return Objects.equals(mLevels, batteryChartViewModel.mLevels)
|
return Objects.equals(mLevels, batteryChartViewModel.mLevels)
|
||||||
&& Objects.equals(mTexts, batteryChartViewModel.mTexts)
|
&& Objects.equals(mTexts, batteryChartViewModel.mTexts)
|
||||||
&& mSelectedIndex == batteryChartViewModel.mSelectedIndex
|
&& mAxisLabelPosition == batteryChartViewModel.mAxisLabelPosition
|
||||||
&& mAxisLabelPosition == batteryChartViewModel.mAxisLabelPosition;
|
&& mSelectedIndex == batteryChartViewModel.mSelectedIndex;
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public String toString() {
|
public String toString() {
|
||||||
return String.format(Locale.ENGLISH,
|
return String.format(Locale.ENGLISH,
|
||||||
"levels: %s\ntexts: %s\nselectedIndex: %d, axisLabelPosition: %s",
|
"levels: %s,\ntexts: %s,\naxisLabelPosition: %s, selectedIndex: %d",
|
||||||
Objects.toString(mLevels), Objects.toString(mTexts), mSelectedIndex,
|
Objects.toString(mLevels), Objects.toString(mTexts), mAxisLabelPosition,
|
||||||
mAxisLabelPosition);
|
mSelectedIndex);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@@ -180,21 +180,21 @@ public final class BatteryChartPreferenceControllerV2Test {
|
|||||||
verify(mHourlyChartView).setViewModel(new BatteryChartViewModel(
|
verify(mHourlyChartView).setViewModel(new BatteryChartViewModel(
|
||||||
List.of(100, 97, 95),
|
List.of(100, 97, 95),
|
||||||
List.of("8 am", "10 am", "12 pm"),
|
List.of("8 am", "10 am", "12 pm"),
|
||||||
BatteryChartViewModel.SELECTED_INDEX_ALL,
|
|
||||||
BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS));
|
BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS));
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void setBatteryChartViewModel_60Hours() {
|
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));
|
mBatteryChartPreferenceController.setBatteryHistoryMap(createBatteryHistoryMap(60));
|
||||||
|
|
||||||
verify(mDailyChartView, atLeastOnce()).setVisibility(View.VISIBLE);
|
verify(mDailyChartView, atLeastOnce()).setVisibility(View.VISIBLE);
|
||||||
verify(mHourlyChartView, atLeastOnce()).setVisibility(View.GONE);
|
verify(mHourlyChartView, atLeastOnce()).setVisibility(View.GONE);
|
||||||
verify(mDailyChartView).setViewModel(new BatteryChartViewModel(
|
verify(mDailyChartView).setViewModel(expectedDailyViewModel);
|
||||||
List.of(100, 83, 59, 41),
|
|
||||||
List.of("Sat", "Sun", "Mon", "Mon"),
|
|
||||||
BatteryChartViewModel.SELECTED_INDEX_ALL,
|
|
||||||
BatteryChartViewModel.AxisLabelPosition.CENTER_OF_TRAPEZOIDS));
|
|
||||||
|
|
||||||
reset(mDailyChartView);
|
reset(mDailyChartView);
|
||||||
reset(mHourlyChartView);
|
reset(mHourlyChartView);
|
||||||
@@ -203,16 +203,13 @@ public final class BatteryChartPreferenceControllerV2Test {
|
|||||||
mBatteryChartPreferenceController.refreshUi();
|
mBatteryChartPreferenceController.refreshUi();
|
||||||
verify(mDailyChartView).setVisibility(View.VISIBLE);
|
verify(mDailyChartView).setVisibility(View.VISIBLE);
|
||||||
verify(mHourlyChartView).setVisibility(View.VISIBLE);
|
verify(mHourlyChartView).setVisibility(View.VISIBLE);
|
||||||
verify(mDailyChartView).setViewModel(new BatteryChartViewModel(
|
|
||||||
List.of(100, 83, 59, 41),
|
expectedDailyViewModel.setSelectedIndex(0);
|
||||||
List.of("Sat", "Sun", "Mon", "Mon"),
|
verify(mDailyChartView).setViewModel(expectedDailyViewModel);
|
||||||
0,
|
|
||||||
BatteryChartViewModel.AxisLabelPosition.CENTER_OF_TRAPEZOIDS));
|
|
||||||
verify(mHourlyChartView).setViewModel(new BatteryChartViewModel(
|
verify(mHourlyChartView).setViewModel(new BatteryChartViewModel(
|
||||||
List.of(100, 97, 95, 93, 91, 89, 87, 85, 83),
|
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",
|
List.of("8 am", "10 am", "12 pm", "2 pm", "4 pm", "6 pm", "8 pm", "10 pm",
|
||||||
"12 am"),
|
"12 am"),
|
||||||
BatteryChartViewModel.SELECTED_INDEX_ALL,
|
|
||||||
BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS));
|
BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS));
|
||||||
|
|
||||||
reset(mDailyChartView);
|
reset(mDailyChartView);
|
||||||
@@ -223,17 +220,15 @@ public final class BatteryChartPreferenceControllerV2Test {
|
|||||||
mBatteryChartPreferenceController.refreshUi();
|
mBatteryChartPreferenceController.refreshUi();
|
||||||
verify(mDailyChartView).setVisibility(View.VISIBLE);
|
verify(mDailyChartView).setVisibility(View.VISIBLE);
|
||||||
verify(mHourlyChartView).setVisibility(View.VISIBLE);
|
verify(mHourlyChartView).setVisibility(View.VISIBLE);
|
||||||
verify(mDailyChartView).setViewModel(new BatteryChartViewModel(
|
expectedDailyViewModel.setSelectedIndex(1);
|
||||||
List.of(100, 83, 59, 41),
|
verify(mDailyChartView).setViewModel(expectedDailyViewModel);
|
||||||
List.of("Sat", "Sun", "Mon", "Mon"),
|
BatteryChartViewModel expectedHourlyViewModel = new BatteryChartViewModel(
|
||||||
1,
|
|
||||||
BatteryChartViewModel.AxisLabelPosition.CENTER_OF_TRAPEZOIDS));
|
|
||||||
verify(mHourlyChartView).setViewModel(new BatteryChartViewModel(
|
|
||||||
List.of(83, 81, 79, 77, 75, 73, 71, 69, 67, 65, 63, 61, 59),
|
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",
|
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"),
|
"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(mDailyChartView);
|
||||||
reset(mHourlyChartView);
|
reset(mHourlyChartView);
|
||||||
@@ -244,16 +239,12 @@ public final class BatteryChartPreferenceControllerV2Test {
|
|||||||
mBatteryChartPreferenceController.refreshUi();
|
mBatteryChartPreferenceController.refreshUi();
|
||||||
verify(mDailyChartView).setVisibility(View.VISIBLE);
|
verify(mDailyChartView).setVisibility(View.VISIBLE);
|
||||||
verify(mHourlyChartView).setVisibility(View.VISIBLE);
|
verify(mHourlyChartView).setVisibility(View.VISIBLE);
|
||||||
verify(mDailyChartView).setViewModel(new BatteryChartViewModel(
|
expectedDailyViewModel.setSelectedIndex(2);
|
||||||
List.of(100, 83, 59, 41),
|
verify(mDailyChartView).setViewModel(expectedDailyViewModel);
|
||||||
List.of("Sat", "Sun", "Mon", "Mon"),
|
|
||||||
2,
|
|
||||||
BatteryChartViewModel.AxisLabelPosition.CENTER_OF_TRAPEZOIDS));
|
|
||||||
verify(mHourlyChartView).setViewModel(new BatteryChartViewModel(
|
verify(mHourlyChartView).setViewModel(new BatteryChartViewModel(
|
||||||
List.of(59, 57, 55, 53, 51, 49, 47, 45, 43, 41),
|
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",
|
List.of("12 am", "2 am", "4 am", "6 am", "8 am", "10 am", "12 pm", "2 pm",
|
||||||
"4 pm", "6 pm"),
|
"4 pm", "6 pm"),
|
||||||
BatteryChartViewModel.SELECTED_INDEX_ALL,
|
|
||||||
BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS));
|
BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@@ -102,10 +102,11 @@ public final class BatteryChartViewV2Test {
|
|||||||
@Test
|
@Test
|
||||||
public void onClick_invokesCallback() {
|
public void onClick_invokesCallback() {
|
||||||
final int originalSelectedIndex = 2;
|
final int originalSelectedIndex = 2;
|
||||||
mBatteryChartView.setViewModel(
|
BatteryChartViewModel batteryChartViewModel = new BatteryChartViewModel(
|
||||||
new BatteryChartViewModel(List.of(90, 80, 70, 60), List.of("", "", "", ""),
|
List.of(90, 80, 70, 60), List.of("", "", "", ""),
|
||||||
originalSelectedIndex,
|
BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS);
|
||||||
BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS));
|
batteryChartViewModel.setSelectedIndex(originalSelectedIndex);
|
||||||
|
mBatteryChartView.setViewModel(batteryChartViewModel);
|
||||||
for (int i = 0; i < mBatteryChartView.mTrapezoidSlots.length; i++) {
|
for (int i = 0; i < mBatteryChartView.mTrapezoidSlots.length; i++) {
|
||||||
mBatteryChartView.mTrapezoidSlots[i] = new BatteryChartViewV2.TrapezoidSlot();
|
mBatteryChartView.mTrapezoidSlots[i] = new BatteryChartViewV2.TrapezoidSlot();
|
||||||
mBatteryChartView.mTrapezoidSlots[i].mLeft = i;
|
mBatteryChartView.mTrapezoidSlots[i].mLeft = i;
|
||||||
@@ -192,8 +193,7 @@ public final class BatteryChartViewV2Test {
|
|||||||
levels.add(index + 1);
|
levels.add(index + 1);
|
||||||
texts.add("");
|
texts.add("");
|
||||||
}
|
}
|
||||||
mBatteryChartView.setViewModel(new BatteryChartViewModel(
|
mBatteryChartView.setViewModel(new BatteryChartViewModel(levels, texts,
|
||||||
levels, texts, BatteryChartViewModel.SELECTED_INDEX_ALL,
|
|
||||||
BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS));
|
BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS));
|
||||||
mBatteryChartView.setClickableForce(true);
|
mBatteryChartView.setClickableForce(true);
|
||||||
when(mPowerUsageFeatureProvider.isChartGraphSlotsEnabled(mContext))
|
when(mPowerUsageFeatureProvider.isChartGraphSlotsEnabled(mContext))
|
||||||
|
Reference in New Issue
Block a user