From 691cfdb494fd6d4e194b1e9dc07470ab64564bfe Mon Sep 17 00:00:00 2001
From: Joshua Trask <joshtrask@google.com>
Date: Wed, 8 Feb 2023 22:25:29 +0000
Subject: Convert NotSelectableTargetInfo subtree to factories

Clients of the `NotSelectableTargetInfo` factory methods won't notice
any API change, but those methods will now return instances of
`ImmutableTargetInfo`, built to match the same behavior of the
legacy subtype (note no clients have any remaining `instanceof` checks
in this part of the inheritance graph). The class now just hosts the
static factory methods and some private static helpers, and no longer
inherits from `ChooserTargetInfo` (or any other base). The two
concrete `NotSelectableTargetInfo` subclasses are removed altogether.

This change is a near-pure refactoring; the only other theoretical
difference is that these `NotSelectableTargetInfo` targets will now
get a plausible implementation of `cloneFilledIn()` instead of
returning null, but in practice that method will never actually be
invoked on these particular `TargetInfo` types. We're also *slightly*
more eager in building `IconHolder` instances, but in practice we
would always do that at nearly the same time anyways (and there's no
particular concern about the overhead).

There are also two changes to tests:

 1. The device config permission change seemed to be necessary in
    order to run `TargetInfoTest` in isolation, even though those
    same tests could run as part of `IntentResolverUnitTests` already.
    I'm not sure why the discrepancy, but the fix seems reasonable.

 2. I needed to do a better job of setting up the placeholder targets'
    icon for tests since the new `ImmutableTargetInfo.Builder` API
    doesn't offer a mechanism to override `hasDisplayIcon()` directly.
    With the better support in place, I made some extra assertions.

This is the first (low-hanging-fruit) conversion of this type, and
in future CLs we should proceed through the rest of the hierarchy
before eventually flattening all the APIs into a single class.

Test: `atest TargetInfoTest`
Bug: 202167050
Change-Id: Ibabfee6ef2349de0db0be97f1d2f894d0672cbfd
---
 .../intentresolver/chooser/TargetInfoTest.kt       | 24 +++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

(limited to 'java/tests/src')

diff --git a/java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt b/java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt
index 69948cc9..e9dbe00e 100644
--- a/java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt
+++ b/java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt
@@ -22,18 +22,30 @@ import android.content.ComponentName
 import android.content.Intent
 import android.content.pm.ActivityInfo
 import android.content.pm.ResolveInfo
+import android.graphics.drawable.AnimatedVectorDrawable
 import android.os.UserHandle
+import android.test.UiThreadTest
 import androidx.test.platform.app.InstrumentationRegistry
 import com.android.intentresolver.createChooserTarget
 import com.android.intentresolver.createShortcutInfo
 import com.android.intentresolver.mock
 import com.android.intentresolver.ResolverDataProvider
 import com.google.common.truth.Truth.assertThat
+import org.junit.Before
 import org.junit.Test
 
 class TargetInfoTest {
     private val context = InstrumentationRegistry.getInstrumentation().getContext()
 
+    @Before
+    fun setup() {
+        // SelectableTargetInfo reads DeviceConfig and needs a permission for that.
+        InstrumentationRegistry
+            .getInstrumentation()
+            .getUiAutomation()
+            .adoptShellPermissionIdentity("android.permission.READ_DEVICE_CONFIG")
+    }
+
     @Test
     fun testNewEmptyTargetInfo() {
         val info = NotSelectableTargetInfo.newEmptyTargetInfo()
@@ -43,13 +55,19 @@ class TargetInfoTest {
         assertThat(info.getDisplayIconHolder().getDisplayIcon()).isNull()
     }
 
+    @UiThreadTest  // AnimatedVectorDrawable needs to start from a thread with a Looper.
     @Test
     fun testNewPlaceholderTargetInfo() {
         val info = NotSelectableTargetInfo.newPlaceHolderTargetInfo(context)
-        assertThat(info.isPlaceHolderTargetInfo()).isTrue()
-        assertThat(info.isChooserTargetInfo()).isTrue()  // From legacy inheritance model.
+        assertThat(info.isPlaceHolderTargetInfo).isTrue()
+        assertThat(info.isChooserTargetInfo).isTrue()  // From legacy inheritance model.
         assertThat(info.hasDisplayIcon()).isTrue()
-        // TODO: test infrastructure isn't set up to assert anything about the icon itself.
+        assertThat(info.displayIconHolder.displayIcon)
+                .isInstanceOf(AnimatedVectorDrawable::class.java)
+        // TODO: assert that the animation is pre-started/running (IIUC this requires synchronizing
+        // with some "render thread" per the `AnimatedVectorDrawable` docs). I believe this is
+        // possible using `AnimatorTestRule` but I couldn't find any sample usage in Kotlin nor get
+        // it working myself.
     }
 
     @Test
-- 
cgit v1.2.3-59-g8ed1b