Fix boot time BatteryStats reset failure

We need some services to create a battery stats snapshot
captured at the time of battery reset.
The issue is that if the battery stats reset occurs during boot,
before the system is ready, those services may not be prepared.

The fix is to postpone battery stats reset until after the
system server is ready.

Also, we will now reset the stats if battery got partially charged
and discharged repeatedly without ever reaching the full charge.
This will prevent battery sessions from going on forever and
overflowing internal data structures.

This CL partially reverts https://cs.android.com/android/_/android/platform/frameworks/base/+/0ed84f12639c4cb7ba6623dc232ae31602827bae

Bug: 192252233
Test: run PowerOnOffTest#testPowerOnOff repeatedly
Change-Id: Ib486e4caeeac7c933288c3a1055259c05f45c6c8

7 files changed, 62 insertions, 11 deletions

diff --git a/core/java/com/android/internal/os/BatteryStatsImpl.java b/core/java/com/android/internal/os/BatteryStatsImpl.java
index dab3e9fa15fa..be059a1e195b 100644
--- a/core/java/com/android/internal/os/BatteryStatsImpl.java
+++ b/core/java/com/android/internal/os/BatteryStatsImpl.java
@@ -191,6 +191,11 @@ public class BatteryStatsImpl extends BatteryStats {
     @VisibleForTesting
     public static final int WAKE_LOCK_WEIGHT = 50;
 
+    public static final int RESET_REASON_CORRUPT_FILE = 1;
+    public static final int RESET_REASON_ADB_COMMAND = 2;
+    public static final int RESET_REASON_FULL_CHARGE = 3;
+    public static final int RESET_REASON_MEASURED_ENERGY_BUCKETS_CHANGE = 4;
+
     protected Clocks mClocks;
 
     private final AtomicFile mStatsFile;
@@ -348,8 +353,9 @@ public class BatteryStatsImpl extends BatteryStats {
 
         /**
          * Callback invoked immediately prior to resetting battery stats.
+         * @param resetReason One of the RESET_REASON_* constants.
          */
-        void prepareForBatteryStatsReset();
+        void prepareForBatteryStatsReset(int resetReason);
     }
 
     private BatteryResetListener mBatteryResetListener;
@@ -747,6 +753,7 @@ public class BatteryStatsImpl extends BatteryStats {
     // CPU update, even if we aren't currently running wake locks.
     boolean mDistributeWakelockCpu;
 
+    private boolean mSystemReady;
     boolean mShuttingDown;
 
     final HistoryEventTracker mActiveEvents = new HistoryEventTracker();
@@ -11210,7 +11217,7 @@ public class BatteryStatsImpl extends BatteryStats {
         long uptimeUs = mSecUptime * 1000;
         long mSecRealtime = mClocks.elapsedRealtime();
         long realtimeUs = mSecRealtime * 1000;
-        resetAllStatsLocked(mSecUptime, mSecRealtime);
+        resetAllStatsLocked(mSecUptime, mSecRealtime, RESET_REASON_ADB_COMMAND);
         mDischargeStartLevel = mHistoryCur.batteryLevel;
         pullPendingStateUpdatesLocked();
         addHistoryRecordLocked(mSecRealtime, mSecUptime);
@@ -11239,9 +11246,10 @@ public class BatteryStatsImpl extends BatteryStats {
         initActiveHistoryEventsLocked(mSecRealtime, mSecUptime);
     }
 
-    private void resetAllStatsLocked(long uptimeMillis, long elapsedRealtimeMillis) {
+    private void resetAllStatsLocked(long uptimeMillis, long elapsedRealtimeMillis,
+            int resetReason) {
         if (mBatteryResetListener != null) {
-            mBatteryResetListener.prepareForBatteryStatsReset();
+            mBatteryResetListener.prepareForBatteryStatsReset(resetReason);
         }
 
         final long uptimeUs = uptimeMillis * 1000;
@@ -13477,6 +13485,13 @@ public class BatteryStatsImpl extends BatteryStats {
         return false;
     }
 
+    /**
+     * Notifies BatteryStatsImpl that the system server is ready.
+     */
+    public void onSystemReady() {
+        mSystemReady = true;
+    }
+
     @GuardedBy("this")
     protected void setOnBatteryLocked(final long mSecRealtime, final long mSecUptime,
             final boolean onBattery, final int oldStatus, final int level, final int chargeUah) {
@@ -13493,10 +13508,17 @@ public class BatteryStatsImpl extends BatteryStats {
             // battery was last full, or the level is at 100, or
             // we have gone through a significant charge (from a very low
             // level to a now very high level).
+            // Also, we will reset the stats if battery got partially charged
+            // and discharged repeatedly without ever reaching the full charge.
+            // This reset is done in order to prevent stats sessions from going on forever.
+            // Exceedingly long battery sessions would lead to an overflow of
+            // data structures such as mWakeupReasonStats.
             boolean reset = false;
-            if (!mNoAutoReset && (oldStatus == BatteryManager.BATTERY_STATUS_FULL
+            if (!mNoAutoReset && mSystemReady
+                    && (oldStatus == BatteryManager.BATTERY_STATUS_FULL
                     || level >= 90
-                    || (mDischargeCurrentLevel < 20 && level >= 80))) {
+                    || (mDischargeCurrentLevel < 20 && level >= 80)
+                    || getHighDischargeAmountSinceCharge() >= 200)) {
                 Slog.i(TAG, "Resetting battery stats: level=" + level + " status=" + oldStatus
                         + " dischargeLevel=" + mDischargeCurrentLevel
                         + " lowAmount=" + getLowDischargeAmountSinceCharge()
@@ -13534,7 +13556,7 @@ public class BatteryStatsImpl extends BatteryStats {
                     });
                 }
                 doWrite = true;
-                resetAllStatsLocked(mSecUptime, mSecRealtime);
+                resetAllStatsLocked(mSecUptime, mSecRealtime, RESET_REASON_FULL_CHARGE);
                 if (chargeUah > 0 && level > 0) {
                     // Only use the reported coulomb charge value if it is supported and reported.
                     mEstimatedBatteryCapacityMah = (int) ((chargeUah / 1000) / (level / 100.0));
@@ -14502,7 +14524,8 @@ public class BatteryStatsImpl extends BatteryStats {
                     ? null : new MeasuredEnergyStats(supportedStandardBuckets, customBucketNames);
             // Supported power buckets changed since last boot.
             // Existing data is no longer reliable.
-            resetAllStatsLocked(SystemClock.uptimeMillis(), SystemClock.elapsedRealtime());
+            resetAllStatsLocked(SystemClock.uptimeMillis(), SystemClock.elapsedRealtime(),
+                    RESET_REASON_MEASURED_ENERGY_BUCKETS_CHANGE);
         }
     }
 
@@ -14949,7 +14972,8 @@ public class BatteryStatsImpl extends BatteryStats {
             }
         } catch (Exception e) {
             Slog.e(TAG, "Error reading battery statistics", e);
-            resetAllStatsLocked(SystemClock.uptimeMillis(), SystemClock.elapsedRealtime());
+            resetAllStatsLocked(SystemClock.uptimeMillis(), SystemClock.elapsedRealtime(),
+                    RESET_REASON_CORRUPT_FILE);
         } finally {
             stats.recycle();
         }
diff --git a/core/java/com/android/internal/os/BatteryUsageStatsStore.java b/core/java/com/android/internal/os/BatteryUsageStatsStore.java
index 5c976025d39d..fd54b32bd12f 100644
--- a/core/java/com/android/internal/os/BatteryUsageStatsStore.java
+++ b/core/java/com/android/internal/os/BatteryUsageStatsStore.java
@@ -69,6 +69,7 @@ public class BatteryUsageStatsStore {
 
     private final Context mContext;
     private final BatteryStatsImpl mBatteryStats;
+    private boolean mSystemReady;
     private final File mStoreDir;
     private final File mLockFile;
     private final AtomicFile mConfigFile;
@@ -95,7 +96,18 @@ public class BatteryUsageStatsStore {
         mBatteryUsageStatsProvider = new BatteryUsageStatsProvider(mContext, mBatteryStats);
     }
 
-    private void prepareForBatteryStatsReset() {
+    /**
+     * Notifies BatteryUsageStatsStore that the system server is ready.
+     */
+    public void onSystemReady() {
+        mSystemReady = true;
+    }
+
+    private void prepareForBatteryStatsReset(int resetReason) {
+        if (resetReason == BatteryStatsImpl.RESET_REASON_CORRUPT_FILE || !mSystemReady) {
+            return;
+        }
+
         final List<BatteryUsageStats> stats =
                 mBatteryUsageStatsProvider.getBatteryUsageStats(BATTERY_USAGE_STATS_QUERY);
         if (stats.isEmpty()) {
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 cbd67c8324f4..0147cdb186f3 100644
--- a/core/tests/coretests/src/com/android/internal/os/BatteryUsageStatsProviderTest.java
+++ b/core/tests/coretests/src/com/android/internal/os/BatteryUsageStatsProviderTest.java
@@ -204,6 +204,7 @@ public class BatteryUsageStatsProviderTest {
         BatteryUsageStatsStore batteryUsageStatsStore = new BatteryUsageStatsStore(context,
                 batteryStats, new File(context.getCacheDir(), "BatteryUsageStatsProviderTest"),
                 new TestHandler(), Integer.MAX_VALUE);
+        batteryUsageStatsStore.onSystemReady();
 
         BatteryUsageStatsProvider provider = new BatteryUsageStatsProvider(context,
                 batteryStats, batteryUsageStatsStore);
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 961e85950b29..083090c54619 100644
--- a/core/tests/coretests/src/com/android/internal/os/BatteryUsageStatsRule.java
+++ b/core/tests/coretests/src/com/android/internal/os/BatteryUsageStatsRule.java
@@ -66,6 +66,7 @@ public class BatteryUsageStatsRule implements TestRule {
         mMockClocks.currentTime = currentTime;
         mBatteryStats = new MockBatteryStatsImpl(mMockClocks);
         mBatteryStats.setPowerProfile(mPowerProfile);
+        mBatteryStats.onSystemReady();
     }
 
     public BatteryUsageStatsRule setAveragePower(String key, double value) {
diff --git a/core/tests/coretests/src/com/android/internal/os/BatteryUsageStatsStoreTest.java b/core/tests/coretests/src/com/android/internal/os/BatteryUsageStatsStoreTest.java
index 141a9fa30c85..e478cd776558 100644
--- a/core/tests/coretests/src/com/android/internal/os/BatteryUsageStatsStoreTest.java
+++ b/core/tests/coretests/src/com/android/internal/os/BatteryUsageStatsStoreTest.java
@@ -59,6 +59,7 @@ public class BatteryUsageStatsStoreTest {
         mBatteryStats = new MockBatteryStatsImpl(mMockClocks);
         mBatteryStats.setNoAutoReset(true);
         mBatteryStats.setPowerProfile(mock(PowerProfile.class));
+        mBatteryStats.onSystemReady();
 
         Context context = InstrumentationRegistry.getContext();
 
@@ -67,6 +68,7 @@ public class BatteryUsageStatsStoreTest {
 
         mBatteryUsageStatsStore = new BatteryUsageStatsStore(context, mBatteryStats,
                 mStoreDirectory, new TestHandler(), MAX_BATTERY_STATS_SNAPSHOT_STORAGE_BYTES);
+        mBatteryUsageStatsStore.onSystemReady();
 
         mBatteryUsageStatsProvider = new BatteryUsageStatsProvider(context, mBatteryStats);
     }
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index a04edc760dfb..aa103f45ad75 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -7629,6 +7629,7 @@ public class ActivityManagerService extends IActivityManager.Stub
         t.traceEnd();
 
         t.traceBegin("ActivityManagerStartApps");
+        mBatteryStatsService.onSystemReady();
         mBatteryStatsService.noteEvent(BatteryStats.HistoryItem.EVENT_USER_RUNNING_START,
                 Integer.toString(currentUserId), currentUserId);
         mBatteryStatsService.noteEvent(BatteryStats.HistoryItem.EVENT_USER_FOREGROUND_START,
diff --git a/services/core/java/com/android/server/am/BatteryStatsService.java b/services/core/java/com/android/server/am/BatteryStatsService.java
index 7d9d78969ca9..9f41c8ba9626 100644
--- a/services/core/java/com/android/server/am/BatteryStatsService.java
+++ b/services/core/java/com/android/server/am/BatteryStatsService.java
@@ -126,7 +126,7 @@ public final class BatteryStatsService extends IBatteryStats.Stub
         Watchdog.Monitor {
     static final String TAG = "BatteryStatsService";
     static final boolean DBG = false;
-    private static final boolean BATTERY_USAGE_STORE_ENABLED = false;
+    private static final boolean BATTERY_USAGE_STORE_ENABLED = true;
 
     private static IBatteryStats sService;
 
@@ -398,6 +398,16 @@ public final class BatteryStatsService extends IBatteryStats.Stub
         registerStatsCallbacks();
     }
 
+    /**
+     * Notifies BatteryStatsService that the system server is ready.
+     */
+    public void onSystemReady() {
+        mStats.onSystemReady();
+        if (BATTERY_USAGE_STORE_ENABLED) {
+            mBatteryUsageStatsStore.onSystemReady();
+        }
+    }
+
     private final class LocalService extends BatteryStatsInternal {
         @Override
         public String[] getWifiIfaces() {