Merge "Fix double-detach DualTimer bug" into oc-dev

core/java/com/android/internal/os/BatteryStatsImpl.java

@@ -2071,15 +2071,16 @@ public class BatteryStatsImpl extends BatteryStats {
         @Override
         public boolean reset(boolean detachIfReset) {
             boolean active = false;
+            // Do not detach the subTimer explicitly since that'll be done by DualTimer.detach().
+            active |= !mSubTimer.reset(false);
             active |= !super.reset(detachIfReset);
-            active |= !mSubTimer.reset(detachIfReset);
             return !active;
         }
 
         @Override
         public void detach() {
-            super.detach();
             mSubTimer.detach();
+            super.detach();
         }
 
         @Override

core/tests/coretests/src/com/android/internal/os/BatteryStatsDualTimerTest.java

@@ -0,0 +1,60 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.android.internal.os;
+
+import android.os.BatteryStats;
+import android.support.test.filters.SmallTest;
+
+import junit.framework.TestCase;
+
+/**
+ * Test BatteryStatsImpl.DualTimer.
+ */
+public class BatteryStatsDualTimerTest extends TestCase {
+
+    @SmallTest
+    public void testResetDetach() throws Exception {
+        final MockClocks clocks = new MockClocks();
+        clocks.realtime = clocks.uptime = 100;
+
+        final BatteryStatsImpl.TimeBase timeBase = new BatteryStatsImpl.TimeBase();
+        timeBase.init(clocks.uptimeMillis(), clocks.elapsedRealtime());
+        final BatteryStatsImpl.TimeBase subTimeBase = new BatteryStatsImpl.TimeBase();
+        subTimeBase.init(clocks.uptimeMillis(), clocks.elapsedRealtime());
+
+        final BatteryStatsImpl.DualTimer timer = new BatteryStatsImpl.DualTimer(clocks,
+                null, BatteryStats.WAKE_TYPE_PARTIAL, null, timeBase, subTimeBase);
+
+        assertTrue(timeBase.hasObserver(timer));
+        assertFalse(subTimeBase.hasObserver(timer));
+        assertFalse(timeBase.hasObserver(timer.getSubTimer()));
+        assertTrue(subTimeBase.hasObserver(timer.getSubTimer()));
+
+        // Timer is running so resetting it should not remove it from timerbases.
+        clocks.realtime = clocks.uptime = 200;
+        timer.startRunningLocked(clocks.realtime);
+        timer.reset(true);
+        assertTrue(timeBase.hasObserver(timer));
+        assertTrue(subTimeBase.hasObserver(timer.getSubTimer()));
+
+        // Stop timer and ensure that resetting removes it from timebases.
+        clocks.realtime = clocks.uptime = 300;
+        timer.stopRunningLocked(clocks.realtime);
+        timer.reset(true);
+        assertFalse(timeBase.hasObserver(timer));
+        assertFalse(timeBase.hasObserver(timer.getSubTimer()));
+    }
+}

core/tests/coretests/src/com/android/internal/os/BatteryStatsSensorTest.java

@@ -371,4 +371,48 @@ public class BatteryStatsSensorTest extends TestCase {
         // Test: UID_2 - background count
         assertEquals(2, bgTimer2.getCountLocked(BatteryStats.STATS_SINCE_CHARGED));
     }
+
+    @SmallTest
+    public void testSensorReset() throws Exception {
+        final MockClocks clocks = new MockClocks();
+        MockBatteryStatsImpl bi = new MockBatteryStatsImpl(clocks);
+        bi.mForceOnBattery = true;
+        clocks.realtime = 100;
+        clocks.uptime = 100;
+        bi.getOnBatteryTimeBase().setRunning(true, 100_000, 100_000);
+        bi.noteUidProcessStateLocked(UID, ActivityManager.PROCESS_STATE_RECEIVER);
+
+        clocks.realtime += 100;
+        clocks.uptime += 100;
+
+        bi.noteStartSensorLocked(UID, SENSOR_ID);
+
+        clocks.realtime += 100;
+        clocks.uptime += 100;
+
+        // The sensor is started and the timer has been created.
+        final BatteryStats.Uid uid = bi.getUidStats().get(UID);
+        assertNotNull(uid);
+
+        BatteryStats.Uid.Sensor sensor = uid.getSensorStats().get(SENSOR_ID);
+        assertNotNull(sensor);
+        assertNotNull(sensor.getSensorTime());
+        assertNotNull(sensor.getSensorBackgroundTime());
+
+        // Reset the stats. Since the sensor is still running, we should still see the timer
+        bi.getUidStatsLocked(UID).reset();
+
+        sensor = uid.getSensorStats().get(SENSOR_ID);
+        assertNotNull(sensor);
+        assertNotNull(sensor.getSensorTime());
+        assertNotNull(sensor.getSensorBackgroundTime());
+
+        bi.noteStopSensorLocked(UID, SENSOR_ID);
+
+        // Now the sensor timer has stopped so this reset should also take out the sensor.
+        bi.getUidStatsLocked(UID).reset();
+
+        sensor = uid.getSensorStats().get(SENSOR_ID);
+        assertNull(sensor);
+    }
 }

core/tests/coretests/src/com/android/internal/os/BatteryStatsTests.java

@@ -5,6 +5,7 @@ import org.junit.runners.Suite;
 
 @RunWith(Suite.class)
 @Suite.SuiteClasses({
+        BatteryStatsDualTimerTest.class,
         BatteryStatsDurationTimerTest.class,
         BatteryStatsSamplingTimerTest.class,
         BatteryStatsServTest.class,