diff options
author | Dmitri Plotnikov <dplotnikov@google.com> | 2021-08-16 14:47:55 -0700 |
---|---|---|
committer | Dmitri Plotnikov <dplotnikov@google.com> | 2021-09-20 18:35:09 +0000 |
commit | 0c561475c3aff676a50358c9280cb805b2c99f57 (patch) | |
tree | 1ec385fc48f87494e62c4412f104c5af420ee34e | |
parent | 21cb293e884c1d2bfc60bc9f4252253eac77d419 (diff) |
Check custom component names before aggregating BatteryUsageStats snapshots
If the lists of custom power components do not match, a crash will occur.
Instead of causing a crash, simply skip incompatible snapshots.
Bug: 196040329
Bug: 200511361
Test: atest FrameworksCoreTests:com.android.internal.os.BatteryUsageStatsProviderTest
Change-Id: I87ba605371a5f3119dcff33f6109e94ee46ab57d
(cherry picked from commit a1ea9ecd56fce8bb84fc673650b6be7282ea0e99)
5 files changed, 61 insertions, 7 deletions
diff --git a/core/java/android/os/BatteryUsageStats.java b/core/java/android/os/BatteryUsageStats.java index f48375246616..0f94cbef3886 100644 --- a/core/java/android/os/BatteryUsageStats.java +++ b/core/java/android/os/BatteryUsageStats.java @@ -271,6 +271,16 @@ public final class BatteryUsageStats implements Parcelable { } /** + * Returns the names of custom power components in order, so the first name in the array + * corresponds to the custom componentId + * {@link BatteryConsumer.FIRST_CUSTOM_POWER_COMPONENT_ID}. + */ + @NonNull + public String[] getCustomPowerComponentNames() { + return mCustomPowerComponentNames; + } + + /** * Returns an iterator for {@link android.os.BatteryStats.HistoryItem}'s. */ @NonNull diff --git a/core/java/com/android/internal/os/BatteryUsageStatsProvider.java b/core/java/com/android/internal/os/BatteryUsageStatsProvider.java index 00385793b62b..980aec196079 100644 --- a/core/java/com/android/internal/os/BatteryUsageStatsProvider.java +++ b/core/java/com/android/internal/os/BatteryUsageStatsProvider.java @@ -29,6 +29,7 @@ import android.util.SparseArray; import com.android.internal.annotations.VisibleForTesting; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Map; @@ -234,8 +235,9 @@ public class BatteryUsageStatsProvider { final boolean includePowerModels = (query.getFlags() & BatteryUsageStatsQuery.FLAG_BATTERY_USAGE_STATS_INCLUDE_POWER_MODELS) != 0; + final String[] customEnergyConsumerNames = mStats.getCustomEnergyConsumerNames(); final BatteryUsageStats.Builder builder = new BatteryUsageStats.Builder( - mStats.getCustomEnergyConsumerNames(), includePowerModels); + customEnergyConsumerNames, includePowerModels); if (mBatteryUsageStatsStore == null) { Log.e(TAG, "BatteryUsageStatsStore is unavailable"); return builder.build(); @@ -247,7 +249,14 @@ public class BatteryUsageStatsProvider { final BatteryUsageStats snapshot = mBatteryUsageStatsStore.loadBatteryUsageStats(timestamp); if (snapshot != null) { - builder.add(snapshot); + if (Arrays.equals(snapshot.getCustomPowerComponentNames(), + customEnergyConsumerNames)) { + builder.add(snapshot); + } else { + Log.w(TAG, "Ignoring older BatteryUsageStats snapshot, which has different " + + "custom power components: " + + Arrays.toString(snapshot.getCustomPowerComponentNames())); + } } } } diff --git a/core/tests/coretests/src/com/android/internal/os/BatteryStatsNoteTest.java b/core/tests/coretests/src/com/android/internal/os/BatteryStatsNoteTest.java index b7dc1c59c90c..d4799a8f5fd3 100644 --- a/core/tests/coretests/src/com/android/internal/os/BatteryStatsNoteTest.java +++ b/core/tests/coretests/src/com/android/internal/os/BatteryStatsNoteTest.java @@ -592,7 +592,7 @@ public class BatteryStatsNoteTest extends TestCase { public void testUpdateDisplayMeasuredEnergyStatsLocked() { final MockClocks clocks = new MockClocks(); // holds realtime and uptime in ms final MockBatteryStatsImpl bi = new MockBatteryStatsImpl(clocks); - bi.initMeasuredEnergyStats(); + bi.initMeasuredEnergyStats(new String[]{"FOO", "BAR"}); clocks.realtime = 0; int screen = Display.STATE_OFF; @@ -677,7 +677,7 @@ public class BatteryStatsNoteTest extends TestCase { public void testUpdateCustomMeasuredEnergyStatsLocked_neverCalled() { final MockClocks clocks = new MockClocks(); // holds realtime and uptime in ms final MockBatteryStatsImpl bi = new MockBatteryStatsImpl(clocks); - bi.initMeasuredEnergyStats(); + bi.initMeasuredEnergyStats(new String[]{"FOO", "BAR"}); bi.setOnBatteryInternal(true); final int uid1 = 11500; @@ -691,7 +691,7 @@ public class BatteryStatsNoteTest extends TestCase { public void testUpdateCustomMeasuredEnergyStatsLocked() { final MockClocks clocks = new MockClocks(); // holds realtime and uptime in ms final MockBatteryStatsImpl bi = new MockBatteryStatsImpl(clocks); - bi.initMeasuredEnergyStats(); + bi.initMeasuredEnergyStats(new String[]{"FOO", "BAR"}); final int bucketA = 0; // Custom bucket 0 final int bucketB = 1; // Custom bucket 1 diff --git a/core/tests/coretests/src/com/android/internal/os/BatteryUsageStatsProviderTest.java b/core/tests/coretests/src/com/android/internal/os/BatteryUsageStatsProviderTest.java index 0147cdb186f3..74b6dbe76a16 100644 --- a/core/tests/coretests/src/com/android/internal/os/BatteryUsageStatsProviderTest.java +++ b/core/tests/coretests/src/com/android/internal/os/BatteryUsageStatsProviderTest.java @@ -18,6 +18,9 @@ package com.android.internal.os; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + import android.app.ActivityManager; import android.content.Context; import android.os.BatteryConsumer; @@ -263,6 +266,39 @@ public class BatteryUsageStatsProviderTest { .of(180.0); } + @Test + public void testAggregateBatteryStats_incompatibleSnapshot() { + Context context = InstrumentationRegistry.getContext(); + MockBatteryStatsImpl batteryStats = mStatsRule.getBatteryStats(); + batteryStats.initMeasuredEnergyStats(new String[]{"FOO", "BAR"}); + + BatteryUsageStatsStore batteryUsageStatsStore = mock(BatteryUsageStatsStore.class); + + when(batteryUsageStatsStore.listBatteryUsageStatsTimestamps()) + .thenReturn(new long[]{1000, 2000}); + + when(batteryUsageStatsStore.loadBatteryUsageStats(1000)).thenReturn( + new BatteryUsageStats.Builder(batteryStats.getCustomEnergyConsumerNames()) + .setStatsDuration(1234).build()); + + // Add a snapshot, with a different set of custom power components. It should + // be skipped by the aggregation. + when(batteryUsageStatsStore.loadBatteryUsageStats(2000)).thenReturn( + new BatteryUsageStats.Builder(new String[]{"different"}) + .setStatsDuration(4321).build()); + + BatteryUsageStatsProvider provider = new BatteryUsageStatsProvider(context, + batteryStats, batteryUsageStatsStore); + + BatteryUsageStatsQuery query = new BatteryUsageStatsQuery.Builder() + .aggregateSnapshots(0, 3000) + .build(); + final BatteryUsageStats stats = provider.getBatteryUsageStats(query); + assertThat(stats.getCustomPowerComponentNames()) + .isEqualTo(batteryStats.getCustomEnergyConsumerNames()); + assertThat(stats.getStatsDuration()).isEqualTo(1234); + } + private static class TestHandler extends Handler { TestHandler() { super(Looper.getMainLooper()); diff --git a/core/tests/coretests/src/com/android/internal/os/MockBatteryStatsImpl.java b/core/tests/coretests/src/com/android/internal/os/MockBatteryStatsImpl.java index 99d576d259ec..cee1a0352a7e 100644 --- a/core/tests/coretests/src/com/android/internal/os/MockBatteryStatsImpl.java +++ b/core/tests/coretests/src/com/android/internal/os/MockBatteryStatsImpl.java @@ -57,11 +57,10 @@ public class MockBatteryStatsImpl extends BatteryStatsImpl { this(new MockClocks()); } - public void initMeasuredEnergyStats() { + public void initMeasuredEnergyStats(String[] customBucketNames) { final boolean[] supportedStandardBuckets = new boolean[MeasuredEnergyStats.NUMBER_STANDARD_POWER_BUCKETS]; Arrays.fill(supportedStandardBuckets, true); - final String[] customBucketNames = {"FOO", "BAR"}; mGlobalMeasuredEnergyStats = new MeasuredEnergyStats(supportedStandardBuckets, customBucketNames); } |