diff options
author | 2024-11-05 15:11:35 -0800 | |
---|---|---|
committer | 2024-11-06 08:36:12 -0800 | |
commit | 34f3b5d5992bf07674abb007a76f2b469e4e6ad8 (patch) | |
tree | c70f7a9d0832924b9c766f9bdba730d13510b0fc | |
parent | 40389e404f72cb13f9632fa86cd0f13905f682ea (diff) |
Reload targets by recreating adapters in response to target pinning.
Use Shareousel's target list updating logic in response to target pinning
and application changed broadcasts. Create new adapters and attach them
to the views after data is loaded, resulting in a cleaner visual update.
Fix: 230703572
Test: manual testing
Test: atest IntentResolver-tests-unit
Flag: com.android.intentresolver.rebuild_adapters_on_target_pinning
Change-Id: I1e7e2fd820195134ec4940f2e76bf7b3eac9e086
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" |