From e18ac502cc01c0c7acf58cd4b082a3aac41c28f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=BBywek?= Date: Sat, 10 Oct 2026 11:20:49 +0200 Subject: [PATCH] Time out slow image captions, and end each caption request once Image captioning gives up on a caption that takes more than 5 seconds, says what it has so far while the image is still focused, and moves on to the next screenshot. The timeout was set with System.currentTimeMillis() on a Handler, which counts uptime instead, so it was due decades later and never fired. A slow part, such as text recognition that did not answer, held the next screenshot until its own 10-second timeout. The timeout now fires after 5 seconds. It used to clear every list, including screenshot requests that were waiting, such as a Describe image the user had just asked for; that went unnoticed only because it never fired. It now ends the slow caption's requests and lets the next screenshot go ahead. A caption with nothing to do drops its timeout at once, so that the timeout cannot give up on a Describe image started within those 5 seconds. Describe image with Gemini or the on-device model does not use this timeout; it has its own. A request could also end twice. A screenshot result that came after its 3-second timeout ended its request a second time, and as the list always removed its first request, it removed the next one while that was still taking its screenshot, and started the one after it. A result that came after its caption had been dropped did the same. Each request now ends once, with a result, an error, its timeout or a cancel, and whatever comes after that is ignored. The list removes the request that ended, not whichever is first, and clearing it cancels its requests and their timeouts. A caption result keeps the screenshot request it came from, and ends that one when it is complete or dropped. The time between screenshots, and the time each request takes, are now measured with SystemClock.uptimeMillis() rather than the wall clock, so setting the clock back no longer holds the next screenshot back by that much time. Limitations: Describe image still stops working after the on-device model runs out of memory or times out, since GeminiActor ends those requests as cancelled without telling ImageCaptioner; that needs a fix of its own. Not tried on a device. - Request: finish() ends a request once, cancel() ends it without a result, and its times are in uptime - RequestList: performNextRequest(Request) removes the request that ended, clear() cancels the requests, and the time between them is in uptime - ScreenshotCaptureRequest: passes itself to its listener, and recycles a screenshot that comes after the request ended - CaptionRequest: ignores a result or an error after the request ended - ImageCaptioner: ends the screenshot request each caption came from, the 5-second timeout ends only the slow caption, a caption with nothing to do drops its timeout, and shutdown() cancels all requests - ImageCaptionerRequestsTest and RequestListTest; without this change, the tests of a late screenshot and of a slow caption fail --- .../talkback/actor/ImageCaptioner.java | 101 ++++-- .../talkback/imagecaption/CaptionRequest.java | 7 +- .../talkback/imagecaption/Request.java | 58 +++- .../talkback/imagecaption/RequestList.java | 93 +++--- .../ScreenshotCaptureRequest.java | 18 +- .../actor/ImageCaptionerRequestsTest.kt | 316 ++++++++++++++++++ .../talkback/imagecaption/RequestListTest.kt | 168 ++++++++++ 7 files changed, 673 insertions(+), 88 deletions(-) create mode 100644 talkback/src/test/java/com/google/android/accessibility/talkback/actor/ImageCaptionerRequestsTest.kt create mode 100644 talkback/src/test/java/com/google/android/accessibility/talkback/imagecaption/RequestListTest.kt diff --git a/talkback/src/main/java/com/google/android/accessibility/talkback/actor/ImageCaptioner.java b/talkback/src/main/java/com/google/android/accessibility/talkback/actor/ImageCaptioner.java index ae826f3b0..09f01b47e 100644 --- a/talkback/src/main/java/com/google/android/accessibility/talkback/actor/ImageCaptioner.java +++ b/talkback/src/main/java/com/google/android/accessibility/talkback/actor/ImageCaptioner.java @@ -613,7 +613,7 @@ void startIconDetector() { @VisibleForTesting void shutdownIconDetector() { removeMessages(MSG_RESULT_TIMEOUT); - captionResults.cleanUp(); + abandonCaptionResults(); if (iconAnnotationsDetector != null) { synchronized (this) { @@ -689,6 +689,8 @@ private void shutdownImageDescription() { } public void shutdown() { + // Ends all requests, so that results that come later are ignored. + clearRequests(); shutdownIconDetector(); shutdownImageDescription(); moduleStateManager.shutdown(); @@ -927,8 +929,9 @@ public boolean captionWithGemini(AccessibilityNodeInfoCompat node) { service, node, this::onScreenshotCapturePending, - (focusedNode, screenCapture, isUserRequested, captureByWindow) -> { + (request, focusedNode, screenCapture, isUserRequested, captureByWindow) -> { processImageCaptioningWithGemini( + request, focusedNode, screenCapture, isUserRequested, @@ -969,8 +972,9 @@ public boolean captionWithOnDeviceGemini(AccessibilityNodeInfoCompat node) { service, node, this::onScreenshotCapturePending, - (focusedNode, screenCapture, isUserRequested, captureByWindow) -> { + (request, focusedNode, screenCapture, isUserRequested, captureByWindow) -> { processImageCaptioningWithGemini( + request, focusedNode, screenCapture, isUserRequested, @@ -983,13 +987,14 @@ public boolean captionWithOnDeviceGemini(AccessibilityNodeInfoCompat node) { } private void processImageCaptioningWithGemini( + ScreenshotCaptureRequest request, AccessibilityNodeInfoCompat focusedNode, Bitmap screenCapture, boolean isUserRequested, boolean captureByWindow, boolean isToAiCore) { if (screenCapture == null) { - handleScreenshotCaptureFailure(isUserRequested); + handleScreenshotCaptureFailure(request, isUserRequested); return; } @@ -1002,6 +1007,7 @@ private void processImageCaptioningWithGemini( captureByWindow); CaptionResult captionResult = new CaptionResult( + request, AccessibilityNode.takeOwnership(focusedNode), isUserRequested, /* isImageDescriptionByGemini= */ true); @@ -1066,21 +1072,22 @@ private void processImageCaptioningWithGemini( return; } - screenshotRequests.performNextRequest(); + screenshotRequests.performNextRequest(request); } private void processScreenDescription( + ScreenshotCaptureRequest request, AccessibilityNodeInfoCompat focusedNode, Bitmap screenCapture, boolean isUserRequested, boolean captureByWindow) { if (screenCapture == null) { - handleScreenshotCaptureFailure(isUserRequested); + handleScreenshotCaptureFailure(request, isUserRequested); return; } byte[] imageBytes = DataFieldUtils.encodeImageToByteArray(screenCapture); if (imageBytes == null) { - handleScreenshotCaptureFailure(isUserRequested); + handleScreenshotCaptureFailure(request, isUserRequested); return; } @@ -1092,7 +1099,7 @@ private void processScreenDescription( screenCapture.recycle(); } - screenshotRequests.performNextRequest(); + screenshotRequests.performNextRequest(request); } /** @@ -1304,12 +1311,13 @@ private static void blackenBlock(Rect bounds, int[] screenshotPixels, int width) @VisibleForTesting void handleScreenshotCaptureResponse( + ScreenshotCaptureRequest request, AccessibilityNodeInfoCompat node, @Nullable Bitmap screenCapture, boolean isUserRequested, boolean captureByWindow) { if (screenCapture == null) { - handleScreenshotCaptureFailure(isUserRequested); + handleScreenshotCaptureFailure(request, isUserRequested); return; } @@ -1320,6 +1328,7 @@ void handleScreenshotCaptureResponse( if (!isUserRequested) { LogUtils.v(TAG, "onScreenCaptureFinish(): auto-caption with Gemini"); processImageCaptioningWithGemini( + request, node, screenCapture, /* isUserRequested= */ false, @@ -1341,7 +1350,7 @@ void handleScreenshotCaptureResponse( captureByWindow); CaptionResult captionResult = - new CaptionResult(AccessibilityNode.takeOwnership(node), isUserRequested); + new CaptionResult(request, AccessibilityNode.takeOwnership(node), isUserRequested); captionResults.addResultAndCleanOldOnes(requestId, captionResult, isUserRequested); // For Bitmap recycle. captionResult.addUsedScreenshot(screens.blockedScreenCapture()); @@ -1409,10 +1418,15 @@ void handleScreenshotCaptureResponse( } LogUtils.v(TAG, "No caption request for the screenshot. Perform the next screenshot request."); - screenshotRequests.performNextRequest(); + // No result will finish the caption result, so it goes now, with its timeout. + captionResult.recycleAndClearScreenshots(); + captionResults.removeCaptionResult(requestId); + removeMessages(MSG_RESULT_TIMEOUT); + screenshotRequests.performNextRequest(request); } - private void handleScreenshotCaptureFailure(boolean isUserRequested) { + private void handleScreenshotCaptureFailure( + ScreenshotCaptureRequest request, boolean isUserRequested) { analytics.onImageCaptionEvent(IMAGE_CAPTION_EVENT_SCREENSHOT_FAILED); LogUtils.e(TAG, "onScreenCaptureFinish() taking screenshot has failed."); @@ -1420,7 +1434,7 @@ private void handleScreenshotCaptureFailure(boolean isUserRequested) { returnFeedback(R.string.gemini_screenshot_unavailable); } - screenshotRequests.performNextRequest(); + screenshotRequests.performNextRequest(request); } @VisibleForTesting @@ -1430,9 +1444,9 @@ void onScreenshotCapturePending(boolean scheduled, Duration intervalTime) { return; } + // The list could not schedule the request, so it has dropped its requests. analytics.onImageCaptionEvent( TalkBackAnalytics.IMAGE_CAPTION_EVENT_SCHEDULE_SCREENSHOT_CAPTURE_FAILURE); - screenshotRequests.performNextRequest(); } @VisibleForTesting @@ -1448,7 +1462,7 @@ void onCharacterCaptionFinish( + StringBuilderUtils.joinFields( StringBuilderUtils.optionalSubObj("result", result.text()), StringBuilderUtils.optionalSubObj("node", node))); - characterCaptionRequests.performNextRequest(); + characterCaptionRequests.performNextRequest(request); handleResult(request.getRequestId(), node, result, isUserRequested); imageCaptionStorage.updateCharacterCaptionResult(node, result); @@ -1471,7 +1485,7 @@ void addCaptionRequest( } analytics.onImageCaptionEvent(IMAGE_CAPTION_EVENT_OCR_PERFORM_FAIL); LogUtils.v(TAG, "onError(), error= %s", Request.errorName(errorCode)); - characterCaptionRequests.performNextRequest(); + characterCaptionRequests.performNextRequest(errorRequest); handleResult( errorRequest.getRequestId(), AccessibilityNode.takeOwnership(node), @@ -1489,7 +1503,7 @@ void onIconDetectionFinish( } analytics.onImageCaptionEvent(IMAGE_CAPTION_EVENT_ICON_DETECT_SUCCEED); LogUtils.v(TAG, "onIconDetectionFinish() result=%s node=%s", result.text(), node); - iconDetectionRequests.performNextRequest(); + iconDetectionRequests.performNextRequest(request); handleResult(request.getRequestId(), node, result, isUserRequested); imageCaptionStorage.updateDetectedIconLabel(node, result); @@ -1513,7 +1527,7 @@ void addIconDetectionRequest( } analytics.onImageCaptionEvent(IMAGE_CAPTION_EVENT_ICON_DETECT_FAIL); LogUtils.v(TAG, "onError(), error=%s", Request.errorName(errorCode)); - iconDetectionRequests.performNextRequest(); + iconDetectionRequests.performNextRequest(errorRequest); handleResult( errorRequest.getRequestId(), AccessibilityNode.takeOwnership(node), @@ -1536,7 +1550,7 @@ private void onImageDescriptionFinish( + StringBuilderUtils.joinFields( StringBuilderUtils.optionalSubObj("result", result.text()), StringBuilderUtils.optionalSubObj("node", node))); - imageDescriptionRequests.performNextRequest(); + imageDescriptionRequests.performNextRequest(request); handleResult(request.getRequestId(), node, result, isUserRequested); imageCaptionStorage.updateImageDescriptionResult(node, result); @@ -1560,7 +1574,7 @@ void addImageDescriptionRequest( } analytics.onImageCaptionEvent(IMAGE_CAPTION_EVENT_IMAGE_DESCRIBE_FAIL); LogUtils.v(TAG, "onError(), error=%s", Request.errorName(errorCode)); - imageDescriptionRequests.performNextRequest(); + imageDescriptionRequests.performNextRequest(errorRequest); handleResult( errorRequest.getRequestId(), AccessibilityNode.takeOwnership(node), @@ -1642,10 +1656,23 @@ Locale getSpeechLocale() { @VisibleForTesting void clearRequests() { + removeMessages(MSG_RESULT_TIMEOUT); screenshotRequests.clear(); characterCaptionRequests.clear(); iconDetectionRequests.clear(); - captionResults.cleanUp(); + imageDescriptionRequests.clear(); + abandonCaptionResults(); + } + + /** + * Drops the caption results that wait for more results, and ends their screenshot requests, so + * that the next screenshot request is performed. + */ + private void abandonCaptionResults() { + for (CaptionResult captionResult : captionResults.removeAll()) { + captionResult.recycleAndClearScreenshots(); + screenshotRequests.performNextRequest(captionResult.screenshotRequest); + } } @VisibleForTesting @@ -1718,7 +1745,8 @@ void handleResult(int id, AccessibilityNode node, Result result, boolean isUserR default -> {} } } - screenshotRequests.performNextRequest(); + // Its screenshot request ended when its caption result was dropped. The first screenshot + // request is another one, so it stays. } else { switch (result.type()) { case OCR -> { @@ -1747,10 +1775,10 @@ void handleResult(int id, AccessibilityNode node, Result result, boolean isUserR if (needFeedbackToUser) { returnCaptionResult(id, captionResult); } - screenshotRequests.performNextRequest(); captionResult.recycleAndClearScreenshots(); captionResults.removeCaptionResult(id); removeMessages(MSG_RESULT_TIMEOUT); + screenshotRequests.performNextRequest(captionResult.screenshotRequest); } } @@ -1958,7 +1986,7 @@ private void sendResultTimeoutMessage() { message.what = MSG_RESULT_TIMEOUT; message.arg1 = requestId; - sendMessageAtTime(message, System.currentTimeMillis() + RESULT_MAX_WAITING_TIME_MS); + sendMessageDelayed(message, RESULT_MAX_WAITING_TIME_MS); } @Override @@ -1977,11 +2005,12 @@ public void handleMessage(@NonNull Message msg) { } } LogUtils.w(TAG, "Caption request is timeout."); - screenshotRequests.clear(); + // Ends the caption requests, so that their results are ignored when they come. The screenshot + // requests that wait stay, such as one the user has just made. characterCaptionRequests.clear(); iconDetectionRequests.clear(); imageDescriptionRequests.clear(); - captionResults.cleanUp(); + abandonCaptionResults(); } } @@ -2108,6 +2137,9 @@ private static ProcessedScreens create( /** Stores all caption results to announce them together. */ private static class CaptionResult { + /** The request that took the screenshot. It stays first in its list until this is finished. */ + private final ScreenshotCaptureRequest screenshotRequest; + private final AccessibilityNode node; private final boolean isUserRequest; private boolean isOcrFinished; @@ -2121,12 +2153,17 @@ private static class CaptionResult { private boolean isGeminiFinishedWithError; private boolean isNetworkError; - private CaptionResult(AccessibilityNode node, boolean isUserRequest) { - this(node, isUserRequest, /* isImageDescriptionByGemini= */ false); + private CaptionResult( + ScreenshotCaptureRequest screenshotRequest, AccessibilityNode node, boolean isUserRequest) { + this(screenshotRequest, node, isUserRequest, /* isImageDescriptionByGemini= */ false); } private CaptionResult( - AccessibilityNode node, boolean isUserRequest, boolean isImageDescriptionByGemini) { + ScreenshotCaptureRequest screenshotRequest, + AccessibilityNode node, + boolean isUserRequest, + boolean isImageDescriptionByGemini) { + this.screenshotRequest = screenshotRequest; this.node = node; this.isUserRequest = isUserRequest; this.isImageDescriptionByGemini = isImageDescriptionByGemini; @@ -2306,9 +2343,11 @@ public CaptionResults() { captionResults = new HashMap<>(); } - public void cleanUp() { - captionResults.forEach((key, value) -> value.recycleAndClearScreenshots()); + /** Removes all caption results and returns them. */ + public List removeAll() { + List results = new ArrayList<>(captionResults.values()); captionResults.clear(); + return results; } @Nullable diff --git a/talkback/src/main/java/com/google/android/accessibility/talkback/imagecaption/CaptionRequest.java b/talkback/src/main/java/com/google/android/accessibility/talkback/imagecaption/CaptionRequest.java index 04a378c6b..dbc6e110f 100644 --- a/talkback/src/main/java/com/google/android/accessibility/talkback/imagecaption/CaptionRequest.java +++ b/talkback/src/main/java/com/google/android/accessibility/talkback/imagecaption/CaptionRequest.java @@ -116,6 +116,9 @@ protected void onCaptionStart() { } protected void onCaptionFinish(Result result) { + if (!finish()) { + return; + } setEndTimestamp(); LogUtils.v( TAG, @@ -131,8 +134,10 @@ protected void onCaptionFinish(Result result) { @Override protected void onError(@ErrorCode int errorCode) { + if (!finish()) { + return; + } setEndTimestamp(); - stopTimeoutRunnable(); LogUtils.e( TAG, "onError() " diff --git a/talkback/src/main/java/com/google/android/accessibility/talkback/imagecaption/Request.java b/talkback/src/main/java/com/google/android/accessibility/talkback/imagecaption/Request.java index 4cbef786c..4cde5a4f3 100644 --- a/talkback/src/main/java/com/google/android/accessibility/talkback/imagecaption/Request.java +++ b/talkback/src/main/java/com/google/android/accessibility/talkback/imagecaption/Request.java @@ -17,14 +17,18 @@ import android.os.Handler; import android.os.Looper; +import android.os.SystemClock; import androidx.annotation.IntDef; import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; import com.google.android.libraries.accessibility.utils.log.LogUtils; import java.time.Duration; -import java.time.Instant; -/** An action of image captioning. */ +/** + * An action of image captioning. It ends once: with a result, an error, its timeout or a cancel. + * Whatever comes after that is ignored, so that a late result cannot end it again. Use it on the + * looper that created it. + */ public abstract class Request { /** A listener to be invoked when the request has been suspended. */ @@ -44,6 +48,9 @@ public interface OnPendingListener { /** Represents the duration is invalid. */ public static final int INVALID_DURATION = -1; + /** Represents the time is not set. */ + public static final long INVALID_TIME = -1; + /** The reasons of image captions. */ @IntDef({ ERROR_UNKNOWN, @@ -74,8 +81,10 @@ public interface OnPendingListener { private final Runnable timeoutRunnable; @Nullable private final OnPendingListener onPendingListener; @Nullable private final Duration timeout; - @Nullable private Instant startTimestamp; - @Nullable private Instant endTimestamp; + // In SystemClock.uptimeMillis(), which does not jump when the wall clock is set. + private long startTimeMs = INVALID_TIME; + private long endTimeMs = INVALID_TIME; + private boolean finished; @VisibleForTesting Request() { @@ -94,15 +103,18 @@ public Request(@Nullable OnPendingListener onPendingListener, @Nullable Duration } public long getDurationMillis() { - if (startTimestamp == null || endTimestamp == null) { + if (startTimeMs == INVALID_TIME || endTimeMs == INVALID_TIME) { return INVALID_DURATION; } - return Duration.between(startTimestamp, endTimestamp).toMillis(); + return endTimeMs - startTimeMs; } - @Nullable - public Instant getEndTimestamp() { - return endTimestamp; + /** + * Returns the time at which the request is done, in {@link SystemClock#uptimeMillis()}, or {@link + * #INVALID_TIME} if it is not done. + */ + public long getEndTimeMillis() { + return endTimeMs; } /** Starts the action. */ @@ -120,15 +132,39 @@ protected void onPending(boolean result, Duration intervalTime) { /** Sets the time at which the caption request started to perform. */ protected void setStartTimestamp() { - startTimestamp = Instant.now(); + startTimeMs = SystemClock.uptimeMillis(); } /** Sets the time at which the caption request is done. */ protected void setEndTimestamp() { - endTimestamp = Instant.now(); + endTimeMs = SystemClock.uptimeMillis(); + } + + /** + * Ends the request and stops its timeout. Returns false if it has already ended, such as when its + * result comes after its timeout or after it was cancelled. The caller then ignores the result. + */ + protected boolean finish() { + stopTimeoutRunnable(); + if (finished) { + LogUtils.w(TAG, "Ignore the end of a request that has already ended. " + this); + return false; + } + finished = true; + return true; + } + + /** Ends the request without a result, so that its result is ignored if it comes later. */ + void cancel() { + stopTimeoutRunnable(); + finished = true; } protected void runTimeoutRunnable() { + if (finished) { + // It ended before its timeout was set, such as when it failed at once. + return; + } if (timeout == null) { LogUtils.w(TAG, "runTimeoutRunnable() with invalid timeout."); return; diff --git a/talkback/src/main/java/com/google/android/accessibility/talkback/imagecaption/RequestList.java b/talkback/src/main/java/com/google/android/accessibility/talkback/imagecaption/RequestList.java index a6c0945c4..0263b8d18 100644 --- a/talkback/src/main/java/com/google/android/accessibility/talkback/imagecaption/RequestList.java +++ b/talkback/src/main/java/com/google/android/accessibility/talkback/imagecaption/RequestList.java @@ -21,18 +21,18 @@ import android.os.Handler; import android.os.Looper; import android.os.Message; +import android.os.SystemClock; import androidx.annotation.NonNull; -import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; import com.google.android.libraries.accessibility.utils.log.LogUtils; import java.time.Duration; -import java.time.Instant; import java.util.ArrayDeque; /** * A list of requests. Adds and starts a request to the list via {@link * RequestList#addRequest(Request)}. After a request is finished, invoke {@link - * RequestList#performNextRequest()} to perform the next one. + * RequestList#performNextRequest(Request)} with it to perform the next one. Use it on the looper + * that created it. */ public class RequestList { @@ -51,12 +51,16 @@ public void handleMessage(@NonNull Message msg) { super.handleMessage(msg); if (msg.what == MSG_RETRY_TO_PERFORM) { LogUtils.v(TAG, "Retry to perform request"); - performNextRequest(/* shouldRemoveFinishedRequest= */ false); + performFirstRequest(); } } }; - private Instant lastRequestExecutionTime = Instant.EPOCH; + /** + * When the last request was performed or finished, in {@link SystemClock#uptimeMillis()}, which + * does not jump when the wall clock is set. + */ + private long lastRequestExecutionTimeMs = Request.INVALID_TIME; public RequestList(int capacity) { this(capacity, /* minIntervalTime= */ Duration.ZERO); @@ -81,68 +85,71 @@ public void addRequest(T request) { requests.size() - 1, request.getClass().getSimpleName()); } else { - performNextRequest(/* shouldRemoveFinishedRequest= */ false); + performFirstRequest(); } } /** - * Removed the finished request and performs the first pending request. + * Removes the finished request and performs the first pending request. Does nothing if the + * request is not the one being performed, such as when it was removed by {@link #clear()}, so + * that a late result cannot remove another request. * *

