From 53705d85ff4092c94592b281d3a05403bae01665 Mon Sep 17 00:00:00 2001
From: Joshua Trask <joshtrask@google.com>
Date: Thu, 3 Nov 2022 15:14:38 -0400
Subject: Don't expose `ChooserTarget` from `TargetInfo` API.

`ChooserTarget` is a deprecated API left over from the (now long-gone)
`ChooserTargetService` system, but internally, `SelectableTargetInfo`
still relies on this type for some basic record-keeping. The
`ChooserTarget`s today come from two sources; they're either passed in
from the caller via `EXTRA_CHOOSER_TARGETS`, or synthesized internally
to shoehorn non-`ChooserTarget` data into a `SelectableTargetInfo`.

Prior to this CL, clients throughout the application might reach
through to the composed-in `ChooserTarget` for some fields stored in
that record type. After this CL, any fields accessed in that way are
instead lifted to the `TargetInfo` API so that clients can access them
directly, without going through a `ChooserTarget`. Thus "consuming"
clients don't need to be aware of the `ChooserTarget`, and from their
perspective it's an internal implementation detail of
`SelectableTargetInfo`.

In a first subsequent CL, these fields can be precomputed from the
`ChooserTarget` during the construction of the `SelectableTargetInfo`
so that we don't retain any instances once we've extracted the
relevant data. In a second subsequent CL, we can expose those fields
as initialization parameters; then instead of synthesizing
intermediate `ChooserTarget`s, we can build our synthetic targets as
`SelectableTargetInfo`s directly. The usage in `EXTRA_CHOOSER_TARGETS`
is regrettably part of our public API that was overlooked in the
`ChooserTargetService` deprecation and won't be going away any time
soon, but if we can adapt them to the `SelectableTargetInfo`
representation immediately when we read them out of the request, we
should be able to cut out any other remaining references to
`ChooserTarget`.

Test: atest IntentResolverUnitTests
Bug: 202167050
Change-Id: I931234b0912952a4ccf5f94a88694ac82b527ae4
---
 .../com/android/intentresolver/ShortcutSelectionLogicTest.kt  | 11 +++++++++--
 .../src/com/android/intentresolver/chooser/TargetInfoTest.kt  |  6 ++++--
 2 files changed, 13 insertions(+), 4 deletions(-)

(limited to 'java/tests/src')

diff --git a/java/tests/src/com/android/intentresolver/ShortcutSelectionLogicTest.kt b/java/tests/src/com/android/intentresolver/ShortcutSelectionLogicTest.kt
index f45d592f..8581ed0c 100644
--- a/java/tests/src/com/android/intentresolver/ShortcutSelectionLogicTest.kt
+++ b/java/tests/src/com/android/intentresolver/ShortcutSelectionLogicTest.kt
@@ -261,6 +261,8 @@ class ShortcutSelectionLogicTest {
         )
     }
 
+    // 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? = ""
     ) {
@@ -268,8 +270,13 @@ class ShortcutSelectionLogicTest {
         for (i in expected.indices) {
             assertEquals(
                 "Unexpected item at position $i",
-                expected[i],
-                actual[i].chooserTarget
+                expected[i].componentName,
+                actual[i].chooserTargetComponentName
+            )
+            assertEquals(
+                "Unexpected item at position $i",
+                expected[i].title,
+                actual[i].displayLabel
             )
         }
     }
diff --git a/java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt b/java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt
index b6d0962b..ae9c0f8d 100644
--- a/java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt
+++ b/java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt
@@ -76,7 +76,9 @@ class TargetInfoTest {
         assertThat(targetInfo.isSelectableTargetInfo()).isTrue()
         assertThat(targetInfo.isChooserTargetInfo()).isTrue()  // From legacy inheritance model.
         assertThat(targetInfo.getDisplayResolveInfo()).isSameInstanceAs(displayInfo)
-        assertThat(targetInfo.getChooserTarget()).isSameInstanceAs(chooserTarget)
+        assertThat(targetInfo.getChooserTargetComponentName())
+            .isEqualTo(chooserTarget.getComponentName())
+        assertThat(targetInfo.getDirectShareShortcutId()).isEqualTo(shortcutInfo.getId())
         assertThat(targetInfo.getDirectShareShortcutInfo()).isSameInstanceAs(shortcutInfo)
         assertThat(targetInfo.getDirectShareAppTarget()).isSameInstanceAs(appTarget)
         // TODO: make more meaningful assertions about the behavior of a selectable target.
@@ -147,4 +149,4 @@ class TargetInfoTest {
         // TODO: consider exercising activity-start behavior.
         // TODO: consider exercising DisplayResolveInfo base class behavior.
     }
-}
\ No newline at end of file
+}
-- 
cgit v1.2.3-59-g8ed1b