Merge "Add ID field to the Callback" into qt-dev

This commit is contained in:
Lucas Dupin
2019-04-23 16:11:20 +00:00
committed by Android (Google) Code Review
2 changed files with 96 additions and 29 deletions

View File

@@ -75,6 +75,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.
*/
@@ -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);
}
}
}

View File

@@ -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;
@@ -215,6 +217,53 @@ public class AttentionDetectorTest extends AndroidTestCase {
verify(mOnUserAttention).run();
}
@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();
@@ -232,7 +281,6 @@ public class AttentionDetectorTest extends AndroidTestCase {
}
private class TestableAttentionDetector extends AttentionDetector {
private boolean mAttentionServiceSupported;
TestableAttentionDetector() {