From 06928183f0c71a3531104b3ec0a5362ce20686aa Mon Sep 17 00:00:00 2001 From: 1 Date: Thu, 30 Mar 2023 14:31:32 +0000 Subject: [PATCH] Remove SCREENSHOT_METADATA_REFACTOR flag usage Code has been in droidfood for months, remove flag to reduce duplication. Bug: 268484562 Test: atest TakeScreenshotServiceTest Change-Id: I3d265b77934ded4d7130a82042380e662cef100e --- .../screenshot/ScreenshotController.java | 131 ---------- .../screenshot/TakeScreenshotService.java | 72 +----- .../screenshot/TakeScreenshotServiceTest.kt | 230 ------------------ 3 files changed, 11 insertions(+), 422 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotController.java b/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotController.java index a9af1a2457f96..6f85c45a66148 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotController.java +++ b/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotController.java @@ -44,7 +44,6 @@ import android.app.ICompatCameraControlCallback; import android.app.Notification; import android.app.assist.AssistContent; import android.content.BroadcastReceiver; -import android.content.ComponentName; import android.content.Context; import android.content.Intent; import android.content.IntentFilter; @@ -525,39 +524,6 @@ public class ScreenshotController { mWindowManager.getCurrentWindowMetrics().getWindowInsets()); } - @MainThread - void takeScreenshotFullscreen(ComponentName topComponent, Consumer finisher, - RequestCallback requestCallback) { - Assert.isMainThread(); - mCurrentRequestCallback = requestCallback; - takeScreenshotInternal(topComponent, finisher, getFullScreenRect()); - } - - @MainThread - void handleImageAsScreenshot(Bitmap screenshot, Rect screenshotScreenBounds, - Insets visibleInsets, int taskId, int userId, ComponentName topComponent, - Consumer finisher, RequestCallback requestCallback) { - Assert.isMainThread(); - if (screenshot == null) { - Log.e(TAG, "Got null bitmap from screenshot message"); - mNotificationsController.notifyScreenshotError( - R.string.screenshot_failed_to_capture_text); - requestCallback.reportError(); - return; - } - - boolean showFlash = false; - if (screenshotScreenBounds == null - || !aspectRatiosMatch(screenshot, visibleInsets, screenshotScreenBounds)) { - showFlash = true; - visibleInsets = Insets.NONE; - screenshotScreenBounds = new Rect(0, 0, screenshot.getWidth(), screenshot.getHeight()); - } - mCurrentRequestCallback = requestCallback; - saveScreenshot(screenshot, finisher, screenshotScreenBounds, visibleInsets, topComponent, - showFlash, UserHandle.of(userId)); - } - /** * Clears current screenshot */ @@ -695,103 +661,6 @@ public class ScreenshotController { setContentView(mScreenshotView); } - /** - * Takes a screenshot of the current display and shows an animation. - */ - private void takeScreenshotInternal(ComponentName topComponent, Consumer finisher, - Rect crop) { - mScreenshotTakenInPortrait = - mContext.getResources().getConfiguration().orientation == ORIENTATION_PORTRAIT; - - // copy the input Rect, since SurfaceControl.screenshot can mutate it - Rect screenRect = new Rect(crop); - Bitmap screenshot = mImageCapture.captureDisplay(mDisplayTracker.getDefaultDisplayId(), - crop); - - if (screenshot == null) { - Log.e(TAG, "takeScreenshotInternal: Screenshot bitmap was null"); - mNotificationsController.notifyScreenshotError( - R.string.screenshot_failed_to_capture_text); - if (mCurrentRequestCallback != null) { - mCurrentRequestCallback.reportError(); - } - return; - } - - saveScreenshot(screenshot, finisher, screenRect, Insets.NONE, topComponent, true, - Process.myUserHandle()); - - mBroadcastSender.sendBroadcast(new Intent(ClipboardOverlayController.SCREENSHOT_ACTION), - ClipboardOverlayController.SELF_PERMISSION); - } - - private void saveScreenshot(Bitmap screenshot, Consumer finisher, Rect screenRect, - Insets screenInsets, ComponentName topComponent, boolean showFlash, UserHandle owner) { - withWindowAttached(() -> { - if (mUserManager.isManagedProfile(owner.getIdentifier())) { - mScreenshotView.announceForAccessibility(mContext.getResources().getString( - R.string.screenshot_saving_work_profile_title)); - } else { - mScreenshotView.announceForAccessibility( - mContext.getResources().getString(R.string.screenshot_saving_title)); - } - }); - - mScreenshotView.reset(); - - if (mScreenshotView.isAttachedToWindow()) { - // if we didn't already dismiss for another reason - if (!mScreenshotView.isDismissing()) { - mUiEventLogger.log(ScreenshotEvent.SCREENSHOT_REENTERED, 0, mPackageName); - } - if (DEBUG_WINDOW) { - Log.d(TAG, "saveScreenshot: screenshotView is already attached, resetting. " - + "(dismissing=" + mScreenshotView.isDismissing() + ")"); - } - } - mPackageName = topComponent == null ? "" : topComponent.getPackageName(); - mScreenshotView.setPackageName(mPackageName); - - mScreenshotView.updateOrientation( - mWindowManager.getCurrentWindowMetrics().getWindowInsets()); - - mScreenBitmap = screenshot; - - if (!isUserSetupComplete(owner)) { - Log.w(TAG, "User setup not complete, displaying toast only"); - // User setup isn't complete, so we don't want to show any UI beyond a toast, as editing - // and sharing shouldn't be exposed to the user. - saveScreenshotAndToast(owner, finisher); - return; - } - - // Optimizations - mScreenBitmap.setHasAlpha(false); - mScreenBitmap.prepareToDraw(); - - saveScreenshotInWorkerThread(owner, finisher, this::showUiOnActionsReady, - this::showUiOnQuickShareActionReady); - - // The window is focusable by default - setWindowFocusable(true); - mScreenshotView.requestFocus(); - - enqueueScrollCaptureRequest(owner); - - attachWindow(); - prepareAnimation(screenRect, showFlash, () -> { - mMessageContainerController.onScreenshotTaken(owner); - }); - - mScreenshotView.badgeScreenshot(mContext.getPackageManager().getUserBadgedIcon( - mContext.getDrawable(R.drawable.overlay_badge_background), owner)); - mScreenshotView.setScreenshot(mScreenBitmap, screenInsets); - // ignore system bar insets for the purpose of window layout - mWindow.getDecorView().setOnApplyWindowInsetsListener( - (v, insets) -> WindowInsets.CONSUMED); - mScreenshotHandler.cancelTimeout(); // restarted after animation - } - private void prepareAnimation(Rect screenRect, boolean showFlash, Runnable onAnimationComplete) { mScreenshotView.getViewTreeObserver().addOnPreDrawListener( diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotService.java b/packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotService.java index f3d2828072be0..1cdad83fb0aa4 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotService.java +++ b/packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotService.java @@ -36,9 +36,6 @@ import android.content.ComponentName; import android.content.Context; import android.content.Intent; import android.content.IntentFilter; -import android.graphics.Bitmap; -import android.graphics.Insets; -import android.graphics.Rect; import android.net.Uri; import android.os.Handler; import android.os.IBinder; @@ -49,7 +46,6 @@ import android.os.RemoteException; import android.os.UserHandle; import android.os.UserManager; import android.util.Log; -import android.view.WindowManager; import android.widget.Toast; import com.android.internal.annotations.VisibleForTesting; @@ -58,7 +54,6 @@ import com.android.internal.util.ScreenshotRequest; import com.android.systemui.R; import com.android.systemui.dagger.qualifiers.Background; import com.android.systemui.flags.FeatureFlags; -import com.android.systemui.flags.Flags; import java.util.concurrent.Executor; import java.util.function.Consumer; @@ -222,30 +217,17 @@ public class TakeScreenshotService extends Service { return; } - if (mFeatureFlags.isEnabled(Flags.SCREENSHOT_METADATA_REFACTOR)) { - Log.d(TAG, "Processing screenshot data"); - ScreenshotData screenshotData = ScreenshotData.fromRequest(request); - try { - mProcessor.processAsync(screenshotData, - (data) -> dispatchToController(data, onSaved, callback)); - } catch (IllegalStateException e) { - Log.e(TAG, "Failed to process screenshot request!", e); - logFailedRequest(request); - mNotificationsController.notifyScreenshotError( - R.string.screenshot_failed_to_capture_text); - callback.reportError(); - } - } else { - try { - mProcessor.processAsync(request, - (r) -> dispatchToController(r, onSaved, callback)); - } catch (IllegalStateException e) { - Log.e(TAG, "Failed to process screenshot request!", e); - logFailedRequest(request); - mNotificationsController.notifyScreenshotError( - R.string.screenshot_failed_to_capture_text); - callback.reportError(); - } + Log.d(TAG, "Processing screenshot data"); + ScreenshotData screenshotData = ScreenshotData.fromRequest(request); + try { + mProcessor.processAsync(screenshotData, + (data) -> dispatchToController(data, onSaved, callback)); + } catch (IllegalStateException e) { + Log.e(TAG, "Failed to process screenshot request!", e); + logFailedRequest(request); + mNotificationsController.notifyScreenshotError( + R.string.screenshot_failed_to_capture_text); + callback.reportError(); } } @@ -257,38 +239,6 @@ public class TakeScreenshotService extends Service { mScreenshot.handleScreenshot(screenshot, uriConsumer, callback); } - private void dispatchToController(ScreenshotRequest request, - Consumer uriConsumer, RequestCallback callback) { - ComponentName topComponent = request.getTopComponent(); - String packageName = topComponent == null ? "" : topComponent.getPackageName(); - mUiEventLogger.log( - ScreenshotEvent.getScreenshotSource(request.getSource()), 0, packageName); - - switch (request.getType()) { - case WindowManager.TAKE_SCREENSHOT_FULLSCREEN: - if (DEBUG_SERVICE) { - Log.d(TAG, "handleMessage: TAKE_SCREENSHOT_FULLSCREEN"); - } - mScreenshot.takeScreenshotFullscreen(topComponent, uriConsumer, callback); - break; - case WindowManager.TAKE_SCREENSHOT_PROVIDED_IMAGE: - if (DEBUG_SERVICE) { - Log.d(TAG, "handleMessage: TAKE_SCREENSHOT_PROVIDED_IMAGE"); - } - Bitmap screenshot = request.getBitmap(); - Rect screenBounds = request.getBoundsInScreen(); - Insets insets = request.getInsets(); - int taskId = request.getTaskId(); - int userId = request.getUserId(); - - mScreenshot.handleImageAsScreenshot(screenshot, screenBounds, insets, - taskId, userId, topComponent, uriConsumer, callback); - break; - default: - Log.wtf(TAG, "Invalid screenshot option: " + request.getType()); - } - } - private void logFailedRequest(ScreenshotRequest request) { ComponentName topComponent = request.getTopComponent(); String packageName = topComponent == null ? "" : topComponent.getPackageName(); diff --git a/packages/SystemUI/tests/src/com/android/systemui/screenshot/TakeScreenshotServiceTest.kt b/packages/SystemUI/tests/src/com/android/systemui/screenshot/TakeScreenshotServiceTest.kt index 47d88a50695f6..77f7426474976 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/screenshot/TakeScreenshotServiceTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/screenshot/TakeScreenshotServiceTest.kt @@ -23,28 +23,21 @@ import android.content.ComponentName import android.graphics.Bitmap import android.graphics.Bitmap.Config.HARDWARE import android.graphics.ColorSpace -import android.graphics.Insets -import android.graphics.Rect import android.hardware.HardwareBuffer import android.os.UserHandle import android.os.UserManager import android.testing.AndroidTestingRunner import android.view.WindowManager.ScreenshotSource.SCREENSHOT_KEY_OTHER -import android.view.WindowManager.ScreenshotSource.SCREENSHOT_OVERVIEW import android.view.WindowManager.TAKE_SCREENSHOT_FULLSCREEN -import android.view.WindowManager.TAKE_SCREENSHOT_PROVIDED_IMAGE import androidx.test.filters.SmallTest import com.android.internal.logging.testing.UiEventLoggerFake import com.android.internal.util.ScreenshotRequest import com.android.systemui.SysuiTestCase import com.android.systemui.flags.FakeFeatureFlags -import com.android.systemui.flags.Flags.SCREENSHOT_METADATA_REFACTOR import com.android.systemui.screenshot.ScreenshotEvent.SCREENSHOT_CAPTURE_FAILED import com.android.systemui.screenshot.ScreenshotEvent.SCREENSHOT_REQUESTED_KEY_OTHER -import com.android.systemui.screenshot.ScreenshotEvent.SCREENSHOT_REQUESTED_OVERVIEW import com.android.systemui.screenshot.TakeScreenshotService.RequestCallback import com.android.systemui.util.mockito.any -import com.android.systemui.util.mockito.argThat import com.android.systemui.util.mockito.eq import com.android.systemui.util.mockito.mock import com.android.systemui.util.mockito.whenever @@ -61,9 +54,6 @@ import org.mockito.Mockito.times import org.mockito.Mockito.verify import org.mockito.Mockito.verifyZeroInteractions -private const val USER_ID = 1 -private const val TASK_ID = 11 - @RunWith(AndroidTestingRunner::class) @SmallTest class TakeScreenshotServiceTest : SysuiTestCase() { @@ -123,9 +113,6 @@ class TakeScreenshotServiceTest : SysuiTestCase() { .whenever(requestProcessor) .processAsync(/* screenshot= */ any(ScreenshotData::class.java), /* callback= */ any()) - // Flipped in selected test cases - flags.set(SCREENSHOT_METADATA_REFACTOR, false) - service.attach( mContext, /* thread = */ null, @@ -157,39 +144,6 @@ class TakeScreenshotServiceTest : SysuiTestCase() { service.handleRequest(request, { /* onSaved */}, callback) - verify(controller, times(1)) - .takeScreenshotFullscreen( - eq(topComponent), - /* onSavedListener = */ any(), - /* requestCallback = */ any() - ) - - assertEquals("Expected one UiEvent", 1, eventLogger.numLogs()) - val logEvent = eventLogger.get(0) - - assertEquals( - "Expected SCREENSHOT_REQUESTED UiEvent", - logEvent.eventId, - SCREENSHOT_REQUESTED_KEY_OTHER.id - ) - assertEquals( - "Expected supplied package name", - topComponent.packageName, - eventLogger.get(0).packageName - ) - } - - @Test - fun takeScreenshotFullscreen_screenshotDataEnabled() { - flags.set(SCREENSHOT_METADATA_REFACTOR, true) - - val request = - ScreenshotRequest.Builder(TAKE_SCREENSHOT_FULLSCREEN, SCREENSHOT_KEY_OTHER) - .setTopComponent(topComponent) - .build() - - service.handleRequest(request, { /* onSaved */}, callback) - verify(controller, times(1)) .handleScreenshot( eq(ScreenshotData.fromRequest(request)), @@ -212,54 +166,8 @@ class TakeScreenshotServiceTest : SysuiTestCase() { ) } - @Test - fun takeScreenshotProvidedImage() { - val bounds = Rect(50, 50, 150, 150) - val bitmap = makeHardwareBitmap(100, 100) - - val request = - ScreenshotRequest.Builder(TAKE_SCREENSHOT_PROVIDED_IMAGE, SCREENSHOT_OVERVIEW) - .setTopComponent(topComponent) - .setTaskId(TASK_ID) - .setUserId(USER_ID) - .setBitmap(bitmap) - .setBoundsOnScreen(bounds) - .setInsets(Insets.NONE) - .build() - - service.handleRequest(request, { /* onSaved */}, callback) - - verify(controller, times(1)) - .handleImageAsScreenshot( - argThat { b -> b.equalsHardwareBitmap(bitmap) }, - eq(bounds), - eq(Insets.NONE), - eq(TASK_ID), - eq(USER_ID), - eq(topComponent), - /* onSavedListener = */ any(), - /* requestCallback = */ any() - ) - - assertEquals("Expected one UiEvent", 1, eventLogger.numLogs()) - val logEvent = eventLogger.get(0) - - assertEquals( - "Expected SCREENSHOT_REQUESTED_* UiEvent", - logEvent.eventId, - SCREENSHOT_REQUESTED_OVERVIEW.id - ) - assertEquals( - "Expected supplied package name", - topComponent.packageName, - eventLogger.get(0).packageName - ) - } - @Test fun takeScreenshotFullscreen_userLocked() { - flags.set(SCREENSHOT_METADATA_REFACTOR, true) - whenever(userManager.isUserUnlocked).thenReturn(false) val request = @@ -300,8 +208,6 @@ class TakeScreenshotServiceTest : SysuiTestCase() { @Test fun takeScreenshotFullscreen_screenCaptureDisabled_allUsers() { - flags.set(SCREENSHOT_METADATA_REFACTOR, true) - whenever(devicePolicyManager.getScreenCaptureDisabled(isNull(), eq(UserHandle.USER_ALL))) .thenReturn(true) @@ -349,144 +255,8 @@ class TakeScreenshotServiceTest : SysuiTestCase() { ) } - @Test - fun takeScreenshotFullscreen_userLocked_metadataDisabled() { - flags.set(SCREENSHOT_METADATA_REFACTOR, false) - whenever(userManager.isUserUnlocked).thenReturn(false) - - val request = - ScreenshotRequest.Builder(TAKE_SCREENSHOT_FULLSCREEN, SCREENSHOT_KEY_OTHER) - .setTopComponent(topComponent) - .build() - - service.handleRequest(request, { /* onSaved */}, callback) - - verify(notificationsController, times(1)).notifyScreenshotError(anyInt()) - verify(callback, times(1)).reportError() - verifyZeroInteractions(controller) - - assertEquals("Expected two UiEvents", 2, eventLogger.numLogs()) - val requestEvent = eventLogger.get(0) - assertEquals( - "Expected SCREENSHOT_REQUESTED_* UiEvent", - SCREENSHOT_REQUESTED_KEY_OTHER.id, - requestEvent.eventId - ) - assertEquals( - "Expected supplied package name", - topComponent.packageName, - requestEvent.packageName - ) - val failureEvent = eventLogger.get(1) - assertEquals( - "Expected SCREENSHOT_CAPTURE_FAILED UiEvent", - SCREENSHOT_CAPTURE_FAILED.id, - failureEvent.eventId - ) - assertEquals( - "Expected supplied package name", - topComponent.packageName, - failureEvent.packageName - ) - } - - @Test - fun takeScreenshotFullscreen_screenCaptureDisabled_allUsers_metadataDisabled() { - flags.set(SCREENSHOT_METADATA_REFACTOR, false) - - whenever(devicePolicyManager.getScreenCaptureDisabled(isNull(), eq(UserHandle.USER_ALL))) - .thenReturn(true) - - whenever( - devicePolicyResourcesManager.getString( - eq(SCREENSHOT_BLOCKED_BY_ADMIN), - /* Supplier */ - any(), - ) - ) - .thenReturn("SCREENSHOT_BLOCKED_BY_ADMIN") - - val request = - ScreenshotRequest.Builder(TAKE_SCREENSHOT_FULLSCREEN, SCREENSHOT_KEY_OTHER) - .setTopComponent(topComponent) - .build() - - service.handleRequest(request, { /* onSaved */}, callback) - - // error shown: Toast.makeText(...).show(), untestable - verify(callback, times(1)).reportError() - verifyZeroInteractions(controller) - assertEquals("Expected two UiEvents", 2, eventLogger.numLogs()) - val requestEvent = eventLogger.get(0) - assertEquals( - "Expected SCREENSHOT_REQUESTED_* UiEvent", - SCREENSHOT_REQUESTED_KEY_OTHER.id, - requestEvent.eventId - ) - assertEquals( - "Expected supplied package name", - topComponent.packageName, - requestEvent.packageName - ) - val failureEvent = eventLogger.get(1) - assertEquals( - "Expected SCREENSHOT_CAPTURE_FAILED UiEvent", - SCREENSHOT_CAPTURE_FAILED.id, - failureEvent.eventId - ) - assertEquals( - "Expected supplied package name", - topComponent.packageName, - failureEvent.packageName - ) - } - - @Test - fun takeScreenshot_workProfile_nullBitmap_metadataDisabled() { - flags.set(SCREENSHOT_METADATA_REFACTOR, false) - - val request = - ScreenshotRequest.Builder(TAKE_SCREENSHOT_FULLSCREEN, SCREENSHOT_KEY_OTHER) - .setTopComponent(topComponent) - .build() - - doThrow(IllegalStateException::class.java) - .whenever(requestProcessor) - .processAsync(any(ScreenshotRequest::class.java), any()) - - service.handleRequest(request, { /* onSaved */}, callback) - - verify(callback, times(1)).reportError() - verify(notificationsController, times(1)).notifyScreenshotError(anyInt()) - verifyZeroInteractions(controller) - assertEquals("Expected two UiEvents", 2, eventLogger.numLogs()) - val requestEvent = eventLogger.get(0) - assertEquals( - "Expected SCREENSHOT_REQUESTED_* UiEvent", - SCREENSHOT_REQUESTED_KEY_OTHER.id, - requestEvent.eventId - ) - assertEquals( - "Expected supplied package name", - topComponent.packageName, - requestEvent.packageName - ) - val failureEvent = eventLogger.get(1) - assertEquals( - "Expected SCREENSHOT_CAPTURE_FAILED UiEvent", - SCREENSHOT_CAPTURE_FAILED.id, - failureEvent.eventId - ) - assertEquals( - "Expected supplied package name", - topComponent.packageName, - failureEvent.packageName - ) - } @Test fun takeScreenshot_workProfile_nullBitmap() { - flags.set(SCREENSHOT_METADATA_REFACTOR, true) - val request = ScreenshotRequest.Builder(TAKE_SCREENSHOT_FULLSCREEN, SCREENSHOT_KEY_OTHER) .setTopComponent(topComponent)