From ba87437a7d3583cac78602794cee9e2ff2ca7972 Mon Sep 17 00:00:00 2001 From: Benjamin Schwartz Date: Thu, 18 Feb 2021 08:32:42 -0800 Subject: [PATCH] powerstats: Clients of PowerStatsInternal must nullcheck Added comments and annotations to the PowerStatsInternal and PowerStatsHALWrapper classes. Also fixed the native code to return partial results if available. Bug: 179684579 Test: On battery I no longer see crash Change-Id: Ic97fb56af6559a3fa65f857c527b10303238ac42 --- .../android/power/PowerStatsInternal.java | 16 ++- .../server/am/BatteryStatsService.java | 4 +- .../powerstats/PowerStatsHALWrapper.java | 7 ++ .../powerstats/StatsPullAtomCallbackImpl.java | 4 + ...id_server_powerstats_PowerStatsService.cpp | 100 ++++++++---------- 5 files changed, 67 insertions(+), 64 deletions(-) diff --git a/services/core/java/android/power/PowerStatsInternal.java b/services/core/java/android/power/PowerStatsInternal.java index 40107a5d15781..27da14230110d 100644 --- a/services/core/java/android/power/PowerStatsInternal.java +++ b/services/core/java/android/power/PowerStatsInternal.java @@ -50,7 +50,8 @@ public abstract class PowerStatsInternal { * requested. * * @return A Future containing a list of {@link EnergyConsumerResult} objects containing energy - * consumer results for all listed {@link EnergyConsumerId}. + * consumer results for all listed {@link EnergyConsumerId}. null if + * {@link EnergyConsumer} not supported */ @NonNull public abstract CompletableFuture getEnergyConsumedAsync( @@ -59,8 +60,10 @@ public abstract class PowerStatsInternal { /** * Returns the power entity info for all available {@link PowerEntity} * - * @return List of available {@link PowerEntity} + * @return List of available {@link PowerEntity} or null if {@link PowerEntity} not + * supported */ + @Nullable public abstract PowerEntity[] getPowerEntityInfo(); /** @@ -71,7 +74,8 @@ public abstract class PowerStatsInternal { * requested. * * @return A Future containing a list of {@link StateResidencyResult} objects containing state - * residency results for all listed {@link PowerEntity.id}. + * residency results for all listed {@link PowerEntity.id}. null if {@link PowerEntity} + * not supported */ @NonNull public abstract CompletableFuture getStateResidencyAsync( @@ -80,8 +84,9 @@ public abstract class PowerStatsInternal { /** * Returns the channel info for all available {@link Channel} * - * @return List of available {@link Channel} + * @return List of available {@link Channel} or null if {@link Channel} not supported */ + @Nullable public abstract Channel[] getEnergyMeterInfo(); /** @@ -91,7 +96,8 @@ public abstract class PowerStatsInternal { * @param channelIds Array of {@link Channel.id} for accumulated energy is being requested. * * @return A Future containing a list of {@link EnergyMeasurement} objects containing - * accumulated energy measurements for all listed {@link Channel.id}. + * accumulated energy measurements for all listed {@link Channel.id}. null if + * {@link Channel} not supported */ @NonNull public abstract CompletableFuture readEnergyMeterAsync( diff --git a/services/core/java/com/android/server/am/BatteryStatsService.java b/services/core/java/com/android/server/am/BatteryStatsService.java index 6500f6d1b6d3f..167c2b1ad66c7 100644 --- a/services/core/java/com/android/server/am/BatteryStatsService.java +++ b/services/core/java/com/android/server/am/BatteryStatsService.java @@ -216,6 +216,8 @@ public final class BatteryStatsService extends IBatteryStats.Stub return; } + if (results == null) return; + for (int i = 0; i < results.length; i++) { final StateResidencyResult result = results[i]; RpmStats.PowerStateSubsystem subsystem = @@ -257,7 +259,7 @@ public final class BatteryStatsService extends IBatteryStats.Stub return EMPTY; } - if (results.length == 0) return EMPTY; + if (results == null || results.length == 0) return EMPTY; int charsLeft = MAX_LOW_POWER_STATS_SIZE; StringBuilder builder = new StringBuilder("SubsystemPowerState"); diff --git a/services/core/java/com/android/server/powerstats/PowerStatsHALWrapper.java b/services/core/java/com/android/server/powerstats/PowerStatsHALWrapper.java index 9a91848475fea..f382d10e08462 100644 --- a/services/core/java/com/android/server/powerstats/PowerStatsHALWrapper.java +++ b/services/core/java/com/android/server/powerstats/PowerStatsHALWrapper.java @@ -16,6 +16,7 @@ package com.android.server.powerstats; +import android.annotation.Nullable; import android.hardware.power.stats.Channel; import android.hardware.power.stats.EnergyMeasurement; import android.hardware.power.stats.IPowerStats; @@ -51,6 +52,7 @@ public final class PowerStatsHALWrapper { * * @return List of information on each PowerEntity. */ + @Nullable android.hardware.power.stats.PowerEntity[] getPowerEntityInfo(); /** @@ -70,6 +72,7 @@ public final class PowerStatsHALWrapper { * * @return StateResidency since boot for each requested PowerEntity */ + @Nullable android.hardware.power.stats.StateResidencyResult[] getStateResidency(int[] powerEntityIds); /** @@ -81,6 +84,7 @@ public final class PowerStatsHALWrapper { * * @return List of EnergyConsumers all available energy consumers. */ + @Nullable android.hardware.power.stats.EnergyConsumer[] getEnergyConsumerInfo(); /** @@ -96,6 +100,7 @@ public final class PowerStatsHALWrapper { * @return List of EnergyConsumerResult objects containing energy consumer results for all * available energy consumers (power models). */ + @Nullable android.hardware.power.stats.EnergyConsumerResult[] getEnergyConsumed( int[] energyConsumerIds); @@ -105,6 +110,7 @@ public final class PowerStatsHALWrapper { * @return List of Channel objects containing channel info for all available energy * meters. */ + @Nullable android.hardware.power.stats.Channel[] getEnergyMeterInfo(); /** @@ -120,6 +126,7 @@ public final class PowerStatsHALWrapper { * @return List of EnergyMeasurement objects containing energy measurements for all * available energy meters. */ + @Nullable android.hardware.power.stats.EnergyMeasurement[] readEnergyMeter(int[] channelIds); /** diff --git a/services/core/java/com/android/server/powerstats/StatsPullAtomCallbackImpl.java b/services/core/java/com/android/server/powerstats/StatsPullAtomCallbackImpl.java index bdabefbbeee60..ba778b3a481c5 100644 --- a/services/core/java/com/android/server/powerstats/StatsPullAtomCallbackImpl.java +++ b/services/core/java/com/android/server/powerstats/StatsPullAtomCallbackImpl.java @@ -87,6 +87,8 @@ public class StatsPullAtomCallbackImpl implements StatsManager.StatsPullAtomCall return StatsManager.PULL_SKIP; } + if (energyMeasurements == null) return StatsManager.PULL_SKIP; + for (int i = 0; i < energyMeasurements.length; i++) { // Only report energy measurements that have been accumulated since boot final EnergyMeasurement energyMeasurement = energyMeasurements[i]; @@ -135,6 +137,8 @@ public class StatsPullAtomCallbackImpl implements StatsManager.StatsPullAtomCall return StatsManager.PULL_SKIP; } + if (results == null) return StatsManager.PULL_SKIP; + for (int i = 0; i < results.length; i++) { final StateResidencyResult result = results[i]; for (int j = 0; j < result.stateResidencyData.length; j++) { diff --git a/services/core/jni/com_android_server_powerstats_PowerStatsService.cpp b/services/core/jni/com_android_server_powerstats_PowerStatsService.cpp index 3f54529211dde..0e21aaf33ec76 100644 --- a/services/core/jni/com_android_server_powerstats_PowerStatsService.cpp +++ b/services/core/jni/com_android_server_powerstats_PowerStatsService.cpp @@ -217,41 +217,30 @@ static jobjectArray nativeGetStateResidency(JNIEnv *env, jclass clazz, jintArray jobjectArray stateResidencyResultArray = nullptr; Return ret = gPowerStatsHalV1_0_ptr->getPowerEntityStateResidencyData( powerEntityIdVector, [&env, &stateResidencyResultArray](auto results, auto status) { - if (status != Status::SUCCESS) { - ALOGE("Error getting power entity state residency data"); - } else { - stateResidencyResultArray = - env->NewObjectArray(results.size(), class_SRR, nullptr); - for (int i = 0; i < results.size(); i++) { - jobjectArray stateResidencyArray = - env->NewObjectArray(results[i].stateResidencyData.size(), class_SR, - nullptr); - for (int j = 0; j < results[i].stateResidencyData.size(); j++) { - jobject stateResidency = env->NewObject(class_SR, method_SR_init); - env->SetIntField(stateResidency, field_SR_id, - results[i].stateResidencyData[j].powerEntityStateId); - env->SetLongField(stateResidency, field_SR_totalTimeInStateMs, - results[i].stateResidencyData[j].totalTimeInStateMs); - env->SetLongField(stateResidency, field_SR_totalStateEntryCount, - results[i] - .stateResidencyData[j] - .totalStateEntryCount); - env->SetLongField(stateResidency, field_SR_lastEntryTimestampMs, - results[i] - .stateResidencyData[j] - .lastEntryTimestampMs); - env->SetObjectArrayElement(stateResidencyArray, j, stateResidency); - env->DeleteLocalRef(stateResidency); - } - jobject stateResidencyResult = env->NewObject(class_SRR, method_SRR_init); - env->SetIntField(stateResidencyResult, field_SRR_id, - results[i].powerEntityId); - env->SetObjectField(stateResidencyResult, field_SRR_stateResidencyData, - stateResidencyArray); - env->SetObjectArrayElement(stateResidencyResultArray, i, - stateResidencyResult); - env->DeleteLocalRef(stateResidencyResult); + stateResidencyResultArray = env->NewObjectArray(results.size(), class_SRR, nullptr); + for (int i = 0; i < results.size(); i++) { + jobjectArray stateResidencyArray = + env->NewObjectArray(results[i].stateResidencyData.size(), class_SR, + nullptr); + for (int j = 0; j < results[i].stateResidencyData.size(); j++) { + jobject stateResidency = env->NewObject(class_SR, method_SR_init); + env->SetIntField(stateResidency, field_SR_id, + results[i].stateResidencyData[j].powerEntityStateId); + env->SetLongField(stateResidency, field_SR_totalTimeInStateMs, + results[i].stateResidencyData[j].totalTimeInStateMs); + env->SetLongField(stateResidency, field_SR_totalStateEntryCount, + results[i].stateResidencyData[j].totalStateEntryCount); + env->SetLongField(stateResidency, field_SR_lastEntryTimestampMs, + results[i].stateResidencyData[j].lastEntryTimestampMs); + env->SetObjectArrayElement(stateResidencyArray, j, stateResidency); + env->DeleteLocalRef(stateResidency); } + jobject stateResidencyResult = env->NewObject(class_SRR, method_SRR_init); + env->SetIntField(stateResidencyResult, field_SRR_id, results[i].powerEntityId); + env->SetObjectField(stateResidencyResult, field_SRR_stateResidencyData, + stateResidencyArray); + env->SetObjectArrayElement(stateResidencyResultArray, i, stateResidencyResult); + env->DeleteLocalRef(stateResidencyResult); } }); if (!checkResult(ret, __func__)) { @@ -320,30 +309,25 @@ static jobjectArray nativeReadEnergyMeters(JNIEnv *env, jclass clazz, jintArray gPowerStatsHalV1_0_ptr ->getEnergyData(channelIdVector, [&env, &energyMeasurementArray](auto energyData, auto status) { - if (status != Status::SUCCESS) { - ALOGW("Error getting energy data"); - } else { - energyMeasurementArray = - env->NewObjectArray(energyData.size(), class_EM, - nullptr); - for (int i = 0; i < energyData.size(); i++) { - jobject energyMeasurement = - env->NewObject(class_EM, method_EM_init); - env->SetIntField(energyMeasurement, field_EM_id, - energyData[i].index); - env->SetLongField(energyMeasurement, - field_EM_timestampMs, - energyData[i].timestamp); - env->SetLongField(energyMeasurement, - field_EM_durationMs, - energyData[i].timestamp); - env->SetLongField(energyMeasurement, - field_EM_energyUWs, - energyData[i].energy); - env->SetObjectArrayElement(energyMeasurementArray, - i, energyMeasurement); - env->DeleteLocalRef(energyMeasurement); - } + energyMeasurementArray = + env->NewObjectArray(energyData.size(), class_EM, + nullptr); + for (int i = 0; i < energyData.size(); i++) { + jobject energyMeasurement = + env->NewObject(class_EM, method_EM_init); + env->SetIntField(energyMeasurement, field_EM_id, + energyData[i].index); + env->SetLongField(energyMeasurement, + field_EM_timestampMs, + energyData[i].timestamp); + env->SetLongField(energyMeasurement, + field_EM_durationMs, + energyData[i].timestamp); + env->SetLongField(energyMeasurement, field_EM_energyUWs, + energyData[i].energy); + env->SetObjectArrayElement(energyMeasurementArray, i, + energyMeasurement); + env->DeleteLocalRef(energyMeasurement); } });