diff options
4 files changed, 160 insertions, 77 deletions
diff --git a/aconfig/FeatureFlags.aconfig b/aconfig/FeatureFlags.aconfig index c23b51ae..e2b2f57b 100644 --- a/aconfig/FeatureFlags.aconfig +++ b/aconfig/FeatureFlags.aconfig @@ -117,6 +117,16 @@ flag { } flag { + name: "rebuild_adapters_on_target_pinning" + namespace: "intentresolver" + description: "Rebuild and swap adapters when a target gets (un)pinned to avoid flickering." + bug: "230703572" + metadata { + purpose: PURPOSE_BUGFIX + } +} + +flag { name: "target_hover_and_keyboard_focus_states" namespace: "intentresolver" description: "Adopt Launcher pointer hover and keyboard novigation focus effects for targets." diff --git a/java/src/com/android/intentresolver/ChooserActivity.java b/java/src/com/android/intentresolver/ChooserActivity.java index 250edaf2..f7cc9291 100644 --- a/java/src/com/android/intentresolver/ChooserActivity.java +++ b/java/src/com/android/intentresolver/ChooserActivity.java @@ -25,6 +25,7 @@ import static androidx.lifecycle.LifecycleKt.getCoroutineScope; import static com.android.intentresolver.ChooserActionFactory.EDIT_SOURCE; import static com.android.intentresolver.Flags.fixShortcutsFlashing; import static com.android.intentresolver.Flags.keyboardNavigationFix; +import static com.android.intentresolver.Flags.rebuildAdaptersOnTargetPinning; import static com.android.intentresolver.Flags.shareouselUpdateExcludeComponentsExtra; import static com.android.intentresolver.Flags.unselectFinalItem; import static com.android.intentresolver.ext.CreationExtrasExtKt.replaceDefaultArgs; @@ -1545,10 +1546,14 @@ public class ChooserActivity extends Hilt_ChooserActivity implements private void handlePackagesChanged(@Nullable ResolverListAdapter listAdapter) { // Refresh pinned items mPinnedSharedPrefs = getPinnedSharedPrefs(this); - if (listAdapter == null) { - mChooserMultiProfilePagerAdapter.refreshPackagesInAllTabs(); + if (rebuildAdaptersOnTargetPinning()) { + recreatePagerAdapter(); } else { - listAdapter.handlePackagesChanged(); + if (listAdapter == null) { + mChooserMultiProfilePagerAdapter.refreshPackagesInAllTabs(); + } else { + listAdapter.handlePackagesChanged(); + } } } diff --git a/java/src/com/android/intentresolver/ShortcutSelectionLogic.java b/java/src/com/android/intentresolver/ShortcutSelectionLogic.java index 2d5ec451..3a1a51e3 100644 --- a/java/src/com/android/intentresolver/ShortcutSelectionLogic.java +++ b/java/src/com/android/intentresolver/ShortcutSelectionLogic.java @@ -16,6 +16,8 @@ package com.android.intentresolver; +import static com.android.intentresolver.Flags.rebuildAdaptersOnTargetPinning; + import android.app.prediction.AppTarget; import android.content.Context; import android.content.Intent; @@ -171,16 +173,21 @@ public class ShortcutSelectionLogic { List<TargetInfo> serviceTargets) { // Check for duplicates and abort if found - for (TargetInfo otherTargetInfo : serviceTargets) { + for (int i = 0; i < serviceTargets.size(); i++) { + TargetInfo otherTargetInfo = serviceTargets.get(i); if (chooserTargetInfo.isSimilar(otherTargetInfo)) { + if (rebuildAdaptersOnTargetPinning() + && chooserTargetInfo.isPinned() != otherTargetInfo.isPinned()) { + serviceTargets.set(i, chooserTargetInfo); + return true; + } return false; } } int currentSize = serviceTargets.size(); final float newScore = chooserTargetInfo.getModifiedScore(); - for (int i = 0; i < Math.min(currentSize, maxRankedTargets); - i++) { + for (int i = 0; i < Math.min(currentSize, maxRankedTargets); i++) { final TargetInfo serviceTarget = serviceTargets.get(i); if (serviceTarget == null) { serviceTargets.set(i, chooserTargetInfo); diff --git a/tests/unit/src/com/android/intentresolver/ShortcutSelectionLogicTest.kt b/tests/unit/src/com/android/intentresolver/ShortcutSelectionLogicTest.kt index e26dffb8..d591d928 100644 --- a/tests/unit/src/com/android/intentresolver/ShortcutSelectionLogicTest.kt +++ b/tests/unit/src/com/android/intentresolver/ShortcutSelectionLogicTest.kt @@ -21,13 +21,18 @@ import android.content.Context import android.content.Intent import android.content.pm.ShortcutInfo import android.os.UserHandle +import android.platform.test.annotations.EnableFlags +import android.platform.test.flag.junit.SetFlagsRule import android.service.chooser.ChooserTarget import androidx.test.filters.SmallTest import androidx.test.platform.app.InstrumentationRegistry +import com.android.intentresolver.Flags.FLAG_REBUILD_ADAPTERS_ON_TARGET_PINNING import com.android.intentresolver.chooser.DisplayResolveInfo import com.android.intentresolver.chooser.TargetInfo -import org.junit.Assert.assertEquals -import org.junit.Assert.assertTrue +import com.google.common.truth.Correspondence +import com.google.common.truth.Truth.assertThat +import com.google.common.truth.Truth.assertWithMessage +import org.junit.Rule import org.junit.Test import org.mockito.kotlin.doReturn import org.mockito.kotlin.mock @@ -36,10 +41,12 @@ private const val PACKAGE_A = "package.a" private const val PACKAGE_B = "package.b" private const val CLASS_NAME = "./MainActivity" +private val PERSONAL_USER_HANDLE: UserHandle = + InstrumentationRegistry.getInstrumentation().targetContext.user + @SmallTest class ShortcutSelectionLogicTest { - private val PERSONAL_USER_HANDLE: UserHandle = - InstrumentationRegistry.getInstrumentation().getTargetContext().getUser() + @get:Rule val flagRule = SetFlagsRule() private val packageTargets = HashMap<String, Array<ChooserTarget>>().apply { @@ -57,6 +64,14 @@ class ShortcutSelectionLogicTest { this[pkg] = targets } } + private val targetInfoChooserTargetCorrespondence = + Correspondence.from<TargetInfo, ChooserTarget>( + { actual, expected -> + actual.chooserTargetComponentName == expected.componentName && + actual.displayLabel == expected.title + }, + "", + ) private val baseDisplayInfo = DisplayResolveInfo.newDisplayResolveInfo( @@ -64,7 +79,7 @@ class ShortcutSelectionLogicTest { ResolverDataProvider.createResolveInfo(3, 0, PERSONAL_USER_HANDLE), "label", "extended info", - Intent() + Intent(), ) private val otherBaseDisplayInfo = @@ -73,7 +88,7 @@ class ShortcutSelectionLogicTest { ResolverDataProvider.createResolveInfo(4, 0, PERSONAL_USER_HANDLE), "label 2", "extended info 2", - Intent() + Intent(), ) private operator fun Map<String, Array<ChooserTarget>>.get(pkg: String, idx: Int) = @@ -87,7 +102,7 @@ class ShortcutSelectionLogicTest { val testSubject = ShortcutSelectionLogic( /* maxShortcutTargetsPerApp = */ 1, - /* applySharingAppLimits = */ false + /* applySharingAppLimits = */ false, ) val isUpdated = @@ -102,15 +117,15 @@ class ShortcutSelectionLogicTest { /* targetIntent = */ mock(), /* refererFillInIntent = */ mock(), /* maxRankedTargets = */ 4, - /* serviceTargets = */ serviceResults + /* serviceTargets = */ serviceResults, ) - assertTrue("Updates are expected", isUpdated) - assertShortcutsInOrder( - listOf(sc2, sc1), - serviceResults, - "Two shortcuts are expected as we do not apply per-app shortcut limit" - ) + assertWithMessage("Updates are expected").that(isUpdated).isTrue() + assertWithMessage("Two shortcuts are expected as we do not apply per-app shortcut limit") + .that(serviceResults) + .comparingElementsUsing(targetInfoChooserTargetCorrespondence) + .containsExactly(sc2, sc1) + .inOrder() } @Test @@ -121,7 +136,7 @@ class ShortcutSelectionLogicTest { val testSubject = ShortcutSelectionLogic( /* maxShortcutTargetsPerApp = */ 1, - /* applySharingAppLimits = */ true + /* applySharingAppLimits = */ true, ) val isUpdated = @@ -136,15 +151,15 @@ class ShortcutSelectionLogicTest { /* targetIntent = */ mock(), /* refererFillInIntent = */ mock(), /* maxRankedTargets = */ 4, - /* serviceTargets = */ serviceResults + /* serviceTargets = */ serviceResults, ) - assertTrue("Updates are expected", isUpdated) - assertShortcutsInOrder( - listOf(sc2), - serviceResults, - "One shortcut is expected as we apply per-app shortcut limit" - ) + assertWithMessage("Updates are expected").that(isUpdated).isTrue() + assertWithMessage("One shortcut is expected as we apply per-app shortcut limit") + .that(serviceResults) + .comparingElementsUsing(targetInfoChooserTargetCorrespondence) + .containsExactly(sc2) + .inOrder() } @Test @@ -155,7 +170,7 @@ class ShortcutSelectionLogicTest { val testSubject = ShortcutSelectionLogic( /* maxShortcutTargetsPerApp = */ 1, - /* applySharingAppLimits = */ false + /* applySharingAppLimits = */ false, ) val isUpdated = @@ -170,15 +185,15 @@ class ShortcutSelectionLogicTest { /* targetIntent = */ mock(), /* refererFillInIntent = */ mock(), /* maxRankedTargets = */ 1, - /* serviceTargets = */ serviceResults + /* serviceTargets = */ serviceResults, ) - assertTrue("Updates are expected", isUpdated) - assertShortcutsInOrder( - listOf(sc2), - serviceResults, - "One shortcut is expected as we apply overall shortcut limit" - ) + assertWithMessage("Updates are expected").that(isUpdated).isTrue() + assertWithMessage("One shortcut is expected as we apply overall shortcut limit") + .that(serviceResults) + .comparingElementsUsing(targetInfoChooserTargetCorrespondence) + .containsExactly(sc2) + .inOrder() } @Test @@ -191,7 +206,7 @@ class ShortcutSelectionLogicTest { val testSubject = ShortcutSelectionLogic( /* maxShortcutTargetsPerApp = */ 1, - /* applySharingAppLimits = */ true + /* applySharingAppLimits = */ true, ) testSubject.addServiceResults( @@ -205,7 +220,7 @@ class ShortcutSelectionLogicTest { /* targetIntent = */ mock(), /* refererFillInIntent = */ mock(), /* maxRankedTargets = */ 4, - /* serviceTargets = */ serviceResults + /* serviceTargets = */ serviceResults, ) testSubject.addServiceResults( /* origTarget = */ otherBaseDisplayInfo, @@ -218,14 +233,14 @@ class ShortcutSelectionLogicTest { /* targetIntent = */ mock(), /* refererFillInIntent = */ mock(), /* maxRankedTargets = */ 4, - /* serviceTargets = */ serviceResults + /* serviceTargets = */ serviceResults, ) - assertShortcutsInOrder( - listOf(pkgBsc2, pkgAsc2), - serviceResults, - "Two shortcuts are expected as we apply per-app shortcut limit" - ) + assertWithMessage("Two shortcuts are expected as we apply per-app shortcut limit") + .that(serviceResults) + .comparingElementsUsing(targetInfoChooserTargetCorrespondence) + .containsExactly(pkgBsc2, pkgAsc2) + .inOrder() } @Test @@ -236,7 +251,7 @@ class ShortcutSelectionLogicTest { val testSubject = ShortcutSelectionLogic( /* maxShortcutTargetsPerApp = */ 1, - /* applySharingAppLimits = */ false + /* applySharingAppLimits = */ false, ) val isUpdated = @@ -256,15 +271,15 @@ class ShortcutSelectionLogicTest { /* targetIntent = */ mock(), /* refererFillInIntent = */ mock(), /* maxRankedTargets = */ 4, - /* serviceTargets = */ serviceResults + /* serviceTargets = */ serviceResults, ) - assertTrue("Updates are expected", isUpdated) - assertShortcutsInOrder( - listOf(sc1, sc2), - serviceResults, - "Two shortcuts are expected as we do not apply per-app shortcut limit" - ) + assertWithMessage("Updates are expected").that(isUpdated).isTrue() + assertWithMessage("Two shortcuts are expected as we do not apply per-app shortcut limit") + .that(serviceResults) + .comparingElementsUsing(targetInfoChooserTargetCorrespondence) + .containsExactly(sc1, sc2) + .inOrder() } @Test @@ -276,7 +291,7 @@ class ShortcutSelectionLogicTest { val testSubject = ShortcutSelectionLogic( /* maxShortcutTargetsPerApp = */ 1, - /* applySharingAppLimits = */ true + /* applySharingAppLimits = */ true, ) val context = mock<Context> { on { packageManager } doReturn (mock()) } @@ -291,36 +306,82 @@ class ShortcutSelectionLogicTest { /* targetIntent = */ mock(), /* refererFillInIntent = */ mock(), /* maxRankedTargets = */ 4, - /* serviceTargets = */ serviceResults + /* serviceTargets = */ serviceResults, ) - assertShortcutsInOrder( - listOf(sc3, sc2), - serviceResults, - "At most two caller-provided shortcuts are allowed" - ) + assertWithMessage("At most two caller-provided shortcuts are allowed") + .that(serviceResults) + .comparingElementsUsing(targetInfoChooserTargetCorrespondence) + .containsExactly(sc3, sc2) + .inOrder() } - // TODO: consider renaming. Not all `ChooserTarget`s are "shortcuts" and many of our test cases - // add results with `isShortcutResult = false` and `directShareToShortcutInfos = emptyMap()`. - private fun assertShortcutsInOrder( - expected: List<ChooserTarget>, - actual: List<TargetInfo>, - msg: String? = "" - ) { - assertEquals(msg, expected.size, actual.size) - for (i in expected.indices) { - assertEquals( - "Unexpected item at position $i", - expected[i].componentName, - actual[i].chooserTargetComponentName + @Test + @EnableFlags(FLAG_REBUILD_ADAPTERS_ON_TARGET_PINNING) + fun addServiceResults_sameShortcutWithDifferentPinnedStatus_shortcutUpdated() { + val serviceResults = ArrayList<TargetInfo>() + val sc1 = + createChooserTarget( + title = "Shortcut", + score = 1f, + ComponentName(PACKAGE_A, CLASS_NAME), + PACKAGE_A.shortcutId(0), ) - assertEquals( - "Unexpected item at position $i", - expected[i].title, - actual[i].displayLabel + val sc2 = + createChooserTarget( + title = "Shortcut", + score = 1f, + ComponentName(PACKAGE_A, CLASS_NAME), + PACKAGE_A.shortcutId(0), ) - } + val testSubject = + ShortcutSelectionLogic( + /* maxShortcutTargetsPerApp = */ 1, + /* applySharingAppLimits = */ false, + ) + + testSubject.addServiceResults( + /* origTarget = */ baseDisplayInfo, + /* origTargetScore = */ 0.1f, + /* targets = */ listOf(sc1), + /* isShortcutResult = */ true, + /* directShareToShortcutInfos = */ mapOf( + sc1 to createShortcutInfo(PACKAGE_A.shortcutId(1), sc1.componentName, 1) + ), + /* directShareToAppTargets = */ emptyMap(), + /* userContext = */ mock(), + /* targetIntent = */ mock(), + /* refererFillInIntent = */ mock(), + /* maxRankedTargets = */ 4, + /* serviceTargets = */ serviceResults, + ) + val isUpdated = + testSubject.addServiceResults( + /* origTarget = */ baseDisplayInfo, + /* origTargetScore = */ 0.1f, + /* targets = */ listOf(sc1), + /* isShortcutResult = */ true, + /* directShareToShortcutInfos = */ mapOf( + sc1 to + createShortcutInfo(PACKAGE_A.shortcutId(1), sc1.componentName, 1).apply { + addFlags(ShortcutInfo.FLAG_PINNED) + } + ), + /* directShareToAppTargets = */ emptyMap(), + /* userContext = */ mock(), + /* targetIntent = */ mock(), + /* refererFillInIntent = */ mock(), + /* maxRankedTargets = */ 4, + /* serviceTargets = */ serviceResults, + ) + + assertWithMessage("Updates are expected").that(isUpdated).isTrue() + assertWithMessage("Updated shortcut is expected") + .that(serviceResults) + .comparingElementsUsing(targetInfoChooserTargetCorrespondence) + .containsExactly(sc2) + .inOrder() + assertThat(serviceResults[0].isPinned).isTrue() } private fun String.shortcutId(id: Int) = "$this.$id" |