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) + } +}