From 691cfdb494fd6d4e194b1e9dc07470ab64564bfe Mon Sep 17 00:00:00 2001 From: Joshua Trask 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 --- .../chooser/NotSelectableTargetInfo.java | 147 +++++++-------------- .../intentresolver/chooser/TargetInfoTest.kt | 24 +++- 2 files changed, 66 insertions(+), 105 deletions(-) (limited to 'java') diff --git a/java/src/com/android/intentresolver/chooser/NotSelectableTargetInfo.java b/java/src/com/android/intentresolver/chooser/NotSelectableTargetInfo.java index 9a2c971f..c63ebc8c 100644 --- a/java/src/com/android/intentresolver/chooser/NotSelectableTargetInfo.java +++ b/java/src/com/android/intentresolver/chooser/NotSelectableTargetInfo.java @@ -18,33 +18,28 @@ package com.android.intentresolver.chooser; import android.annotation.Nullable; import android.app.Activity; -import android.content.ComponentName; import android.content.Context; -import android.content.Intent; -import android.content.pm.ResolveInfo; import android.graphics.drawable.AnimatedVectorDrawable; import android.graphics.drawable.Drawable; import android.os.Bundle; import android.os.UserHandle; import com.android.intentresolver.R; -import com.android.intentresolver.ResolverActivity; -import java.util.List; +import java.util.function.Supplier; /** * Distinguish between targets that selectable by the user, vs those that are * placeholders for the system while information is loading in an async manner. */ -public abstract class NotSelectableTargetInfo extends ChooserTargetInfo { +public final class NotSelectableTargetInfo { /** Create a non-selectable {@link TargetInfo} with no content. */ public static TargetInfo newEmptyTargetInfo() { - return new NotSelectableTargetInfo() { - @Override - public boolean isEmptyTargetInfo() { - return true; - } - }; + return ImmutableTargetInfo.newBuilder() + .setLegacyType(ImmutableTargetInfo.LegacyTargetType.EMPTY_TARGET_INFO) + .setDisplayIconHolder(makeReadOnlyIconHolder(() -> null)) + .setActivityStarter(makeNoOpActivityStarter()) + .build(); } /** @@ -52,108 +47,56 @@ public abstract class NotSelectableTargetInfo extends ChooserTargetInfo { * unless/until it can be replaced by the result of a pending asynchronous load. */ public static TargetInfo newPlaceHolderTargetInfo(Context context) { - return new NotSelectableTargetInfo() { - @Override - public boolean isPlaceHolderTargetInfo() { - return true; - } - - @Override - public IconHolder getDisplayIconHolder() { - return new IconHolder() { - @Override - public Drawable getDisplayIcon() { - AnimatedVectorDrawable avd = (AnimatedVectorDrawable) - context.getDrawable( - R.drawable.chooser_direct_share_icon_placeholder); - avd.start(); // Start animation after generation. - return avd; - } - - @Override - public void setDisplayIcon(Drawable icon) {} - }; - } - - @Override - public boolean hasDisplayIcon() { - return true; - } - }; - } - - public final boolean isNotSelectableTargetInfo() { - return true; - } - - public Intent getResolvedIntent() { - return null; - } - - public ComponentName getResolvedComponentName() { - return null; - } - - public boolean start(Activity activity, Bundle options) { - return false; - } - - public boolean startAsCaller(ResolverActivity activity, Bundle options, int userId) { - return false; - } - - @Nullable - @Override - public Intent getTargetIntent() { - return null; + return ImmutableTargetInfo.newBuilder() + .setLegacyType(ImmutableTargetInfo.LegacyTargetType.PLACEHOLDER_TARGET_INFO) + .setDisplayIconHolder( + makeReadOnlyIconHolder(() -> makeStartedPlaceholderDrawable(context))) + .setActivityStarter(makeNoOpActivityStarter()) + .build(); } - public boolean startAsUser(Activity activity, Bundle options, UserHandle user) { - return false; + private static Drawable makeStartedPlaceholderDrawable(Context context) { + AnimatedVectorDrawable avd = (AnimatedVectorDrawable) context.getDrawable( + R.drawable.chooser_direct_share_icon_placeholder); + avd.start(); // Start animation after generation. + return avd; } - public ResolveInfo getResolveInfo() { - return null; - } - - public CharSequence getDisplayLabel() { - return null; - } - - public CharSequence getExtendedInfo() { - return null; - } - - public TargetInfo cloneFilledIn(Intent fillInIntent, int flags) { - return null; - } - - public List getAllSourceIntents() { - return null; - } - - public float getModifiedScore() { - return -0.1f; - } + private static ImmutableTargetInfo.IconHolder makeReadOnlyIconHolder( + Supplier iconProvider) { + return new ImmutableTargetInfo.IconHolder() { + @Override + @Nullable + public Drawable getDisplayIcon() { + return iconProvider.get(); + } - public boolean isSuspended() { - return false; + @Override + public void setDisplayIcon(Drawable icon) {} + }; } - public boolean isPinned() { - return false; - } + private static ImmutableTargetInfo.TargetActivityStarter makeNoOpActivityStarter() { + return new ImmutableTargetInfo.TargetActivityStarter() { + @Override + public boolean start(TargetInfo target, Activity activity, Bundle options) { + return false; + } - @Override - public IconHolder getDisplayIconHolder() { - return new IconHolder() { @Override - public Drawable getDisplayIcon() { - return null; + public boolean startAsCaller( + TargetInfo target, Activity activity, Bundle options, int userId) { + return false; } @Override - public void setDisplayIcon(Drawable icon) {} + public boolean startAsUser( + TargetInfo target, Activity activity, Bundle options, UserHandle user) { + return false; + } }; } + + // TODO: merge all the APIs up to a single `TargetInfo` class. + private NotSelectableTargetInfo() {} } 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