From a606c18684ff0128db15dcfde984a0abd3c0bab6 Mon Sep 17 00:00:00 2001 From: Marcello Galhardo Date: Wed, 1 Mar 2023 17:16:49 +0000 Subject: [PATCH] Use current user to launch note task It retrieves the system user using the UserTracker, so the NoteTask is associated with the current logged in user or system user if locked out (b/267634412). Additionally, clean-up the code by removing the unnecessary optional parameters (b/269724992) and adds the missing flags for opening note task correctly (b/271280243). Test: atest SystemUITests:NoteTaskControllerTest Fixes: b/267634412 Fixes: b/269724992 Fixes: b/271280243 Change-Id: Id6e1261dac7855c35c6d4a00e362d84709816339 --- .../systemui/notetask/NoteTaskController.kt | 29 ++++++--- .../systemui/notetask/NoteTaskInfoResolver.kt | 6 +- .../systemui/notetask/NoteTaskModule.kt | 15 ----- .../notetask/NoteTaskControllerTest.kt | 64 +++++++++---------- .../notetask/NoteTaskInfoResolverTest.kt | 5 +- 5 files changed, 57 insertions(+), 62 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/notetask/NoteTaskController.kt b/packages/SystemUI/src/com/android/systemui/notetask/NoteTaskController.kt index c65f0aaab91f1..5b36e93ec5e1a 100644 --- a/packages/SystemUI/src/com/android/systemui/notetask/NoteTaskController.kt +++ b/packages/SystemUI/src/com/android/systemui/notetask/NoteTaskController.kt @@ -22,6 +22,9 @@ import android.content.ActivityNotFoundException import android.content.ComponentName import android.content.Context import android.content.Intent +import android.content.Intent.FLAG_ACTIVITY_MULTIPLE_TASK +import android.content.Intent.FLAG_ACTIVITY_NEW_DOCUMENT +import android.content.Intent.FLAG_ACTIVITY_NEW_TASK import android.content.pm.PackageManager import android.os.Build import android.os.UserManager @@ -54,8 +57,8 @@ constructor( private val resolver: NoteTaskInfoResolver, private val eventLogger: NoteTaskEventLogger, private val optionalBubbles: Optional, - private val optionalUserManager: Optional, - private val optionalKeyguardManager: Optional, + private val userManager: UserManager, + private val keyguardManager: KeyguardManager, @NoteTaskEnabledKey private val isEnabled: Boolean, private val devicePolicyManager: DevicePolicyManager, private val userTracker: UserTracker, @@ -106,8 +109,6 @@ constructor( if (!isEnabled) return val bubbles = optionalBubbles.getOrNull() ?: return - val userManager = optionalUserManager.getOrNull() ?: return - val keyguardManager = optionalKeyguardManager.getOrNull() ?: return // TODO(b/249954038): We should handle direct boot (isUserUnlocked). For now, we do nothing. if (!userManager.isUserUnlocked) return @@ -140,12 +141,13 @@ constructor( logDebug { "onShowNoteTask - start: $info" } when (info.launchMode) { is NoteTaskLaunchMode.AppBubble -> { + // TODO(b/267634412, b/268351693): Should use `showOrHideAppBubbleAsUser` bubbles.showOrHideAppBubble(intent) // App bubble logging happens on `onBubbleExpandChanged`. logDebug { "onShowNoteTask - opened as app bubble: $info" } } is NoteTaskLaunchMode.Activity -> { - context.startActivity(intent) + context.startActivityAsUser(intent, userTracker.userHandle) eventLogger.logNoteTaskOpened(info) logDebug { "onShowNoteTask - opened as activity: $info" } } @@ -198,12 +200,21 @@ constructor( } private fun createNoteIntent(info: NoteTaskInfo): Intent = - Intent(NoteTaskController.ACTION_CREATE_NOTE) - .setPackage(info.packageName) - .addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) + Intent(NoteTaskController.ACTION_CREATE_NOTE).apply { + setPackage(info.packageName) + // EXTRA_USE_STYLUS_MODE does not mean a stylus is in-use, but a stylus entrypoint // was used to start it. - .putExtra(NoteTaskController.INTENT_EXTRA_USE_STYLUS_MODE, true) + putExtra(NoteTaskController.INTENT_EXTRA_USE_STYLUS_MODE, true) + + addFlags(FLAG_ACTIVITY_NEW_TASK) + // We should ensure the note experience can be open both as a full screen (lock screen) + // and inside the app bubble (contextual). These additional flags will do that. + if (info.launchMode == NoteTaskLaunchMode.Activity) { + addFlags(FLAG_ACTIVITY_MULTIPLE_TASK) + addFlags(FLAG_ACTIVITY_NEW_DOCUMENT) + } + } private inline fun logDebug(message: () -> String) { if (Build.IS_DEBUGGABLE) { diff --git a/packages/SystemUI/src/com/android/systemui/notetask/NoteTaskInfoResolver.kt b/packages/SystemUI/src/com/android/systemui/notetask/NoteTaskInfoResolver.kt index 0f75f9591074e..7be491f0b3a62 100644 --- a/packages/SystemUI/src/com/android/systemui/notetask/NoteTaskInfoResolver.kt +++ b/packages/SystemUI/src/com/android/systemui/notetask/NoteTaskInfoResolver.kt @@ -17,19 +17,19 @@ package com.android.systemui.notetask import android.app.role.RoleManager -import android.content.Context import android.content.pm.PackageManager import android.content.pm.PackageManager.ApplicationInfoFlags import android.os.UserHandle import android.util.Log +import com.android.systemui.settings.UserTracker import javax.inject.Inject class NoteTaskInfoResolver @Inject constructor( - private val context: Context, private val roleManager: RoleManager, private val packageManager: PackageManager, + private val userTracker: UserTracker, ) { fun resolveInfo( @@ -38,7 +38,7 @@ constructor( isKeyguardLocked: Boolean = false, ): NoteTaskInfo? { // TODO(b/267634412): Select UserHandle depending on where the user initiated note-taking. - val user = context.user + val user = userTracker.userHandle val packageName = roleManager.getRoleHoldersAsUser(RoleManager.ROLE_NOTES, user).firstOrNull() diff --git a/packages/SystemUI/src/com/android/systemui/notetask/NoteTaskModule.kt b/packages/SystemUI/src/com/android/systemui/notetask/NoteTaskModule.kt index ba8999c068e35..6278c699498c9 100644 --- a/packages/SystemUI/src/com/android/systemui/notetask/NoteTaskModule.kt +++ b/packages/SystemUI/src/com/android/systemui/notetask/NoteTaskModule.kt @@ -17,11 +17,7 @@ package com.android.systemui.notetask import android.app.Activity -import android.app.KeyguardManager import android.app.role.RoleManager -import android.content.Context -import android.os.UserManager -import androidx.core.content.getSystemService import com.android.systemui.flags.FeatureFlags import com.android.systemui.flags.Flags import com.android.systemui.notetask.quickaffordance.NoteTaskQuickAffordanceModule @@ -32,7 +28,6 @@ import dagger.Module import dagger.Provides import dagger.multibindings.ClassKey import dagger.multibindings.IntoMap -import java.util.Optional /** Compose all dependencies required by Note Task feature. */ @Module(includes = [NoteTaskQuickAffordanceModule::class]) @@ -55,15 +50,5 @@ interface NoteTaskModule { val isFeatureEnabled = featureFlags.isEnabled(Flags.NOTE_TASKS) return isRoleAvailable && isFeatureEnabled } - - @Provides - fun provideOptionalKeyguardManager(context: Context): Optional { - return Optional.ofNullable(context.getSystemService()) - } - - @Provides - fun provideOptionalUserManager(context: Context): Optional { - return Optional.ofNullable(context.getSystemService()) - } } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/notetask/NoteTaskControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/notetask/NoteTaskControllerTest.kt index 376b7cc701500..4efc30f0aee29 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/notetask/NoteTaskControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/notetask/NoteTaskControllerTest.kt @@ -20,12 +20,17 @@ import android.app.admin.DevicePolicyManager import android.content.ComponentName import android.content.Context import android.content.Intent +import android.content.Intent.FLAG_ACTIVITY_MULTIPLE_TASK +import android.content.Intent.FLAG_ACTIVITY_NEW_DOCUMENT +import android.content.Intent.FLAG_ACTIVITY_NEW_TASK import android.content.pm.PackageManager +import android.os.UserHandle import android.os.UserManager import androidx.test.filters.SmallTest import androidx.test.runner.AndroidJUnit4 import com.android.systemui.SysuiTestCase import com.android.systemui.notetask.shortcut.CreateNoteTaskShortcutActivity +import com.android.systemui.settings.FakeUserTracker import com.android.systemui.settings.UserTracker import com.android.systemui.util.mockito.any import com.android.systemui.util.mockito.argumentCaptor @@ -57,8 +62,8 @@ internal class NoteTaskControllerTest : SysuiTestCase() { @Mock lateinit var keyguardManager: KeyguardManager @Mock lateinit var userManager: UserManager @Mock lateinit var eventLogger: NoteTaskEventLogger - @Mock private lateinit var userTracker: UserTracker @Mock private lateinit var devicePolicyManager: DevicePolicyManager + private val userTracker: UserTracker = FakeUserTracker() private val noteTaskInfo = NoteTaskInfo(packageName = NOTES_PACKAGE_NAME, uid = NOTES_UID) @@ -81,16 +86,14 @@ internal class NoteTaskControllerTest : SysuiTestCase() { private fun createNoteTaskController( isEnabled: Boolean = true, bubbles: Bubbles? = this.bubbles, - keyguardManager: KeyguardManager? = this.keyguardManager, - userManager: UserManager? = this.userManager, ): NoteTaskController = NoteTaskController( context = context, resolver = resolver, eventLogger = eventLogger, optionalBubbles = Optional.ofNullable(bubbles), - optionalUserManager = Optional.ofNullable(userManager), - optionalKeyguardManager = Optional.ofNullable(keyguardManager), + userManager = userManager, + keyguardManager = keyguardManager, isEnabled = isEnabled, devicePolicyManager = devicePolicyManager, userTracker = userTracker, @@ -225,13 +228,19 @@ internal class NoteTaskControllerTest : SysuiTestCase() { ) val intentCaptor = argumentCaptor() - verify(context).startActivity(capture(intentCaptor)) + val userCaptor = argumentCaptor() + verify(context).startActivityAsUser(capture(intentCaptor), capture(userCaptor)) intentCaptor.value.let { intent -> assertThat(intent.action).isEqualTo(Intent.ACTION_CREATE_NOTE) assertThat(intent.`package`).isEqualTo(NOTES_PACKAGE_NAME) - assertThat(intent.flags).isEqualTo(Intent.FLAG_ACTIVITY_NEW_TASK) + assertThat(intent.flags and FLAG_ACTIVITY_NEW_TASK).isEqualTo(FLAG_ACTIVITY_NEW_TASK) + assertThat(intent.flags and FLAG_ACTIVITY_MULTIPLE_TASK) + .isEqualTo(FLAG_ACTIVITY_MULTIPLE_TASK) + assertThat(intent.flags and FLAG_ACTIVITY_NEW_DOCUMENT) + .isEqualTo(FLAG_ACTIVITY_NEW_DOCUMENT) assertThat(intent.getBooleanExtra(Intent.EXTRA_USE_STYLUS_MODE, false)).isTrue() } + assertThat(userCaptor.value).isEqualTo(userTracker.userHandle) verify(eventLogger).logNoteTaskOpened(expectedInfo) verifyZeroInteractions(bubbles) } @@ -259,7 +268,7 @@ internal class NoteTaskControllerTest : SysuiTestCase() { intentCaptor.value.let { intent -> assertThat(intent.action).isEqualTo(Intent.ACTION_CREATE_NOTE) assertThat(intent.`package`).isEqualTo(NOTES_PACKAGE_NAME) - assertThat(intent.flags).isEqualTo(Intent.FLAG_ACTIVITY_NEW_TASK) + assertThat(intent.flags).isEqualTo(FLAG_ACTIVITY_NEW_TASK) assertThat(intent.getBooleanExtra(Intent.EXTRA_USE_STYLUS_MODE, false)).isTrue() } verifyZeroInteractions(eventLogger) @@ -283,13 +292,22 @@ internal class NoteTaskControllerTest : SysuiTestCase() { ) val intentCaptor = argumentCaptor() - verify(context).startActivity(capture(intentCaptor)) + val userCaptor = argumentCaptor() + verify(context).startActivityAsUser(capture(intentCaptor), capture(userCaptor)) + + (intentCaptor.value.flags and FLAG_ACTIVITY_NEW_TASK) == FLAG_ACTIVITY_NEW_TASK + intentCaptor.value.let { intent -> assertThat(intent.action).isEqualTo(Intent.ACTION_CREATE_NOTE) assertThat(intent.`package`).isEqualTo(NOTES_PACKAGE_NAME) - assertThat(intent.flags).isEqualTo(Intent.FLAG_ACTIVITY_NEW_TASK) + assertThat(intent.flags and FLAG_ACTIVITY_NEW_TASK).isEqualTo(FLAG_ACTIVITY_NEW_TASK) + assertThat(intent.flags and FLAG_ACTIVITY_MULTIPLE_TASK) + .isEqualTo(FLAG_ACTIVITY_MULTIPLE_TASK) + assertThat(intent.flags and FLAG_ACTIVITY_NEW_DOCUMENT) + .isEqualTo(FLAG_ACTIVITY_NEW_DOCUMENT) assertThat(intent.getBooleanExtra(Intent.EXTRA_USE_STYLUS_MODE, false)).isTrue() } + assertThat(userCaptor.value).isEqualTo(userTracker.userHandle) verify(eventLogger).logNoteTaskOpened(expectedInfo) verifyZeroInteractions(bubbles) } @@ -305,28 +323,6 @@ internal class NoteTaskControllerTest : SysuiTestCase() { verifyZeroInteractions(context, bubbles, eventLogger) } - @Test - fun showNoteTask_keyguardManagerIsNull_shouldDoNothing() { - createNoteTaskController(keyguardManager = null) - .showNoteTask( - entryPoint = NoteTaskEntryPoint.TAIL_BUTTON, - isInMultiWindowMode = false, - ) - - verifyZeroInteractions(context, bubbles, eventLogger) - } - - @Test - fun showNoteTask_userManagerIsNull_shouldDoNothing() { - createNoteTaskController(userManager = null) - .showNoteTask( - entryPoint = NoteTaskEntryPoint.TAIL_BUTTON, - isInMultiWindowMode = false, - ) - - verifyZeroInteractions(context, bubbles, eventLogger) - } - @Test fun showNoteTask_intentResolverReturnsNull_shouldDoNothing() { whenever(resolver.resolveInfo(any(), any(), any())).thenReturn(null) @@ -460,7 +456,7 @@ internal class NoteTaskControllerTest : SysuiTestCase() { intentCaptor.value.let { intent -> assertThat(intent.action).isEqualTo(NoteTaskController.ACTION_CREATE_NOTE) assertThat(intent.`package`).isEqualTo(NOTES_PACKAGE_NAME) - assertThat(intent.flags).isEqualTo(Intent.FLAG_ACTIVITY_NEW_TASK) + assertThat(intent.flags).isEqualTo(FLAG_ACTIVITY_NEW_TASK) assertThat(intent.getBooleanExtra(Intent.EXTRA_USE_STYLUS_MODE, false)).isTrue() } } @@ -487,7 +483,7 @@ internal class NoteTaskControllerTest : SysuiTestCase() { intentCaptor.value.let { intent -> assertThat(intent.action).isEqualTo(NoteTaskController.ACTION_CREATE_NOTE) assertThat(intent.`package`).isEqualTo(NOTES_PACKAGE_NAME) - assertThat(intent.flags).isEqualTo(Intent.FLAG_ACTIVITY_NEW_TASK) + assertThat(intent.flags).isEqualTo(FLAG_ACTIVITY_NEW_TASK) assertThat(intent.getBooleanExtra(Intent.EXTRA_USE_STYLUS_MODE, false)).isTrue() } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/notetask/NoteTaskInfoResolverTest.kt b/packages/SystemUI/tests/src/com/android/systemui/notetask/NoteTaskInfoResolverTest.kt index 7f64f8a123e01..0c945dfa4b4c2 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/notetask/NoteTaskInfoResolverTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/notetask/NoteTaskInfoResolverTest.kt @@ -22,6 +22,8 @@ import android.content.pm.PackageManager import android.test.suitebuilder.annotation.SmallTest import androidx.test.runner.AndroidJUnit4 import com.android.systemui.SysuiTestCase +import com.android.systemui.settings.FakeUserTracker +import com.android.systemui.settings.UserTracker import com.android.systemui.util.mockito.eq import com.android.systemui.util.mockito.whenever import com.google.common.truth.Truth.assertThat @@ -44,13 +46,14 @@ internal class NoteTaskInfoResolverTest : SysuiTestCase() { @Mock lateinit var packageManager: PackageManager @Mock lateinit var roleManager: RoleManager + private val userTracker: UserTracker = FakeUserTracker() private lateinit var underTest: NoteTaskInfoResolver @Before fun setUp() { MockitoAnnotations.initMocks(this) - underTest = NoteTaskInfoResolver(context, roleManager, packageManager) + underTest = NoteTaskInfoResolver(roleManager, packageManager, userTracker) } @Test