From 85257d68d79fe1d1188b7d4365f35f7d72801355 Mon Sep 17 00:00:00 2001 From: Joshua Trask Date: Tue, 15 Nov 2022 10:46:47 -0500 Subject: SelectableTargetInfo "immutability"/other cleanup This CL makes many small "pure refactoring" changes as part of my ongoing go/chooser-targetinfo-cleanup. 1. Most important: introduce APIs to manage "functional"/"mutable" aspects of `SelectableTargetInfo`: `IconHolder` (to manage icon getting/setting), `TargetActivityStarter` (for the various `start` methods in `TargetInfo`), and `TargetHashProvider` (to compute a hash we use for metrics logging). I pulled `IconHolder` up into the base API and exposed it directly to clients (since they're really the ones that manipulate the "set" icon, and eventually they shold take over more of the icon-management responsibilities, as I've noted in a new TODO comment). I've left the others in `SelectableTargetInfo` for now since their responsibilities aren't as concerning in an "immutable" `TargetInfo` -- they defer some computation that's parameterized on the caller arguments, but they don't directly mutate any state in the target object. Eventually, the other interfaces can move to the upcoming new `ImmutableTargetInfo` type (where it will be convenient having the ability to copy these sub-components as whole objects). 2. Precompute any other fields in `SelectableTargetInfo` to show that we're now basically ready to implement immutability. 3. Extract `ChooserTarget` fields in the `SelectableTargetInfo` "factory method" -- that deprecated type is no longer part of the API of this `TargetInfo` implementation. Also add a new factory overload to skip the `ChooserTarget` representation for synthetic targets where it was only used as an intermediary in shuttling around these particular fields. 4. Implement `SelectableTargetInfo` "copy constructor" in terms of its primary (internal) constructor to make sure we perform any necessary initialization steps consistently. Test: atest IntentResolverUnitTests Bug: 202167050 Change-Id: If6f0cceebc4627fcd9a796948738c34195269e6d --- .../intentresolver/ChooserListAdapterTest.kt | 10 +++++--- .../intentresolver/ShortcutSelectionLogicTest.kt | 27 ++++++++++++++++------ .../intentresolver/chooser/TargetInfoTest.kt | 4 +++- 3 files changed, 30 insertions(+), 11 deletions(-) (limited to 'java/tests/src') diff --git a/java/tests/src/com/android/intentresolver/ChooserListAdapterTest.kt b/java/tests/src/com/android/intentresolver/ChooserListAdapterTest.kt index d054e7fa..080708fe 100644 --- a/java/tests/src/com/android/intentresolver/ChooserListAdapterTest.kt +++ b/java/tests/src/com/android/intentresolver/ChooserListAdapterTest.kt @@ -27,6 +27,7 @@ import android.widget.TextView import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.platform.app.InstrumentationRegistry import com.android.intentresolver.ChooserListAdapter.LoadDirectShareIconTask +import com.android.intentresolver.chooser.DisplayResolveInfo import com.android.intentresolver.chooser.SelectableTargetInfo import com.android.intentresolver.chooser.TargetInfo import com.android.internal.R @@ -112,9 +113,11 @@ class ChooserListAdapterTest { verify(testTaskProvider, times(1)).invoke() } - private fun createSelectableTargetInfo(): TargetInfo = - SelectableTargetInfo.newSelectableTargetInfo( - /* sourceInfo = */ mock(), + private fun createSelectableTargetInfo(): TargetInfo { + val displayInfo: DisplayResolveInfo = mock() + whenever(displayInfo.getAllSourceIntents()).thenReturn(listOf(mock())) + return SelectableTargetInfo.newSelectableTargetInfo( + /* sourceInfo = */ displayInfo, /* backupResolveInfo = */ mock(), /* resolvedIntent = */ Intent(), /* chooserTarget = */ createChooserTarget( @@ -125,6 +128,7 @@ class ChooserListAdapterTest { /* appTarget */ null, /* referrerFillInIntent = */ Intent() ) + } private fun createView(): View { val view = FrameLayout(context) diff --git a/java/tests/src/com/android/intentresolver/ShortcutSelectionLogicTest.kt b/java/tests/src/com/android/intentresolver/ShortcutSelectionLogicTest.kt index 2c56e613..e114d38d 100644 --- a/java/tests/src/com/android/intentresolver/ShortcutSelectionLogicTest.kt +++ b/java/tests/src/com/android/intentresolver/ShortcutSelectionLogicTest.kt @@ -20,6 +20,7 @@ import android.content.ComponentName import android.content.Context import android.content.pm.ShortcutInfo import android.service.chooser.ChooserTarget +import com.android.intentresolver.chooser.DisplayResolveInfo import com.android.intentresolver.chooser.TargetInfo import androidx.test.filters.SmallTest import org.junit.Assert.assertEquals @@ -59,9 +60,11 @@ class ShortcutSelectionLogicTest { /* maxShortcutTargetsPerApp = */ 1, /* applySharingAppLimits = */ false ) + val displayInfo: DisplayResolveInfo = mock() + whenever(displayInfo.getAllSourceIntents()).thenReturn(listOf(mock())) val isUpdated = testSubject.addServiceResults( - /* origTarget = */ mock(), + /* origTarget = */ displayInfo, /* origTargetScore = */ 0.1f, /* targets = */ listOf(sc1, sc2), /* isShortcutResult = */ true, @@ -91,9 +94,11 @@ class ShortcutSelectionLogicTest { /* maxShortcutTargetsPerApp = */ 1, /* applySharingAppLimits = */ true ) + val displayInfo: DisplayResolveInfo = mock() + whenever(displayInfo.getAllSourceIntents()).thenReturn(listOf(mock())) val isUpdated = testSubject.addServiceResults( - /* origTarget = */ mock(), + /* origTarget = */ displayInfo, /* origTargetScore = */ 0.1f, /* targets = */ listOf(sc1, sc2), /* isShortcutResult = */ true, @@ -123,9 +128,11 @@ class ShortcutSelectionLogicTest { /* maxShortcutTargetsPerApp = */ 1, /* applySharingAppLimits = */ false ) + val displayInfo: DisplayResolveInfo = mock() + whenever(displayInfo.getAllSourceIntents()).thenReturn(listOf(mock())) val isUpdated = testSubject.addServiceResults( - /* origTarget = */ mock(), + /* origTarget = */ displayInfo, /* origTargetScore = */ 0.1f, /* targets = */ listOf(sc1, sc2), /* isShortcutResult = */ true, @@ -157,9 +164,13 @@ class ShortcutSelectionLogicTest { /* maxShortcutTargetsPerApp = */ 1, /* applySharingAppLimits = */ true ) + val displayInfo: DisplayResolveInfo = mock() + whenever(displayInfo.getAllSourceIntents()).thenReturn(listOf(mock())) + val displayInfo2: DisplayResolveInfo = mock() + whenever(displayInfo2.getAllSourceIntents()).thenReturn(listOf(mock())) - testSubject.addServiceResults( - /* origTarget = */ mock(), + val isUpdated = testSubject.addServiceResults( + /* origTarget = */ displayInfo, /* origTargetScore = */ 0.1f, /* targets = */ listOf(pkgAsc1, pkgAsc2), /* isShortcutResult = */ true, @@ -172,7 +183,7 @@ class ShortcutSelectionLogicTest { /* serviceTargets = */ serviceResults ) testSubject.addServiceResults( - /* origTarget = */ mock(), + /* origTarget = */ displayInfo2, /* origTargetScore = */ 0.2f, /* targets = */ listOf(pkgBsc1, pkgBsc2), /* isShortcutResult = */ true, @@ -201,9 +212,11 @@ class ShortcutSelectionLogicTest { /* maxShortcutTargetsPerApp = */ 1, /* applySharingAppLimits = */ false ) + val displayInfo: DisplayResolveInfo = mock() + whenever(displayInfo.getAllSourceIntents()).thenReturn(listOf(mock())) val isUpdated = testSubject.addServiceResults( - /* origTarget = */ mock(), + /* origTarget = */ displayInfo, /* origTargetScore = */ 0.1f, /* targets = */ listOf(sc1, sc2), /* isShortcutResult = */ true, diff --git a/java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt b/java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt index 11837e08..c29de0be 100644 --- a/java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt +++ b/java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt @@ -28,6 +28,7 @@ import com.android.intentresolver.createChooserTarget import com.android.intentresolver.createShortcutInfo import com.android.intentresolver.mock import com.android.intentresolver.ResolverDataProvider +import com.android.intentresolver.whenever import com.google.common.truth.Truth.assertThat import org.junit.Test @@ -40,7 +41,7 @@ class TargetInfoTest { assertThat(info.isEmptyTargetInfo()).isTrue() assertThat(info.isChooserTargetInfo()).isTrue() // From legacy inheritance model. assertThat(info.hasDisplayIcon()).isFalse() - assertThat(info.getDisplayIcon()).isNull() + assertThat(info.getDisplayIconHolder().getDisplayIcon()).isNull() } @Test @@ -55,6 +56,7 @@ class TargetInfoTest { @Test fun testNewSelectableTargetInfo() { val displayInfo: DisplayResolveInfo = mock() + whenever(displayInfo.getAllSourceIntents()).thenReturn(listOf(mock())) val chooserTarget = createChooserTarget( "title", 0.3f, ResolverDataProvider.createComponentName(1), "test_shortcut_id") val shortcutInfo = createShortcutInfo("id", ResolverDataProvider.createComponentName(2), 3) -- cgit v1.2.3-59-g8ed1b