From 2a1e0c46c4c00aa183537b9daad7db688a8b2842 Mon Sep 17 00:00:00 2001 From: Alex Salo Date: Wed, 7 Oct 2020 15:30:00 -0700 Subject: [PATCH] Listen DeviceConfig changes DeviceConfig reads are not free, and AttentionDetector code sits on a critical system path. Avoid reading DeviceConfig on each invocation, add a onChangeListener instead. Bug: 169427927 Test: manually and atest AttentionDetectorTest Change-Id: Icb89f78f07207ee8e80a11f16b0f03ab578f4ea6 --- .../server/power/AttentionDetector.java | 82 ++++++++++++++----- .../server/power/AttentionDetectorTest.java | 30 ++----- 2 files changed, 68 insertions(+), 44 deletions(-) diff --git a/services/core/java/com/android/server/power/AttentionDetector.java b/services/core/java/com/android/server/power/AttentionDetector.java index 06edd19a376a8..1a4a22203e57b 100644 --- a/services/core/java/com/android/server/power/AttentionDetector.java +++ b/services/core/java/com/android/server/power/AttentionDetector.java @@ -18,6 +18,7 @@ package com.android.server.power; import static android.provider.DeviceConfig.NAMESPACE_ATTENTION_MANAGER_SERVICE; +import android.annotation.NonNull; import android.app.ActivityManager; import android.app.SynchronousUserSwitchObserver; import android.attention.AttentionManagerInternal; @@ -42,6 +43,7 @@ import com.android.server.LocalServices; import com.android.server.wm.WindowManagerInternal; import java.io.PrintWriter; +import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; @@ -66,6 +68,9 @@ public class AttentionDetector { /** Default value in absence of {@link DeviceConfig} override. */ static final long DEFAULT_PRE_DIM_CHECK_DURATION_MILLIS = 2_000; + @VisibleForTesting + protected long mPreDimCheckDurationMillis; + /** DeviceConfig flag name, describes how long to run the check beyond the screen dim event. */ static final String KEY_POST_DIM_CHECK_DURATION_MILLIS = "post_dim_check_duration_millis"; @@ -73,12 +78,28 @@ public class AttentionDetector { /** Default value in absence of {@link DeviceConfig} override. */ static final long DEFAULT_POST_DIM_CHECK_DURATION_MILLIS = 0; + private long mRequestedPostDimTimeoutMillis; + + /** + * Keep the last used post dim timeout for the dumpsys (it can't be longer the dim duration). + */ + private long mEffectivePostDimTimeoutMillis; + /** * DeviceConfig flag name, describes the limit of how long the device can remain unlocked due to * attention checking. */ static final String KEY_MAX_EXTENSION_MILLIS = "max_extension_millis"; + /** + * The default value for the maximum time, in millis, that the phone can stay unlocked because + * of attention events, triggered by any user. + */ + @VisibleForTesting + protected long mDefaultMaximumExtensionMillis; + + private long mMaximumExtensionMillis; + private Context mContext; private boolean mIsSettingEnabled; @@ -88,13 +109,6 @@ public class AttentionDetector { */ private final Runnable mOnUserAttention; - /** - * The default value for the maximum time, in millis, that the phone can stay unlocked because - * of attention events, triggered by any user. - */ - @VisibleForTesting - protected long mDefaultMaximumExtensionMillis; - private final Object mLock; /** @@ -137,9 +151,6 @@ public class AttentionDetector { @VisibleForTesting AttentionCallbackInternalImpl mCallback; - /** Keep the last used post dim timeout for the dumpsys. */ - private long mLastPostDimTimeout; - public AttentionDetector(Runnable onUserAttention, Object lock) { mOnUserAttention = onUserAttention; mLock = lock; @@ -180,6 +191,11 @@ public class AttentionDetector { updateEnabledFromSettings(context); } }, UserHandle.USER_ALL); + + readValuesFromDeviceConfig(); + DeviceConfig.addOnPropertiesChangedListener(NAMESPACE_ATTENTION_MANAGER_SERVICE, + context.getMainExecutor(), + (properties) -> onDeviceConfigChange(properties.getKeyset())); } /** To be called in {@link PowerManagerService#updateUserActivitySummaryLocked}. */ @@ -192,8 +208,8 @@ public class AttentionDetector { } final long now = SystemClock.uptimeMillis(); - final long whenToCheck = nextScreenDimming - getPreDimCheckDurationMillis(); - final long whenToStopExtending = mLastUserActivityTime + getMaxExtensionMillis(); + final long whenToCheck = nextScreenDimming - mPreDimCheckDurationMillis; + final long whenToStopExtending = mLastUserActivityTime + mMaximumExtensionMillis; if (now < whenToCheck) { if (DEBUG) { Slog.d(TAG, "Do not check for attention yet, wait " + (whenToCheck - now)); @@ -220,9 +236,11 @@ public class AttentionDetector { mRequestId++; mLastActedOnNextScreenDimming = nextScreenDimming; mCallback = new AttentionCallbackInternalImpl(mRequestId); + mEffectivePostDimTimeoutMillis = Math.min(mRequestedPostDimTimeoutMillis, + dimDurationMillis); Slog.v(TAG, "Checking user attention, ID: " + mRequestId); final boolean sent = mAttentionManager.checkAttention( - getPreDimCheckDurationMillis() + getPostDimCheckDurationMillis(dimDurationMillis), + mPreDimCheckDurationMillis + mEffectivePostDimTimeoutMillis, mCallback); if (!sent) { mRequested.set(false); @@ -294,9 +312,9 @@ public class AttentionDetector { public void dump(PrintWriter pw) { pw.println("AttentionDetector:"); pw.println(" mIsSettingEnabled=" + mIsSettingEnabled); - pw.println(" mMaxExtensionMillis=" + getMaxExtensionMillis()); - pw.println(" preDimCheckDurationMillis=" + getPreDimCheckDurationMillis()); - pw.println(" postDimCheckDurationMillis=" + mLastPostDimTimeout); + pw.println(" mMaxExtensionMillis=" + mMaximumExtensionMillis); + pw.println(" mPreDimCheckDurationMillis=" + mPreDimCheckDurationMillis); + pw.println(" mEffectivePostDimTimeout=" + mEffectivePostDimTimeoutMillis); pw.println(" mLastUserActivityTime(excludingAttention)=" + mLastUserActivityTime); pw.println(" mAttentionServiceSupported=" + isAttentionServiceSupported()); pw.println(" mRequested=" + mRequested); @@ -319,7 +337,7 @@ public class AttentionDetector { /** How long to check after the screen dims, capped at the dim duration. */ @VisibleForTesting - protected long getPostDimCheckDurationMillis(long dimDurationMillis) { + protected long getPostDimCheckDurationMillis() { final long millis = DeviceConfig.getLong(NAMESPACE_ATTENTION_MANAGER_SERVICE, KEY_POST_DIM_CHECK_DURATION_MILLIS, DEFAULT_POST_DIM_CHECK_DURATION_MILLIS); @@ -328,9 +346,7 @@ public class AttentionDetector { Slog.w(TAG, "Bad flag value supplied for: " + KEY_POST_DIM_CHECK_DURATION_MILLIS); return DEFAULT_POST_DIM_CHECK_DURATION_MILLIS; } - - mLastPostDimTimeout = Math.min(millis, dimDurationMillis); - return mLastPostDimTimeout; + return millis; } /** How long the device can remain unlocked due to attention checking. */ @@ -348,6 +364,32 @@ public class AttentionDetector { return millis; } + private void onDeviceConfigChange(@NonNull Set keys) { + for (String key : keys) { + switch (key) { + case KEY_MAX_EXTENSION_MILLIS: + case KEY_POST_DIM_CHECK_DURATION_MILLIS: + case KEY_PRE_DIM_CHECK_DURATION_MILLIS: + readValuesFromDeviceConfig(); + return; + default: + Slog.i(TAG, "Ignoring change on " + key); + } + } + } + + private void readValuesFromDeviceConfig() { + mMaximumExtensionMillis = getMaxExtensionMillis(); + mPreDimCheckDurationMillis = getPreDimCheckDurationMillis(); + mRequestedPostDimTimeoutMillis = getPostDimCheckDurationMillis(); + + Slog.i(TAG, "readValuesFromDeviceConfig():" + + "\nmMaximumExtensionMillis=" + mMaximumExtensionMillis + + "\nmPreDimCheckDurationMillis=" + mPreDimCheckDurationMillis + + "\nmRequestedPostDimTimeoutMillis=" + mRequestedPostDimTimeoutMillis); + + } + @VisibleForTesting final class AttentionCallbackInternalImpl extends AttentionCallbackInternal { private final int mId; diff --git a/services/tests/servicestests/src/com/android/server/power/AttentionDetectorTest.java b/services/tests/servicestests/src/com/android/server/power/AttentionDetectorTest.java index af161ee8b8181..ca0270deb3982 100644 --- a/services/tests/servicestests/src/com/android/server/power/AttentionDetectorTest.java +++ b/services/tests/servicestests/src/com/android/server/power/AttentionDetectorTest.java @@ -70,7 +70,6 @@ public class AttentionDetectorTest extends AndroidTestCase { private Runnable mOnUserAttention; private TestableAttentionDetector mAttentionDetector; private AttentionDetector mRealAttentionDetector; - private long mPreDimCheckDuration; private long mNextDimming; private int mIsSettingEnabled; @@ -342,23 +341,14 @@ public class AttentionDetectorTest extends AndroidTestCase { public void testGetPostDimCheckDurationMillis_handlesGoodFlagValue() { DeviceConfig.setProperty(NAMESPACE_ATTENTION_MANAGER_SERVICE, KEY_POST_DIM_CHECK_DURATION_MILLIS, "333", false); - assertThat(mRealAttentionDetector.getPostDimCheckDurationMillis( - DEFAULT_DIM_DURATION_MILLIS)).isEqualTo(333); - } - - @Test - public void testGetPostDimCheckDurationMillis_capsGoodFlagValueByMaxDimDuration() { - DeviceConfig.setProperty(NAMESPACE_ATTENTION_MANAGER_SERVICE, - KEY_POST_DIM_CHECK_DURATION_MILLIS, "7000", false); - assertThat(mRealAttentionDetector.getPostDimCheckDurationMillis(6500)).isEqualTo(6500); + assertThat(mRealAttentionDetector.getPostDimCheckDurationMillis()).isEqualTo(333); } @Test public void testGetPostDimCheckDurationMillis_rejectsNegativeValue() { DeviceConfig.setProperty(NAMESPACE_ATTENTION_MANAGER_SERVICE, KEY_POST_DIM_CHECK_DURATION_MILLIS, "-50", false); - assertThat(mRealAttentionDetector.getPostDimCheckDurationMillis( - DEFAULT_DIM_DURATION_MILLIS)).isEqualTo( + assertThat(mRealAttentionDetector.getPostDimCheckDurationMillis()).isEqualTo( DEFAULT_POST_DIM_CHECK_DURATION_MILLIS); } @@ -366,8 +356,7 @@ public class AttentionDetectorTest extends AndroidTestCase { public void testGetPostDimCheckDurationMillis_rejectsTooBigValue() { DeviceConfig.setProperty(NAMESPACE_ATTENTION_MANAGER_SERVICE, KEY_POST_DIM_CHECK_DURATION_MILLIS, "20000", false); - assertThat(mRealAttentionDetector.getPostDimCheckDurationMillis( - DEFAULT_DIM_DURATION_MILLIS)).isEqualTo( + assertThat(mRealAttentionDetector.getPostDimCheckDurationMillis()).isEqualTo( DEFAULT_POST_DIM_CHECK_DURATION_MILLIS); } @@ -375,14 +364,12 @@ public class AttentionDetectorTest extends AndroidTestCase { public void testGetPostDimCheckDurationMillis_handlesBadFlagValue() { DeviceConfig.setProperty(NAMESPACE_ATTENTION_MANAGER_SERVICE, KEY_POST_DIM_CHECK_DURATION_MILLIS, "20000k", false); - assertThat(mRealAttentionDetector.getPostDimCheckDurationMillis( - DEFAULT_DIM_DURATION_MILLIS)).isEqualTo( + assertThat(mRealAttentionDetector.getPostDimCheckDurationMillis()).isEqualTo( DEFAULT_POST_DIM_CHECK_DURATION_MILLIS); DeviceConfig.setProperty(NAMESPACE_ATTENTION_MANAGER_SERVICE, KEY_POST_DIM_CHECK_DURATION_MILLIS, "0.25", false); - assertThat(mRealAttentionDetector.getPostDimCheckDurationMillis( - DEFAULT_DIM_DURATION_MILLIS)).isEqualTo( + assertThat(mRealAttentionDetector.getPostDimCheckDurationMillis()).isEqualTo( DEFAULT_POST_DIM_CHECK_DURATION_MILLIS); } @@ -423,7 +410,7 @@ public class AttentionDetectorTest extends AndroidTestCase { } private long registerAttention() { - mPreDimCheckDuration = 4000L; + mAttentionDetector.mPreDimCheckDurationMillis = 4000L; mAttentionDetector.onUserActivity(SystemClock.uptimeMillis(), PowerManager.USER_ACTIVITY_EVENT_TOUCH); return mAttentionDetector.updateUserActivity(mNextDimming, DEFAULT_DIM_DURATION_MILLIS); @@ -447,10 +434,5 @@ public class AttentionDetectorTest extends AndroidTestCase { public boolean isAttentionServiceSupported() { return mAttentionServiceSupported; } - - @Override - public long getPreDimCheckDurationMillis() { - return mPreDimCheckDuration; - } } }