diff options
23 files changed, 353 insertions, 551 deletions
diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/bouncer/domain/interactor/AlternateBouncerInteractorTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/bouncer/domain/interactor/AlternateBouncerInteractorTest.kt index adf4fc6c8ae3..b253309104d6 100644 --- a/packages/SystemUI/multivalentTests/src/com/android/systemui/bouncer/domain/interactor/AlternateBouncerInteractorTest.kt +++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/bouncer/domain/interactor/AlternateBouncerInteractorTest.kt @@ -140,6 +140,14 @@ class AlternateBouncerInteractorTest : SysuiTestCase() { } @Test + fun canShowAlternateBouncerForFingerprint_primaryBouncerShowing() { + givenCanShowAlternateBouncer() + bouncerRepository.setPrimaryShow(true) + + assertFalse(underTest.canShowAlternateBouncerForFingerprint()) + } + + @Test fun show_whenCannotShow() { givenCannotShowAlternateBouncer() @@ -202,7 +210,7 @@ class AlternateBouncerInteractorTest : SysuiTestCase() { } else { bouncerRepository.setAlternateBouncerUIAvailable(true) } - + bouncerRepository.setPrimaryShow(false) biometricSettingsRepository.setIsFingerprintAuthEnrolledAndEnabled(true) biometricSettingsRepository.setIsFingerprintAuthCurrentlyAllowed(true) whenever(keyguardUpdateMonitor.isFingerprintLockedOut).thenReturn(false) diff --git a/packages/SystemUI/src/com/android/systemui/bouncer/domain/interactor/AlternateBouncerInteractor.kt b/packages/SystemUI/src/com/android/systemui/bouncer/domain/interactor/AlternateBouncerInteractor.kt index af32eb534155..000f03a8c6ec 100644 --- a/packages/SystemUI/src/com/android/systemui/bouncer/domain/interactor/AlternateBouncerInteractor.kt +++ b/packages/SystemUI/src/com/android/systemui/bouncer/domain/interactor/AlternateBouncerInteractor.kt @@ -109,7 +109,8 @@ constructor( biometricSettingsRepository.isFingerprintAuthCurrentlyAllowed.value && !keyguardUpdateMonitor.isFingerprintLockedOut && !keyguardStateController.isUnlocked && - !statusBarStateController.isDozing + !statusBarStateController.isDozing && + !bouncerRepository.primaryBouncerShow.value } /** diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/ReferenceScreenshotModule.java b/packages/SystemUI/src/com/android/systemui/screenshot/ReferenceScreenshotModule.java index 6224e1bf2414..afb0280a3917 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/ReferenceScreenshotModule.java +++ b/packages/SystemUI/src/com/android/systemui/screenshot/ReferenceScreenshotModule.java @@ -16,6 +16,7 @@ package com.android.systemui.screenshot; +import dagger.Binds; import dagger.Module; import dagger.Provides; @@ -29,4 +30,9 @@ public interface ReferenceScreenshotModule { static ScreenshotNotificationSmartActionsProvider providesScrnshtNotifSmartActionsProvider() { return new ScreenshotNotificationSmartActionsProvider(); } + + /** */ + @Binds + ScreenshotActionsProvider.Factory bindScreenshotActionsProviderFactory( + DefaultScreenshotActionsProvider.Factory defaultScreenshotActionsProviderFactory); } diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotActionsProvider.kt b/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotActionsProvider.kt index f69021f34ebb..328742543184 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotActionsProvider.kt +++ b/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotActionsProvider.kt @@ -28,8 +28,7 @@ import com.android.systemui.screenshot.ActionIntentCreator.createShareWithSubjec import com.android.systemui.screenshot.ScreenshotEvent.SCREENSHOT_EDIT_TAPPED import com.android.systemui.screenshot.ScreenshotEvent.SCREENSHOT_PREVIEW_TAPPED import com.android.systemui.screenshot.ScreenshotEvent.SCREENSHOT_SHARE_TAPPED -import com.android.systemui.screenshot.ScreenshotEvent.SCREENSHOT_SMART_ACTION_TAPPED -import com.android.systemui.screenshot.ui.viewmodel.ActionButtonViewModel +import com.android.systemui.screenshot.ui.viewmodel.ActionButtonAppearance import com.android.systemui.screenshot.ui.viewmodel.ScreenshotViewModel import dagger.assisted.Assisted import dagger.assisted.AssistedFactory @@ -59,7 +58,6 @@ class DefaultScreenshotActionsProvider constructor( private val context: Context, private val viewModel: ScreenshotViewModel, - private val smartActionsProvider: SmartActionsProvider, private val uiEventLogger: UiEventLogger, @Assisted val request: ScreenshotData, @Assisted val requestId: String, @@ -81,81 +79,52 @@ constructor( } } viewModel.addAction( - ActionButtonViewModel( + ActionButtonAppearance( AppCompatResources.getDrawable(context, R.drawable.ic_screenshot_edit), context.resources.getString(R.string.screenshot_edit_label), context.resources.getString(R.string.screenshot_edit_description), - ) { - debugLog(LogConfig.DEBUG_ACTIONS) { "Edit tapped" } - uiEventLogger.log(SCREENSHOT_EDIT_TAPPED, 0, request.packageNameString) - onDeferrableActionTapped { result -> - actionExecutor.startSharedTransition( - createEdit(result.uri, context), - result.user, - true - ) - } + ) + ) { + debugLog(LogConfig.DEBUG_ACTIONS) { "Edit tapped" } + uiEventLogger.log(SCREENSHOT_EDIT_TAPPED, 0, request.packageNameString) + onDeferrableActionTapped { result -> + actionExecutor.startSharedTransition( + createEdit(result.uri, context), + result.user, + true + ) } - ) + } + viewModel.addAction( - ActionButtonViewModel( + ActionButtonAppearance( AppCompatResources.getDrawable(context, R.drawable.ic_screenshot_share), context.resources.getString(R.string.screenshot_share_label), context.resources.getString(R.string.screenshot_share_description), - ) { - debugLog(LogConfig.DEBUG_ACTIONS) { "Share tapped" } - uiEventLogger.log(SCREENSHOT_SHARE_TAPPED, 0, request.packageNameString) - onDeferrableActionTapped { result -> - actionExecutor.startSharedTransition( - createShareWithSubject(result.uri, result.subject), - result.user, - false - ) - } - } - ) - smartActionsProvider.requestQuickShare(request, requestId) { quickShare -> - if (!quickShare.actionIntent.isImmutable) { - viewModel.addAction( - ActionButtonViewModel( - quickShare.getIcon().loadDrawable(context), - quickShare.title, - quickShare.title - ) { - debugLog(LogConfig.DEBUG_ACTIONS) { "Quickshare tapped" } - onDeferrableActionTapped { result -> - uiEventLogger.log( - SCREENSHOT_SMART_ACTION_TAPPED, - 0, - request.packageNameString - ) - val pendingIntentWithUri = - smartActionsProvider.wrapIntent( - quickShare, - result.uri, - result.subject, - requestId - ) - actionExecutor.sendPendingIntent(pendingIntentWithUri) - } - } + ) + ) { + debugLog(LogConfig.DEBUG_ACTIONS) { "Share tapped" } + uiEventLogger.log(SCREENSHOT_SHARE_TAPPED, 0, request.packageNameString) + onDeferrableActionTapped { result -> + actionExecutor.startSharedTransition( + createShareWithSubject(result.uri, result.subject), + result.user, + false ) - } else { - Log.w(TAG, "Received immutable quick share pending intent; ignoring") } } } override fun onScrollChipReady(onClick: Runnable) { viewModel.addAction( - ActionButtonViewModel( + ActionButtonAppearance( AppCompatResources.getDrawable(context, R.drawable.ic_screenshot_scroll), context.resources.getString(R.string.screenshot_scroll_label), context.resources.getString(R.string.screenshot_scroll_label), - ) { - onClick.run() - } - ) + ) + ) { + onClick.run() + } } override fun setCompletedScreenshot(result: ScreenshotSavedResult) { @@ -165,15 +134,6 @@ constructor( } this.result = result pendingAction?.invoke(result) - smartActionsProvider.requestSmartActions(request, requestId, result) { smartActions -> - viewModel.addActions( - smartActions.map { - ActionButtonViewModel(it.getIcon().loadDrawable(context), it.title, it.title) { - actionExecutor.sendPendingIntent(it.actionIntent) - } - } - ) - } } private fun onDeferrableActionTapped(onResult: (ScreenshotSavedResult) -> Unit) { diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotController.java b/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotController.java index f43882c9ec28..9d42580ce83d 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotController.java +++ b/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotController.java @@ -176,11 +176,12 @@ public class ScreenshotController { // These strings are used for communicating the action invoked to // ScreenshotNotificationSmartActionsProvider. - static final String EXTRA_ACTION_TYPE = "android:screenshot_action_type"; - static final String EXTRA_ID = "android:screenshot_id"; - static final String EXTRA_SMART_ACTIONS_ENABLED = "android:smart_actions_enabled"; - static final String EXTRA_ACTION_INTENT = "android:screenshot_action_intent"; - static final String EXTRA_ACTION_INTENT_FILLIN = "android:screenshot_action_intent_fillin"; + public static final String EXTRA_ACTION_TYPE = "android:screenshot_action_type"; + public static final String EXTRA_ID = "android:screenshot_id"; + public static final String EXTRA_SMART_ACTIONS_ENABLED = "android:smart_actions_enabled"; + public static final String EXTRA_ACTION_INTENT = "android:screenshot_action_intent"; + public static final String EXTRA_ACTION_INTENT_FILLIN = + "android:screenshot_action_intent_fillin"; // From WizardManagerHelper.java diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotNotificationSmartActionsProvider.java b/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotNotificationSmartActionsProvider.java index 3eafbfbf37d7..23f05e0d8337 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotNotificationSmartActionsProvider.java +++ b/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotNotificationSmartActionsProvider.java @@ -44,7 +44,7 @@ public class ScreenshotNotificationSmartActionsProvider { public static final String DEFAULT_ACTION_TYPE = "Smart Action"; /* Define phases of screenshot execution. */ - protected enum ScreenshotOp { + public enum ScreenshotOp { OP_UNKNOWN, RETRIEVE_SMART_ACTIONS, REQUEST_SMART_ACTIONS, @@ -52,7 +52,7 @@ public class ScreenshotNotificationSmartActionsProvider { } /* Enum to report success or failure for screenshot execution phases. */ - protected enum ScreenshotOpStatus { + public enum ScreenshotOpStatus { OP_STATUS_UNKNOWN, SUCCESS, ERROR, diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/SmartActionsProvider.kt b/packages/SystemUI/src/com/android/systemui/screenshot/SmartActionsProvider.kt deleted file mode 100644 index a895b300b900..000000000000 --- a/packages/SystemUI/src/com/android/systemui/screenshot/SmartActionsProvider.kt +++ /dev/null @@ -1,285 +0,0 @@ -/* - * Copyright (C) 2024 The Android Open Source Project - * - * 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.android.systemui.screenshot - -import android.app.Notification -import android.app.PendingIntent -import android.content.ClipData -import android.content.ClipDescription -import android.content.ComponentName -import android.content.Context -import android.content.Intent -import android.graphics.Bitmap -import android.net.Uri -import android.os.Bundle -import android.os.Process -import android.os.SystemClock -import android.os.UserHandle -import android.provider.DeviceConfig -import android.util.Log -import com.android.internal.config.sysui.SystemUiDeviceConfigFlags -import com.android.systemui.log.DebugLogger.debugLog -import com.android.systemui.screenshot.LogConfig.DEBUG_ACTIONS -import com.android.systemui.screenshot.ScreenshotNotificationSmartActionsProvider.ScreenshotSmartActionType.QUICK_SHARE_ACTION -import com.android.systemui.screenshot.ScreenshotNotificationSmartActionsProvider.ScreenshotSmartActionType.REGULAR_SMART_ACTIONS -import java.util.concurrent.CompletableFuture -import java.util.concurrent.TimeUnit -import java.util.concurrent.TimeoutException -import javax.inject.Inject -import kotlin.random.Random - -/** - * Handle requesting smart/quickshare actions from the provider and executing an action when the - * action futures complete. - */ -class SmartActionsProvider -@Inject -constructor( - private val context: Context, - private val smartActions: ScreenshotNotificationSmartActionsProvider, -) { - /** - * Requests quick share action for a given screenshot. - * - * @param data the ScreenshotData request - * @param id the request id for the screenshot - * @param onAction callback to run when quick share action is returned - */ - fun requestQuickShare( - data: ScreenshotData, - id: String, - onAction: (Notification.Action) -> Unit - ) { - val bitmap = data.bitmap ?: return - val component = data.topComponent ?: ComponentName("", "") - requestQuickShareAction(id, bitmap, component, data.getUserOrDefault()) { quickShare -> - onAction(quickShare) - } - } - - /** - * Requests smart actions for a given screenshot. - * - * @param data the ScreenshotData request - * @param id the request id for the screenshot - * @param result the data for the saved image - * @param onActions callback to run when actions are returned - */ - fun requestSmartActions( - data: ScreenshotData, - id: String, - result: ScreenshotSavedResult, - onActions: (List<Notification.Action>) -> Unit - ) { - val bitmap = data.bitmap ?: return - val component = data.topComponent ?: ComponentName("", "") - requestSmartActions( - id, - bitmap, - component, - data.getUserOrDefault(), - result.uri, - REGULAR_SMART_ACTIONS - ) { actions -> - onActions(actions) - } - } - - /** - * Wraps the given quick share action in a broadcast intent. - * - * @param quickShare the quick share action to wrap - * @param uri the URI of the saved screenshot - * @param subject the subject/title for the screenshot - * @param id the request ID of the screenshot - * @return the pending intent with correct URI - */ - fun wrapIntent( - quickShare: Notification.Action, - uri: Uri, - subject: String, - id: String - ): PendingIntent { - val wrappedIntent: Intent = - Intent(context, SmartActionsReceiver::class.java) - .putExtra(ScreenshotController.EXTRA_ACTION_INTENT, quickShare.actionIntent) - .putExtra( - ScreenshotController.EXTRA_ACTION_INTENT_FILLIN, - createFillInIntent(uri, subject) - ) - .addFlags(Intent.FLAG_RECEIVER_FOREGROUND) - val extras: Bundle = quickShare.extras - val actionType = - extras.getString( - ScreenshotNotificationSmartActionsProvider.ACTION_TYPE, - ScreenshotNotificationSmartActionsProvider.DEFAULT_ACTION_TYPE - ) - // We only query for quick share actions when smart actions are enabled, so we can assert - // that it's true here. - wrappedIntent - .putExtra(ScreenshotController.EXTRA_ACTION_TYPE, actionType) - .putExtra(ScreenshotController.EXTRA_ID, id) - .putExtra(ScreenshotController.EXTRA_SMART_ACTIONS_ENABLED, true) - return PendingIntent.getBroadcast( - context, - Random.nextInt(), - wrappedIntent, - PendingIntent.FLAG_CANCEL_CURRENT or PendingIntent.FLAG_IMMUTABLE - ) - } - - private fun createFillInIntent(uri: Uri, subject: String): Intent { - val fillIn = Intent() - fillIn.setType("image/png") - fillIn.putExtra(Intent.EXTRA_STREAM, uri) - fillIn.putExtra(Intent.EXTRA_SUBJECT, subject) - // Include URI in ClipData also, so that grantPermission picks it up. - // We don't use setData here because some apps interpret this as "to:". - val clipData = - ClipData(ClipDescription("content", arrayOf("image/png")), ClipData.Item(uri)) - fillIn.clipData = clipData - fillIn.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION) - return fillIn - } - - private fun requestQuickShareAction( - id: String, - image: Bitmap, - component: ComponentName, - user: UserHandle, - timeoutMs: Long = 500, - onAction: (Notification.Action) -> Unit - ) { - requestSmartActions(id, image, component, user, null, QUICK_SHARE_ACTION, timeoutMs) { - it.firstOrNull()?.let { action -> onAction(action) } - } - } - - private fun requestSmartActions( - id: String, - image: Bitmap, - component: ComponentName, - user: UserHandle, - uri: Uri?, - actionType: ScreenshotNotificationSmartActionsProvider.ScreenshotSmartActionType, - timeoutMs: Long = 500, - onActions: (List<Notification.Action>) -> Unit - ) { - val enabled = isSmartActionsEnabled(user) - debugLog(DEBUG_ACTIONS) { - ("getSmartActionsFuture id=$id, uri=$uri, provider=$smartActions, " + - "actionType=$actionType, smartActionsEnabled=$enabled, userHandle=$user") - } - if (!enabled) { - debugLog(DEBUG_ACTIONS) { "Screenshot Intelligence not enabled, returning empty list" } - onActions(listOf()) - return - } - if (image.config != Bitmap.Config.HARDWARE) { - debugLog(DEBUG_ACTIONS) { - "Bitmap expected: Hardware, Bitmap found: ${image.config}. Returning empty list." - } - onActions(listOf()) - return - } - val smartActionsFuture: CompletableFuture<List<Notification.Action>> - val startTimeMs = SystemClock.uptimeMillis() - try { - smartActionsFuture = - smartActions.getActions(id, uri, image, component, actionType, user) - } catch (e: Throwable) { - val waitTimeMs = SystemClock.uptimeMillis() - startTimeMs - debugLog(DEBUG_ACTIONS, error = e) { - "Failed to get future for screenshot notification smart actions." - } - notifyScreenshotOp( - id, - ScreenshotNotificationSmartActionsProvider.ScreenshotOp.REQUEST_SMART_ACTIONS, - ScreenshotNotificationSmartActionsProvider.ScreenshotOpStatus.ERROR, - waitTimeMs - ) - onActions(listOf()) - return - } - try { - val actions = smartActionsFuture.get(timeoutMs, TimeUnit.MILLISECONDS) - val waitTimeMs = SystemClock.uptimeMillis() - startTimeMs - debugLog(DEBUG_ACTIONS) { - ("Got ${actions.size} smart actions. Wait time: $waitTimeMs ms, " + - "actionType=$actionType") - } - notifyScreenshotOp( - id, - ScreenshotNotificationSmartActionsProvider.ScreenshotOp.WAIT_FOR_SMART_ACTIONS, - ScreenshotNotificationSmartActionsProvider.ScreenshotOpStatus.SUCCESS, - waitTimeMs - ) - onActions(actions) - } catch (e: Throwable) { - val waitTimeMs = SystemClock.uptimeMillis() - startTimeMs - debugLog(DEBUG_ACTIONS, error = e) { - "Error getting smart actions. Wait time: $waitTimeMs ms, actionType=$actionType" - } - val status = - if (e is TimeoutException) { - ScreenshotNotificationSmartActionsProvider.ScreenshotOpStatus.TIMEOUT - } else { - ScreenshotNotificationSmartActionsProvider.ScreenshotOpStatus.ERROR - } - notifyScreenshotOp( - id, - ScreenshotNotificationSmartActionsProvider.ScreenshotOp.WAIT_FOR_SMART_ACTIONS, - status, - waitTimeMs - ) - onActions(listOf()) - } - } - - private fun notifyScreenshotOp( - screenshotId: String, - op: ScreenshotNotificationSmartActionsProvider.ScreenshotOp, - status: ScreenshotNotificationSmartActionsProvider.ScreenshotOpStatus, - durationMs: Long - ) { - debugLog(DEBUG_ACTIONS) { - "$smartActions notifyOp: $op id=$screenshotId, status=$status, durationMs=$durationMs" - } - try { - smartActions.notifyOp(screenshotId, op, status, durationMs) - } catch (e: Throwable) { - Log.e(TAG, "Error in notifyScreenshotOp: ", e) - } - } - - private fun isSmartActionsEnabled(user: UserHandle): Boolean { - // Smart actions don't yet work for cross-user saves. - val savingToOtherUser = user !== Process.myUserHandle() - val actionsEnabled = - DeviceConfig.getBoolean( - DeviceConfig.NAMESPACE_SYSTEMUI, - SystemUiDeviceConfigFlags.ENABLE_SCREENSHOT_NOTIFICATION_SMART_ACTIONS, - true - ) - return !savingToOtherUser && actionsEnabled - } - - companion object { - private const val TAG = "SmartActionsProvider" - private const val SCREENSHOT_SHARE_SUBJECT_TEMPLATE = "Screenshot (%s)" - } -} diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/dagger/ScreenshotModule.java b/packages/SystemUI/src/com/android/systemui/screenshot/dagger/ScreenshotModule.java index 2ce6d8380e36..b3eaa91b8dff 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/dagger/ScreenshotModule.java +++ b/packages/SystemUI/src/com/android/systemui/screenshot/dagger/ScreenshotModule.java @@ -22,12 +22,10 @@ import android.app.Service; import android.view.accessibility.AccessibilityManager; import com.android.systemui.dagger.SysUISingleton; -import com.android.systemui.screenshot.DefaultScreenshotActionsProvider; import com.android.systemui.screenshot.ImageCapture; import com.android.systemui.screenshot.ImageCaptureImpl; import com.android.systemui.screenshot.LegacyScreenshotViewProxy; import com.android.systemui.screenshot.RequestProcessor; -import com.android.systemui.screenshot.ScreenshotActionsProvider; import com.android.systemui.screenshot.ScreenshotPolicy; import com.android.systemui.screenshot.ScreenshotPolicyImpl; import com.android.systemui.screenshot.ScreenshotProxyService; @@ -93,10 +91,6 @@ public abstract class ScreenshotModule { abstract ScreenshotSoundController bindScreenshotSoundController( ScreenshotSoundControllerImpl screenshotSoundProviderImpl); - @Binds - abstract ScreenshotActionsProvider.Factory bindScreenshotActionsProviderFactory( - DefaultScreenshotActionsProvider.Factory defaultScreenshotActionsProviderFactory); - @Provides @SysUISingleton static ScreenshotViewModel providesScreenshotViewModel( diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/ui/binder/ActionButtonViewBinder.kt b/packages/SystemUI/src/com/android/systemui/screenshot/ui/binder/ActionButtonViewBinder.kt index a6374ae3304d..3c5a0ec107f8 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/ui/binder/ActionButtonViewBinder.kt +++ b/packages/SystemUI/src/com/android/systemui/screenshot/ui/binder/ActionButtonViewBinder.kt @@ -28,16 +28,16 @@ object ActionButtonViewBinder { fun bind(view: View, viewModel: ActionButtonViewModel) { val iconView = view.requireViewById<ImageView>(R.id.overlay_action_chip_icon) val textView = view.requireViewById<TextView>(R.id.overlay_action_chip_text) - iconView.setImageDrawable(viewModel.icon) - textView.text = viewModel.name - setMargins(iconView, textView, viewModel.name?.isNotEmpty() ?: false) + iconView.setImageDrawable(viewModel.appearance.icon) + textView.text = viewModel.appearance.label + setMargins(iconView, textView, viewModel.appearance.label?.isNotEmpty() ?: false) if (viewModel.onClicked != null) { view.setOnClickListener { viewModel.onClicked.invoke() } } else { view.setOnClickListener(null) } view.tag = viewModel.id - view.contentDescription = viewModel.description + view.contentDescription = viewModel.appearance.description view.visibility = View.VISIBLE view.alpha = 1f } diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/ui/viewmodel/ActionButtonAppearance.kt b/packages/SystemUI/src/com/android/systemui/screenshot/ui/viewmodel/ActionButtonAppearance.kt new file mode 100644 index 000000000000..55a2ad21e292 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/screenshot/ui/viewmodel/ActionButtonAppearance.kt @@ -0,0 +1,26 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * 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.android.systemui.screenshot.ui.viewmodel + +import android.graphics.drawable.Drawable + +/** Data describing how an action should be shown to the user. */ +data class ActionButtonAppearance( + val icon: Drawable?, + val label: CharSequence?, + val description: CharSequence, +) diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/ui/viewmodel/ActionButtonViewModel.kt b/packages/SystemUI/src/com/android/systemui/screenshot/ui/viewmodel/ActionButtonViewModel.kt index 97b24c1b7df7..64b0105a98a0 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/ui/viewmodel/ActionButtonViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/screenshot/ui/viewmodel/ActionButtonViewModel.kt @@ -16,19 +16,19 @@ package com.android.systemui.screenshot.ui.viewmodel -import android.graphics.drawable.Drawable - data class ActionButtonViewModel( - val icon: Drawable?, - val name: CharSequence?, - val description: CharSequence, + val appearance: ActionButtonAppearance, + val id: Int, val onClicked: (() -> Unit)?, ) { - val id: Int = getId() - companion object { private var nextId = 0 private fun getId() = nextId.also { nextId += 1 } + + fun withNextId( + appearance: ActionButtonAppearance, + onClicked: (() -> Unit)? + ): ActionButtonViewModel = ActionButtonViewModel(appearance, getId(), onClicked) } } diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/ui/viewmodel/ScreenshotViewModel.kt b/packages/SystemUI/src/com/android/systemui/screenshot/ui/viewmodel/ScreenshotViewModel.kt index ddfa69b687eb..fa3480343ea0 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/ui/viewmodel/ScreenshotViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/screenshot/ui/viewmodel/ScreenshotViewModel.kt @@ -17,6 +17,7 @@ package com.android.systemui.screenshot.ui.viewmodel import android.graphics.Bitmap +import android.util.Log import android.view.accessibility.AccessibilityManager import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow @@ -39,16 +40,34 @@ class ScreenshotViewModel(private val accessibilityManager: AccessibilityManager _previewAction.value = onClick } - fun addAction(action: ActionButtonViewModel) { + fun addAction(actionAppearance: ActionButtonAppearance, onClicked: (() -> Unit)): Int { val actionList = _actions.value.toMutableList() + val action = ActionButtonViewModel.withNextId(actionAppearance, onClicked) actionList.add(action) _actions.value = actionList + return action.id } - fun addActions(actions: List<ActionButtonViewModel>) { + fun updateActionAppearance(actionId: Int, appearance: ActionButtonAppearance) { val actionList = _actions.value.toMutableList() - actionList.addAll(actions) - _actions.value = actionList + val index = actionList.indexOfFirst { it.id == actionId } + if (index >= 0) { + actionList[index] = + ActionButtonViewModel(appearance, actionId, actionList[index].onClicked) + _actions.value = actionList + } else { + Log.w(TAG, "Attempted to update unknown action id $actionId") + } + } + + fun removeAction(actionId: Int) { + val actionList = _actions.value.toMutableList() + if (actionList.removeIf { it.id == actionId }) { + // Update if something was removed. + _actions.value = actionList + } else { + Log.w(TAG, "Attempted to remove unknown action id $actionId") + } } fun reset() { @@ -56,4 +75,8 @@ class ScreenshotViewModel(private val accessibilityManager: AccessibilityManager _previewAction.value = null _actions.value = listOf() } + + companion object { + const val TAG = "ScreenshotViewModel" + } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/screenshot/DefaultScreenshotActionsProviderTest.kt b/packages/SystemUI/tests/src/com/android/systemui/screenshot/DefaultScreenshotActionsProviderTest.kt index bde821b79469..853e50a12ea5 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/screenshot/DefaultScreenshotActionsProviderTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/screenshot/DefaultScreenshotActionsProviderTest.kt @@ -16,8 +16,6 @@ package com.android.systemui.screenshot -import android.app.Notification -import android.app.PendingIntent import android.content.Intent import android.net.Uri import android.os.Process @@ -27,8 +25,6 @@ import android.view.accessibility.AccessibilityManager import androidx.test.filters.SmallTest import com.android.internal.logging.UiEventLogger import com.android.systemui.SysuiTestCase -import com.android.systemui.clipboardoverlay.EditTextActivity -import com.android.systemui.res.R import com.android.systemui.screenshot.ui.viewmodel.ScreenshotViewModel import com.android.systemui.util.mockito.argumentCaptor import com.android.systemui.util.mockito.capture @@ -41,10 +37,7 @@ import org.junit.Assert.assertNotNull import org.junit.Before import org.junit.runner.RunWith import org.mockito.Mockito.verifyNoMoreInteractions -import org.mockito.kotlin.any -import org.mockito.kotlin.never import org.mockito.kotlin.verify -import org.mockito.kotlin.whenever @RunWith(AndroidTestingRunner::class) @SmallTest @@ -52,7 +45,6 @@ class DefaultScreenshotActionsProviderTest : SysuiTestCase() { private val actionExecutor = mock<ActionExecutor>() private val accessibilityManager = mock<AccessibilityManager>() private val uiEventLogger = mock<UiEventLogger>() - private val smartActionsProvider = mock<SmartActionsProvider>() private val request = ScreenshotData.forTesting() private val validResult = ScreenshotSavedResult(Uri.EMPTY, Process.myUserHandle(), 0) @@ -119,42 +111,10 @@ class DefaultScreenshotActionsProviderTest : SysuiTestCase() { assertThat(intentCaptor.value.action).isEqualTo(Intent.ACTION_CHOOSER) } - @Test - fun quickShareTapped_wrapsAndSendsIntent() = runTest { - val quickShare = - Notification.Action( - R.drawable.ic_screenshot_edit, - "TestQuickShare", - PendingIntent.getActivity( - context, - 0, - Intent(context, EditTextActivity::class.java), - PendingIntent.FLAG_MUTABLE - ) - ) - whenever(smartActionsProvider.requestQuickShare(any(), any(), any())).then { - (it.getArgument(2) as ((Notification.Action) -> Unit)).invoke(quickShare) - } - whenever(smartActionsProvider.wrapIntent(any(), any(), any(), any())).thenAnswer { - (it.getArgument(0) as Notification.Action).actionIntent - } - actionsProvider = createActionsProvider() - - viewModel.actions.value[2].onClicked?.invoke() - verify(uiEventLogger, never()) - .log(eq(ScreenshotEvent.SCREENSHOT_SMART_ACTION_TAPPED), any(), any()) - verify(smartActionsProvider, never()).wrapIntent(any(), any(), any(), any()) - actionsProvider.setCompletedScreenshot(validResult) - verify(smartActionsProvider) - .wrapIntent(eq(quickShare), eq(validResult.uri), eq(validResult.subject), eq("testid")) - verify(uiEventLogger).log(eq(ScreenshotEvent.SCREENSHOT_SMART_ACTION_TAPPED), eq(0), eq("")) - } - private fun createActionsProvider(): ScreenshotActionsProvider { return DefaultScreenshotActionsProvider( context, viewModel, - smartActionsProvider, uiEventLogger, request, "testid", diff --git a/packages/SystemUI/tests/src/com/android/systemui/screenshot/ui/viewmodel/ScreenshotViewModelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/screenshot/ui/viewmodel/ScreenshotViewModelTest.kt new file mode 100644 index 000000000000..d44e26c266fc --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/screenshot/ui/viewmodel/ScreenshotViewModelTest.kt @@ -0,0 +1,77 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * 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.android.systemui.screenshot.ui.viewmodel + +import android.view.accessibility.AccessibilityManager +import androidx.test.filters.SmallTest +import com.google.common.truth.Truth.assertThat +import org.junit.Test +import org.mockito.Mockito.mock + +@SmallTest +class ScreenshotViewModelTest { + private val accessibilityManager: AccessibilityManager = mock(AccessibilityManager::class.java) + private val appearance = ActionButtonAppearance(null, "Label", "Description") + private val onclick = {} + + @Test + fun testAddAction() { + val viewModel = ScreenshotViewModel(accessibilityManager) + + assertThat(viewModel.actions.value).isEmpty() + + viewModel.addAction(appearance, onclick) + + assertThat(viewModel.actions.value).hasSize(1) + + val added = viewModel.actions.value[0] + assertThat(added.appearance).isEqualTo(appearance) + assertThat(added.onClicked).isEqualTo(onclick) + } + + @Test + fun testRemoveAction() { + val viewModel = ScreenshotViewModel(accessibilityManager) + val firstId = viewModel.addAction(ActionButtonAppearance(null, "", ""), {}) + val secondId = viewModel.addAction(appearance, onclick) + + assertThat(viewModel.actions.value).hasSize(2) + assertThat(firstId).isNotEqualTo(secondId) + + viewModel.removeAction(firstId) + + assertThat(viewModel.actions.value).hasSize(1) + + val remaining = viewModel.actions.value[0] + assertThat(remaining.appearance).isEqualTo(appearance) + assertThat(remaining.onClicked).isEqualTo(onclick) + } + + @Test + fun testUpdateActionAppearance() { + val viewModel = ScreenshotViewModel(accessibilityManager) + val id = viewModel.addAction(appearance, onclick) + val otherAppearance = ActionButtonAppearance(null, "Other", "Other") + + viewModel.updateActionAppearance(id, otherAppearance) + + assertThat(viewModel.actions.value).hasSize(1) + val updated = viewModel.actions.value[0] + assertThat(updated.appearance).isEqualTo(otherAppearance) + assertThat(updated.onClicked).isEqualTo(onclick) + } +} diff --git a/services/core/java/com/android/server/audio/AudioDeviceBroker.java b/services/core/java/com/android/server/audio/AudioDeviceBroker.java index df84653135fa..d435c2484b49 100644 --- a/services/core/java/com/android/server/audio/AudioDeviceBroker.java +++ b/services/core/java/com/android/server/audio/AudioDeviceBroker.java @@ -1535,8 +1535,9 @@ public class AudioDeviceBroker { sendLMsgNoDelay(MSG_L_SYNCHRONIZE_ADI_DEVICES_IN_INVENTORY, SENDMSG_QUEUE, deviceState); } - /*package*/ void postUpdatedAdiDeviceState(AdiDeviceState deviceState) { - sendLMsgNoDelay(MSG_L_UPDATED_ADI_DEVICE_STATE, SENDMSG_QUEUE, deviceState); + /*package*/ void postUpdatedAdiDeviceState(AdiDeviceState deviceState, boolean initSA) { + sendILMsgNoDelay( + MSG_IL_UPDATED_ADI_DEVICE_STATE, SENDMSG_QUEUE, initSA ? 1 : 0, deviceState); } /*package*/ static final class CommunicationDeviceInfo { @@ -2068,8 +2069,8 @@ public class AudioDeviceBroker { } } break; - case MSG_L_UPDATED_ADI_DEVICE_STATE: - mAudioService.onUpdatedAdiDeviceState((AdiDeviceState) msg.obj); + case MSG_IL_UPDATED_ADI_DEVICE_STATE: + mAudioService.onUpdatedAdiDeviceState((AdiDeviceState) msg.obj, msg.arg1 == 1); break; default: @@ -2156,7 +2157,7 @@ public class AudioDeviceBroker { private static final int MSG_CHECK_COMMUNICATION_ROUTE_CLIENT_STATE = 56; private static final int MSG_I_UPDATE_LE_AUDIO_GROUP_ADDRESSES = 57; private static final int MSG_L_SYNCHRONIZE_ADI_DEVICES_IN_INVENTORY = 58; - private static final int MSG_L_UPDATED_ADI_DEVICE_STATE = 59; + private static final int MSG_IL_UPDATED_ADI_DEVICE_STATE = 59; diff --git a/services/core/java/com/android/server/audio/AudioDeviceInventory.java b/services/core/java/com/android/server/audio/AudioDeviceInventory.java index 8b64d93e4b30..ecb08c2088e6 100644 --- a/services/core/java/com/android/server/audio/AudioDeviceInventory.java +++ b/services/core/java/com/android/server/audio/AudioDeviceInventory.java @@ -169,7 +169,7 @@ public class AudioDeviceInventory { if (ads != null) { if (ads.getAudioDeviceCategory() != category) { ads.setAudioDeviceCategory(category); - mDeviceBroker.postUpdatedAdiDeviceState(ads); + mDeviceBroker.postUpdatedAdiDeviceState(ads, false /*initSA*/); mDeviceBroker.postPersistAudioDeviceSettings(); } mDeviceBroker.postSynchronizeAdiDevicesInInventory(ads); @@ -182,7 +182,7 @@ public class AudioDeviceInventory { mDeviceInventory.put(ads.getDeviceId(), ads); checkDeviceInventorySize_l(); - mDeviceBroker.postUpdatedAdiDeviceState(ads); + mDeviceBroker.postUpdatedAdiDeviceState(ads, true /*initSA*/); mDeviceBroker.postPersistAudioDeviceSettings(); } } @@ -212,7 +212,7 @@ public class AudioDeviceInventory { checkDeviceInventorySize_l(); } if (updatedCategory.get()) { - mDeviceBroker.postUpdatedAdiDeviceState(deviceState); + mDeviceBroker.postUpdatedAdiDeviceState(deviceState, false /*initSA*/); } mDeviceBroker.postSynchronizeAdiDevicesInInventory(deviceState); } @@ -314,7 +314,7 @@ public class AudioDeviceInventory { } ads2.setAudioDeviceCategory(updatedDevice.getAudioDeviceCategory()); - mDeviceBroker.postUpdatedAdiDeviceState(ads2); + mDeviceBroker.postUpdatedAdiDeviceState(ads2, false /*initSA*/); AudioService.sDeviceLogger.enqueue(new EventLogger.StringEvent( "synchronizeBleDeviceInInventory synced device pair ads1=" + updatedDevice + " ads2=" + ads2).printLog(TAG)); @@ -335,7 +335,7 @@ public class AudioDeviceInventory { } ads2.setAudioDeviceCategory(updatedDevice.getAudioDeviceCategory()); - mDeviceBroker.postUpdatedAdiDeviceState(ads2); + mDeviceBroker.postUpdatedAdiDeviceState(ads2, false /*initSA*/); AudioService.sDeviceLogger.enqueue(new EventLogger.StringEvent( "synchronizeBleDeviceInInventory synced device pair ads1=" + updatedDevice + " peer ads2=" + ads2).printLog(TAG)); @@ -360,7 +360,7 @@ public class AudioDeviceInventory { } ads.setAudioDeviceCategory(updatedDevice.getAudioDeviceCategory()); - mDeviceBroker.postUpdatedAdiDeviceState(ads); + mDeviceBroker.postUpdatedAdiDeviceState(ads, false /*initSA*/); AudioService.sDeviceLogger.enqueue(new EventLogger.StringEvent( "synchronizeDeviceProfilesInInventory synced device pair ads1=" + updatedDevice + " ads2=" + ads).printLog(TAG)); diff --git a/services/core/java/com/android/server/audio/AudioService.java b/services/core/java/com/android/server/audio/AudioService.java index d4f04b5ad760..45076d04fbbd 100644 --- a/services/core/java/com/android/server/audio/AudioService.java +++ b/services/core/java/com/android/server/audio/AudioService.java @@ -11253,7 +11253,8 @@ public class AudioService extends IAudioService.Stub mDeviceBroker.addOrUpdateBtAudioDeviceCategoryInInventory(deviceState); mDeviceBroker.postPersistAudioDeviceSettings(); - mSpatializerHelper.refreshDevice(deviceState.getAudioDeviceAttributes()); + mSpatializerHelper.refreshDevice(deviceState.getAudioDeviceAttributes(), + false /* initState */); mSoundDoseHelper.setAudioDeviceCategory(addr, internalType, btAudioDeviceCategory == AUDIO_DEVICE_CATEGORY_HEADPHONES); } @@ -11324,11 +11325,11 @@ public class AudioService extends IAudioService.Stub /** Update the sound dose and spatializer state based on the new AdiDeviceState. */ @VisibleForTesting(visibility = PACKAGE) - public void onUpdatedAdiDeviceState(AdiDeviceState deviceState) { + public void onUpdatedAdiDeviceState(AdiDeviceState deviceState, boolean initSA) { if (deviceState == null) { return; } - mSpatializerHelper.refreshDevice(deviceState.getAudioDeviceAttributes()); + mSpatializerHelper.refreshDevice(deviceState.getAudioDeviceAttributes(), initSA); mSoundDoseHelper.setAudioDeviceCategory(deviceState.getDeviceAddress(), deviceState.getInternalDeviceType(), deviceState.getAudioDeviceCategory() == AUDIO_DEVICE_CATEGORY_HEADPHONES); diff --git a/services/core/java/com/android/server/audio/SpatializerHelper.java b/services/core/java/com/android/server/audio/SpatializerHelper.java index 3b5fa7f00891..38fa79f7f44a 100644 --- a/services/core/java/com/android/server/audio/SpatializerHelper.java +++ b/services/core/java/com/android/server/audio/SpatializerHelper.java @@ -295,11 +295,11 @@ public class SpatializerHelper { // could have been called another time after boot in case of audioserver restart addCompatibleAudioDevice( new AudioDeviceAttributes(AudioSystem.DEVICE_OUT_SPEAKER, ""), - false /*forceEnable*/); + false /*forceEnable*/, false /*forceInit*/); // not force-enabling as this device might already be in the device list addCompatibleAudioDevice( new AudioDeviceAttributes(AudioSystem.DEVICE_OUT_WIRED_HEADPHONE, ""), - false /*forceEnable*/); + false /*forceEnable*/, false /*forceInit*/); } catch (RemoteException e) { resetCapabilities(); } finally { @@ -526,7 +526,7 @@ public class SpatializerHelper { } synchronized void addCompatibleAudioDevice(@NonNull AudioDeviceAttributes ada) { - addCompatibleAudioDevice(ada, true /*forceEnable*/); + addCompatibleAudioDevice(ada, true /*forceEnable*/, false /*forceInit*/); } /** @@ -540,7 +540,7 @@ public class SpatializerHelper { */ @GuardedBy("this") private void addCompatibleAudioDevice(@NonNull AudioDeviceAttributes ada, - boolean forceEnable) { + boolean forceEnable, boolean forceInit) { if (!isDeviceCompatibleWithSpatializationModes(ada)) { return; } @@ -548,6 +548,9 @@ public class SpatializerHelper { final AdiDeviceState deviceState = findSACompatibleDeviceStateForAudioDeviceAttributes(ada); AdiDeviceState updatedDevice = null; // non-null on update. if (deviceState != null) { + if (forceInit) { + initSAState(deviceState); + } if (forceEnable && !deviceState.isSAEnabled()) { updatedDevice = deviceState; updatedDevice.setSAEnabled(true); @@ -756,10 +759,10 @@ public class SpatializerHelper { } } - synchronized void refreshDevice(@NonNull AudioDeviceAttributes ada) { + synchronized void refreshDevice(@NonNull AudioDeviceAttributes ada, boolean initState) { final AdiDeviceState deviceState = findSACompatibleDeviceStateForAudioDeviceAttributes(ada); if (isAvailableForAdiDeviceState(deviceState)) { - addCompatibleAudioDevice(ada, /*forceEnable=*/deviceState.isSAEnabled()); + addCompatibleAudioDevice(ada, /*forceEnable=*/deviceState.isSAEnabled(), initState); setHeadTrackerEnabled(deviceState.isHeadTrackerEnabled(), ada); } else { removeCompatibleAudioDevice(ada); diff --git a/services/core/java/com/android/server/wm/LetterboxConfiguration.java b/services/core/java/com/android/server/wm/LetterboxConfiguration.java index 45cf10bd3f5e..5aa0ed7ce76c 100644 --- a/services/core/java/com/android/server/wm/LetterboxConfiguration.java +++ b/services/core/java/com/android/server/wm/LetterboxConfiguration.java @@ -327,14 +327,14 @@ final class LetterboxConfiguration { R.dimen.config_letterboxBackgroundWallpaperBlurRadius); mLetterboxBackgroundWallpaperDarkScrimAlpha = mContext.getResources().getFloat( R.dimen.config_letterboxBackgroundWallaperDarkScrimAlpha); - mLetterboxHorizontalPositionMultiplier = mContext.getResources().getFloat( - R.dimen.config_letterboxHorizontalPositionMultiplier); - mLetterboxVerticalPositionMultiplier = mContext.getResources().getFloat( - R.dimen.config_letterboxVerticalPositionMultiplier); - mLetterboxBookModePositionMultiplier = mContext.getResources().getFloat( - R.dimen.config_letterboxBookModePositionMultiplier); - mLetterboxTabletopModePositionMultiplier = mContext.getResources().getFloat( - R.dimen.config_letterboxTabletopModePositionMultiplier); + setLetterboxHorizontalPositionMultiplier(mContext.getResources().getFloat( + R.dimen.config_letterboxHorizontalPositionMultiplier)); + setLetterboxVerticalPositionMultiplier(mContext.getResources().getFloat( + R.dimen.config_letterboxVerticalPositionMultiplier)); + setLetterboxBookModePositionMultiplier(mContext.getResources().getFloat( + R.dimen.config_letterboxBookModePositionMultiplier)); + setLetterboxTabletopModePositionMultiplier(mContext.getResources() + .getFloat(R.dimen.config_letterboxTabletopModePositionMultiplier)); mIsHorizontalReachabilityEnabled = mContext.getResources().getBoolean( R.bool.config_letterboxIsHorizontalReachabilityEnabled); mIsVerticalReachabilityEnabled = mContext.getResources().getBoolean( @@ -657,29 +657,8 @@ final class LetterboxConfiguration { * right side. */ float getLetterboxHorizontalPositionMultiplier(boolean isInBookMode) { - if (isInBookMode) { - if (mLetterboxBookModePositionMultiplier < 0.0f - || mLetterboxBookModePositionMultiplier > 1.0f) { - Slog.w(TAG, - "mLetterboxBookModePositionMultiplier out of bounds (isInBookMode=true): " - + mLetterboxBookModePositionMultiplier); - // Default to left position if invalid value is provided. - return 0.0f; - } else { - return mLetterboxBookModePositionMultiplier; - } - } else { - if (mLetterboxHorizontalPositionMultiplier < 0.0f - || mLetterboxHorizontalPositionMultiplier > 1.0f) { - Slog.w(TAG, - "mLetterboxBookModePositionMultiplier out of bounds (isInBookMode=false):" - + mLetterboxBookModePositionMultiplier); - // Default to central position if invalid value is provided. - return 0.5f; - } else { - return mLetterboxHorizontalPositionMultiplier; - } - } + return isInBookMode ? mLetterboxBookModePositionMultiplier + : mLetterboxHorizontalPositionMultiplier; } /* @@ -689,37 +668,28 @@ final class LetterboxConfiguration { * bottom side. */ float getLetterboxVerticalPositionMultiplier(boolean isInTabletopMode) { - if (isInTabletopMode) { - return (mLetterboxTabletopModePositionMultiplier < 0.0f - || mLetterboxTabletopModePositionMultiplier > 1.0f) - // Default to top position if invalid value is provided. - ? 0.0f : mLetterboxTabletopModePositionMultiplier; - } else { - return (mLetterboxVerticalPositionMultiplier < 0.0f - || mLetterboxVerticalPositionMultiplier > 1.0f) - // Default to central position if invalid value is provided. - ? 0.5f : mLetterboxVerticalPositionMultiplier; - } + return isInTabletopMode ? mLetterboxTabletopModePositionMultiplier + : mLetterboxVerticalPositionMultiplier; } /** - * Overrides horizontal position of a center of the letterboxed app window. If given value < 0 - * or > 1, then it and a value of {@link - * com.android.internal.R.dimen.config_letterboxHorizontalPositionMultiplier} are ignored and - * central position (0.5) is used. + * Overrides horizontal position of a center of the letterboxed app window. + * + * @throws IllegalArgumentException If given value < 0 or > 1. */ void setLetterboxHorizontalPositionMultiplier(float multiplier) { - mLetterboxHorizontalPositionMultiplier = multiplier; + mLetterboxHorizontalPositionMultiplier = assertValidMultiplier(multiplier, + "mLetterboxHorizontalPositionMultiplier"); } /** - * Overrides vertical position of a center of the letterboxed app window. If given value < 0 - * or > 1, then it and a value of {@link - * com.android.internal.R.dimen.config_letterboxVerticalPositionMultiplier} are ignored and - * central position (0.5) is used. + * Overrides vertical position of a center of the letterboxed app window. + * + * @throws IllegalArgumentException If given value < 0 or > 1. */ void setLetterboxVerticalPositionMultiplier(float multiplier) { - mLetterboxVerticalPositionMultiplier = multiplier; + mLetterboxVerticalPositionMultiplier = assertValidMultiplier(multiplier, + "mLetterboxVerticalPositionMultiplier"); } /** @@ -740,6 +710,28 @@ final class LetterboxConfiguration { com.android.internal.R.dimen.config_letterboxVerticalPositionMultiplier); } + /** + * Sets tabletop mode position multiplier. + * + * @throws IllegalArgumentException If given value < 0 or > 1. + */ + @VisibleForTesting + void setLetterboxTabletopModePositionMultiplier(float multiplier) { + mLetterboxTabletopModePositionMultiplier = assertValidMultiplier(multiplier, + "mLetterboxTabletopModePositionMultiplier"); + } + + /** + * Sets tabletop mode position multiplier. + * + * @throws IllegalArgumentException If given value < 0 or > 1. + */ + @VisibleForTesting + void setLetterboxBookModePositionMultiplier(float multiplier) { + mLetterboxBookModePositionMultiplier = assertValidMultiplier(multiplier, + "mLetterboxBookModePositionMultiplier"); + } + /* * Whether horizontal reachability repositioning is allowed for letterboxed fullscreen apps in * landscape device orientation. @@ -1356,4 +1348,21 @@ final class LetterboxConfiguration { void resetUserAppAspectRatioFullscreenEnabled() { setUserAppAspectRatioFullscreenOverrideEnabled(false); } + + /** + * Checks whether the multiplier is between [0,1]. + * + * @param multiplierName sent in the exception if multiplier is invalid, for easier debugging. + * + * @return multiplier, if valid + * @throws IllegalArgumentException if outside bounds. + */ + private float assertValidMultiplier(float multiplier, String multiplierName) + throws IllegalArgumentException { + if (multiplier < 0.0f || multiplier > 1.0f) { + throw new IllegalArgumentException("Trying to set " + multiplierName + + " out of bounds: " + multiplier); + } + return multiplier; + } } diff --git a/services/core/java/com/android/server/wm/WindowManagerShellCommand.java b/services/core/java/com/android/server/wm/WindowManagerShellCommand.java index a6db310f4e63..e877d17fddb4 100644 --- a/services/core/java/com/android/server/wm/WindowManagerShellCommand.java +++ b/services/core/java/com/android/server/wm/WindowManagerShellCommand.java @@ -842,7 +842,12 @@ public class WindowManagerShellCommand extends ShellCommand { return -1; } synchronized (mInternal.mGlobalLock) { - mLetterboxConfiguration.setLetterboxHorizontalPositionMultiplier(multiplier); + try { + mLetterboxConfiguration.setLetterboxHorizontalPositionMultiplier(multiplier); + } catch (IllegalArgumentException e) { + getErrPrintWriter().println("Error: invalid multiplier value " + e); + return -1; + } } return 0; } @@ -861,7 +866,12 @@ public class WindowManagerShellCommand extends ShellCommand { return -1; } synchronized (mInternal.mGlobalLock) { - mLetterboxConfiguration.setLetterboxVerticalPositionMultiplier(multiplier); + try { + mLetterboxConfiguration.setLetterboxVerticalPositionMultiplier(multiplier); + } catch (IllegalArgumentException e) { + getErrPrintWriter().println("Error: invalid multiplier value " + e); + return -1; + } } return 0; } diff --git a/services/tests/servicestests/src/com/android/server/audio/AudioDeviceBrokerTest.java b/services/tests/servicestests/src/com/android/server/audio/AudioDeviceBrokerTest.java index f1c1dc365b90..59f4d56b44f1 100644 --- a/services/tests/servicestests/src/com/android/server/audio/AudioDeviceBrokerTest.java +++ b/services/tests/servicestests/src/com/android/server/audio/AudioDeviceBrokerTest.java @@ -24,6 +24,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.any; @@ -315,7 +316,7 @@ public class AudioDeviceBrokerTest { mFakeBtDevice.getAddress())); verify(mMockAudioService, timeout(MAX_MESSAGE_HANDLING_DELAY_MS).times(0)).onUpdatedAdiDeviceState( - eq(devState)); + eq(devState), anyBoolean()); } // metadata set @@ -326,7 +327,7 @@ public class AudioDeviceBrokerTest { mFakeBtDevice.getAddress())); verify(mMockAudioService, timeout(MAX_MESSAGE_HANDLING_DELAY_MS)).onUpdatedAdiDeviceState( - any()); + any(), anyBoolean()); } } finally { // reset the metadata device type @@ -354,7 +355,7 @@ public class AudioDeviceBrokerTest { verify(mMockAudioService, timeout(MAX_MESSAGE_HANDLING_DELAY_MS).atLeast(1)).onUpdatedAdiDeviceState( ArgumentMatchers.argThat(devState -> devState.getAudioDeviceCategory() - == AudioManager.AUDIO_DEVICE_CATEGORY_OTHER)); + == AudioManager.AUDIO_DEVICE_CATEGORY_OTHER), anyBoolean()); } private void doTestConnectionDisconnectionReconnection(int delayAfterDisconnection, diff --git a/services/tests/wmtests/src/com/android/server/wm/LetterboxConfigurationTest.java b/services/tests/wmtests/src/com/android/server/wm/LetterboxConfigurationTest.java index 80e169d8d579..b90fa21cb2b1 100644 --- a/services/tests/wmtests/src/com/android/server/wm/LetterboxConfigurationTest.java +++ b/services/tests/wmtests/src/com/android/server/wm/LetterboxConfigurationTest.java @@ -25,6 +25,8 @@ import static com.android.server.wm.LetterboxConfiguration.LETTERBOX_VERTICAL_RE import static com.android.server.wm.LetterboxConfiguration.LETTERBOX_VERTICAL_REACHABILITY_POSITION_CENTER; import static com.android.server.wm.LetterboxConfiguration.LETTERBOX_VERTICAL_REACHABILITY_POSITION_TOP; +import static com.android.server.wm.testing.Assert.assertThrows; + import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; @@ -37,6 +39,8 @@ import android.platform.test.annotations.Presubmit; import androidx.test.filters.SmallTest; +import com.android.server.wm.testing.Assert; + import org.junit.Before; import org.junit.Test; @@ -288,4 +292,56 @@ public class LetterboxConfigurationTest { false /* forTabletopMode */, LETTERBOX_VERTICAL_REACHABILITY_POSITION_TOP); } + + @Test + public void test_setLetterboxHorizontalPositionMultiplier_validValues() { + assertThrows(IllegalArgumentException.class, + () -> mLetterboxConfiguration.setLetterboxHorizontalPositionMultiplier(-1)); + assertThrows(IllegalArgumentException.class, + () -> mLetterboxConfiguration.setLetterboxHorizontalPositionMultiplier(2)); + + // Does not throw an exception for values [0,1]. + mLetterboxConfiguration.setLetterboxHorizontalPositionMultiplier(0); + mLetterboxConfiguration.setLetterboxHorizontalPositionMultiplier(0.5f); + mLetterboxConfiguration.setLetterboxHorizontalPositionMultiplier(1); + } + + @Test + public void test_setLetterboxVerticalPositionMultiplier_validValues() { + assertThrows(IllegalArgumentException.class, + () -> mLetterboxConfiguration.setLetterboxVerticalPositionMultiplier(-1)); + assertThrows(IllegalArgumentException.class, + () -> mLetterboxConfiguration.setLetterboxVerticalPositionMultiplier(2)); + + // Does not throw an exception for values [0,1]. + mLetterboxConfiguration.setLetterboxVerticalPositionMultiplier(0); + mLetterboxConfiguration.setLetterboxVerticalPositionMultiplier(0.5f); + mLetterboxConfiguration.setLetterboxVerticalPositionMultiplier(1); + } + + @Test + public void test_setLetterboxBookModePositionMultiplier_validValues() { + assertThrows(IllegalArgumentException.class, + () -> mLetterboxConfiguration.setLetterboxBookModePositionMultiplier(-1)); + assertThrows(IllegalArgumentException.class, + () -> mLetterboxConfiguration.setLetterboxBookModePositionMultiplier(2)); + + // Does not throw an exception for values [0,1]. + mLetterboxConfiguration.setLetterboxBookModePositionMultiplier(0); + mLetterboxConfiguration.setLetterboxBookModePositionMultiplier(0.5f); + mLetterboxConfiguration.setLetterboxBookModePositionMultiplier(1); + } + + @Test + public void test_setLetterboxTabletopModePositionMultiplier_validValues() { + assertThrows(IllegalArgumentException.class, + () -> mLetterboxConfiguration.setLetterboxTabletopModePositionMultiplier(-1)); + assertThrows(IllegalArgumentException.class, + () -> mLetterboxConfiguration.setLetterboxTabletopModePositionMultiplier(2)); + + // Does not throw an exception for values [0,1]. + mLetterboxConfiguration.setLetterboxTabletopModePositionMultiplier(0); + mLetterboxConfiguration.setLetterboxTabletopModePositionMultiplier(0.5f); + mLetterboxConfiguration.setLetterboxTabletopModePositionMultiplier(1); + } } diff --git a/services/tests/wmtests/src/com/android/server/wm/SizeCompatTests.java b/services/tests/wmtests/src/com/android/server/wm/SizeCompatTests.java index faa36a8f6479..dbc54b2b4cf4 100644 --- a/services/tests/wmtests/src/com/android/server/wm/SizeCompatTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/SizeCompatTests.java @@ -4101,31 +4101,6 @@ public class SizeCompatTests extends WindowTestsBase { } @Test - public void testUpdateResolvedBoundsHorizontalPosition_invalidMultiplier_defaultToCenter() { - // Display configured as (2800, 1400). - - // Below 0.0. - assertHorizontalPositionForDifferentDisplayConfigsForPortraitActivity( - /* letterboxHorizontalPositionMultiplier */ -1.0f, - // At launch. - /* fixedOrientationLetterbox */ new Rect(1050, 0, 1750, 1400), - // After 90 degree rotation. - /* sizeCompatUnscaled */ new Rect(350, 0, 1050, 1400), - // After the display is resized to (700, 1400). - /* sizeCompatScaled */ new Rect(525, 0, 875, 700)); - - // Above 1.0 - assertHorizontalPositionForDifferentDisplayConfigsForPortraitActivity( - /* letterboxHorizontalPositionMultiplier */ 2.0f, - // At launch. - /* fixedOrientationLetterbox */ new Rect(1050, 0, 1750, 1400), - // After 90 degree rotation. - /* sizeCompatUnscaled */ new Rect(350, 0, 1050, 1400), - // After the display is resized to (700, 1400). - /* sizeCompatScaled */ new Rect(525, 0, 875, 700)); - } - - @Test public void testUpdateResolvedBoundsHorizontalPosition_right() { // Display configured as (2800, 1400). assertHorizontalPositionForDifferentDisplayConfigsForPortraitActivity( @@ -4331,31 +4306,6 @@ public class SizeCompatTests extends WindowTestsBase { } @Test - public void testUpdateResolvedBoundsVerticalPosition_invalidMultiplier_defaultToCenter() { - // Display configured as (1400, 2800). - - // Below 0.0. - assertVerticalPositionForDifferentDisplayConfigsForLandscapeActivity( - /* letterboxVerticalPositionMultiplier */ -1.0f, - // At launch. - /* fixedOrientationLetterbox */ new Rect(0, 1050, 1400, 1750), - // After 90 degree rotation. - /* sizeCompatUnscaled */ new Rect(700, 350, 2100, 1050), - // After the display is resized to (1400, 700). - /* sizeCompatScaled */ new Rect(0, 525, 700, 875)); - - // Above 1.0 - assertVerticalPositionForDifferentDisplayConfigsForLandscapeActivity( - /* letterboxVerticalPositionMultiplier */ 2.0f, - // At launch. - /* fixedOrientationLetterbox */ new Rect(0, 1050, 1400, 1750), - // After 90 degree rotation. - /* sizeCompatUnscaled */ new Rect(700, 350, 2100, 1050), - // After the display is resized to (1400, 700). - /* sizeCompatScaled */ new Rect(0, 525, 700, 875)); - } - - @Test public void testUpdateResolvedBoundsVerticalPosition_bottom() { // Display configured as (1400, 2800). assertVerticalPositionForDifferentDisplayConfigsForLandscapeActivity( |