Discards the older requests if there are too many requests waiting to be executed. */ - public void performNextRequest() { - performNextRequest(/* shouldRemoveFinishedRequest= */ true); + public void performNextRequest(Request finishedRequest) { + if (requests.isEmpty() || requests.getFirst() != finishedRequest) { + LogUtils.v(TAG, "%s is not being performed", finishedRequest.getClass().getSimpleName()); + return; + } + + requests.removeFirst(); + // Updates lastRequestExecutionTime by the end timestamp which is more accurate than start + // timestamp. + long endTimeMs = finishedRequest.getEndTimeMillis(); + if (endTimeMs != Request.INVALID_TIME) { + lastRequestExecutionTimeMs = endTimeMs; + } + performFirstRequest(); } /** * Performs the first pending request or waits until the take-screenshot function is ready. * *

Discards the older requests if there are too many requests waiting to be executed. - * - * @param shouldRemoveFinishedRequest {@code true} if the first request that is finished should be - * removed */ - private void performNextRequest(boolean shouldRemoveFinishedRequest) { + private void performFirstRequest() { if (requests.isEmpty()) { return; } - if (shouldRemoveFinishedRequest) { - // Updates lastRequestExecutionTime by the end timestamp which is more accurate than start - // timestamp. - T finishedRequest = requests.removeFirst(); - @Nullable Instant finishedRequestEndTimestamp = finishedRequest.getEndTimestamp(); - if (finishedRequestEndTimestamp != null) { - lastRequestExecutionTime = finishedRequestEndTimestamp; - } - if (requests.isEmpty()) { - return; - } - } - while (requests.size() > capacity) { T request = requests.removeFirst(); + request.cancel(); LogUtils.v(TAG, "cancel %s size=%d ", request.getClass().getSimpleName(), requests.size()); } - Duration intervalTime = Duration.between(lastRequestExecutionTime, Instant.now()); - long waitingTime = minIntervalTime.minus(intervalTime).toMillis(); - if (waitingTime > 0) { - LogUtils.v(TAG, "waiting... %d ms", waitingTime); - Message message = new Message(); - message.what = MSG_RETRY_TO_PERFORM; - T request = requests.getFirst(); - if (handler.sendMessageDelayed(message, waitingTime)) { - request.onPending(true, intervalTime); - } else { - LogUtils.e(TAG, "Fail to send message to the handler."); - request.onPending(false, intervalTime); + T request = requests.getFirst(); + long now = SystemClock.uptimeMillis(); + if (lastRequestExecutionTimeMs != Request.INVALID_TIME) { + Duration intervalTime = Duration.ofMillis(now - lastRequestExecutionTimeMs); + long waitingTime = minIntervalTime.minus(intervalTime).toMillis(); + if (waitingTime > 0) { + LogUtils.v(TAG, "waiting... %d ms", waitingTime); + Message message = new Message(); + message.what = MSG_RETRY_TO_PERFORM; + if (handler.sendMessageDelayed(message, waitingTime)) { + request.onPending(true, intervalTime); + } else { + LogUtils.e(TAG, "Fail to send message to the handler."); + request.onPending(false, intervalTime); + // The looper is quitting, so no request can be performed. + clear(); + } + return; } - return; } - T request = requests.getFirst(); - lastRequestExecutionTime = Instant.now(); + lastRequestExecutionTimeMs = now; request.perform(); LogUtils.v(TAG, "perform %s", request.getClass().getSimpleName()); } @@ -156,9 +163,11 @@ Handler getHandler() { return handler; } + /** Removes and cancels all requests, so that their results are ignored if they come later. */ public void clear() { + handler.removeMessages(MSG_RETRY_TO_PERFORM); while (!requests.isEmpty()) { - T unused = requests.removeFirst(); + requests.removeFirst().cancel(); } } diff --git a/talkback/src/main/java/com/google/android/accessibility/talkback/imagecaption/ScreenshotCaptureRequest.java b/talkback/src/main/java/com/google/android/accessibility/talkback/imagecaption/ScreenshotCaptureRequest.java index 9c44a99a3..0e5e9654b 100644 --- a/talkback/src/main/java/com/google/android/accessibility/talkback/imagecaption/ScreenshotCaptureRequest.java +++ b/talkback/src/main/java/com/google/android/accessibility/talkback/imagecaption/ScreenshotCaptureRequest.java @@ -34,8 +34,14 @@ public class ScreenshotCaptureRequest extends Request { /** A listener to be invoked when taking screenshot is finished. */ public interface OnFinishListener { - /** Called when taking screenshot is finished. */ + /** + * Called when taking screenshot is finished. + * + * @param request the request, which stays first in its {@link RequestList} until it is passed + * to {@link RequestList#performNextRequest(Request)} + */ void onFinish( + ScreenshotCaptureRequest request, AccessibilityNodeInfoCompat node, Bitmap bitmap, boolean isUserRequested, @@ -105,7 +111,13 @@ public OnFinishListener getOnFinishListener() { } private void onFinished(@Nullable Bitmap screenCapture, boolean capturedByWindow) { - stopTimeoutRunnable(); + if (!finish()) { + // It timed out or was cancelled, and the next request may have started. + if (screenCapture != null) { + screenCapture.recycle(); + } + return; + } setEndTimestamp(); LogUtils.v( TAG, @@ -116,6 +128,6 @@ private void onFinished(@Nullable Bitmap screenCapture, boolean capturedByWindow StringBuilderUtils.optionalSubObj("screenCapture", screenCapture), StringBuilderUtils.optionalTag("capturedByWindow", capturedByWindow), StringBuilderUtils.optionalSubObj("node", node))); - onFinishListener.onFinish(node, screenCapture, isUserRequested, capturedByWindow); + onFinishListener.onFinish(this, node, screenCapture, isUserRequested, capturedByWindow); } } diff --git a/talkback/src/test/java/com/google/android/accessibility/talkback/actor/ImageCaptionerRequestsTest.kt b/talkback/src/test/java/com/google/android/accessibility/talkback/actor/ImageCaptionerRequestsTest.kt new file mode 100644 index 000000000..fb4608b75 --- /dev/null +++ b/talkback/src/test/java/com/google/android/accessibility/talkback/actor/ImageCaptionerRequestsTest.kt @@ -0,0 +1,316 @@ +/* + * Copyright 2026 Backtalk contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ + +package com.google.android.accessibility.talkback.actor + +import android.accessibilityservice.AccessibilityService +import android.accessibilityservice.AccessibilityService.TakeScreenshotCallback +import android.content.Context +import android.content.ContextWrapper +import android.graphics.Bitmap +import android.graphics.Rect +import android.os.Looper +import android.view.View +import android.view.accessibility.AccessibilityEvent +import android.view.accessibility.AccessibilityNodeInfo +import androidx.core.view.accessibility.AccessibilityNodeInfoCompat +import com.google.android.accessibility.talkback.ActorState +import com.google.android.accessibility.talkback.ActorStateWritable +import com.google.android.accessibility.talkback.Feedback +import com.google.android.accessibility.talkback.Pipeline +import com.google.android.accessibility.talkback.PrimesController +import com.google.android.accessibility.talkback.R +import com.google.android.accessibility.talkback.TalkBackAnalyticsImpl +import com.google.android.accessibility.talkback.TalkBackService +import com.google.android.accessibility.talkback.actor.gemini.AiCoreEndpoint +import com.google.android.accessibility.talkback.actor.gemini.GeminiActor +import com.google.android.accessibility.talkback.actor.gemini.GeminiActor.FinishReason +import com.google.android.accessibility.talkback.dynamicfeature.DownloaderFactory +import com.google.android.accessibility.talkback.dynamicfeature.ModuleStateManager +import com.google.android.accessibility.talkback.focusmanagement.AccessibilityFocusMonitor +import com.google.android.accessibility.talkback.imagecaption.CaptionRequest +import com.google.android.accessibility.talkback.imagecaption.ImageCaptionStorage +import com.google.android.accessibility.talkback.imagecaption.RequestList +import com.google.android.accessibility.talkback.imagecaption.ScreenshotCaptureRequest +import com.google.android.accessibility.talkback.imagedescription.ImageDescriptionProcessor +import com.google.android.accessibility.utils.FormFactorUtils +import com.google.android.accessibility.utils.SharedPreferencesUtils +import java.time.Duration +import java.util.concurrent.Executor +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.Robolectric +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment +import org.robolectric.Shadows.shadowOf +import org.robolectric.annotation.Config + +/** How ImageCaptioner ends its requests: each one once, and on time. */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34]) +class ImageCaptionerRequestsTest { + + /** Records the screenshot requests, which the tests then end. */ + class FakeScreenshotService : AccessibilityService() { + val callbacks = mutableListOf() + + override fun onAccessibilityEvent(event: AccessibilityEvent?) {} + + override fun onInterrupt() {} + + override fun takeScreenshot(displayId: Int, executor: Executor, callback: TakeScreenshotCallback) { + callbacks += callback + } + + override fun takeScreenshotOfWindow( + accessibilityWindowId: Int, + executor: Executor, + callback: TakeScreenshotCallback, + ) { + callbacks += callback + } + } + + private class FocusMonitor(service: AccessibilityService) : + AccessibilityFocusMonitor(service, null, null) { + var focus: AccessibilityNodeInfoCompat? = null + + override fun getAccessibilityFocus(useInputFocusIfEmpty: Boolean): AccessibilityNodeInfoCompat? = + focus?.let { AccessibilityNodeInfoCompat.obtain(it) } + } + + private val context: Context + get() = RuntimeEnvironment.getApplication() + + private val spoken = mutableListOf() + private val geminiRequests = mutableListOf() + private val resultDialogs = mutableListOf() + private val pipeline = + object : Pipeline.FeedbackReturner { + override fun returnFeedback(feedback: Feedback): Boolean { + val part = feedback.failovers().first() + part.speech()?.let { spoken += it.text() } + part.geminiRequest()?.let { geminiRequests += it } + part.geminiResultDialog()?.let { resultDialogs += it.imageDescriptionResult()?.text() } + return true + } + } + private lateinit var service: FakeScreenshotService + private lateinit var focusMonitor: FocusMonitor + private lateinit var captioner: ImageCaptioner + private lateinit var node: AccessibilityNodeInfoCompat + + @Before + fun setUp() { + // TalkBackService does this when it starts. + FormFactorUtils.initialize(context) + service = Robolectric.setupService(FakeScreenshotService::class.java) + focusMonitor = FocusMonitor(service) + captioner = + ImageCaptioner( + service, + // No time between screenshots, so that only the requests themselves decide what runs. + RequestList(ImageCaptioner.SCREENSHOT_REQUEST_CAPACITY, Duration.ZERO), + ImageCaptionStorage(), + ModuleStateManager( + service, + DownloaderFactory.create(service.application), + DownloaderFactory.legacy(service.application), + ), + focusMonitor, + TalkBackAnalyticsImpl(context), + PrimesController(), + ) + captioner.setPipeline(pipeline) + captioner.setActorState(actorStateWithoutAiCore()) + + // Robolectric compares nodes by their view. + val info = AccessibilityNodeInfo(View(context)) + info.setBoundsInScreen(Rect(10, 10, 90, 90)) + info.className = "android.widget.ImageView" + info.isVisibleToUser = true + node = AccessibilityNodeInfoCompat.wrap(info) + } + + /** ImageCaptioner asks the actor state whether the phone has AICore, which it does not. */ + private fun actorStateWithoutAiCore(): ActorState { + val talkBack = TalkBackService() + ContextWrapper::class + .java + .getDeclaredMethod("attachBaseContext", Context::class.java) + .apply { isAccessible = true } + .invoke(talkBack, context) + val aiCore = AiCoreEndpoint(context) + val gemini = GeminiActor(talkBack, TalkBackAnalyticsImpl(context), PrimesController(), aiCore, aiCore) + return ActorState( + ActorStateWritable( + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + gemini.state, + null, + null, + ) + ) + } + + private fun idle() { + shadowOf(Looper.getMainLooper()).idle() + } + + private fun advance(duration: Duration) { + shadowOf(Looper.getMainLooper()).idleFor(duration) + } + + /** Ends a screenshot request with a picture, as the system callback would. */ + private fun takeScreenshot(callback: TakeScreenshotCallback) { + val requestField = + callback.javaClass.declaredFields.first { + ScreenshotCaptureRequest::class.java.isAssignableFrom(it.type) + } + requestField.isAccessible = true + val request = requestField.get(callback) as ScreenshotCaptureRequest + ScreenshotCaptureRequest::class + .java + .getDeclaredMethod("onFinished", Bitmap::class.java, Boolean::class.javaPrimitiveType) + .apply { isAccessible = true } + .invoke(request, Bitmap.createBitmap(100, 100, Bitmap.Config.ARGB_8888), false) + idle() + } + + private fun failScreenshot(callback: TakeScreenshotCallback) { + callback.onFailure(AccessibilityService.ERROR_TAKE_SCREENSHOT_INTERNAL_ERROR) + idle() + } + + @Test + fun aScreenshotThatEndsAfterItsTimeoutDoesNotEndTheNextRequest() { + captioner.caption(node, /* isUserRequested= */ true) // A + captioner.caption(node, /* isUserRequested= */ true) // B, which waits for A + idle() + assertEquals(1, service.callbacks.size) + + // A's screenshot takes too long, so A ends and B starts. + advance(ScreenshotCaptureRequest.SCREENSHOT_CAPTURE_TIMEOUT_MS) + assertEquals(2, service.callbacks.size) + + captioner.caption(node, /* isUserRequested= */ true) // C, which waits for B + idle() + failScreenshot(service.callbacks[0]) // A's screenshot fails, late + assertEquals("C started before B ended", 2, service.callbacks.size) + + failScreenshot(service.callbacks[1]) + assertEquals(3, service.callbacks.size) + } + + @Test + fun aSlowCaptionIsSaidAfterFiveSeconds_andItsLateTimeoutEndsNothingElse() { + focusMonitor.focus = node + // Its image description never answers, so the caption waits for it. + captioner.imageDescriptionProcessor = + ImageDescriptionProcessor(context, TalkBackAnalyticsImpl(context)) + captioner.caption(node, /* isUserRequested= */ true) // A + idle() + takeScreenshot(service.callbacks[0]) + captioner.caption(node, /* isUserRequested= */ true) // B, which waits for A's caption + idle() + val spokenForA = spoken.size + + advance(Duration.ofMillis(4900)) + assertEquals(spokenForA, spoken.size) + assertEquals(1, service.callbacks.size) + + // After 5 seconds, A says what it has, and B starts. + advance(Duration.ofMillis(200)) + assertEquals("A was not said after 5 seconds", spokenForA + 1, spoken.size) + assertEquals("B did not start when A gave up", 2, service.callbacks.size) + + takeScreenshot(service.callbacks[1]) + captioner.caption(node, /* isUserRequested= */ true) // C, which waits for B's caption + idle() + val spokenForB = spoken.size + + // A's own requests would time out now, 10 seconds after they started. They ended with A. + advance(CaptionRequest.CAPTION_TIMEOUT_MS.minusMillis(5100).plusMillis(50)) + assertEquals("A was said again", spokenForB, spoken.size) + assertEquals("C started before B ended", 2, service.callbacks.size) + + // B gives up 5 seconds after its screenshot, and C starts. + advance(Duration.ofMillis(200)) + assertEquals(spokenForB + 1, spoken.size) + assertEquals(3, service.callbacks.size) + } + + /** Describe image with Gemini, as the menu does, up to the request to the model. */ + private fun describeImageWithGemini(): Feedback.GeminiRequest { + val screenshots = service.callbacks.size + captioner.captionWithGemini(node) + idle() + takeScreenshot(service.callbacks[screenshots]) + return geminiRequests.last() + } + + private fun answer(request: Feedback.GeminiRequest, text: String) { + captioner.handleResultFromGemini( + request.requestId(), + text, + /* isSuccess= */ true, + FinishReason.STOP, + /* errorReason= */ null, + /* manualTrigger= */ true, + ) + idle() + } + + @Test + fun aModelThatAnswersAfter30Seconds_isNotCutOffAfter5() { + val request = describeImageWithGemini() + + advance(Duration.ofSeconds(30)) + assertTrue(resultDialogs.isEmpty()) + answer(request, "A cat on a sofa.") + assertEquals(listOf("A cat on a sofa."), resultDialogs) + } + + @Test + fun anAutomaticCaptionWithNothingToDo_leavesNoTimeoutForDescribeImage() { + // Automatic text recognition is off, and there is no icon detector, so the caption has nothing + // to do. + SharedPreferencesUtils.getSharedPreferences(context) + .edit() + .putBoolean(context.getString(R.string.pref_auto_text_recognition_key), false) + .commit() + captioner.caption(node, /* isUserRequested= */ false) + idle() + takeScreenshot(service.callbacks[0]) + + val request = describeImageWithGemini() + advance(Duration.ofSeconds(30)) + answer(request, "A cat on a sofa.") + assertEquals(spoken.toString(), listOf("A cat on a sofa."), resultDialogs) + } +} diff --git a/talkback/src/test/java/com/google/android/accessibility/talkback/imagecaption/RequestListTest.kt b/talkback/src/test/java/com/google/android/accessibility/talkback/imagecaption/RequestListTest.kt new file mode 100644 index 000000000..d0d6e02cc --- /dev/null +++ b/talkback/src/test/java/com/google/android/accessibility/talkback/imagecaption/RequestListTest.kt @@ -0,0 +1,168 @@ +/* + * Copyright 2026 Backtalk contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ + +package com.google.android.accessibility.talkback.imagecaption + +import android.os.Looper +import java.time.Duration +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.Shadows.shadowOf +import org.robolectric.annotation.Config + +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34]) +class RequestListTest { + private val pending = mutableListOf() + + /** A request that the test ends, as its result, its error or its timeout would. */ + private inner class FakeRequest( + private val list: RequestList, + timeout: Duration?, + private val failAtOnce: Boolean, + ) : Request({ scheduled, _ -> pending += scheduled }, timeout) { + var performed = 0 + var results = 0 + val errors = mutableListOf() + + override fun perform() { + performed++ + setStartTimestamp() + if (failAtOnce) { + onError(Request.ERROR_UNKNOWN) + } + runTimeoutRunnable() + } + + override fun onError(errorCode: Int) { + if (finish()) { + setEndTimestamp() + errors += errorCode + list.performNextRequest(this) + } + } + + fun succeed() { + if (finish()) { + setEndTimestamp() + results++ + list.performNextRequest(this) + } + } + } + + private fun RequestList.request( + timeout: Duration? = null, + failAtOnce: Boolean = false, + ) = FakeRequest(this, timeout, failAtOnce) + + private fun advance(duration: Duration) { + shadowOf(Looper.getMainLooper()).idleFor(duration) + } + + @Test + fun aLateResultEndsNeitherItsRequestAgainNorTheNextOne() { + val list = RequestList(/* capacity= */ 1) + val a = list.request(timeout = Duration.ofSeconds(3)) + val b = list.request() + val c = list.request() + list.addRequest(a) + list.addRequest(b) + + advance(Duration.ofSeconds(3)) + assertEquals(listOf(Request.ERROR_TIMEOUT), a.errors) + assertEquals(1, b.performed) + + list.addRequest(c) + a.succeed() + list.performNextRequest(a) + assertEquals(0, a.results) + assertEquals("C started before B ended", 0, c.performed) + + b.succeed() + assertEquals(1, c.performed) + } + + @Test + fun clearCancelsTheRequests_soTheirTimeoutsAndResultsAreIgnored() { + val list = RequestList(/* capacity= */ 10) + val a = list.request(timeout = Duration.ofSeconds(3)) + val b = list.request() + list.addRequest(a) + list.addRequest(b) + + list.clear() + advance(Duration.ofSeconds(3)) + a.succeed() + assertTrue(a.errors.isEmpty()) + assertEquals(0, a.results) + + val d = list.request() + list.addRequest(d) + assertEquals(1, d.performed) + assertEquals(0, b.performed) + } + + @Test + fun aRequestThatFailsAtOnceSetsNoTimeout() { + val list = RequestList(/* capacity= */ 1) + val a = list.request(timeout = Duration.ofSeconds(3), failAtOnce = true) + list.addRequest(a) + + advance(Duration.ofSeconds(3)) + assertEquals(listOf(Request.ERROR_UNKNOWN), a.errors) + } + + @Test + fun waitsTheIntervalInUptime() { + val list = RequestList(/* capacity= */ 1, Duration.ofSeconds(1)) + val a = list.request() + val b = list.request() + list.addRequest(a) + a.succeed() + + list.addRequest(b) + assertEquals(listOf(true), pending) + advance(Duration.ofMillis(999)) + assertEquals(0, b.performed) + advance(Duration.ofMillis(1)) + assertEquals(1, b.performed) + } + + @Test + fun keepsTheNewestWaitingRequests_andCancelsTheOthers() { + val list = RequestList(/* capacity= */ 1) + val a = list.request() + val b = list.request() + val c = list.request() + list.addRequest(a) + list.addRequest(b) + list.addRequest(c) + + a.succeed() + assertEquals(0, b.performed) + assertEquals(1, c.performed) + + b.succeed() + assertEquals(0, b.results) + val d = list.request() + list.addRequest(d) + assertEquals("D started before C ended", 0, d.performed) + } +}