From 3bf1ea36ee0f13a5c2b35d4b9b3a727b13da6cba Mon Sep 17 00:00:00 2001 From: Miranda Kephart Date: Thu, 21 May 2020 15:27:49 -0400 Subject: [PATCH] Fix logging for successive screenshots When two screenshots are taken in quick succession, we don't show UI for the first one (but do still continue the task for saving the screenshot in the background). However, the logging was done as part of the UI flow, meaning that we currently don't log when the first screenshot is saved. This change fixes the logic flow so that we log success/failure of the saving task in all cases, but only show UI for the last successive screenshot. Bug: 157244123 Fix: 157244123 Test: manual; tested that we don't see the log before the fix and do after Change-Id: I7646f4a3cb6cfd0f903472e101fb385cef58ddea --- .../systemui/screenshot/GlobalScreenshot.java | 96 +++++++++++-------- .../screenshot/SaveImageInBackgroundTask.java | 15 ++- 2 files changed, 64 insertions(+), 47 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/GlobalScreenshot.java b/packages/SystemUI/src/com/android/systemui/screenshot/GlobalScreenshot.java index f2d2eb3a0836d..33d692f8e1e57 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/GlobalScreenshot.java +++ b/packages/SystemUI/src/com/android/systemui/screenshot/GlobalScreenshot.java @@ -431,7 +431,13 @@ public class GlobalScreenshot implements ViewTreeObserver.OnComputeInternalInset data.createDeleteAction = false; if (mSaveInBgTask != null) { - mSaveInBgTask.ignoreResult(); + // just log success/failure for the pre-existing screenshot + mSaveInBgTask.setActionsReadyListener(new ActionsReadyListener() { + @Override + void onActionsReady(SavedImageData imageData) { + logSuccessOnActionsReady(imageData); + } + }); } mSaveInBgTask = new SaveImageInBackgroundTask(mContext, data); @@ -636,6 +642,52 @@ public class GlobalScreenshot implements ViewTreeObserver.OnComputeInternalInset mScreenshotPreview.setTranslationY(0); } + /** + * Sets up the action shade and its entrance animation, once we get the screenshot URI. + */ + private void showUiOnActionsReady(SavedImageData imageData) { + logSuccessOnActionsReady(imageData); + if (imageData.uri != null) { + mScreenshotHandler.post(() -> { + if (mScreenshotAnimation != null && mScreenshotAnimation.isRunning()) { + mScreenshotAnimation.addListener(new AnimatorListenerAdapter() { + @Override + public void onAnimationEnd(Animator animation) { + super.onAnimationEnd(animation); + createScreenshotActionsShadeAnimation(imageData).start(); + } + }); + } else { + createScreenshotActionsShadeAnimation(imageData).start(); + } + + AccessibilityManager accessibilityManager = (AccessibilityManager) + mContext.getSystemService(Context.ACCESSIBILITY_SERVICE); + long timeoutMs = accessibilityManager.getRecommendedTimeoutMillis( + SCREENSHOT_CORNER_DEFAULT_TIMEOUT_MILLIS, + AccessibilityManager.FLAG_CONTENT_CONTROLS); + + mScreenshotHandler.removeMessages(MESSAGE_CORNER_TIMEOUT); + mScreenshotHandler.sendMessageDelayed( + mScreenshotHandler.obtainMessage(MESSAGE_CORNER_TIMEOUT), + timeoutMs); + }); + } + } + + /** + * Logs success/failure of the screenshot saving task, and shows an error if it failed. + */ + private void logSuccessOnActionsReady(SavedImageData imageData) { + if (imageData.uri == null) { + mUiEventLogger.log(ScreenshotEvent.SCREENSHOT_NOT_SAVED); + mNotificationsController.notifyScreenshotError( + R.string.screenshot_failed_to_capture_text); + } else { + mUiEventLogger.log(ScreenshotEvent.SCREENSHOT_SAVED); + } + } + /** * Starts the animation after taking the screenshot */ @@ -651,43 +703,11 @@ public class GlobalScreenshot implements ViewTreeObserver.OnComputeInternalInset mScreenshotAnimation = createScreenshotDropInAnimation(w, h, screenRect); saveScreenshotInWorkerThread(finisher, new ActionsReadyListener() { - @Override - void onActionsReady(SavedImageData imageData) { - finisher.accept(imageData.uri); - if (imageData.uri == null) { - mUiEventLogger.log(ScreenshotEvent.SCREENSHOT_NOT_SAVED); - mNotificationsController.notifyScreenshotError( - R.string.screenshot_failed_to_capture_text); - } else { - mUiEventLogger.log(ScreenshotEvent.SCREENSHOT_SAVED); - mScreenshotHandler.post(() -> { - if (mScreenshotAnimation != null && mScreenshotAnimation.isRunning()) { - mScreenshotAnimation.addListener( - new AnimatorListenerAdapter() { - @Override - public void onAnimationEnd(Animator animation) { - super.onAnimationEnd(animation); - createScreenshotActionsShadeAnimation(imageData) - .start(); - } - }); - } else { - createScreenshotActionsShadeAnimation(imageData).start(); - } - AccessibilityManager accessibilityManager = (AccessibilityManager) - mContext.getSystemService(Context.ACCESSIBILITY_SERVICE); - long timeoutMs = accessibilityManager.getRecommendedTimeoutMillis( - SCREENSHOT_CORNER_DEFAULT_TIMEOUT_MILLIS, - AccessibilityManager.FLAG_CONTENT_CONTROLS); - - mScreenshotHandler.removeMessages(MESSAGE_CORNER_TIMEOUT); - mScreenshotHandler.sendMessageDelayed( - mScreenshotHandler.obtainMessage(MESSAGE_CORNER_TIMEOUT), - timeoutMs); - }); - } - } - }); + @Override + void onActionsReady(SavedImageData imageData) { + showUiOnActionsReady(imageData); + } + }); mScreenshotHandler.post(() -> { if (!mScreenshotLayout.isAttachedToWindow()) { mWindowManager.addView(mScreenshotLayout, mWindowLayoutParams); diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/SaveImageInBackgroundTask.java b/packages/SystemUI/src/com/android/systemui/screenshot/SaveImageInBackgroundTask.java index e88ce5a96f87e..a82061587e087 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/SaveImageInBackgroundTask.java +++ b/packages/SystemUI/src/com/android/systemui/screenshot/SaveImageInBackgroundTask.java @@ -215,6 +215,7 @@ class SaveImageInBackgroundTask extends AsyncTask { mImageData.deleteAction = createDeleteAction(mContext, mContext.getResources(), uri); mParams.mActionsReadyListener.onActionsReady(mImageData); + mParams.finisher.accept(mImageData.uri); mParams.image = null; mParams.errorMsgResId = 0; } catch (Exception e) { @@ -225,22 +226,18 @@ class SaveImageInBackgroundTask extends AsyncTask { mParams.errorMsgResId = R.string.screenshot_failed_to_save_text; mImageData.reset(); mParams.mActionsReadyListener.onActionsReady(mImageData); + mParams.finisher.accept(null); } return null; } /** - * If we get a new screenshot request while this one is saving, we want to continue saving in - * the background but not return anything. + * Update the listener run when the saving task completes. Used to avoid showing UI for the + * first screenshot when a second one is taken. */ - void ignoreResult() { - mParams.mActionsReadyListener = new GlobalScreenshot.ActionsReadyListener() { - @Override - void onActionsReady(GlobalScreenshot.SavedImageData imageData) { - // do nothing - } - }; + void setActionsReadyListener(GlobalScreenshot.ActionsReadyListener listener) { + mParams.mActionsReadyListener = listener; } @Override