Merge "Add ID field to the Callback" into qt-dev
diff --git a/services/core/java/com/android/server/power/AttentionDetector.java b/services/core/java/com/android/server/power/AttentionDetector.java
index 5e829b2..a65a812 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 @@
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 @@
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 @@
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 @@
// 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 @@
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 4de00f7..c30a7dd 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 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 @@
}
@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 @@
}
private class TestableAttentionDetector extends AttentionDetector {
-
private boolean mAttentionServiceSupported;
TestableAttentionDetector() {