From 5ea5d77a768eb429c314068868ff80b464806b66 Mon Sep 17 00:00:00 2001
From: Richard Chang <richardycc@google.com>
Date: Wed, 13 Oct 2021 09:42:32 +0000
Subject: [PATCH] ThermalManagerService: fix different system clock comparison

ThermalManagerService tried to compare
SystemClock.elapsedRealtime() and System.currentTimeMillis().
It may lead to more unnecessary update tasks.

Ref: https://developer.android.com/reference/android/os/SystemClock
Add testcase testTemperatureWatcherGetForecastUpdate

Bug: 203028173
Test: atest ThermalManagerServiceTest
Change-Id: Ic0c579cb124fd8d86bff9f6d9e21e33ca9d9f0ff
---
 .../server/power/ThermalManagerService.java   |  9 ++++--
 .../power/ThermalManagerServiceTest.java      | 30 +++++++++++++++++++
 2 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/services/core/java/com/android/server/power/ThermalManagerService.java b/services/core/java/com/android/server/power/ThermalManagerService.java
index db408488deba2..96823c8ae9aba 100644
--- a/services/core/java/com/android/server/power/ThermalManagerService.java
+++ b/services/core/java/com/android/server/power/ThermalManagerService.java
@@ -1088,6 +1088,10 @@ public class ThermalManagerService extends SystemService {
         @GuardedBy("mSamples")
         private long mLastForecastCallTimeMillis = 0;
 
+        private static final int INACTIVITY_THRESHOLD_MILLIS = 10000;
+        @VisibleForTesting
+        long mInactivityThresholdMillis = INACTIVITY_THRESHOLD_MILLIS;
+
         void updateSevereThresholds() {
             synchronized (mSamples) {
                 List<TemperatureThreshold> thresholds =
@@ -1107,13 +1111,12 @@ public class ThermalManagerService extends SystemService {
             }
         }
 
-        private static final int INACTIVITY_THRESHOLD_MILLIS = 10000;
         private static final int RING_BUFFER_SIZE = 30;
 
         private void updateTemperature() {
             synchronized (mSamples) {
                 if (SystemClock.elapsedRealtime() - mLastForecastCallTimeMillis
-                        < INACTIVITY_THRESHOLD_MILLIS) {
+                        < mInactivityThresholdMillis) {
                     // Trigger this again after a second as long as forecast has been called more
                     // recently than the inactivity timeout
                     mHandler.postDelayed(this::updateTemperature, 1000);
@@ -1199,7 +1202,7 @@ public class ThermalManagerService extends SystemService {
 
         float getForecast(int forecastSeconds) {
             synchronized (mSamples) {
-                mLastForecastCallTimeMillis = System.currentTimeMillis();
+                mLastForecastCallTimeMillis = SystemClock.elapsedRealtime();
                 if (mSamples.isEmpty()) {
                     updateTemperature();
                 }
diff --git a/services/tests/servicestests/src/com/android/server/power/ThermalManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/power/ThermalManagerServiceTest.java
index aaa74dd832afe..36e988f36577d 100644
--- a/services/tests/servicestests/src/com/android/server/power/ThermalManagerServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/power/ThermalManagerServiceTest.java
@@ -17,6 +17,7 @@
 package com.android.server.power;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
@@ -457,4 +458,33 @@ public class ThermalManagerServiceTest {
         watcher.mSevereThresholds.erase();
         assertTrue(Float.isNaN(watcher.getForecast(0)));
     }
+
+    @Test
+    public void testTemperatureWatcherGetForecastUpdate() throws Exception {
+        ThermalManagerService.TemperatureWatcher watcher = mService.mTemperatureWatcher;
+
+        // Reduce the inactivity threshold to speed up testing
+        watcher.mInactivityThresholdMillis = 2000;
+
+        // Make sure mSamples is empty before updateTemperature
+        assertTrue(isWatcherSamplesEmpty(watcher));
+
+        // Call getForecast once to trigger updateTemperature
+        watcher.getForecast(0);
+
+        // After 1 second, the samples should be updated
+        Thread.sleep(1000);
+        assertFalse(isWatcherSamplesEmpty(watcher));
+
+        // After mInactivityThresholdMillis, the samples should be cleared
+        Thread.sleep(watcher.mInactivityThresholdMillis);
+        assertTrue(isWatcherSamplesEmpty(watcher));
+    }
+
+    // Helper function to hold mSamples lock, avoid GuardedBy lint errors
+    private boolean isWatcherSamplesEmpty(ThermalManagerService.TemperatureWatcher watcher) {
+        synchronized (watcher.mSamples) {
+            return watcher.mSamples.isEmpty();
+        }
+    }
 }