diff options
| -rw-r--r-- | services/core/java/com/android/server/power/AttentionDetector.java | 75 | ||||
| -rw-r--r-- | services/tests/servicestests/src/com/android/server/power/AttentionDetectorTest.java | 50 |
2 files changed, 96 insertions, 29 deletions
diff --git a/services/core/java/com/android/server/power/AttentionDetector.java b/services/core/java/com/android/server/power/AttentionDetector.java index 5e829b2d6067..a65a81263780 100644 --- a/services/core/java/com/android/server/power/AttentionDetector.java +++ b/services/core/java/com/android/server/power/AttentionDetector.java @@ -76,6 +76,12 @@ public class AttentionDetector { private final AtomicBoolean mRequested; /** + * Monotonously increasing ID for the requests sent. + */ + @VisibleForTesting + protected int mRequestId; + + /** * {@link android.service.attention.AttentionService} API timeout. */ private long mMaxAttentionApiTimeoutMillis; @@ -105,36 +111,13 @@ public class AttentionDetector { private AtomicLong mConsecutiveTimeoutExtendedCount = new AtomicLong(0); @VisibleForTesting - final AttentionCallbackInternal mCallback = new AttentionCallbackInternal() { - @Override - public void onSuccess(int result, long timestamp) { - Slog.v(TAG, "onSuccess: " + result); - if (mRequested.getAndSet(false)) { - synchronized (mLock) { - if (mWakefulness != PowerManagerInternal.WAKEFULNESS_AWAKE) { - if (DEBUG) Slog.d(TAG, "Device slept before receiving callback."); - return; - } - if (result == AttentionService.ATTENTION_SUCCESS_PRESENT) { - mOnUserAttention.run(); - } else { - resetConsecutiveExtensionCount(); - } - } - } - } - - @Override - public void onFailure(int error) { - Slog.i(TAG, "Failed to check attention: " + error); - mRequested.set(false); - } - }; + AttentionCallbackInternalImpl mCallback; public AttentionDetector(Runnable onUserAttention, Object lock) { mOnUserAttention = onUserAttention; mLock = lock; mRequested = new AtomicBoolean(false); + mRequestId = 0; // Device starts with an awake state upon boot. mWakefulness = PowerManagerInternal.WAKEFULNESS_AWAKE; @@ -196,9 +179,7 @@ public class AttentionDetector { return nextScreenDimming; } else if (mRequested.get()) { if (DEBUG) { - // TODO(b/128134941): consider adding a member ID increasing counter in - // AttentionCallbackInternal to track this better. - Slog.d(TAG, "Pending attention callback, wait."); + Slog.d(TAG, "Pending attention callback with ID=" + mCallback.mId + ", wait."); } return whenToCheck; } @@ -208,6 +189,8 @@ public class AttentionDetector { // This means that we must assume that the request was successful, and then cancel it // afterwards if AttentionManager couldn't deliver it. mRequested.set(true); + mRequestId++; + mCallback = new AttentionCallbackInternalImpl(mRequestId); final boolean sent = mAttentionManager.checkAttention(getAttentionTimeout(), mCallback); if (!sent) { mRequested.set(false); @@ -301,4 +284,40 @@ public class AttentionDetector { pw.print(" mAttentionServiceSupported=" + isAttentionServiceSupported()); pw.print(" mRequested=" + mRequested); } + + @VisibleForTesting + final class AttentionCallbackInternalImpl extends AttentionCallbackInternal { + private final int mId; + + AttentionCallbackInternalImpl(int id) { + this.mId = id; + } + + @Override + public void onSuccess(int result, long timestamp) { + Slog.v(TAG, "onSuccess: " + result + ", ID: " + mId); + // If we don't check for request ID it's possible to get into a loop: success leads + // to the onUserAttention(), which in turn triggers updateUserActivity(), which will + // call back onSuccess() instantaneously if there is a cached value, and circle repeats. + if (mId == mRequestId && mRequested.getAndSet(false)) { + synchronized (mLock) { + if (mWakefulness != PowerManagerInternal.WAKEFULNESS_AWAKE) { + if (DEBUG) Slog.d(TAG, "Device slept before receiving callback."); + return; + } + if (result == AttentionService.ATTENTION_SUCCESS_PRESENT) { + mOnUserAttention.run(); + } else { + resetConsecutiveExtensionCount(); + } + } + } + } + + @Override + public void onFailure(int error) { + Slog.i(TAG, "Failed to check attention: " + error + ", ID: " + mId); + mRequested.set(false); + } + } } 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 4de00f7b5565..c30a7dd8321c 100644 --- a/services/tests/servicestests/src/com/android/server/power/AttentionDetectorTest.java +++ b/services/tests/servicestests/src/com/android/server/power/AttentionDetectorTest.java @@ -22,6 +22,8 @@ import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.Mockito.atMost; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.never; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.verify; @@ -216,6 +218,53 @@ public class AttentionDetectorTest extends AndroidTestCase { } @Test + public void testCallbackOnSuccess_doesNotCallNonCurrentCallback() { + mAttentionDetector.mRequestId = 5; + registerAttention(); // mRequestId = 6; + mAttentionDetector.mRequestId = 55; + + mAttentionDetector.mCallback.onSuccess(AttentionService.ATTENTION_SUCCESS_PRESENT, + SystemClock.uptimeMillis()); + verify(mOnUserAttention, never()).run(); + } + + @Test + public void testCallbackOnSuccess_callsCallbackAfterOldCallbackCame() { + mAttentionDetector.mRequestId = 5; + registerAttention(); // mRequestId = 6; + mAttentionDetector.mRequestId = 55; + + mAttentionDetector.mCallback.onSuccess(AttentionService.ATTENTION_SUCCESS_PRESENT, + SystemClock.uptimeMillis()); // old callback came + mAttentionDetector.mRequestId = 6; // now back to current + mAttentionDetector.mCallback.onSuccess(AttentionService.ATTENTION_SUCCESS_PRESENT, + SystemClock.uptimeMillis()); + verify(mOnUserAttention).run(); + } + + @Test + public void testCallbackOnSuccess_DoesNotGoIntoInfiniteLoop() { + // Mimic real behavior + doAnswer((invocation) -> { + // Mimic a cache hit: calling onSuccess() immediately + registerAttention(); + mAttentionDetector.mRequestId++; + mAttentionDetector.mCallback.onSuccess(AttentionService.ATTENTION_SUCCESS_PRESENT, + SystemClock.uptimeMillis()); + return null; + }).when(mOnUserAttention).run(); + + registerAttention(); + // This test fails with literal stack overflow: + // e.g. java.lang.StackOverflowError: stack size 1039KB + mAttentionDetector.mCallback.onSuccess(AttentionService.ATTENTION_SUCCESS_PRESENT, + SystemClock.uptimeMillis()); + + // We don't actually get here when the test fails + verify(mOnUserAttention, atMost(1)).run(); + } + + @Test public void testCallbackOnFailure_unregistersCurrentRequestCode() { registerAttention(); mAttentionDetector.mCallback.onFailure(AttentionService.ATTENTION_FAILURE_UNKNOWN); @@ -232,7 +281,6 @@ public class AttentionDetectorTest extends AndroidTestCase { } private class TestableAttentionDetector extends AttentionDetector { - private boolean mAttentionServiceSupported; TestableAttentionDetector() { |