From e07408d447d10db7f14117515d1c53728aa8db4a Mon Sep 17 00:00:00 2001 From: Dmitri Plotnikov Date: Thu, 16 Dec 2021 19:14:03 -0800 Subject: [PATCH] Include saved battery history buffers into BatteryUsageStats parcel Bug: 209297031 Test: atest FrameworksCoreTests:BatteryUsageStatsTest FrameworksCoreTests:BatteryUsageStatsProviderTest Change-Id: I369d863b6f6fd488030aa031cc465bef6ce99ab8 --- core/java/android/os/BatteryUsageStats.java | 25 ++++---- .../internal/os/BatteryStatsHistory.java | 60 ++++++++++++++++--- .../os/BatteryUsageStatsProvider.java | 9 ++- .../os/BatteryUsageStatsProviderTest.java | 9 ++- .../internal/os/BatteryUsageStatsRule.java | 9 ++- 5 files changed, 84 insertions(+), 28 deletions(-) diff --git a/core/java/android/os/BatteryUsageStats.java b/core/java/android/os/BatteryUsageStats.java index a23dae80625d4..abe5f81088b12 100644 --- a/core/java/android/os/BatteryUsageStats.java +++ b/core/java/android/os/BatteryUsageStats.java @@ -135,7 +135,7 @@ public final class BatteryUsageStats implements Parcelable, Closeable { private final List mUidBatteryConsumers; private final List mUserBatteryConsumers; private final AggregateBatteryConsumer[] mAggregateBatteryConsumers; - private final Parcel mHistoryBuffer; + private final BatteryStatsHistory mBatteryStatsHistory; private CursorWindow mBatteryConsumersCursorWindow; private BatteryUsageStats(@NonNull Builder builder) { @@ -146,7 +146,7 @@ public final class BatteryUsageStats implements Parcelable, Closeable { mDischargePercentage = builder.mDischargePercentage; mDischargedPowerLowerBound = builder.mDischargedPowerLowerBoundMah; mDischargedPowerUpperBound = builder.mDischargedPowerUpperBoundMah; - mHistoryBuffer = builder.mHistoryBuffer; + mBatteryStatsHistory = builder.mBatteryStatsHistory; mBatteryTimeRemainingMs = builder.mBatteryTimeRemainingMs; mChargeTimeRemainingMs = builder.mChargeTimeRemainingMs; mCustomPowerComponentNames = builder.mCustomPowerComponentNames; @@ -301,11 +301,11 @@ public final class BatteryUsageStats implements Parcelable, Closeable { */ @NonNull public BatteryStatsHistoryIterator iterateBatteryStatsHistory() { - if (mHistoryBuffer == null) { + if (mBatteryStatsHistory == null) { throw new IllegalStateException( "Battery history was not requested in the BatteryUsageStatsQuery"); } - return new BatteryStatsHistoryIterator(new BatteryStatsHistory(mHistoryBuffer)); + return new BatteryStatsHistoryIterator(mBatteryStatsHistory); } @Override @@ -363,12 +363,9 @@ public final class BatteryUsageStats implements Parcelable, Closeable { } if (source.readBoolean()) { - final byte[] historyBlob = source.readBlob(); - - mHistoryBuffer = Parcel.obtain(); - mHistoryBuffer.unmarshall(historyBlob, 0, historyBlob.length); + mBatteryStatsHistory = BatteryStatsHistory.createFromBatteryUsageStatsParcel(source); } else { - mHistoryBuffer = null; + mBatteryStatsHistory = null; } } @@ -389,9 +386,9 @@ public final class BatteryUsageStats implements Parcelable, Closeable { mBatteryConsumersCursorWindow.writeToParcel(dest, flags); - if (mHistoryBuffer != null) { + if (mBatteryStatsHistory != null) { dest.writeBoolean(true); - dest.writeBlob(mHistoryBuffer.marshall()); + mBatteryStatsHistory.writeToBatteryUsageStatsParcel(dest); } else { dest.writeBoolean(false); } @@ -770,7 +767,7 @@ public final class BatteryUsageStats implements Parcelable, Closeable { new SparseArray<>(); private final SparseArray mUserBatteryConsumerBuilders = new SparseArray<>(); - private Parcel mHistoryBuffer; + private BatteryStatsHistory mBatteryStatsHistory; public Builder(@NonNull String[] customPowerComponentNames) { this(customPowerComponentNames, false, false); @@ -895,8 +892,8 @@ public final class BatteryUsageStats implements Parcelable, Closeable { * Sets the parceled recent history. */ @NonNull - public Builder setBatteryHistory(Parcel historyBuffer) { - mHistoryBuffer = historyBuffer; + public Builder setBatteryHistory(BatteryStatsHistory batteryStatsHistory) { + mBatteryStatsHistory = batteryStatsHistory; return this; } diff --git a/core/java/com/android/internal/os/BatteryStatsHistory.java b/core/java/com/android/internal/os/BatteryStatsHistory.java index 879e0a8cfe10f..b0fce8f18742a 100644 --- a/core/java/com/android/internal/os/BatteryStatsHistory.java +++ b/core/java/com/android/internal/os/BatteryStatsHistory.java @@ -160,6 +160,11 @@ public class BatteryStatsHistory { mHistoryDir = null; mHistoryBuffer = historyBuffer; } + + public File getHistoryDirectory() { + return mHistoryDir; + } + /** * Set the active file that mHistoryBuffer is backed up into. * @@ -375,12 +380,26 @@ public class BatteryStatsHistory { } /** - * Read all history files and serialize into a big Parcel. This is to send history files to - * Settings app since Settings app can not access /data/system directory. - * Checkin file also call this method. + * Read all history files and serialize into a big Parcel. + * Checkin file calls this method. + * * @param out the output parcel */ public void writeToParcel(Parcel out) { + writeToParcel(out, false /* useBlobs */); + } + + /** + * This is for Settings app, when Settings app receives big history parcel, it call + * this method to parse it into list of parcels. + * @param out the output parcel + */ + public void writeToBatteryUsageStatsParcel(Parcel out) { + out.writeBlob(mHistoryBuffer.marshall()); + writeToParcel(out, true /* useBlobs */); + } + + private void writeToParcel(Parcel out, boolean useBlobs) { final long start = SystemClock.uptimeMillis(); out.writeInt(mFileNumbers.size() - 1); for(int i = 0; i < mFileNumbers.size() - 1; i++) { @@ -391,7 +410,12 @@ public class BatteryStatsHistory { } catch(Exception e) { Slog.e(TAG, "Error reading file "+ file.getBaseFile().getPath(), e); } - out.writeByteArray(raw); + if (useBlobs) { + out.writeBlob(raw); + } else { + // Avoiding blobs in the check-in file for compatibility + out.writeByteArray(raw); + } } if (DEBUG) { Slog.d(TAG, "writeToParcel duration ms:" + (SystemClock.uptimeMillis() - start)); @@ -399,18 +423,36 @@ public class BatteryStatsHistory { } /** - * This is for Settings app, when Settings app receives big history parcel, it call - * this method to parse it into list of parcels. - * Checkin file also call this method. + * Reads a BatteryStatsHistory from a parcel written with + * the {@link #writeToBatteryUsageStatsParcel} method. + */ + public static BatteryStatsHistory createFromBatteryUsageStatsParcel(Parcel in) { + final byte[] historyBlob = in.readBlob(); + + Parcel historyBuffer = Parcel.obtain(); + historyBuffer.unmarshall(historyBlob, 0, historyBlob.length); + + BatteryStatsHistory history = new BatteryStatsHistory(historyBuffer); + history.readFromParcel(in, true /* useBlobs */); + return history; + } + + /** + * This is for the check-in file, which has all history files embedded. + * * @param in the input parcel. */ public void readFromParcel(Parcel in) { + readFromParcel(in, false /* useBlobs */); + } + + private void readFromParcel(Parcel in, boolean useBlobs) { final long start = SystemClock.uptimeMillis(); mHistoryParcels = new ArrayList<>(); final int count = in.readInt(); for(int i = 0; i < count; i++) { - byte[] temp = in.createByteArray(); - if (temp.length == 0) { + byte[] temp = useBlobs ? in.readBlob() : in.createByteArray(); + if (temp == null || temp.length == 0) { continue; } Parcel p = Parcel.obtain(); diff --git a/core/java/com/android/internal/os/BatteryUsageStatsProvider.java b/core/java/com/android/internal/os/BatteryUsageStatsProvider.java index c1111ec259c9d..69b7b4e8df0ff 100644 --- a/core/java/com/android/internal/os/BatteryUsageStatsProvider.java +++ b/core/java/com/android/internal/os/BatteryUsageStatsProvider.java @@ -30,6 +30,7 @@ import android.util.SparseArray; import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; +import java.io.File; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -200,7 +201,13 @@ public class BatteryUsageStatsProvider { Parcel historyBuffer = Parcel.obtain(); historyBuffer.appendFrom(batteryStatsImpl.mHistoryBuffer, 0, batteryStatsImpl.mHistoryBuffer.dataSize()); - batteryUsageStatsBuilder.setBatteryHistory(historyBuffer); + + final File systemDir = + batteryStatsImpl.mBatteryStatsHistory.getHistoryDirectory().getParentFile(); + final BatteryStatsHistory batteryStatsHistory = + new BatteryStatsHistory(batteryStatsImpl, systemDir, historyBuffer); + + batteryUsageStatsBuilder.setBatteryHistory(batteryStatsHistory); } return batteryUsageStatsBuilder.build(); 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 7db31fb0ace5f..9b3876f6d4ca5 100644 --- a/core/tests/coretests/src/com/android/internal/os/BatteryUsageStatsProviderTest.java +++ b/core/tests/coretests/src/com/android/internal/os/BatteryUsageStatsProviderTest.java @@ -39,6 +39,8 @@ import androidx.test.InstrumentationRegistry; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; +import libcore.testing.io.TestIoUtils; + import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -52,9 +54,12 @@ public class BatteryUsageStatsProviderTest { private static final int APP_UID = Process.FIRST_APPLICATION_UID + 42; private static final long MINUTE_IN_MS = 60 * 1000; + private final File mHistoryDir = + TestIoUtils.createTemporaryDirectory(getClass().getSimpleName()); @Rule - public final BatteryUsageStatsRule mStatsRule = new BatteryUsageStatsRule(12345) - .setAveragePower(PowerProfile.POWER_FLASHLIGHT, 360.0); + public final BatteryUsageStatsRule mStatsRule = + new BatteryUsageStatsRule(12345, mHistoryDir) + .setAveragePower(PowerProfile.POWER_FLASHLIGHT, 360.0); @Test public void test_getBatteryUsageStats() { diff --git a/core/tests/coretests/src/com/android/internal/os/BatteryUsageStatsRule.java b/core/tests/coretests/src/com/android/internal/os/BatteryUsageStatsRule.java index f75a6df0a325f..b3056e222477e 100644 --- a/core/tests/coretests/src/com/android/internal/os/BatteryUsageStatsRule.java +++ b/core/tests/coretests/src/com/android/internal/os/BatteryUsageStatsRule.java @@ -40,6 +40,7 @@ import org.junit.runner.Description; import org.junit.runners.model.Statement; import org.mockito.stubbing.Answer; +import java.io.File; import java.util.Arrays; public class BatteryUsageStatsRule implements TestRule { @@ -57,14 +58,18 @@ public class BatteryUsageStatsRule implements TestRule { private boolean mScreenOn; public BatteryUsageStatsRule() { - this(0); + this(0, null); } public BatteryUsageStatsRule(long currentTime) { + this(currentTime, null); + } + + public BatteryUsageStatsRule(long currentTime, File historyDir) { Context context = InstrumentationRegistry.getContext(); mPowerProfile = spy(new PowerProfile(context, true /* forTest */)); mMockClock.currentTime = currentTime; - mBatteryStats = new MockBatteryStatsImpl(mMockClock); + mBatteryStats = new MockBatteryStatsImpl(mMockClock, historyDir); mBatteryStats.setPowerProfile(mPowerProfile); mBatteryStats.onSystemReady(); }