From 8ad27b4ab8bc802d5c5b564db29ab72dfb0c5ab3 Mon Sep 17 00:00:00 2001 From: Joshua Trask Date: Wed, 7 Jun 2023 18:58:21 +0000 Subject: Clean up ResolverActivity.getResolveInfoUserHandle This function was a shim to read from ResolveInfo.userHandle, which we added as a stopgap to unblock development on cloned-app support before the field was actually populated from PackageManager (since we could make reasonable "predictions" for the right value with logic based on our own application UI). The PackageManager support has since caught up, so this has been inlined to use the "real" field and we can now remove the shim. This CL also extends ResolverDataProvider to set userHandles in the fake ResolveInfo instances we build for our tests. That wasn't necessary to get our current tests to pass, but since we have logic that depends on these fields, it seemed preferable to provide plausible values that would better reflect any future issues. Bug: 273294251 Test: IntentResolverUnitTests, CtsSharesheetDeviceTest Change-Id: I541220f9e3ab945a1753cda0f8a53f22ab708876 --- .../com/android/intentresolver/ChooserActivity.java | 14 +++----------- .../android/intentresolver/ChooserListAdapter.java | 3 +-- .../com/android/intentresolver/ResolverActivity.java | 20 +++----------------- .../android/intentresolver/icons/LoadIconTask.java | 4 +--- 4 files changed, 8 insertions(+), 33 deletions(-) (limited to 'java/src') diff --git a/java/src/com/android/intentresolver/ChooserActivity.java b/java/src/com/android/intentresolver/ChooserActivity.java index a2dff970..a9dd8a5f 100644 --- a/java/src/com/android/intentresolver/ChooserActivity.java +++ b/java/src/com/android/intentresolver/ChooserActivity.java @@ -871,9 +871,7 @@ public class ChooserActivity extends ResolverActivity implements targetList, // Adding userHandle from ResolveInfo allows the app icon in Dialog Box to be // resolved correctly within the same tab. - getResolveInfoUserHandle( - targetInfo.getResolveInfo(), - mChooserMultiProfilePagerAdapter.getCurrentUserHandle()), + targetInfo.getResolveInfo().userHandle, shortcutIdKey, shortcutTitle, isShortcutPinned, @@ -914,9 +912,7 @@ public class ChooserActivity extends ResolverActivity implements getSupportFragmentManager(), mti, which, - getResolveInfoUserHandle( - targetInfo.getResolveInfo(), - mChooserMultiProfilePagerAdapter.getCurrentUserHandle())); + targetInfo.getResolveInfo().userHandle); return; } } @@ -1134,11 +1130,7 @@ public class ChooserActivity extends ResolverActivity implements // Adding two stage comparator, first stage compares using displayLabel, next stage // compares using resolveInfo.userHandle mComparator = Comparator.comparing(DisplayResolveInfo::getDisplayLabel, collator) - .thenComparingInt(displayResolveInfo -> - getResolveInfoUserHandle( - displayResolveInfo.getResolveInfo(), - // TODO: User resolveInfo.userHandle, once its available. - UserHandle.SYSTEM).getIdentifier()); + .thenComparingInt(target -> target.getResolveInfo().userHandle.getIdentifier()); } @Override diff --git a/java/src/com/android/intentresolver/ChooserListAdapter.java b/java/src/com/android/intentresolver/ChooserListAdapter.java index b1fa16b0..de947bd1 100644 --- a/java/src/com/android/intentresolver/ChooserListAdapter.java +++ b/java/src/com/android/intentresolver/ChooserListAdapter.java @@ -384,8 +384,7 @@ public class ChooserListAdapter extends ResolverListAdapter { .collect(Collectors.groupingBy(target -> target.getResolvedComponentName().getPackageName() + "#" + target.getDisplayLabel() - + '#' + ResolverActivity.getResolveInfoUserHandle( - target.getResolveInfo(), getUserHandle()).getIdentifier() + + '#' + target.getResolveInfo().userHandle.getIdentifier() )) .values() .stream() diff --git a/java/src/com/android/intentresolver/ResolverActivity.java b/java/src/com/android/intentresolver/ResolverActivity.java index 57871532..3efe942c 100644 --- a/java/src/com/android/intentresolver/ResolverActivity.java +++ b/java/src/com/android/intentresolver/ResolverActivity.java @@ -1649,10 +1649,9 @@ public class ResolverActivity extends FragmentActivity implements /** Start the activity specified by the {@link TargetInfo}.*/ public final void safelyStartActivity(TargetInfo cti) { // In case cloned apps are present, we would want to start those apps in cloned user - // space, which will not be same as adaptor's userHandle. resolveInfo.userHandle + // space, which will not be same as the adapter's userHandle. resolveInfo.userHandle // identifies the correct user space in such cases. - UserHandle activityUserHandle = getResolveInfoUserHandle( - cti.getResolveInfo(), mMultiProfilePagerAdapter.getCurrentUserHandle()); + UserHandle activityUserHandle = cti.getResolveInfo().userHandle; safelyStartActivityAsUser(cti, activityUserHandle, null); } @@ -2267,11 +2266,7 @@ public class ResolverActivity extends FragmentActivity implements && Objects.equals(lhs.activityInfo.packageName, rhs.activityInfo.packageName) // Comparing against resolveInfo.userHandle in case cloned apps are present, // as they will have the same activityInfo. - && Objects.equals( - getResolveInfoUserHandle(lhs, - mMultiProfilePagerAdapter.getActiveListAdapter().getUserHandle()), - getResolveInfoUserHandle(rhs, - mMultiProfilePagerAdapter.getActiveListAdapter().getUserHandle())); + && Objects.equals(lhs.userHandle, rhs.userHandle); } private boolean inactiveListAdapterHasItems() { @@ -2409,13 +2404,4 @@ public class ResolverActivity extends FragmentActivity implements } return userList; } - - /** - * This function is temporary in nature, and its usages will be replaced with just - * resolveInfo.userHandle, once it is available, once sharesheet is stable. - */ - public static UserHandle getResolveInfoUserHandle(ResolveInfo resolveInfo, - UserHandle predictedHandle) { - return resolveInfo.userHandle; - } } diff --git a/java/src/com/android/intentresolver/icons/LoadIconTask.java b/java/src/com/android/intentresolver/icons/LoadIconTask.java index 37ce4093..75132208 100644 --- a/java/src/com/android/intentresolver/icons/LoadIconTask.java +++ b/java/src/com/android/intentresolver/icons/LoadIconTask.java @@ -24,7 +24,6 @@ import android.os.Trace; import android.os.UserHandle; import android.util.Log; -import com.android.intentresolver.ResolverActivity; import com.android.intentresolver.TargetPresentationGetter; import com.android.intentresolver.chooser.DisplayResolveInfo; @@ -64,8 +63,7 @@ class LoadIconTask extends BaseLoadIconTask { protected final Drawable loadIconForResolveInfo(ResolveInfo ri) { // Load icons based on userHandle from ResolveInfo. If in work profile/clone profile, icons // should be badged. - return mPresentationFactory.makePresentationGetter(ri) - .getIcon(ResolverActivity.getResolveInfoUserHandle(ri, mUserHandle)); + return mPresentationFactory.makePresentationGetter(ri).getIcon(ri.userHandle); } } -- cgit v1.2.3-59-g8ed1b From 7cbff193529acbfbccce1d270127cd889c65b68c Mon Sep 17 00:00:00 2001 From: Joshua Trask Date: Wed, 7 Jun 2023 16:50:57 +0000 Subject: Move two params to ResolverActivity.onCreate() Namely, this is the data that subclasses previously configured by calling up to `ResolverActivity.setSafeForwardingMode()` and `ResolverActivity.setAdditionalTargets()`. Both parameters are constant within a session (even though we still don't have a great place to hold them as `final`) -- and both can affect the behavior of `ResolverActivity.onCreate()`, so it's important that we have the data prepared for that call (and, especially in the case of "additional targets" it's not necessarily clear what would happen if we made additional changes afterward). Chooser was already setting these at the right time, but this makes the dependency explicit and prevents accidental misuse. This is a pure refactoring that trivially has no side effects in the IntentResolver codebase. The API change in ResolverActivity would break other subclasses (if they were depending on the IntentResolver fork instead of the framework version), but that's not relevant right now, and besides, the only client that would seem to be broken is CarResolverActivity, which currently uses these APIs incorrectly. Test: IntentResolverUnitTests, CtsSharesheetDeviceTest Bug: 286249609 Change-Id: I7309a4fb33244cfac20c8dffc81e738a91381904 --- .../android/intentresolver/ChooserActivity.java | 12 ++- .../android/intentresolver/ResolverActivity.java | 98 ++++++++++++---------- 2 files changed, 57 insertions(+), 53 deletions(-) (limited to 'java/src') diff --git a/java/src/com/android/intentresolver/ChooserActivity.java b/java/src/com/android/intentresolver/ChooserActivity.java index a2dff970..f5558a00 100644 --- a/java/src/com/android/intentresolver/ChooserActivity.java +++ b/java/src/com/android/intentresolver/ChooserActivity.java @@ -286,10 +286,6 @@ public class ChooserActivity extends ResolverActivity implements mEnterTransitionAnimationDelegate, new HeadlineGeneratorImpl(this)); - setAdditionalTargets(mChooserRequest.getAdditionalTargets()); - - setSafeForwardingMode(true); - mPinnedSharedPrefs = getPinnedSharedPrefs(this); mMaxTargetsPerRow = getResources().getInteger(R.integer.config_chooser_max_targets_per_row); @@ -307,12 +303,14 @@ public class ChooserActivity extends ResolverActivity implements super.onCreate( savedInstanceState, mChooserRequest.getTargetIntent(), + mChooserRequest.getAdditionalTargets(), mChooserRequest.getTitle(), mChooserRequest.getDefaultTitleResource(), mChooserRequest.getInitialIntents(), - /* rList: List = */ null, - /* supportsAlwaysUseOption = */ false, - new DefaultTargetDataLoader(this, getLifecycle(), false)); + /* resolutionList= */ null, + /* supportsAlwaysUseOption= */ false, + new DefaultTargetDataLoader(this, getLifecycle(), false), + /* safeForwardingMode= */ true); mChooserShownTime = System.currentTimeMillis(); final long systemCost = mChooserShownTime - intentReceivedTime; diff --git a/java/src/com/android/intentresolver/ResolverActivity.java b/java/src/com/android/intentresolver/ResolverActivity.java index 57871532..f6637d0f 100644 --- a/java/src/com/android/intentresolver/ResolverActivity.java +++ b/java/src/com/android/intentresolver/ResolverActivity.java @@ -119,6 +119,7 @@ import com.android.internal.util.LatencyTracker; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Objects; @@ -143,7 +144,14 @@ public class ResolverActivity extends FragmentActivity implements mIsIntentPicker = isIntentPicker; } + /** + * Whether to enable a launch mode that is safe to use when forwarding intents received from + * applications and running in system processes. This mode uses Activity.startActivityAsCaller + * instead of the normal Activity.startActivity for launching the activity selected + * by the user. + */ private boolean mSafeForwardingMode; + private Button mAlwaysButton; private Button mOnceButton; protected View mProfileView; @@ -332,38 +340,55 @@ public class ResolverActivity extends FragmentActivity implements mResolvingHome = true; } - setSafeForwardingMode(true); - - onCreate(savedInstanceState, intent, null, 0, null, null, true, createIconLoader()); + onCreate( + savedInstanceState, + intent, + /* additionalTargets= */ null, + /* title= */ null, + /* defaultTitleRes= */ 0, + /* initialIntents= */ null, + /* resolutionList= */ null, + /* supportsAlwaysUseOption= */ true, + createIconLoader(), + /* safeForwardingMode= */ true); } /** * Compatibility version for other bundled services that use this overload without * a default title resource */ - protected void onCreate(Bundle savedInstanceState, Intent intent, - CharSequence title, Intent[] initialIntents, - List rList, boolean supportsAlwaysUseOption) { + protected void onCreate( + Bundle savedInstanceState, + Intent intent, + CharSequence title, + Intent[] initialIntents, + List resolutionList, + boolean supportsAlwaysUseOption, + boolean safeForwardingMode) { onCreate( savedInstanceState, intent, + null, title, 0, initialIntents, - rList, + resolutionList, supportsAlwaysUseOption, - createIconLoader()); + createIconLoader(), + safeForwardingMode); } protected void onCreate( Bundle savedInstanceState, Intent intent, + Intent[] additionalTargets, CharSequence title, int defaultTitleRes, Intent[] initialIntents, - List rList, + List resolutionList, boolean supportsAlwaysUseOption, - TargetDataLoader targetDataLoader) { + TargetDataLoader targetDataLoader, + boolean safeForwardingMode) { setTheme(appliedThemeResId()); super.onCreate(savedInstanceState); @@ -381,12 +406,17 @@ public class ResolverActivity extends FragmentActivity implements mReferrerPackage = getReferrerPackageName(); - // Add our initial intent as the first item, regardless of what else has already been added. + // The initial intent must come before any other targets that are to be added. mIntents.add(0, new Intent(intent)); + if (additionalTargets != null) { + Collections.addAll(mIntents, additionalTargets); + } + mTitle = title; mDefaultTitleResId = defaultTitleRes; mSupportsAlwaysUseOption = supportsAlwaysUseOption; + mSafeForwardingMode = safeForwardingMode; // The last argument of createResolverListAdapter is whether to do special handling // of the last used choice to highlight it in the list. We need to always @@ -399,7 +429,7 @@ public class ResolverActivity extends FragmentActivity implements boolean filterLastUsed = mSupportsAlwaysUseOption && !isVoiceInteraction() && !shouldShowTabs() && !hasCloneProfile(); mMultiProfilePagerAdapter = createMultiProfilePagerAdapter( - initialIntents, rList, filterLastUsed, targetDataLoader); + initialIntents, resolutionList, filterLastUsed, targetDataLoader); if (configureContentView(targetDataLoader)) { return; } @@ -455,17 +485,17 @@ public class ResolverActivity extends FragmentActivity implements protected AbstractMultiProfilePagerAdapter createMultiProfilePagerAdapter( Intent[] initialIntents, - List rList, + List resolutionList, boolean filterLastUsed, TargetDataLoader targetDataLoader) { AbstractMultiProfilePagerAdapter resolverMultiProfilePagerAdapter = null; if (shouldShowTabs()) { resolverMultiProfilePagerAdapter = createResolverMultiProfilePagerAdapterForTwoProfiles( - initialIntents, rList, filterLastUsed, targetDataLoader); + initialIntents, resolutionList, filterLastUsed, targetDataLoader); } else { resolverMultiProfilePagerAdapter = createResolverMultiProfilePagerAdapterForOneProfile( - initialIntents, rList, filterLastUsed, targetDataLoader); + initialIntents, resolutionList, filterLastUsed, targetDataLoader); } return resolverMultiProfilePagerAdapter; } @@ -1043,7 +1073,7 @@ public class ResolverActivity extends FragmentActivity implements Context context, List payloadIntents, Intent[] initialIntents, - List rList, + List resolutionList, boolean filterLastUsed, UserHandle userHandle, TargetDataLoader targetDataLoader) { @@ -1054,7 +1084,7 @@ public class ResolverActivity extends FragmentActivity implements context, payloadIntents, initialIntents, - rList, + resolutionList, filterLastUsed, createListController(userHandle), userHandle, @@ -1142,14 +1172,14 @@ public class ResolverActivity extends FragmentActivity implements private ResolverMultiProfilePagerAdapter createResolverMultiProfilePagerAdapterForOneProfile( Intent[] initialIntents, - List rList, + List resolutionList, boolean filterLastUsed, TargetDataLoader targetDataLoader) { ResolverListAdapter adapter = createResolverListAdapter( /* context */ this, /* payloadIntents */ mIntents, initialIntents, - rList, + resolutionList, filterLastUsed, /* userHandle */ getPersonalProfileUserHandle(), targetDataLoader); @@ -1170,7 +1200,7 @@ public class ResolverActivity extends FragmentActivity implements private ResolverMultiProfilePagerAdapter createResolverMultiProfilePagerAdapterForTwoProfiles( Intent[] initialIntents, - List rList, + List resolutionList, boolean filterLastUsed, TargetDataLoader targetDataLoader) { // In the edge case when we have 0 apps in the current profile and >1 apps in the other, @@ -1197,7 +1227,7 @@ public class ResolverActivity extends FragmentActivity implements /* context */ this, /* payloadIntents */ mIntents, selectedProfile == PROFILE_PERSONAL ? initialIntents : null, - rList, + resolutionList, (filterLastUsed && UserHandle.myUserId() == getPersonalProfileUserHandle().getIdentifier()), /* userHandle */ getPersonalProfileUserHandle(), @@ -1207,7 +1237,7 @@ public class ResolverActivity extends FragmentActivity implements /* context */ this, /* payloadIntents */ mIntents, selectedProfile == PROFILE_WORK ? initialIntents : null, - rList, + resolutionList, (filterLastUsed && UserHandle.myUserId() == workProfileUserHandle.getIdentifier()), /* userHandle */ workProfileUserHandle, @@ -1365,14 +1395,6 @@ public class ResolverActivity extends FragmentActivity implements return new Option(target.getDisplayLabel(), index); } - protected final void setAdditionalTargets(Intent[] intents) { - if (intents != null) { - for (Intent intent : intents) { - mIntents.add(intent); - } - } - } - public final Intent getTargetIntent() { return mIntents.isEmpty() ? null : mIntents.get(0); } @@ -1433,22 +1455,6 @@ public class ResolverActivity extends FragmentActivity implements () -> getString(R.string.forward_intent_to_work)); } - /** - * Turn on launch mode that is safe to use when forwarding intents received from - * applications and running in system processes. This mode uses Activity.startActivityAsCaller - * instead of the normal Activity.startActivity for launching the activity selected - * by the user. - * - *

This mode is set to true by default if the activity is initialized through - * {@link #onCreate(android.os.Bundle)}. If a subclass calls one of the other onCreate - * methods, it is set to false by default. You must set it before calling one of the - * more detailed onCreate methods, so that it will be set correctly in the case where - * there is only one intent to resolve and it is thus started immediately.

- */ - public final void setSafeForwardingMode(boolean safeForwarding) { - mSafeForwardingMode = safeForwarding; - } - protected final CharSequence getTitleForAction(Intent intent, int defaultTitleRes) { final ActionTitle title = mResolvingHome ? ActionTitle.HOME -- cgit v1.2.3-59-g8ed1b From f609ae4c5d04918b328ba12e4aa99ca8fecfde88 Mon Sep 17 00:00:00 2001 From: 1 Date: Fri, 16 Jun 2023 14:56:17 +0000 Subject: Add an extra to sharesheet's image edit intent To allow the editor to customize the behavior for the sharesheet flow if desired. Test: Validate that EXTRA is received by editor. Bug: 287621918 Change-Id: Ia70cc645e85054fd73f77f989cb4267cdc524cd4 --- java/src/com/android/intentresolver/ChooserActionFactory.java | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'java/src') diff --git a/java/src/com/android/intentresolver/ChooserActionFactory.java b/java/src/com/android/intentresolver/ChooserActionFactory.java index 6ec62753..4e595c96 100644 --- a/java/src/com/android/intentresolver/ChooserActionFactory.java +++ b/java/src/com/android/intentresolver/ChooserActionFactory.java @@ -78,6 +78,11 @@ public final class ChooserActionFactory implements ChooserContentPreviewUi.Actio | Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION | Intent.FLAG_GRANT_PREFIX_URI_PERMISSION; + // Boolean extra used to inform the editor that it may want to customize the editing experience + // for the sharesheet editing flow. + private static final String EDIT_SOURCE = "edit_source"; + private static final String EDIT_SOURCE_SHARESHEET = "sharesheet"; + private static final String CHIP_LABEL_METADATA_KEY = "android.service.chooser.chip_label"; private static final String CHIP_ICON_METADATA_KEY = "android.service.chooser.chip_icon"; @@ -284,6 +289,7 @@ public final class ChooserActionFactory implements ChooserContentPreviewUi.Actio resolveIntent.setFlags(originalIntent.getFlags() & URI_PERMISSION_INTENT_FLAGS); resolveIntent.setComponent(editorComponent); resolveIntent.setAction(Intent.ACTION_EDIT); + resolveIntent.putExtra(EDIT_SOURCE, EDIT_SOURCE_SHARESHEET); String originalAction = originalIntent.getAction(); if (Intent.ACTION_SEND.equals(originalAction)) { if (resolveIntent.getData() == null) { -- cgit v1.2.3-59-g8ed1b From a1c9dc6af9fb0ff4b50ed1b8e7f59bdfccbfa682 Mon Sep 17 00:00:00 2001 From: Andrey Epin Date: Tue, 13 Jun 2023 10:45:20 -0700 Subject: Provide copy action only when a text is sent The copy button will be present to the user only when a text without images is sent i.e. intent action is ACTION_SEND, EXTRA_STREAM does not have any URI and EXTRA_TEXT is not null. Fix: 287061873 Fix: 275382122 Fix: 288425116 Test: manual testing Test: atest ChooserActionFactoryTest Change-Id: I26c6eef24e7f909842eb066a73286a7525124d21 --- .../intentresolver/ChooserActionFactory.java | 75 +++++----- .../intentresolver/ChooserActionFactoryTest.kt | 152 +++++++++++++++------ 2 files changed, 151 insertions(+), 76 deletions(-) (limited to 'java/src') diff --git a/java/src/com/android/intentresolver/ChooserActionFactory.java b/java/src/com/android/intentresolver/ChooserActionFactory.java index 6ec62753..2308dda5 100644 --- a/java/src/com/android/intentresolver/ChooserActionFactory.java +++ b/java/src/com/android/intentresolver/ChooserActionFactory.java @@ -84,6 +84,8 @@ public final class ChooserActionFactory implements ChooserContentPreviewUi.Actio private static final String IMAGE_EDITOR_SHARED_ELEMENT = "screenshot_preview_image"; private final Context mContext; + + @Nullable private final Runnable mCopyButtonRunnable; private final Runnable mEditButtonRunnable; private final ImmutableList mCustomActions; @@ -140,7 +142,7 @@ public final class ChooserActionFactory implements ChooserContentPreviewUi.Actio @VisibleForTesting ChooserActionFactory( Context context, - Runnable copyButtonRunnable, + @Nullable Runnable copyButtonRunnable, Runnable editButtonRunnable, List customActions, @Nullable ChooserAction modifyShareAction, @@ -219,49 +221,24 @@ public final class ChooserActionFactory implements ChooserContentPreviewUi.Actio return mExcludeSharedTextAction; } + @Nullable private static Runnable makeCopyButtonRunnable( Context context, Intent targetIntent, String referrerPackageName, Consumer finishCallback, ChooserActivityLogger logger) { + final ClipData clipData; + try { + clipData = extractTextToCopy(targetIntent); + } catch (Throwable t) { + Log.e(TAG, "Failed to extract data to copy", t); + return null; + } + if (clipData == null) { + return null; + } return () -> { - if (targetIntent == null) { - finishCallback.accept(null); - return; - } - - final String action = targetIntent.getAction(); - - ClipData clipData = null; - if (Intent.ACTION_SEND.equals(action)) { - String extraText = targetIntent.getStringExtra(Intent.EXTRA_TEXT); - Uri extraStream = targetIntent.getParcelableExtra(Intent.EXTRA_STREAM); - - if (extraText != null) { - clipData = ClipData.newPlainText(null, extraText); - } else if (extraStream != null) { - clipData = ClipData.newUri(context.getContentResolver(), null, extraStream); - } else { - Log.w(TAG, "No data available to copy to clipboard"); - return; - } - } else if (Intent.ACTION_SEND_MULTIPLE.equals(action)) { - final ArrayList streams = targetIntent.getParcelableArrayListExtra( - Intent.EXTRA_STREAM); - clipData = ClipData.newUri(context.getContentResolver(), null, streams.get(0)); - for (int i = 1; i < streams.size(); i++) { - clipData.addItem( - context.getContentResolver(), - new ClipData.Item(streams.get(i))); - } - } else { - // expected to only be visible with ACTION_SEND or ACTION_SEND_MULTIPLE - // so warn about unexpected action - Log.w(TAG, "Action (" + action + ") not supported for copying to clipboard"); - return; - } - ClipboardManager clipboardManager = (ClipboardManager) context.getSystemService( Context.CLIPBOARD_SERVICE); clipboardManager.setPrimaryClipAsPackage(clipData, referrerPackageName); @@ -271,6 +248,30 @@ public final class ChooserActionFactory implements ChooserContentPreviewUi.Actio }; } + @Nullable + private static ClipData extractTextToCopy(Intent targetIntent) { + if (targetIntent == null) { + return null; + } + + final String action = targetIntent.getAction(); + + ClipData clipData = null; + if (Intent.ACTION_SEND.equals(action)) { + String extraText = targetIntent.getStringExtra(Intent.EXTRA_TEXT); + + if (extraText != null) { + clipData = ClipData.newPlainText(null, extraText); + } else { + Log.w(TAG, "No data available to copy to clipboard"); + } + } else { + // expected to only be visible with ACTION_SEND (when a text is shared) + Log.d(TAG, "Action (" + action + ") not supported for copying to clipboard"); + } + return clipData; + } + private static TargetInfo getEditSharingTarget( Context context, Intent originalIntent, diff --git a/java/tests/src/com/android/intentresolver/ChooserActionFactoryTest.kt b/java/tests/src/com/android/intentresolver/ChooserActionFactoryTest.kt index d72c9aa6..8d994f08 100644 --- a/java/tests/src/com/android/intentresolver/ChooserActionFactoryTest.kt +++ b/java/tests/src/com/android/intentresolver/ChooserActionFactoryTest.kt @@ -25,48 +25,45 @@ import android.content.IntentFilter import android.content.res.Resources import android.graphics.drawable.Icon import android.service.chooser.ChooserAction -import android.view.View import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.platform.app.InstrumentationRegistry -import com.android.intentresolver.flags.FeatureFlagRepository import com.google.common.collect.ImmutableList import com.google.common.truth.Truth.assertThat +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import java.util.function.Consumer import org.junit.After import org.junit.Assert.assertEquals import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.mockito.Mockito -import java.util.concurrent.Callable -import java.util.concurrent.CountDownLatch -import java.util.concurrent.TimeUnit -import java.util.function.Consumer @RunWith(AndroidJUnit4::class) class ChooserActionFactoryTest { private val context = InstrumentationRegistry.getInstrumentation().getContext() private val logger = mock() - private val flags = mock() private val actionLabel = "Action label" private val modifyShareLabel = "Modify share" private val testAction = "com.android.intentresolver.testaction" private val countdown = CountDownLatch(1) - private val testReceiver: BroadcastReceiver = object : BroadcastReceiver() { - override fun onReceive(context: Context, intent: Intent) { - // Just doing at most a single countdown per test. - countdown.countDown() + private val testReceiver: BroadcastReceiver = + object : BroadcastReceiver() { + override fun onReceive(context: Context, intent: Intent) { + // Just doing at most a single countdown per test. + countdown.countDown() + } } - } - private object resultConsumer : Consumer { - var latestReturn = Integer.MIN_VALUE + private val resultConsumer = + object : Consumer { + var latestReturn = Integer.MIN_VALUE - override fun accept(resultCode: Int) { - latestReturn = resultCode + override fun accept(resultCode: Int) { + latestReturn = resultCode + } } - } - @Before fun setup() { context.registerReceiver(testReceiver, IntentFilter(testAction)) @@ -91,7 +88,7 @@ class ChooserActionFactoryTest { Mockito.verify(logger).logCustomActionSelected(eq(0)) assertEquals(Activity.RESULT_OK, resultConsumer.latestReturn) - // Verify the pendingintent has been called + // Verify the pending intent has been called countdown.await(500, TimeUnit.MILLISECONDS) } @@ -109,42 +106,119 @@ class ChooserActionFactoryTest { val action = factory.modifyShareAction ?: error("Modify share action should not be null") action.onClicked.run() - Mockito.verify(logger).logActionSelected( - eq(ChooserActivityLogger.SELECTION_TYPE_MODIFY_SHARE)) + Mockito.verify(logger) + .logActionSelected(eq(ChooserActivityLogger.SELECTION_TYPE_MODIFY_SHARE)) assertEquals(Activity.RESULT_OK, resultConsumer.latestReturn) - // Verify the pendingintent has been called + // Verify the pending intent has been called countdown.await(500, TimeUnit.MILLISECONDS) } + @Test + fun nonSendAction_noCopyRunnable() { + val targetIntent = + Intent(Intent.ACTION_SEND_MULTIPLE).apply { + putExtra(Intent.EXTRA_TEXT, "Text to show") + } + + val chooserRequest = + mock { + whenever(this.targetIntent).thenReturn(targetIntent) + whenever(chooserActions).thenReturn(ImmutableList.of()) + } + val testSubject = + ChooserActionFactory( + context, + chooserRequest, + mock(), + logger, + {}, + { null }, + mock(), + {}, + ) + assertThat(testSubject.copyButtonRunnable).isNull() + } + + @Test + fun sendActionNoText_noCopyRunnable() { + val targetIntent = Intent(Intent.ACTION_SEND) + + val chooserRequest = + mock { + whenever(this.targetIntent).thenReturn(targetIntent) + whenever(chooserActions).thenReturn(ImmutableList.of()) + } + val testSubject = + ChooserActionFactory( + context, + chooserRequest, + mock(), + logger, + {}, + { null }, + mock(), + {}, + ) + assertThat(testSubject.copyButtonRunnable).isNull() + } + + @Test + fun sendActionWithText_nonNullCopyRunnable() { + val targetIntent = Intent(Intent.ACTION_SEND).apply { putExtra(Intent.EXTRA_TEXT, "Text") } + + val chooserRequest = + mock { + whenever(this.targetIntent).thenReturn(targetIntent) + whenever(chooserActions).thenReturn(ImmutableList.of()) + } + val testSubject = + ChooserActionFactory( + context, + chooserRequest, + mock(), + logger, + {}, + { null }, + mock(), + {}, + ) + assertThat(testSubject.copyButtonRunnable).isNotNull() + } + private fun createFactory(includeModifyShare: Boolean = false): ChooserActionFactory { - val testPendingIntent = PendingIntent.getActivity(context, 0, Intent(testAction),0) + val testPendingIntent = PendingIntent.getActivity(context, 0, Intent(testAction), 0) val targetIntent = Intent() - val action = ChooserAction.Builder( - Icon.createWithResource("", Resources.ID_NULL), - actionLabel, - testPendingIntent - ).build() + val action = + ChooserAction.Builder( + Icon.createWithResource("", Resources.ID_NULL), + actionLabel, + testPendingIntent + ) + .build() val chooserRequest = mock() whenever(chooserRequest.targetIntent).thenReturn(targetIntent) whenever(chooserRequest.chooserActions).thenReturn(ImmutableList.of(action)) if (includeModifyShare) { - val modifyShare = ChooserAction.Builder( - Icon.createWithResource("", Resources.ID_NULL), - modifyShareLabel, - testPendingIntent - ).build() + val modifyShare = + ChooserAction.Builder( + Icon.createWithResource("", Resources.ID_NULL), + modifyShareLabel, + testPendingIntent + ) + .build() whenever(chooserRequest.modifyShareAction).thenReturn(modifyShare) } return ChooserActionFactory( context, chooserRequest, - mock(), + mock(), logger, - Consumer{}, - Callable{null}, - mock(), - resultConsumer) + {}, + { null }, + mock(), + resultConsumer + ) } -} \ No newline at end of file +} -- cgit v1.2.3-59-g8ed1b From 114b886b4aaf3341b4072f184c96cc71d4763f76 Mon Sep 17 00:00:00 2001 From: Govinda Wasserman Date: Wed, 21 Jun 2023 14:19:39 -0400 Subject: Creates application-level and activity-level dagger graphs Provides Activities and BroadcastReceivers with the ability to use injection in their constructors. Adds ChooserActivity, ResolverActivity, IntentForwarderActivity, and ChooserActivityReEnabler to the dagger graph. Test: Existing tests still pass BUG: 242615621 Change-Id: I0ce3bb225ceab59243833535c3776d9d64672d7d --- Android.bp | 4 ++ AndroidManifest.xml | 14 ++-- .../intentresolver/ApplicationComponentOwner.kt | 15 +++++ .../android/intentresolver/ChooserActivity.java | 3 + .../intentresolver/ChooserActivityReEnabler.kt | 3 +- .../intentresolver/IntentForwarderActivity.java | 7 ++ .../IntentResolverAppComponentFactory.kt | 75 ++++++++++++++++++++++ .../intentresolver/IntentResolverApplication.kt | 28 ++++++++ .../android/intentresolver/ResolverActivity.java | 3 + .../intentresolver/dagger/ActivityBinderModule.kt | 30 +++++++++ .../intentresolver/dagger/ActivityModule.kt | 6 ++ .../android/intentresolver/dagger/ActivityScope.kt | 5 ++ .../intentresolver/dagger/ActivitySubComponent.kt | 18 ++++++ java/src/com/android/intentresolver/dagger/App.kt | 5 ++ .../intentresolver/dagger/ApplicationComponent.kt | 23 +++++++ .../intentresolver/dagger/ApplicationModule.kt | 23 +++++++ .../intentresolver/dagger/ReceiverBinderModule.kt | 18 ++++++ java/tests/AndroidManifest.xml | 7 +- .../com/android/intentresolver/TestApplication.kt | 25 +++++++- 19 files changed, 302 insertions(+), 10 deletions(-) create mode 100644 java/src/com/android/intentresolver/ApplicationComponentOwner.kt create mode 100644 java/src/com/android/intentresolver/IntentResolverAppComponentFactory.kt create mode 100644 java/src/com/android/intentresolver/IntentResolverApplication.kt create mode 100644 java/src/com/android/intentresolver/dagger/ActivityBinderModule.kt create mode 100644 java/src/com/android/intentresolver/dagger/ActivityModule.kt create mode 100644 java/src/com/android/intentresolver/dagger/ActivityScope.kt create mode 100644 java/src/com/android/intentresolver/dagger/ActivitySubComponent.kt create mode 100644 java/src/com/android/intentresolver/dagger/App.kt create mode 100644 java/src/com/android/intentresolver/dagger/ApplicationComponent.kt create mode 100644 java/src/com/android/intentresolver/dagger/ApplicationModule.kt create mode 100644 java/src/com/android/intentresolver/dagger/ReceiverBinderModule.kt (limited to 'java/src') diff --git a/Android.bp b/Android.bp index bab33509..219a79e3 100644 --- a/Android.bp +++ b/Android.bp @@ -75,6 +75,8 @@ android_library { "androidx.lifecycle_lifecycle-extensions", "androidx.lifecycle_lifecycle-runtime-ktx", "androidx.lifecycle_lifecycle-viewmodel-ktx", + "dagger2", + "jsr330", "kotlin-stdlib", "kotlinx_coroutines", "kotlinx-coroutines-android", @@ -83,6 +85,8 @@ android_library { "SystemUIFlagsLib", ], + plugins: ["dagger2-compiler"], + lint: { strict_updatability_linting: false, }, diff --git a/AndroidManifest.xml b/AndroidManifest.xml index da781a22..f9bdc69e 100644 --- a/AndroidManifest.xml +++ b/AndroidManifest.xml @@ -17,10 +17,11 @@ */ --> + xmlns:tools="http://schemas.android.com/tools" + package="com.android.intentresolver" + android:versionCode="0" + android:versionName="2021-11" + coreApp="true"> @@ -39,12 +40,15 @@ + android:supportsRtl="true" + tools:replace="android:appComponentFactory" + android:appComponentFactory=".IntentResolverAppComponentFactory"> Unit) +} diff --git a/java/src/com/android/intentresolver/ChooserActivity.java b/java/src/com/android/intentresolver/ChooserActivity.java index a2dff970..6067266d 100644 --- a/java/src/com/android/intentresolver/ChooserActivity.java +++ b/java/src/com/android/intentresolver/ChooserActivity.java @@ -119,6 +119,8 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.function.Consumer; +import javax.inject.Inject; + /** * The Chooser Activity handles intent resolution specifically for sharing intents - * for example, as generated by {@see android.content.Intent#createChooser(Intent, CharSequence)}. @@ -228,6 +230,7 @@ public class ChooserActivity extends ResolverActivity implements private boolean mExcludeSharedText = false; + @Inject public ChooserActivity() {} @Override diff --git a/java/src/com/android/intentresolver/ChooserActivityReEnabler.kt b/java/src/com/android/intentresolver/ChooserActivityReEnabler.kt index 3236c1be..8c2b3d0d 100644 --- a/java/src/com/android/intentresolver/ChooserActivityReEnabler.kt +++ b/java/src/com/android/intentresolver/ChooserActivityReEnabler.kt @@ -5,11 +5,12 @@ import android.content.ComponentName import android.content.Context import android.content.Intent import android.content.pm.PackageManager +import javax.inject.Inject /** * Ensures that the unbundled version of [ChooserActivity] does not get stuck in a disabled state. */ -class ChooserActivityReEnabler : BroadcastReceiver() { +class ChooserActivityReEnabler @Inject constructor() : BroadcastReceiver() { override fun onReceive(context: Context, intent: Intent) { if (intent.action == Intent.ACTION_BOOT_COMPLETED) { diff --git a/java/src/com/android/intentresolver/IntentForwarderActivity.java b/java/src/com/android/intentresolver/IntentForwarderActivity.java index 5e8945f1..d69a6c71 100644 --- a/java/src/com/android/intentresolver/IntentForwarderActivity.java +++ b/java/src/com/android/intentresolver/IntentForwarderActivity.java @@ -57,6 +57,8 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import javax.inject.Inject; + /** * This is used in conjunction with * {@link DevicePolicyManager#addCrossProfileIntentFilter} to enable intents to @@ -84,6 +86,11 @@ public class IntentForwarderActivity extends Activity { private MetricsLogger mMetricsLogger; protected ExecutorService mExecutorService; + @Inject + public IntentForwarderActivity() { + super(); + } + @Override protected void onDestroy() { super.onDestroy(); diff --git a/java/src/com/android/intentresolver/IntentResolverAppComponentFactory.kt b/java/src/com/android/intentresolver/IntentResolverAppComponentFactory.kt new file mode 100644 index 00000000..eef49ec4 --- /dev/null +++ b/java/src/com/android/intentresolver/IntentResolverAppComponentFactory.kt @@ -0,0 +1,75 @@ +package com.android.intentresolver + +import android.app.Activity +import android.app.Application +import android.content.BroadcastReceiver +import android.content.Intent +import android.util.Log +import androidx.core.app.AppComponentFactory +import com.android.intentresolver.dagger.ActivitySubComponent +import javax.inject.Inject +import javax.inject.Provider + +/** [AppComponentFactory] that performs dagger injection on Android components. */ +class IntentResolverAppComponentFactory : AppComponentFactory() { + + @set:Inject lateinit var activitySubComponentBuilder: Provider + @set:Inject + lateinit var receivers: Map, @JvmSuppressWildcards Provider> + + override fun instantiateApplicationCompat(cl: ClassLoader, className: String): Application { + val app = super.instantiateApplicationCompat(cl, className) + if (app !is ApplicationComponentOwner) { + throw RuntimeException("App must be ApplicationComponentOwner") + } + app.doWhenApplicationComponentReady { it.inject(this) } + return app + } + + override fun instantiateActivityCompat( + cl: ClassLoader, + className: String, + intent: Intent?, + ): Activity { + return runCatching { + val activities = activitySubComponentBuilder.get().build().activities() + instantiate(className, activities) + } + .onFailure { + if (it is UninitializedPropertyAccessException) { + // This should never happen but if it did it would cause errors that could + // be very difficult to identify, so we log it out of an abundance of + // caution. + Log.e(TAG, "Tried to instantiate $className before AppComponent", it) + } + } + .getOrNull() + ?: super.instantiateActivityCompat(cl, className, intent) + } + + override fun instantiateReceiverCompat( + cl: ClassLoader, + className: String, + intent: Intent?, + ): BroadcastReceiver { + return instantiate(className, receivers) + ?: super.instantiateReceiverCompat(cl, className, intent) + } + + private fun instantiate(className: String, providers: Map, Provider>): T? { + return runCatching { providers[Class.forName(className)]?.get() } + .onFailure { + if (it is UninitializedPropertyAccessException) { + // This should never happen but if it did it would cause errors that could + // be very difficult to identify, so we log it out of an abundance of + // caution. + Log.e(TAG, "Tried to instantiate $className before AppComponent", it) + } + } + .getOrNull() + } + + companion object { + private const val TAG = "IRAppComponentFactory" + } +} diff --git a/java/src/com/android/intentresolver/IntentResolverApplication.kt b/java/src/com/android/intentresolver/IntentResolverApplication.kt new file mode 100644 index 00000000..73d15f3c --- /dev/null +++ b/java/src/com/android/intentresolver/IntentResolverApplication.kt @@ -0,0 +1,28 @@ +package com.android.intentresolver + +import android.app.Application +import com.android.intentresolver.dagger.ApplicationComponent +import com.android.intentresolver.dagger.DaggerApplicationComponent + +/** [Application] that maintains the [ApplicationComponent]. */ +class IntentResolverApplication : Application(), ApplicationComponentOwner { + + private lateinit var applicationComponent: ApplicationComponent + + private val pendingDaggerActions = mutableSetOf<(ApplicationComponent) -> Unit>() + + override fun onCreate() { + super.onCreate() + applicationComponent = DaggerApplicationComponent.builder().application(this).build() + pendingDaggerActions.forEach { it.invoke(applicationComponent) } + pendingDaggerActions.clear() + } + + override fun doWhenApplicationComponentReady(action: (ApplicationComponent) -> Unit) { + if (this::applicationComponent.isInitialized) { + action.invoke(applicationComponent) + } else { + pendingDaggerActions.add(action) + } + } +} diff --git a/java/src/com/android/intentresolver/ResolverActivity.java b/java/src/com/android/intentresolver/ResolverActivity.java index 57871532..dfc5a021 100644 --- a/java/src/com/android/intentresolver/ResolverActivity.java +++ b/java/src/com/android/intentresolver/ResolverActivity.java @@ -125,6 +125,8 @@ import java.util.Objects; import java.util.Set; import java.util.function.Supplier; +import javax.inject.Inject; + /** * This is a copy of ResolverActivity to support IntentResolver's ChooserActivity. This code is * *not* the resolver that is actually triggered by the system right now (you want @@ -135,6 +137,7 @@ import java.util.function.Supplier; public class ResolverActivity extends FragmentActivity implements ResolverListAdapter.ResolverListCommunicator { + @Inject public ResolverActivity() { mIsIntentPicker = getClass().equals(ResolverActivity.class); } diff --git a/java/src/com/android/intentresolver/dagger/ActivityBinderModule.kt b/java/src/com/android/intentresolver/dagger/ActivityBinderModule.kt new file mode 100644 index 00000000..59b08c2c --- /dev/null +++ b/java/src/com/android/intentresolver/dagger/ActivityBinderModule.kt @@ -0,0 +1,30 @@ +package com.android.intentresolver.dagger + +import android.app.Activity +import com.android.intentresolver.ChooserActivity +import com.android.intentresolver.IntentForwarderActivity +import com.android.intentresolver.ResolverActivity +import dagger.Binds +import dagger.Module +import dagger.multibindings.ClassKey +import dagger.multibindings.IntoMap + +/** Injection instructions for injectable [Activities][Activity]. */ +@Module +interface ActivityBinderModule { + + @Binds + @IntoMap + @ClassKey(ChooserActivity::class) + fun bindChooserActivity(activity: ChooserActivity): Activity + + @Binds + @IntoMap + @ClassKey(ResolverActivity::class) + fun bindResolverActivity(activity: ResolverActivity): Activity + + @Binds + @IntoMap + @ClassKey(IntentForwarderActivity::class) + fun bindIntentForwarderActivity(activity: IntentForwarderActivity): Activity +} diff --git a/java/src/com/android/intentresolver/dagger/ActivityModule.kt b/java/src/com/android/intentresolver/dagger/ActivityModule.kt new file mode 100644 index 00000000..a1b7551a --- /dev/null +++ b/java/src/com/android/intentresolver/dagger/ActivityModule.kt @@ -0,0 +1,6 @@ +package com.android.intentresolver.dagger + +import dagger.Module + +/** Injections for the [ActivitySubComponent] */ +@Module(includes = [ActivityBinderModule::class]) interface ActivityModule diff --git a/java/src/com/android/intentresolver/dagger/ActivityScope.kt b/java/src/com/android/intentresolver/dagger/ActivityScope.kt new file mode 100644 index 00000000..dc3a8bef --- /dev/null +++ b/java/src/com/android/intentresolver/dagger/ActivityScope.kt @@ -0,0 +1,5 @@ +package com.android.intentresolver.dagger + +import javax.inject.Scope + +@MustBeDocumented @Retention(AnnotationRetention.RUNTIME) @Scope annotation class ActivityScope diff --git a/java/src/com/android/intentresolver/dagger/ActivitySubComponent.kt b/java/src/com/android/intentresolver/dagger/ActivitySubComponent.kt new file mode 100644 index 00000000..092b9088 --- /dev/null +++ b/java/src/com/android/intentresolver/dagger/ActivitySubComponent.kt @@ -0,0 +1,18 @@ +package com.android.intentresolver.dagger + +import android.app.Activity +import dagger.Subcomponent +import javax.inject.Provider + +/** Subcomponent for injections across the life of an Activity. */ +@ActivityScope +@Subcomponent(modules = [ActivityModule::class]) +interface ActivitySubComponent { + + @Subcomponent.Builder + interface Builder { + fun build(): ActivitySubComponent + } + + fun activities(): Map, @JvmSuppressWildcards Provider> +} diff --git a/java/src/com/android/intentresolver/dagger/App.kt b/java/src/com/android/intentresolver/dagger/App.kt new file mode 100644 index 00000000..a16272e8 --- /dev/null +++ b/java/src/com/android/intentresolver/dagger/App.kt @@ -0,0 +1,5 @@ +package com.android.intentresolver.dagger + +import javax.inject.Qualifier + +@MustBeDocumented @Retention(AnnotationRetention.RUNTIME) @Qualifier annotation class App diff --git a/java/src/com/android/intentresolver/dagger/ApplicationComponent.kt b/java/src/com/android/intentresolver/dagger/ApplicationComponent.kt new file mode 100644 index 00000000..ed337e8b --- /dev/null +++ b/java/src/com/android/intentresolver/dagger/ApplicationComponent.kt @@ -0,0 +1,23 @@ +package com.android.intentresolver.dagger + +import android.app.Application +import com.android.intentresolver.IntentResolverAppComponentFactory +import dagger.BindsInstance +import dagger.Component +import javax.inject.Singleton + +/** Top level component for injections across the life of the process. */ +@Singleton +@Component(modules = [ApplicationModule::class]) +interface ApplicationComponent { + + @Component.Builder + interface Builder { + + @BindsInstance fun application(application: Application): Builder + + fun build(): ApplicationComponent + } + + fun inject(appComponentFactory: IntentResolverAppComponentFactory) +} diff --git a/java/src/com/android/intentresolver/dagger/ApplicationModule.kt b/java/src/com/android/intentresolver/dagger/ApplicationModule.kt new file mode 100644 index 00000000..f9285c25 --- /dev/null +++ b/java/src/com/android/intentresolver/dagger/ApplicationModule.kt @@ -0,0 +1,23 @@ +package com.android.intentresolver.dagger + +import android.app.Application +import android.content.Context +import dagger.Module +import dagger.Provides +import javax.inject.Singleton + +/** Injections for the [ApplicationComponent] */ +@Module( + subcomponents = [ActivitySubComponent::class], + includes = [ReceiverBinderModule::class], +) +abstract class ApplicationModule { + + companion object { + @Provides + @Singleton + @App + fun provideApplicationContext(application: Application): Context = + application.applicationContext + } +} diff --git a/java/src/com/android/intentresolver/dagger/ReceiverBinderModule.kt b/java/src/com/android/intentresolver/dagger/ReceiverBinderModule.kt new file mode 100644 index 00000000..1f587f18 --- /dev/null +++ b/java/src/com/android/intentresolver/dagger/ReceiverBinderModule.kt @@ -0,0 +1,18 @@ +package com.android.intentresolver.dagger + +import android.content.BroadcastReceiver +import com.android.intentresolver.ChooserActivityReEnabler +import dagger.Binds +import dagger.Module +import dagger.multibindings.ClassKey +import dagger.multibindings.IntoMap + +/** Injection instructions for injectable [BroadcastReceivers][BroadcastReceiver] */ +@Module +interface ReceiverBinderModule { + + @Binds + @IntoMap + @ClassKey(ChooserActivityReEnabler::class) + fun bindChooserActivityReEnabler(receiver: ChooserActivityReEnabler): BroadcastReceiver +} diff --git a/java/tests/AndroidManifest.xml b/java/tests/AndroidManifest.xml index 05830c4c..b397db5f 100644 --- a/java/tests/AndroidManifest.xml +++ b/java/tests/AndroidManifest.xml @@ -15,7 +15,8 @@ --> + xmlns:tools="http://schemas.android.com/tools" + package="com.android.intentresolver.tests"> @@ -25,7 +26,9 @@ - + diff --git a/java/tests/src/com/android/intentresolver/TestApplication.kt b/java/tests/src/com/android/intentresolver/TestApplication.kt index 849cfbab..f0761fbd 100644 --- a/java/tests/src/com/android/intentresolver/TestApplication.kt +++ b/java/tests/src/com/android/intentresolver/TestApplication.kt @@ -19,9 +19,30 @@ package com.android.intentresolver import android.app.Application import android.content.Context import android.os.UserHandle +import com.android.intentresolver.dagger.ApplicationComponent +import com.android.intentresolver.dagger.DaggerApplicationComponent -class TestApplication : Application() { +class TestApplication : Application(), ApplicationComponentOwner { + + private lateinit var applicationComponent: ApplicationComponent + + private val pendingDaggerActions = mutableSetOf<(ApplicationComponent) -> Unit>() + + override fun onCreate() { + super.onCreate() + applicationComponent = DaggerApplicationComponent.builder().application(this).build() + pendingDaggerActions.forEach { it.invoke(applicationComponent) } + pendingDaggerActions.clear() + } + + override fun doWhenApplicationComponentReady(action: (ApplicationComponent) -> Unit) { + if (this::applicationComponent.isInitialized) { + action.invoke(applicationComponent) + } else { + pendingDaggerActions.add(action) + } + } // return the current context as a work profile doesn't really exist in these tests override fun createContextAsUser(user: UserHandle, flags: Int): Context = this -} \ No newline at end of file +} -- cgit v1.2.3-59-g8ed1b From 3a0ac7d534fb3dda80de82a889511344850b2c3f Mon Sep 17 00:00:00 2001 From: Joshua Trask Date: Mon, 5 Jun 2023 17:38:47 +0000 Subject: Add a null check for a rare error case. This just addresses the immediate NPE and doesn't explain the root cause of the null value. I expect we *shouldn't* be seeing nulls, but in the context of the other legacy code it seems to make sense to take appropriate precautions against them anyways. This could've been a regression from the legacy code, since an older version used an `instanceof` check that would've correctly skipped over nulls. I've chosen the narrowest fix to reproduce that logic, because it's difficult to reason about other more-"elegant" potential improvements (as briefly noted in b/285565219). There's no obvious way to reproduce the null condition, so this isn't tested to "fix"/"work in" the underlying error case overall (although it should be clear from the code that this would prevent the specific NPE we have at hand). Instead the test runs for this CL just validate that the new null-check condition doesn't add regressions to our currently-tested cases (where the result is non-null). Bug: 285565219 Test: UnbundledChooserActivityTest, CtsSharesheetDeviceTest Change-Id: I3b367206b4eefad579dbf613dfd38f485720cf65 --- java/src/com/android/intentresolver/ChooserActivity.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) (limited to 'java/src') diff --git a/java/src/com/android/intentresolver/ChooserActivity.java b/java/src/com/android/intentresolver/ChooserActivity.java index 63ac6435..eb38ad54 100644 --- a/java/src/com/android/intentresolver/ChooserActivity.java +++ b/java/src/com/android/intentresolver/ChooserActivity.java @@ -883,7 +883,7 @@ public class ChooserActivity extends ResolverActivity implements final long selectionCost = System.currentTimeMillis() - mChooserShownTime; - if (targetInfo.isMultiDisplayResolveInfo()) { + if ((targetInfo != null) && targetInfo.isMultiDisplayResolveInfo()) { MultiDisplayResolveInfo mti = (MultiDisplayResolveInfo) targetInfo; if (!mti.hasSelected()) { // Add userHandle based badge to the stackedAppDialogBox. @@ -900,7 +900,17 @@ public class ChooserActivity extends ResolverActivity implements super.startSelected(which, always, filtered); - if (currentListAdapter.getCount() > 0) { + // TODO: both of the conditions around this switch logic *should* be redundant, and + // can be removed if certain invariants can be guaranteed. In particular, it seems + // like targetInfo (from `ChooserListAdapter.targetInfoForPosition()`) is *probably* + // expected to be null only at out-of-bounds indexes where `getPositionTargetType()` + // returns TARGET_BAD; then the switch falls through to a default no-op, and we don't + // need to null-check targetInfo. We only need the null check if it's possible that + // the ChooserListAdapter contains null elements "in the middle" of its list data, + // such that they're classified as belonging to one of the real target types. That + // should probably never happen. But why would this method ever be invoked with a + // null target at all? Even an out-of-bounds index should never be "selected"... + if ((currentListAdapter.getCount() > 0) && (targetInfo != null)) { switch (currentListAdapter.getPositionTargetType(which)) { case ChooserListAdapter.TARGET_SERVICE: getChooserActivityLogger().logShareTargetSelected( -- cgit v1.2.3-59-g8ed1b From e5b06cf0322d094198b73db009dd6905abcd290e Mon Sep 17 00:00:00 2001 From: Andrey Epin Date: Mon, 10 Jul 2023 15:20:31 -0700 Subject: Make ShortcutLoader fully mockable Fix: 290618729 Test: inject a call to ChooserActivity#handlePackagesChanged() into the test, observe the crash; then apply the fix and observe that the crash is not happening anymore. Change-Id: I5ea92e07995790bdbf7f2d23e26739d129f20dcd --- java/src/com/android/intentresolver/shortcuts/ShortcutLoader.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'java/src') diff --git a/java/src/com/android/intentresolver/shortcuts/ShortcutLoader.kt b/java/src/com/android/intentresolver/shortcuts/ShortcutLoader.kt index 3ffbe039..f05542e2 100644 --- a/java/src/com/android/intentresolver/shortcuts/ShortcutLoader.kt +++ b/java/src/com/android/intentresolver/shortcuts/ShortcutLoader.kt @@ -136,7 +136,8 @@ constructor( } /** Clear application targets (see [updateAppTargets] and initiate shrtcuts loading. */ - fun reset() { + @OpenForTesting + open fun reset() { Log.d(TAG, "reset shortcut loader for user $userHandle") appTargetSource.tryEmit(null) shortcutSource.tryEmit(null) -- cgit v1.2.3-59-g8ed1b From 2adcb00136f8486738e7117c920566fa1c1ac5d5 Mon Sep 17 00:00:00 2001 From: Jeff Chang Date: Fri, 21 Jul 2023 03:46:04 +0000 Subject: Ignore launch_adjacent when launch from App picker When a resolver activity is in split screen, starting an activity with launch_adjacent from resolver activity introduces a poor user experience for split screen mode. This CL keeps the resolver activity and the new activity on the same side of spit. Bug: 289909905 Test: 1. Make split (A|ResolverActivity) 2. Launch Activity B from ResolverActivity 3. Verfiy split (A|B) Change-Id: Ia25e62b27f69740f8969e498d44059cc39dabe32 --- java/src/com/android/intentresolver/ResolverActivity.java | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'java/src') diff --git a/java/src/com/android/intentresolver/ResolverActivity.java b/java/src/com/android/intentresolver/ResolverActivity.java index 5057d321..252d0a41 100644 --- a/java/src/com/android/intentresolver/ResolverActivity.java +++ b/java/src/com/android/intentresolver/ResolverActivity.java @@ -27,6 +27,7 @@ import static android.app.admin.DevicePolicyResources.Strings.Core.RESOLVER_PERS import static android.app.admin.DevicePolicyResources.Strings.Core.RESOLVER_WORK_PROFILE_NOT_SUPPORTED; import static android.app.admin.DevicePolicyResources.Strings.Core.RESOLVER_WORK_TAB; import static android.app.admin.DevicePolicyResources.Strings.Core.RESOLVER_WORK_TAB_ACCESSIBILITY; +import static android.content.Intent.FLAG_ACTIVITY_LAUNCH_ADJACENT; import static android.content.Intent.FLAG_ACTIVITY_NEW_TASK; import static android.content.PermissionChecker.PID_UNKNOWN; import static android.stats.devicepolicy.nano.DevicePolicyEnums.RESOLVER_EMPTY_STATE_NO_SHARING_TO_PERSONAL; @@ -1160,6 +1161,12 @@ public class ResolverActivity extends FragmentActivity implements // flag set, we are now losing it. That should be a very rare case // and we can live with this. intent.setFlags(intent.getFlags() & ~Intent.FLAG_ACTIVITY_EXCLUDE_FROM_RECENTS); + + // If FLAG_ACTIVITY_LAUNCH_ADJACENT was set, ResolverActivity was opened in the alternate + // side, which means we want to open the target app on the same side as ResolverActivity. + if ((intent.getFlags() & FLAG_ACTIVITY_LAUNCH_ADJACENT) != 0) { + intent.setFlags(intent.getFlags() & ~FLAG_ACTIVITY_LAUNCH_ADJACENT); + } return intent; } -- cgit v1.2.3-59-g8ed1b From 58534f4139ebdec4ef9fed16688fda0b9798477d Mon Sep 17 00:00:00 2001 From: Mark Renouf Date: Fri, 28 Jul 2023 10:42:00 -0400 Subject: Renames ChooserActivityLogger => logging/EventLog Change-Id: Ia742b6f425c1f1df84b58f7e4fe81eebb8c20117 --- .../intentresolver/ChooserActionFactory.java | 17 +- .../android/intentresolver/ChooserActivity.java | 49 +- .../intentresolver/ChooserActivityLogger.java | 538 -------------------- .../android/intentresolver/ChooserListAdapter.java | 9 +- .../android/intentresolver/logging/EventLog.java | 539 +++++++++++++++++++++ .../model/AbstractResolverComparator.java | 16 +- .../AppPredictionServiceResolverComparator.java | 8 +- .../ResolverRankerServiceResolverComparator.java | 10 +- .../intentresolver/ChooserActionFactoryTest.kt | 5 +- .../intentresolver/ChooserActivityLoggerTest.java | 422 ---------------- .../ChooserActivityOverrideData.java | 5 +- .../intentresolver/ChooserListAdapterTest.kt | 5 +- .../intentresolver/ChooserWrapperActivity.java | 7 +- .../android/intentresolver/IChooserWrapper.java | 3 +- .../UnbundledChooserActivityTest.java | 31 +- .../intentresolver/logging/EventLogTest.java | 422 ++++++++++++++++ 16 files changed, 1048 insertions(+), 1038 deletions(-) delete mode 100644 java/src/com/android/intentresolver/ChooserActivityLogger.java create mode 100644 java/src/com/android/intentresolver/logging/EventLog.java delete mode 100644 java/tests/src/com/android/intentresolver/ChooserActivityLoggerTest.java create mode 100644 java/tests/src/com/android/intentresolver/logging/EventLogTest.java (limited to 'java/src') diff --git a/java/src/com/android/intentresolver/ChooserActionFactory.java b/java/src/com/android/intentresolver/ChooserActionFactory.java index 06c7e8d7..a54e8c62 100644 --- a/java/src/com/android/intentresolver/ChooserActionFactory.java +++ b/java/src/com/android/intentresolver/ChooserActionFactory.java @@ -37,6 +37,7 @@ import android.view.View; import com.android.intentresolver.chooser.DisplayResolveInfo; import com.android.intentresolver.chooser.TargetInfo; import com.android.intentresolver.contentpreview.ChooserContentPreviewUi; +import com.android.intentresolver.logging.EventLog; import com.android.intentresolver.widget.ActionRow; import com.android.internal.annotations.VisibleForTesting; @@ -97,7 +98,7 @@ public final class ChooserActionFactory implements ChooserContentPreviewUi.Actio private final @Nullable ChooserAction mModifyShareAction; private final Consumer mExcludeSharedTextAction; private final Consumer mFinishCallback; - private final ChooserActivityLogger mLogger; + private final EventLog mLogger; /** * @param context @@ -116,7 +117,7 @@ public final class ChooserActionFactory implements ChooserContentPreviewUi.Actio Context context, ChooserRequestParameters chooserRequest, ChooserIntegratedDeviceComponents integratedDeviceComponents, - ChooserActivityLogger logger, + EventLog logger, Consumer onUpdateSharedTextIsExcluded, Callable firstVisibleImageQuery, ActionActivityStarter activityStarter, @@ -152,7 +153,7 @@ public final class ChooserActionFactory implements ChooserContentPreviewUi.Actio List customActions, @Nullable ChooserAction modifyShareAction, Consumer onUpdateSharedTextIsExcluded, - ChooserActivityLogger logger, + EventLog logger, Consumer finishCallback) { mContext = context; mCopyButtonRunnable = copyButtonRunnable; @@ -208,7 +209,7 @@ public final class ChooserActionFactory implements ChooserContentPreviewUi.Actio mModifyShareAction, mFinishCallback, () -> { - mLogger.logActionSelected(ChooserActivityLogger.SELECTION_TYPE_MODIFY_SHARE); + mLogger.logActionSelected(EventLog.SELECTION_TYPE_MODIFY_SHARE); }); } @@ -232,7 +233,7 @@ public final class ChooserActionFactory implements ChooserContentPreviewUi.Actio Intent targetIntent, String referrerPackageName, Consumer finishCallback, - ChooserActivityLogger logger) { + EventLog logger) { final ClipData clipData; try { clipData = extractTextToCopy(targetIntent); @@ -248,7 +249,7 @@ public final class ChooserActionFactory implements ChooserContentPreviewUi.Actio Context.CLIPBOARD_SERVICE); clipboardManager.setPrimaryClipAsPackage(clipData, referrerPackageName); - logger.logActionSelected(ChooserActivityLogger.SELECTION_TYPE_COPY); + logger.logActionSelected(EventLog.SELECTION_TYPE_COPY); finishCallback.accept(Activity.RESULT_OK); }; } @@ -327,10 +328,10 @@ public final class ChooserActionFactory implements ChooserContentPreviewUi.Actio TargetInfo editSharingTarget, Callable firstVisibleImageQuery, ActionActivityStarter activityStarter, - ChooserActivityLogger logger) { + EventLog logger) { return () -> { // Log share completion via edit. - logger.logActionSelected(ChooserActivityLogger.SELECTION_TYPE_EDIT); + logger.logActionSelected(EventLog.SELECTION_TYPE_EDIT); View firstImageView = null; try { diff --git a/java/src/com/android/intentresolver/ChooserActivity.java b/java/src/com/android/intentresolver/ChooserActivity.java index 7ebcf9f9..b041475b 100644 --- a/java/src/com/android/intentresolver/ChooserActivity.java +++ b/java/src/com/android/intentresolver/ChooserActivity.java @@ -92,6 +92,7 @@ import com.android.intentresolver.flags.FeatureFlagRepositoryFactory; import com.android.intentresolver.grid.ChooserGridAdapter; import com.android.intentresolver.icons.DefaultTargetDataLoader; import com.android.intentresolver.icons.TargetDataLoader; +import com.android.intentresolver.logging.EventLog; import com.android.intentresolver.measurements.Tracer; import com.android.intentresolver.model.AbstractResolverComparator; import com.android.intentresolver.model.AppPredictionServiceResolverComparator; @@ -193,7 +194,7 @@ public class ChooserActivity extends ResolverActivity implements private boolean mShouldDisplayLandscape; // statsd logger wrapper - protected ChooserActivityLogger mChooserActivityLogger; + protected EventLog mEventLog; private long mChooserShownTime; protected boolean mIsSuccessfullySelected; @@ -236,7 +237,7 @@ public class ChooserActivity extends ResolverActivity implements final long intentReceivedTime = System.currentTimeMillis(); mLatencyTracker.onActionStart(ACTION_LOAD_SHARE_SHEET); - getChooserActivityLogger().logSharesheetTriggered(); + getEventLog().logSharesheetTriggered(); mFeatureFlagRepository = createFeatureFlagRepository(); mIntegratedDeviceComponents = getIntegratedDeviceComponents(); @@ -314,7 +315,7 @@ public class ChooserActivity extends ResolverActivity implements mChooserShownTime = System.currentTimeMillis(); final long systemCost = mChooserShownTime - intentReceivedTime; - getChooserActivityLogger().logChooserActivityShown( + getEventLog().logChooserActivityShown( isWorkProfile(), mChooserRequest.getTargetType(), systemCost); if (mResolverDrawerLayout != null) { @@ -323,7 +324,7 @@ public class ChooserActivity extends ResolverActivity implements mResolverDrawerLayout.setOnCollapsedChangedListener( isCollapsed -> { mChooserMultiProfilePagerAdapter.setIsCollapsed(isCollapsed); - getChooserActivityLogger().logSharesheetExpansionChanged(isCollapsed); + getEventLog().logSharesheetExpansionChanged(isCollapsed); }); } @@ -331,7 +332,7 @@ public class ChooserActivity extends ResolverActivity implements Log.d(TAG, "System Time Cost is " + systemCost); } - getChooserActivityLogger().logShareStarted( + getEventLog().logShareStarted( getReferrerPackageName(), mChooserRequest.getTargetType(), mChooserRequest.getCallerChooserTargets().size(), @@ -550,7 +551,7 @@ public class ChooserActivity extends ResolverActivity implements if (shouldShowStickyContentPreview() || mChooserMultiProfilePagerAdapter .getCurrentRootAdapter().getSystemRowCount() != 0) { - getChooserActivityLogger().logActionShareWithPreview( + getEventLog().logActionShareWithPreview( mChooserContentPreviewUi.getPreferredContentPreview()); } return postRebuildListInternal(rebuildCompleted); @@ -910,8 +911,8 @@ public class ChooserActivity extends ResolverActivity implements if ((currentListAdapter.getCount() > 0) && (targetInfo != null)) { switch (currentListAdapter.getPositionTargetType(which)) { case ChooserListAdapter.TARGET_SERVICE: - getChooserActivityLogger().logShareTargetSelected( - ChooserActivityLogger.SELECTION_TYPE_SERVICE, + getEventLog().logShareTargetSelected( + EventLog.SELECTION_TYPE_SERVICE, targetInfo.getResolveInfo().activityInfo.processName, which, /* directTargetAlsoRanked= */ getRankedPosition(targetInfo), @@ -924,8 +925,8 @@ public class ChooserActivity extends ResolverActivity implements return; case ChooserListAdapter.TARGET_CALLER: case ChooserListAdapter.TARGET_STANDARD: - getChooserActivityLogger().logShareTargetSelected( - ChooserActivityLogger.SELECTION_TYPE_APP, + getEventLog().logShareTargetSelected( + EventLog.SELECTION_TYPE_APP, targetInfo.getResolveInfo().activityInfo.processName, (which - currentListAdapter.getSurfacedTargetInfo().size()), /* directTargetAlsoRanked= */ -1, @@ -941,8 +942,8 @@ public class ChooserActivity extends ResolverActivity implements // they are from the alphabetical pool. // TODO: why do we log a different selection type if the -1 value already // designates the same condition? - getChooserActivityLogger().logShareTargetSelected( - ChooserActivityLogger.SELECTION_TYPE_STANDARD, + getEventLog().logShareTargetSelected( + EventLog.SELECTION_TYPE_STANDARD, targetInfo.getResolveInfo().activityInfo.processName, /* value= */ -1, /* directTargetAlsoRanked= */ -1, @@ -994,7 +995,7 @@ public class ChooserActivity extends ResolverActivity implements if (profileRecord == null) { return; } - getChooserActivityLogger().logDirectShareTargetReceived( + getEventLog().logDirectShareTargetReceived( MetricsEvent.ACTION_DIRECT_SHARE_TARGETS_LOADED_SHORTCUT_MANAGER, (int) (SystemClock.elapsedRealtime() - profileRecord.loadingStartTime)); } @@ -1128,11 +1129,11 @@ public class ChooserActivity extends ResolverActivity implements } } - protected ChooserActivityLogger getChooserActivityLogger() { - if (mChooserActivityLogger == null) { - mChooserActivityLogger = new ChooserActivityLogger(); + protected EventLog getEventLog() { + if (mEventLog == null) { + mEventLog = new EventLog(); } - return mChooserActivityLogger; + return mEventLog; } public class ChooserListController extends ResolverListController { @@ -1258,7 +1259,7 @@ public class ChooserActivity extends ResolverActivity implements targetIntent, this, context.getPackageManager(), - getChooserActivityLogger(), + getEventLog(), chooserRequest, maxTargetsPerRow, initialIntentsUserSpace, @@ -1282,7 +1283,7 @@ public class ChooserActivity extends ResolverActivity implements AbstractResolverComparator resolverComparator; if (appPredictor != null) { resolverComparator = new AppPredictionServiceResolverComparator(this, getTargetIntent(), - getReferrerPackageName(), appPredictor, userHandle, getChooserActivityLogger(), + getReferrerPackageName(), appPredictor, userHandle, getEventLog(), getIntegratedDeviceComponents().getNearbySharingComponent()); } else { resolverComparator = @@ -1291,7 +1292,7 @@ public class ChooserActivity extends ResolverActivity implements getTargetIntent(), getReferrerPackageName(), null, - getChooserActivityLogger(), + getEventLog(), getResolverRankerServiceUserHandleList(userHandle), getIntegratedDeviceComponents().getNearbySharingComponent()); } @@ -1316,7 +1317,7 @@ public class ChooserActivity extends ResolverActivity implements this, mChooserRequest, mIntegratedDeviceComponents, - getChooserActivityLogger(), + getEventLog(), (isExcluded) -> mExcludeSharedText = isExcluded, this::getFirstVisibleImgPreviewView, new ChooserActionFactory.ActionActivityStarter() { @@ -1531,7 +1532,7 @@ public class ChooserActivity extends ResolverActivity implements Log.d(TAG, "app target loading time " + duration + " ms"); } addCallerChooserTargets(); - getChooserActivityLogger().logSharesheetAppLoadComplete(); + getEventLog().logSharesheetAppLoadComplete(); maybeQueryAdditionalPostProcessingTargets(chooserListAdapter); mLatencyTracker.onActionEnd(ACTION_LOAD_SHARE_SHEET); } @@ -1578,7 +1579,7 @@ public class ChooserActivity extends ResolverActivity implements } logDirectShareTargetReceived(userHandle); sendVoiceChoicesIfNeeded(); - getChooserActivityLogger().logSharesheetDirectLoadComplete(); + getEventLog().logSharesheetDirectLoadComplete(); } private void setupScrollListener() { @@ -1884,7 +1885,7 @@ public class ChooserActivity extends ResolverActivity implements @Override protected void maybeLogProfileChange() { - getChooserActivityLogger().logSharesheetProfileChanged(); + getEventLog().logSharesheetProfileChanged(); } private static class ProfileRecord { diff --git a/java/src/com/android/intentresolver/ChooserActivityLogger.java b/java/src/com/android/intentresolver/ChooserActivityLogger.java deleted file mode 100644 index 1f606f26..00000000 --- a/java/src/com/android/intentresolver/ChooserActivityLogger.java +++ /dev/null @@ -1,538 +0,0 @@ -/* - * Copyright (C) 2020 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.intentresolver; - -import android.annotation.Nullable; -import android.content.Intent; -import android.metrics.LogMaker; -import android.net.Uri; -import android.provider.MediaStore; -import android.util.HashedStringCache; -import android.util.Log; - -import com.android.intentresolver.contentpreview.ContentPreviewType; -import com.android.internal.annotations.VisibleForTesting; -import com.android.internal.logging.InstanceId; -import com.android.internal.logging.InstanceIdSequence; -import com.android.internal.logging.MetricsLogger; -import com.android.internal.logging.UiEvent; -import com.android.internal.logging.UiEventLogger; -import com.android.internal.logging.UiEventLoggerImpl; -import com.android.internal.logging.nano.MetricsProto.MetricsEvent; -import com.android.internal.util.FrameworkStatsLog; - -/** - * Helper for writing Sharesheet atoms to statsd log. - * @hide - */ -public class ChooserActivityLogger { - private static final String TAG = "ChooserActivity"; - private static final boolean DEBUG = true; - - public static final int SELECTION_TYPE_SERVICE = 1; - public static final int SELECTION_TYPE_APP = 2; - public static final int SELECTION_TYPE_STANDARD = 3; - public static final int SELECTION_TYPE_COPY = 4; - public static final int SELECTION_TYPE_NEARBY = 5; - public static final int SELECTION_TYPE_EDIT = 6; - public static final int SELECTION_TYPE_MODIFY_SHARE = 7; - public static final int SELECTION_TYPE_CUSTOM_ACTION = 8; - - /** - * This shim is provided only for testing. In production, clients will only ever use a - * {@link DefaultFrameworkStatsLogger}. - */ - @VisibleForTesting - interface FrameworkStatsLogger { - /** Overload to use for logging {@code FrameworkStatsLog.SHARESHEET_STARTED}. */ - void write( - int frameworkEventId, - int appEventId, - String packageName, - int instanceId, - String mimeType, - int numAppProvidedDirectTargets, - int numAppProvidedAppTargets, - boolean isWorkProfile, - int previewType, - int intentType, - int numCustomActions, - boolean modifyShareActionProvided); - - /** Overload to use for logging {@code FrameworkStatsLog.RANKING_SELECTED}. */ - void write( - int frameworkEventId, - int appEventId, - String packageName, - int instanceId, - int positionPicked, - boolean isPinned); - } - - private static final int SHARESHEET_INSTANCE_ID_MAX = (1 << 13); - - // A small per-notification ID, used for statsd logging. - // TODO: consider precomputing and storing as final. - private static InstanceIdSequence sInstanceIdSequence; - private InstanceId mInstanceId; - - private final UiEventLogger mUiEventLogger; - private final FrameworkStatsLogger mFrameworkStatsLogger; - private final MetricsLogger mMetricsLogger; - - public ChooserActivityLogger() { - this(new UiEventLoggerImpl(), new DefaultFrameworkStatsLogger(), new MetricsLogger()); - } - - @VisibleForTesting - ChooserActivityLogger( - UiEventLogger uiEventLogger, - FrameworkStatsLogger frameworkLogger, - MetricsLogger metricsLogger) { - mUiEventLogger = uiEventLogger; - mFrameworkStatsLogger = frameworkLogger; - mMetricsLogger = metricsLogger; - } - - /** Records metrics for the start time of the {@link ChooserActivity}. */ - public void logChooserActivityShown( - boolean isWorkProfile, String targetMimeType, long systemCost) { - mMetricsLogger.write(new LogMaker(MetricsEvent.ACTION_ACTIVITY_CHOOSER_SHOWN) - .setSubtype( - isWorkProfile ? MetricsEvent.MANAGED_PROFILE : MetricsEvent.PARENT_PROFILE) - .addTaggedData(MetricsEvent.FIELD_SHARESHEET_MIMETYPE, targetMimeType) - .addTaggedData(MetricsEvent.FIELD_TIME_TO_APP_TARGETS, systemCost)); - } - - /** Logs a UiEventReported event for the system sharesheet completing initial start-up. */ - public void logShareStarted( - String packageName, - String mimeType, - int appProvidedDirect, - int appProvidedApp, - boolean isWorkprofile, - int previewType, - String intent, - int customActionCount, - boolean modifyShareActionProvided) { - mFrameworkStatsLogger.write(FrameworkStatsLog.SHARESHEET_STARTED, - /* event_id = 1 */ SharesheetStartedEvent.SHARE_STARTED.getId(), - /* package_name = 2 */ packageName, - /* instance_id = 3 */ getInstanceId().getId(), - /* mime_type = 4 */ mimeType, - /* num_app_provided_direct_targets = 5 */ appProvidedDirect, - /* num_app_provided_app_targets = 6 */ appProvidedApp, - /* is_workprofile = 7 */ isWorkprofile, - /* previewType = 8 */ typeFromPreviewInt(previewType), - /* intentType = 9 */ typeFromIntentString(intent), - /* num_provided_custom_actions = 10 */ customActionCount, - /* modify_share_action_provided = 11 */ modifyShareActionProvided); - } - - /** - * Log that a custom action has been tapped by the user. - * - * @param positionPicked index of the custom action within the list of custom actions. - */ - public void logCustomActionSelected(int positionPicked) { - mFrameworkStatsLogger.write(FrameworkStatsLog.RANKING_SELECTED, - /* event_id = 1 */ - SharesheetTargetSelectedEvent.SHARESHEET_CUSTOM_ACTION_SELECTED.getId(), - /* package_name = 2 */ null, - /* instance_id = 3 */ getInstanceId().getId(), - /* position_picked = 4 */ positionPicked, - /* is_pinned = 5 */ false); - } - - /** - * Logs a UiEventReported event for the system sharesheet when the user selects a target. - * TODO: document parameters and/or consider breaking up by targetType so we don't have to - * support an overly-generic signature. - */ - public void logShareTargetSelected( - int targetType, - String packageName, - int positionPicked, - int directTargetAlsoRanked, - int numCallerProvided, - @Nullable HashedStringCache.HashResult directTargetHashed, - boolean isPinned, - boolean successfullySelected, - long selectionCost) { - mFrameworkStatsLogger.write(FrameworkStatsLog.RANKING_SELECTED, - /* event_id = 1 */ SharesheetTargetSelectedEvent.fromTargetType(targetType).getId(), - /* package_name = 2 */ packageName, - /* instance_id = 3 */ getInstanceId().getId(), - /* position_picked = 4 */ positionPicked, - /* is_pinned = 5 */ isPinned); - - int category = getTargetSelectionCategory(targetType); - if (category != 0) { - LogMaker targetLogMaker = new LogMaker(category).setSubtype(positionPicked); - if (directTargetHashed != null) { - targetLogMaker.addTaggedData( - MetricsEvent.FIELD_HASHED_TARGET_NAME, directTargetHashed.hashedString); - targetLogMaker.addTaggedData( - MetricsEvent.FIELD_HASHED_TARGET_SALT_GEN, - directTargetHashed.saltGeneration); - targetLogMaker.addTaggedData(MetricsEvent.FIELD_RANKED_POSITION, - directTargetAlsoRanked); - } - targetLogMaker.addTaggedData(MetricsEvent.FIELD_IS_CATEGORY_USED, numCallerProvided); - mMetricsLogger.write(targetLogMaker); - } - - if (successfullySelected) { - if (DEBUG) { - Log.d(TAG, "User Selection Time Cost is " + selectionCost); - Log.d(TAG, "position of selected app/service/caller is " + positionPicked); - } - MetricsLogger.histogram( - null, "user_selection_cost_for_smart_sharing", (int) selectionCost); - MetricsLogger.histogram(null, "app_position_for_smart_sharing", positionPicked); - } - } - - /** Log when direct share targets were received. */ - public void logDirectShareTargetReceived(int category, int latency) { - mMetricsLogger.write(new LogMaker(category).setSubtype(latency)); - } - - /** - * Log when we display a preview UI of the specified {@code previewType} as part of our - * Sharesheet session. - */ - public void logActionShareWithPreview(int previewType) { - mMetricsLogger.write( - new LogMaker(MetricsEvent.ACTION_SHARE_WITH_PREVIEW).setSubtype(previewType)); - } - - /** Log when the user selects an action button with the specified {@code targetType}. */ - public void logActionSelected(int targetType) { - if (targetType == SELECTION_TYPE_COPY) { - LogMaker targetLogMaker = new LogMaker( - MetricsEvent.ACTION_ACTIVITY_CHOOSER_PICKED_SYSTEM_TARGET).setSubtype(1); - mMetricsLogger.write(targetLogMaker); - } - mFrameworkStatsLogger.write(FrameworkStatsLog.RANKING_SELECTED, - /* event_id = 1 */ SharesheetTargetSelectedEvent.fromTargetType(targetType).getId(), - /* package_name = 2 */ "", - /* instance_id = 3 */ getInstanceId().getId(), - /* position_picked = 4 */ -1, - /* is_pinned = 5 */ false); - } - - /** Log a warning that we couldn't display the content preview from the supplied {@code uri}. */ - public void logContentPreviewWarning(Uri uri) { - // The ContentResolver already logs the exception. Log something more informative. - Log.w(TAG, "Could not load (" + uri.toString() + ") thumbnail/name for preview. If " - + "desired, consider using Intent#createChooser to launch the ChooserActivity, " - + "and set your Intent's clipData and flags in accordance with that method's " - + "documentation"); - - } - - /** Logs a UiEventReported event for the system sharesheet being triggered by the user. */ - public void logSharesheetTriggered() { - log(SharesheetStandardEvent.SHARESHEET_TRIGGERED, getInstanceId()); - } - - /** Logs a UiEventReported event for the system sharesheet completing loading app targets. */ - public void logSharesheetAppLoadComplete() { - log(SharesheetStandardEvent.SHARESHEET_APP_LOAD_COMPLETE, getInstanceId()); - } - - /** - * Logs a UiEventReported event for the system sharesheet completing loading service targets. - */ - public void logSharesheetDirectLoadComplete() { - log(SharesheetStandardEvent.SHARESHEET_DIRECT_LOAD_COMPLETE, getInstanceId()); - } - - /** - * Logs a UiEventReported event for the system sharesheet timing out loading service targets. - */ - public void logSharesheetDirectLoadTimeout() { - log(SharesheetStandardEvent.SHARESHEET_DIRECT_LOAD_TIMEOUT, getInstanceId()); - } - - /** - * Logs a UiEventReported event for the system sharesheet switching - * between work and main profile. - */ - public void logSharesheetProfileChanged() { - log(SharesheetStandardEvent.SHARESHEET_PROFILE_CHANGED, getInstanceId()); - } - - /** Logs a UiEventReported event for the system sharesheet getting expanded or collapsed. */ - public void logSharesheetExpansionChanged(boolean isCollapsed) { - log(isCollapsed ? SharesheetStandardEvent.SHARESHEET_COLLAPSED : - SharesheetStandardEvent.SHARESHEET_EXPANDED, getInstanceId()); - } - - /** - * Logs a UiEventReported event for the system sharesheet app share ranking timing out. - */ - public void logSharesheetAppShareRankingTimeout() { - log(SharesheetStandardEvent.SHARESHEET_APP_SHARE_RANKING_TIMEOUT, getInstanceId()); - } - - /** - * Logs a UiEventReported event for the system sharesheet when direct share row is empty. - */ - public void logSharesheetEmptyDirectShareRow() { - log(SharesheetStandardEvent.SHARESHEET_EMPTY_DIRECT_SHARE_ROW, getInstanceId()); - } - - /** - * Logs a UiEventReported event for a given share activity - * @param event - * @param instanceId - */ - private void log(UiEventLogger.UiEventEnum event, InstanceId instanceId) { - mUiEventLogger.logWithInstanceId( - event, - 0, - null, - instanceId); - } - - /** - * @return A unique {@link InstanceId} to join across events recorded by this logger instance. - */ - private InstanceId getInstanceId() { - if (mInstanceId == null) { - if (sInstanceIdSequence == null) { - sInstanceIdSequence = new InstanceIdSequence(SHARESHEET_INSTANCE_ID_MAX); - } - mInstanceId = sInstanceIdSequence.newInstanceId(); - } - return mInstanceId; - } - - /** - * The UiEvent enums that this class can log. - */ - enum SharesheetStartedEvent implements UiEventLogger.UiEventEnum { - @UiEvent(doc = "Basic system Sharesheet has started and is visible.") - SHARE_STARTED(228); - - private final int mId; - SharesheetStartedEvent(int id) { - mId = id; - } - @Override - public int getId() { - return mId; - } - } - - /** - * The UiEvent enums that this class can log. - */ - enum SharesheetTargetSelectedEvent implements UiEventLogger.UiEventEnum { - INVALID(0), - @UiEvent(doc = "User selected a service target.") - SHARESHEET_SERVICE_TARGET_SELECTED(232), - @UiEvent(doc = "User selected an app target.") - SHARESHEET_APP_TARGET_SELECTED(233), - @UiEvent(doc = "User selected a standard target.") - SHARESHEET_STANDARD_TARGET_SELECTED(234), - @UiEvent(doc = "User selected the copy target.") - SHARESHEET_COPY_TARGET_SELECTED(235), - @UiEvent(doc = "User selected the nearby target.") - SHARESHEET_NEARBY_TARGET_SELECTED(626), - @UiEvent(doc = "User selected the edit target.") - SHARESHEET_EDIT_TARGET_SELECTED(669), - @UiEvent(doc = "User selected the modify share target.") - SHARESHEET_MODIFY_SHARE_SELECTED(1316), - @UiEvent(doc = "User selected a custom action.") - SHARESHEET_CUSTOM_ACTION_SELECTED(1317); - - private final int mId; - SharesheetTargetSelectedEvent(int id) { - mId = id; - } - @Override public int getId() { - return mId; - } - - public static SharesheetTargetSelectedEvent fromTargetType(int targetType) { - switch(targetType) { - case SELECTION_TYPE_SERVICE: - return SHARESHEET_SERVICE_TARGET_SELECTED; - case SELECTION_TYPE_APP: - return SHARESHEET_APP_TARGET_SELECTED; - case SELECTION_TYPE_STANDARD: - return SHARESHEET_STANDARD_TARGET_SELECTED; - case SELECTION_TYPE_COPY: - return SHARESHEET_COPY_TARGET_SELECTED; - case SELECTION_TYPE_NEARBY: - return SHARESHEET_NEARBY_TARGET_SELECTED; - case SELECTION_TYPE_EDIT: - return SHARESHEET_EDIT_TARGET_SELECTED; - case SELECTION_TYPE_MODIFY_SHARE: - return SHARESHEET_MODIFY_SHARE_SELECTED; - case SELECTION_TYPE_CUSTOM_ACTION: - return SHARESHEET_CUSTOM_ACTION_SELECTED; - default: - return INVALID; - } - } - } - - /** - * The UiEvent enums that this class can log. - */ - enum SharesheetStandardEvent implements UiEventLogger.UiEventEnum { - INVALID(0), - @UiEvent(doc = "User clicked share.") - SHARESHEET_TRIGGERED(227), - @UiEvent(doc = "User changed from work to personal profile or vice versa.") - SHARESHEET_PROFILE_CHANGED(229), - @UiEvent(doc = "User expanded target list.") - SHARESHEET_EXPANDED(230), - @UiEvent(doc = "User collapsed target list.") - SHARESHEET_COLLAPSED(231), - @UiEvent(doc = "Sharesheet app targets is fully populated.") - SHARESHEET_APP_LOAD_COMPLETE(322), - @UiEvent(doc = "Sharesheet direct targets is fully populated.") - SHARESHEET_DIRECT_LOAD_COMPLETE(323), - @UiEvent(doc = "Sharesheet direct targets timed out.") - SHARESHEET_DIRECT_LOAD_TIMEOUT(324), - @UiEvent(doc = "Sharesheet app share ranking timed out.") - SHARESHEET_APP_SHARE_RANKING_TIMEOUT(831), - @UiEvent(doc = "Sharesheet empty direct share row.") - SHARESHEET_EMPTY_DIRECT_SHARE_ROW(828); - - private final int mId; - SharesheetStandardEvent(int id) { - mId = id; - } - @Override public int getId() { - return mId; - } - } - - /** - * Returns the enum used in sharesheet started atom to indicate what preview type was used. - */ - private static int typeFromPreviewInt(int previewType) { - switch(previewType) { - case ContentPreviewType.CONTENT_PREVIEW_IMAGE: - return FrameworkStatsLog.SHARESHEET_STARTED__PREVIEW_TYPE__CONTENT_PREVIEW_IMAGE; - case ContentPreviewType.CONTENT_PREVIEW_FILE: - return FrameworkStatsLog.SHARESHEET_STARTED__PREVIEW_TYPE__CONTENT_PREVIEW_FILE; - case ContentPreviewType.CONTENT_PREVIEW_TEXT: - default: - return FrameworkStatsLog - .SHARESHEET_STARTED__PREVIEW_TYPE__CONTENT_PREVIEW_TYPE_UNKNOWN; - } - } - - /** - * Returns the enum used in sharesheet started atom to indicate what intent triggers the - * ChooserActivity. - */ - private static int typeFromIntentString(String intent) { - if (intent == null) { - return FrameworkStatsLog.SHARESHEET_STARTED__INTENT_TYPE__INTENT_DEFAULT; - } - switch (intent) { - case Intent.ACTION_VIEW: - return FrameworkStatsLog.SHARESHEET_STARTED__INTENT_TYPE__INTENT_ACTION_VIEW; - case Intent.ACTION_EDIT: - return FrameworkStatsLog.SHARESHEET_STARTED__INTENT_TYPE__INTENT_ACTION_EDIT; - case Intent.ACTION_SEND: - return FrameworkStatsLog.SHARESHEET_STARTED__INTENT_TYPE__INTENT_ACTION_SEND; - case Intent.ACTION_SENDTO: - return FrameworkStatsLog.SHARESHEET_STARTED__INTENT_TYPE__INTENT_ACTION_SENDTO; - case Intent.ACTION_SEND_MULTIPLE: - return FrameworkStatsLog - .SHARESHEET_STARTED__INTENT_TYPE__INTENT_ACTION_SEND_MULTIPLE; - case MediaStore.ACTION_IMAGE_CAPTURE: - return FrameworkStatsLog - .SHARESHEET_STARTED__INTENT_TYPE__INTENT_ACTION_IMAGE_CAPTURE; - case Intent.ACTION_MAIN: - return FrameworkStatsLog.SHARESHEET_STARTED__INTENT_TYPE__INTENT_ACTION_MAIN; - default: - return FrameworkStatsLog.SHARESHEET_STARTED__INTENT_TYPE__INTENT_DEFAULT; - } - } - - @VisibleForTesting - static int getTargetSelectionCategory(int targetType) { - switch (targetType) { - case SELECTION_TYPE_SERVICE: - return MetricsEvent.ACTION_ACTIVITY_CHOOSER_PICKED_SERVICE_TARGET; - case SELECTION_TYPE_APP: - return MetricsEvent.ACTION_ACTIVITY_CHOOSER_PICKED_APP_TARGET; - case SELECTION_TYPE_STANDARD: - return MetricsEvent.ACTION_ACTIVITY_CHOOSER_PICKED_STANDARD_TARGET; - default: - return 0; - } - } - - private static class DefaultFrameworkStatsLogger implements FrameworkStatsLogger { - @Override - public void write( - int frameworkEventId, - int appEventId, - String packageName, - int instanceId, - String mimeType, - int numAppProvidedDirectTargets, - int numAppProvidedAppTargets, - boolean isWorkProfile, - int previewType, - int intentType, - int numCustomActions, - boolean modifyShareActionProvided) { - FrameworkStatsLog.write( - frameworkEventId, - /* event_id = 1 */ appEventId, - /* package_name = 2 */ packageName, - /* instance_id = 3 */ instanceId, - /* mime_type = 4 */ mimeType, - /* num_app_provided_direct_targets */ numAppProvidedDirectTargets, - /* num_app_provided_app_targets */ numAppProvidedAppTargets, - /* is_workprofile */ isWorkProfile, - /* previewType = 8 */ previewType, - /* intentType = 9 */ intentType, - /* num_provided_custom_actions = 10 */ numCustomActions, - /* modify_share_action_provided = 11 */ modifyShareActionProvided); - } - - @Override - public void write( - int frameworkEventId, - int appEventId, - String packageName, - int instanceId, - int positionPicked, - boolean isPinned) { - FrameworkStatsLog.write( - frameworkEventId, - /* event_id = 1 */ appEventId, - /* package_name = 2 */ packageName, - /* instance_id = 3 */ instanceId, - /* position_picked = 4 */ positionPicked, - /* is_pinned = 5 */ isPinned); - } - } -} diff --git a/java/src/com/android/intentresolver/ChooserListAdapter.java b/java/src/com/android/intentresolver/ChooserListAdapter.java index de947bd1..e6d6dbf4 100644 --- a/java/src/com/android/intentresolver/ChooserListAdapter.java +++ b/java/src/com/android/intentresolver/ChooserListAdapter.java @@ -49,6 +49,7 @@ import com.android.intentresolver.chooser.NotSelectableTargetInfo; import com.android.intentresolver.chooser.SelectableTargetInfo; import com.android.intentresolver.chooser.TargetInfo; import com.android.intentresolver.icons.TargetDataLoader; +import com.android.intentresolver.logging.EventLog; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.config.sysui.SystemUiDeviceConfigFlags; @@ -80,7 +81,7 @@ public class ChooserListAdapter extends ResolverListAdapter { private final ChooserRequestParameters mChooserRequest; private final int mMaxRankedTargets; - private final ChooserActivityLogger mChooserActivityLogger; + private final EventLog mEventLog; private final Set mRequestedIcons = new HashSet<>(); @@ -139,7 +140,7 @@ public class ChooserListAdapter extends ResolverListAdapter { Intent targetIntent, ResolverListCommunicator resolverListCommunicator, PackageManager packageManager, - ChooserActivityLogger chooserActivityLogger, + EventLog eventLog, ChooserRequestParameters chooserRequest, int maxRankedTargets, UserHandle initialIntentsUserSpace, @@ -165,7 +166,7 @@ public class ChooserListAdapter extends ResolverListAdapter { mPlaceHolderTargetInfo = NotSelectableTargetInfo.newPlaceHolderTargetInfo(context); mTargetDataLoader = targetDataLoader; createPlaceHolders(); - mChooserActivityLogger = chooserActivityLogger; + mEventLog = eventLog; mShortcutSelectionLogic = new ShortcutSelectionLogic( context.getResources().getInteger(R.integer.config_maxShortcutTargetsPerApp), DeviceConfig.getBoolean( @@ -633,7 +634,7 @@ public class ChooserListAdapter extends ResolverListAdapter { mServiceTargets.removeIf(o -> o.isPlaceHolderTargetInfo()); if (mServiceTargets.isEmpty()) { mServiceTargets.add(NotSelectableTargetInfo.newEmptyTargetInfo()); - mChooserActivityLogger.logSharesheetEmptyDirectShareRow(); + mEventLog.logSharesheetEmptyDirectShareRow(); } notifyDataSetChanged(); } diff --git a/java/src/com/android/intentresolver/logging/EventLog.java b/java/src/com/android/intentresolver/logging/EventLog.java new file mode 100644 index 00000000..b30e825b --- /dev/null +++ b/java/src/com/android/intentresolver/logging/EventLog.java @@ -0,0 +1,539 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.intentresolver.logging; + +import android.annotation.Nullable; +import android.content.Intent; +import android.metrics.LogMaker; +import android.net.Uri; +import android.provider.MediaStore; +import android.util.HashedStringCache; +import android.util.Log; + +import com.android.intentresolver.ChooserActivity; +import com.android.intentresolver.contentpreview.ContentPreviewType; +import com.android.internal.annotations.VisibleForTesting; +import com.android.internal.logging.InstanceId; +import com.android.internal.logging.InstanceIdSequence; +import com.android.internal.logging.MetricsLogger; +import com.android.internal.logging.UiEvent; +import com.android.internal.logging.UiEventLogger; +import com.android.internal.logging.UiEventLoggerImpl; +import com.android.internal.logging.nano.MetricsProto.MetricsEvent; +import com.android.internal.util.FrameworkStatsLog; + +/** + * Helper for writing Sharesheet atoms to statsd log. + * @hide + */ +public class EventLog { + private static final String TAG = "ChooserActivity"; + private static final boolean DEBUG = true; + + public static final int SELECTION_TYPE_SERVICE = 1; + public static final int SELECTION_TYPE_APP = 2; + public static final int SELECTION_TYPE_STANDARD = 3; + public static final int SELECTION_TYPE_COPY = 4; + public static final int SELECTION_TYPE_NEARBY = 5; + public static final int SELECTION_TYPE_EDIT = 6; + public static final int SELECTION_TYPE_MODIFY_SHARE = 7; + public static final int SELECTION_TYPE_CUSTOM_ACTION = 8; + + /** + * This shim is provided only for testing. In production, clients will only ever use a + * {@link DefaultFrameworkStatsLogger}. + */ + @VisibleForTesting + interface FrameworkStatsLogger { + /** Overload to use for logging {@code FrameworkStatsLog.SHARESHEET_STARTED}. */ + void write( + int frameworkEventId, + int appEventId, + String packageName, + int instanceId, + String mimeType, + int numAppProvidedDirectTargets, + int numAppProvidedAppTargets, + boolean isWorkProfile, + int previewType, + int intentType, + int numCustomActions, + boolean modifyShareActionProvided); + + /** Overload to use for logging {@code FrameworkStatsLog.RANKING_SELECTED}. */ + void write( + int frameworkEventId, + int appEventId, + String packageName, + int instanceId, + int positionPicked, + boolean isPinned); + } + + private static final int SHARESHEET_INSTANCE_ID_MAX = (1 << 13); + + // A small per-notification ID, used for statsd logging. + // TODO: consider precomputing and storing as final. + private static InstanceIdSequence sInstanceIdSequence; + private InstanceId mInstanceId; + + private final UiEventLogger mUiEventLogger; + private final FrameworkStatsLogger mFrameworkStatsLogger; + private final MetricsLogger mMetricsLogger; + + public EventLog() { + this(new UiEventLoggerImpl(), new DefaultFrameworkStatsLogger(), new MetricsLogger()); + } + + @VisibleForTesting + EventLog( + UiEventLogger uiEventLogger, + FrameworkStatsLogger frameworkLogger, + MetricsLogger metricsLogger) { + mUiEventLogger = uiEventLogger; + mFrameworkStatsLogger = frameworkLogger; + mMetricsLogger = metricsLogger; + } + + /** Records metrics for the start time of the {@link ChooserActivity}. */ + public void logChooserActivityShown( + boolean isWorkProfile, String targetMimeType, long systemCost) { + mMetricsLogger.write(new LogMaker(MetricsEvent.ACTION_ACTIVITY_CHOOSER_SHOWN) + .setSubtype( + isWorkProfile ? MetricsEvent.MANAGED_PROFILE : MetricsEvent.PARENT_PROFILE) + .addTaggedData(MetricsEvent.FIELD_SHARESHEET_MIMETYPE, targetMimeType) + .addTaggedData(MetricsEvent.FIELD_TIME_TO_APP_TARGETS, systemCost)); + } + + /** Logs a UiEventReported event for the system sharesheet completing initial start-up. */ + public void logShareStarted( + String packageName, + String mimeType, + int appProvidedDirect, + int appProvidedApp, + boolean isWorkprofile, + int previewType, + String intent, + int customActionCount, + boolean modifyShareActionProvided) { + mFrameworkStatsLogger.write(FrameworkStatsLog.SHARESHEET_STARTED, + /* event_id = 1 */ SharesheetStartedEvent.SHARE_STARTED.getId(), + /* package_name = 2 */ packageName, + /* instance_id = 3 */ getInstanceId().getId(), + /* mime_type = 4 */ mimeType, + /* num_app_provided_direct_targets = 5 */ appProvidedDirect, + /* num_app_provided_app_targets = 6 */ appProvidedApp, + /* is_workprofile = 7 */ isWorkprofile, + /* previewType = 8 */ typeFromPreviewInt(previewType), + /* intentType = 9 */ typeFromIntentString(intent), + /* num_provided_custom_actions = 10 */ customActionCount, + /* modify_share_action_provided = 11 */ modifyShareActionProvided); + } + + /** + * Log that a custom action has been tapped by the user. + * + * @param positionPicked index of the custom action within the list of custom actions. + */ + public void logCustomActionSelected(int positionPicked) { + mFrameworkStatsLogger.write(FrameworkStatsLog.RANKING_SELECTED, + /* event_id = 1 */ + SharesheetTargetSelectedEvent.SHARESHEET_CUSTOM_ACTION_SELECTED.getId(), + /* package_name = 2 */ null, + /* instance_id = 3 */ getInstanceId().getId(), + /* position_picked = 4 */ positionPicked, + /* is_pinned = 5 */ false); + } + + /** + * Logs a UiEventReported event for the system sharesheet when the user selects a target. + * TODO: document parameters and/or consider breaking up by targetType so we don't have to + * support an overly-generic signature. + */ + public void logShareTargetSelected( + int targetType, + String packageName, + int positionPicked, + int directTargetAlsoRanked, + int numCallerProvided, + @Nullable HashedStringCache.HashResult directTargetHashed, + boolean isPinned, + boolean successfullySelected, + long selectionCost) { + mFrameworkStatsLogger.write(FrameworkStatsLog.RANKING_SELECTED, + /* event_id = 1 */ SharesheetTargetSelectedEvent.fromTargetType(targetType).getId(), + /* package_name = 2 */ packageName, + /* instance_id = 3 */ getInstanceId().getId(), + /* position_picked = 4 */ positionPicked, + /* is_pinned = 5 */ isPinned); + + int category = getTargetSelectionCategory(targetType); + if (category != 0) { + LogMaker targetLogMaker = new LogMaker(category).setSubtype(positionPicked); + if (directTargetHashed != null) { + targetLogMaker.addTaggedData( + MetricsEvent.FIELD_HASHED_TARGET_NAME, directTargetHashed.hashedString); + targetLogMaker.addTaggedData( + MetricsEvent.FIELD_HASHED_TARGET_SALT_GEN, + directTargetHashed.saltGeneration); + targetLogMaker.addTaggedData(MetricsEvent.FIELD_RANKED_POSITION, + directTargetAlsoRanked); + } + targetLogMaker.addTaggedData(MetricsEvent.FIELD_IS_CATEGORY_USED, numCallerProvided); + mMetricsLogger.write(targetLogMaker); + } + + if (successfullySelected) { + if (DEBUG) { + Log.d(TAG, "User Selection Time Cost is " + selectionCost); + Log.d(TAG, "position of selected app/service/caller is " + positionPicked); + } + MetricsLogger.histogram( + null, "user_selection_cost_for_smart_sharing", (int) selectionCost); + MetricsLogger.histogram(null, "app_position_for_smart_sharing", positionPicked); + } + } + + /** Log when direct share targets were received. */ + public void logDirectShareTargetReceived(int category, int latency) { + mMetricsLogger.write(new LogMaker(category).setSubtype(latency)); + } + + /** + * Log when we display a preview UI of the specified {@code previewType} as part of our + * Sharesheet session. + */ + public void logActionShareWithPreview(int previewType) { + mMetricsLogger.write( + new LogMaker(MetricsEvent.ACTION_SHARE_WITH_PREVIEW).setSubtype(previewType)); + } + + /** Log when the user selects an action button with the specified {@code targetType}. */ + public void logActionSelected(int targetType) { + if (targetType == SELECTION_TYPE_COPY) { + LogMaker targetLogMaker = new LogMaker( + MetricsEvent.ACTION_ACTIVITY_CHOOSER_PICKED_SYSTEM_TARGET).setSubtype(1); + mMetricsLogger.write(targetLogMaker); + } + mFrameworkStatsLogger.write(FrameworkStatsLog.RANKING_SELECTED, + /* event_id = 1 */ SharesheetTargetSelectedEvent.fromTargetType(targetType).getId(), + /* package_name = 2 */ "", + /* instance_id = 3 */ getInstanceId().getId(), + /* position_picked = 4 */ -1, + /* is_pinned = 5 */ false); + } + + /** Log a warning that we couldn't display the content preview from the supplied {@code uri}. */ + public void logContentPreviewWarning(Uri uri) { + // The ContentResolver already logs the exception. Log something more informative. + Log.w(TAG, "Could not load (" + uri.toString() + ") thumbnail/name for preview. If " + + "desired, consider using Intent#createChooser to launch the ChooserActivity, " + + "and set your Intent's clipData and flags in accordance with that method's " + + "documentation"); + + } + + /** Logs a UiEventReported event for the system sharesheet being triggered by the user. */ + public void logSharesheetTriggered() { + log(SharesheetStandardEvent.SHARESHEET_TRIGGERED, getInstanceId()); + } + + /** Logs a UiEventReported event for the system sharesheet completing loading app targets. */ + public void logSharesheetAppLoadComplete() { + log(SharesheetStandardEvent.SHARESHEET_APP_LOAD_COMPLETE, getInstanceId()); + } + + /** + * Logs a UiEventReported event for the system sharesheet completing loading service targets. + */ + public void logSharesheetDirectLoadComplete() { + log(SharesheetStandardEvent.SHARESHEET_DIRECT_LOAD_COMPLETE, getInstanceId()); + } + + /** + * Logs a UiEventReported event for the system sharesheet timing out loading service targets. + */ + public void logSharesheetDirectLoadTimeout() { + log(SharesheetStandardEvent.SHARESHEET_DIRECT_LOAD_TIMEOUT, getInstanceId()); + } + + /** + * Logs a UiEventReported event for the system sharesheet switching + * between work and main profile. + */ + public void logSharesheetProfileChanged() { + log(SharesheetStandardEvent.SHARESHEET_PROFILE_CHANGED, getInstanceId()); + } + + /** Logs a UiEventReported event for the system sharesheet getting expanded or collapsed. */ + public void logSharesheetExpansionChanged(boolean isCollapsed) { + log(isCollapsed ? SharesheetStandardEvent.SHARESHEET_COLLAPSED : + SharesheetStandardEvent.SHARESHEET_EXPANDED, getInstanceId()); + } + + /** + * Logs a UiEventReported event for the system sharesheet app share ranking timing out. + */ + public void logSharesheetAppShareRankingTimeout() { + log(SharesheetStandardEvent.SHARESHEET_APP_SHARE_RANKING_TIMEOUT, getInstanceId()); + } + + /** + * Logs a UiEventReported event for the system sharesheet when direct share row is empty. + */ + public void logSharesheetEmptyDirectShareRow() { + log(SharesheetStandardEvent.SHARESHEET_EMPTY_DIRECT_SHARE_ROW, getInstanceId()); + } + + /** + * Logs a UiEventReported event for a given share activity + * @param event + * @param instanceId + */ + private void log(UiEventLogger.UiEventEnum event, InstanceId instanceId) { + mUiEventLogger.logWithInstanceId( + event, + 0, + null, + instanceId); + } + + /** + * @return A unique {@link InstanceId} to join across events recorded by this logger instance. + */ + private InstanceId getInstanceId() { + if (mInstanceId == null) { + if (sInstanceIdSequence == null) { + sInstanceIdSequence = new InstanceIdSequence(SHARESHEET_INSTANCE_ID_MAX); + } + mInstanceId = sInstanceIdSequence.newInstanceId(); + } + return mInstanceId; + } + + /** + * The UiEvent enums that this class can log. + */ + enum SharesheetStartedEvent implements UiEventLogger.UiEventEnum { + @UiEvent(doc = "Basic system Sharesheet has started and is visible.") + SHARE_STARTED(228); + + private final int mId; + SharesheetStartedEvent(int id) { + mId = id; + } + @Override + public int getId() { + return mId; + } + } + + /** + * The UiEvent enums that this class can log. + */ + enum SharesheetTargetSelectedEvent implements UiEventLogger.UiEventEnum { + INVALID(0), + @UiEvent(doc = "User selected a service target.") + SHARESHEET_SERVICE_TARGET_SELECTED(232), + @UiEvent(doc = "User selected an app target.") + SHARESHEET_APP_TARGET_SELECTED(233), + @UiEvent(doc = "User selected a standard target.") + SHARESHEET_STANDARD_TARGET_SELECTED(234), + @UiEvent(doc = "User selected the copy target.") + SHARESHEET_COPY_TARGET_SELECTED(235), + @UiEvent(doc = "User selected the nearby target.") + SHARESHEET_NEARBY_TARGET_SELECTED(626), + @UiEvent(doc = "User selected the edit target.") + SHARESHEET_EDIT_TARGET_SELECTED(669), + @UiEvent(doc = "User selected the modify share target.") + SHARESHEET_MODIFY_SHARE_SELECTED(1316), + @UiEvent(doc = "User selected a custom action.") + SHARESHEET_CUSTOM_ACTION_SELECTED(1317); + + private final int mId; + SharesheetTargetSelectedEvent(int id) { + mId = id; + } + @Override public int getId() { + return mId; + } + + public static SharesheetTargetSelectedEvent fromTargetType(int targetType) { + switch(targetType) { + case SELECTION_TYPE_SERVICE: + return SHARESHEET_SERVICE_TARGET_SELECTED; + case SELECTION_TYPE_APP: + return SHARESHEET_APP_TARGET_SELECTED; + case SELECTION_TYPE_STANDARD: + return SHARESHEET_STANDARD_TARGET_SELECTED; + case SELECTION_TYPE_COPY: + return SHARESHEET_COPY_TARGET_SELECTED; + case SELECTION_TYPE_NEARBY: + return SHARESHEET_NEARBY_TARGET_SELECTED; + case SELECTION_TYPE_EDIT: + return SHARESHEET_EDIT_TARGET_SELECTED; + case SELECTION_TYPE_MODIFY_SHARE: + return SHARESHEET_MODIFY_SHARE_SELECTED; + case SELECTION_TYPE_CUSTOM_ACTION: + return SHARESHEET_CUSTOM_ACTION_SELECTED; + default: + return INVALID; + } + } + } + + /** + * The UiEvent enums that this class can log. + */ + enum SharesheetStandardEvent implements UiEventLogger.UiEventEnum { + INVALID(0), + @UiEvent(doc = "User clicked share.") + SHARESHEET_TRIGGERED(227), + @UiEvent(doc = "User changed from work to personal profile or vice versa.") + SHARESHEET_PROFILE_CHANGED(229), + @UiEvent(doc = "User expanded target list.") + SHARESHEET_EXPANDED(230), + @UiEvent(doc = "User collapsed target list.") + SHARESHEET_COLLAPSED(231), + @UiEvent(doc = "Sharesheet app targets is fully populated.") + SHARESHEET_APP_LOAD_COMPLETE(322), + @UiEvent(doc = "Sharesheet direct targets is fully populated.") + SHARESHEET_DIRECT_LOAD_COMPLETE(323), + @UiEvent(doc = "Sharesheet direct targets timed out.") + SHARESHEET_DIRECT_LOAD_TIMEOUT(324), + @UiEvent(doc = "Sharesheet app share ranking timed out.") + SHARESHEET_APP_SHARE_RANKING_TIMEOUT(831), + @UiEvent(doc = "Sharesheet empty direct share row.") + SHARESHEET_EMPTY_DIRECT_SHARE_ROW(828); + + private final int mId; + SharesheetStandardEvent(int id) { + mId = id; + } + @Override public int getId() { + return mId; + } + } + + /** + * Returns the enum used in sharesheet started atom to indicate what preview type was used. + */ + private static int typeFromPreviewInt(int previewType) { + switch(previewType) { + case ContentPreviewType.CONTENT_PREVIEW_IMAGE: + return FrameworkStatsLog.SHARESHEET_STARTED__PREVIEW_TYPE__CONTENT_PREVIEW_IMAGE; + case ContentPreviewType.CONTENT_PREVIEW_FILE: + return FrameworkStatsLog.SHARESHEET_STARTED__PREVIEW_TYPE__CONTENT_PREVIEW_FILE; + case ContentPreviewType.CONTENT_PREVIEW_TEXT: + default: + return FrameworkStatsLog + .SHARESHEET_STARTED__PREVIEW_TYPE__CONTENT_PREVIEW_TYPE_UNKNOWN; + } + } + + /** + * Returns the enum used in sharesheet started atom to indicate what intent triggers the + * ChooserActivity. + */ + private static int typeFromIntentString(String intent) { + if (intent == null) { + return FrameworkStatsLog.SHARESHEET_STARTED__INTENT_TYPE__INTENT_DEFAULT; + } + switch (intent) { + case Intent.ACTION_VIEW: + return FrameworkStatsLog.SHARESHEET_STARTED__INTENT_TYPE__INTENT_ACTION_VIEW; + case Intent.ACTION_EDIT: + return FrameworkStatsLog.SHARESHEET_STARTED__INTENT_TYPE__INTENT_ACTION_EDIT; + case Intent.ACTION_SEND: + return FrameworkStatsLog.SHARESHEET_STARTED__INTENT_TYPE__INTENT_ACTION_SEND; + case Intent.ACTION_SENDTO: + return FrameworkStatsLog.SHARESHEET_STARTED__INTENT_TYPE__INTENT_ACTION_SENDTO; + case Intent.ACTION_SEND_MULTIPLE: + return FrameworkStatsLog + .SHARESHEET_STARTED__INTENT_TYPE__INTENT_ACTION_SEND_MULTIPLE; + case MediaStore.ACTION_IMAGE_CAPTURE: + return FrameworkStatsLog + .SHARESHEET_STARTED__INTENT_TYPE__INTENT_ACTION_IMAGE_CAPTURE; + case Intent.ACTION_MAIN: + return FrameworkStatsLog.SHARESHEET_STARTED__INTENT_TYPE__INTENT_ACTION_MAIN; + default: + return FrameworkStatsLog.SHARESHEET_STARTED__INTENT_TYPE__INTENT_DEFAULT; + } + } + + @VisibleForTesting + static int getTargetSelectionCategory(int targetType) { + switch (targetType) { + case SELECTION_TYPE_SERVICE: + return MetricsEvent.ACTION_ACTIVITY_CHOOSER_PICKED_SERVICE_TARGET; + case SELECTION_TYPE_APP: + return MetricsEvent.ACTION_ACTIVITY_CHOOSER_PICKED_APP_TARGET; + case SELECTION_TYPE_STANDARD: + return MetricsEvent.ACTION_ACTIVITY_CHOOSER_PICKED_STANDARD_TARGET; + default: + return 0; + } + } + + private static class DefaultFrameworkStatsLogger implements FrameworkStatsLogger { + @Override + public void write( + int frameworkEventId, + int appEventId, + String packageName, + int instanceId, + String mimeType, + int numAppProvidedDirectTargets, + int numAppProvidedAppTargets, + boolean isWorkProfile, + int previewType, + int intentType, + int numCustomActions, + boolean modifyShareActionProvided) { + FrameworkStatsLog.write( + frameworkEventId, + /* event_id = 1 */ appEventId, + /* package_name = 2 */ packageName, + /* instance_id = 3 */ instanceId, + /* mime_type = 4 */ mimeType, + /* num_app_provided_direct_targets */ numAppProvidedDirectTargets, + /* num_app_provided_app_targets */ numAppProvidedAppTargets, + /* is_workprofile */ isWorkProfile, + /* previewType = 8 */ previewType, + /* intentType = 9 */ intentType, + /* num_provided_custom_actions = 10 */ numCustomActions, + /* modify_share_action_provided = 11 */ modifyShareActionProvided); + } + + @Override + public void write( + int frameworkEventId, + int appEventId, + String packageName, + int instanceId, + int positionPicked, + boolean isPinned) { + FrameworkStatsLog.write( + frameworkEventId, + /* event_id = 1 */ appEventId, + /* package_name = 2 */ packageName, + /* instance_id = 3 */ instanceId, + /* position_picked = 4 */ positionPicked, + /* is_pinned = 5 */ isPinned); + } + } +} diff --git a/java/src/com/android/intentresolver/model/AbstractResolverComparator.java b/java/src/com/android/intentresolver/model/AbstractResolverComparator.java index bc54e01e..ff2d6a0f 100644 --- a/java/src/com/android/intentresolver/model/AbstractResolverComparator.java +++ b/java/src/com/android/intentresolver/model/AbstractResolverComparator.java @@ -30,7 +30,7 @@ import android.os.Message; import android.os.UserHandle; import android.util.Log; -import com.android.intentresolver.ChooserActivityLogger; +import com.android.intentresolver.logging.EventLog; import com.android.intentresolver.ResolvedComponentInfo; import com.android.intentresolver.ResolverActivity; import com.android.intentresolver.chooser.TargetInfo; @@ -72,7 +72,7 @@ public abstract class AbstractResolverComparator implements Comparator mAzComparator; - private ChooserActivityLogger mChooserActivityLogger; + private EventLog mEventLog; protected final Handler mHandler = new Handler(Looper.getMainLooper()) { public void handleMessage(Message msg) { @@ -94,8 +94,8 @@ public abstract class AbstractResolverComparator implements Comparator mHandler.sendEmptyMessage(RANKER_SERVICE_RESULT), - getChooserActivityLogger(), + getEventLog(), mUser, mPromoteToFirst); mComparatorModel = buildUpdatedModel(); diff --git a/java/src/com/android/intentresolver/model/ResolverRankerServiceResolverComparator.java b/java/src/com/android/intentresolver/model/ResolverRankerServiceResolverComparator.java index ebaffc36..7d473660 100644 --- a/java/src/com/android/intentresolver/model/ResolverRankerServiceResolverComparator.java +++ b/java/src/com/android/intentresolver/model/ResolverRankerServiceResolverComparator.java @@ -39,7 +39,7 @@ import android.service.resolver.ResolverRankerService; import android.service.resolver.ResolverTarget; import android.util.Log; -import com.android.intentresolver.ChooserActivityLogger; +import com.android.intentresolver.logging.EventLog; import com.android.intentresolver.ResolvedComponentInfo; import com.android.intentresolver.chooser.TargetInfo; import com.android.internal.logging.MetricsLogger; @@ -102,9 +102,9 @@ public class ResolverRankerServiceResolverComparator extends AbstractResolverCom */ public ResolverRankerServiceResolverComparator(Context launchedFromContext, Intent intent, String referrerPackage, Runnable afterCompute, - ChooserActivityLogger chooserActivityLogger, UserHandle targetUserSpace, + EventLog eventLog, UserHandle targetUserSpace, ComponentName promoteToFirst) { - this(launchedFromContext, intent, referrerPackage, afterCompute, chooserActivityLogger, + this(launchedFromContext, intent, referrerPackage, afterCompute, eventLog, Lists.newArrayList(targetUserSpace), promoteToFirst); } @@ -118,7 +118,7 @@ public class ResolverRankerServiceResolverComparator extends AbstractResolverCom */ public ResolverRankerServiceResolverComparator(Context launchedFromContext, Intent intent, String referrerPackage, Runnable afterCompute, - ChooserActivityLogger chooserActivityLogger, List targetUserSpaceList, + EventLog eventLog, List targetUserSpaceList, @Nullable ComponentName promoteToFirst) { super(launchedFromContext, intent, targetUserSpaceList, promoteToFirst); mCollator = Collator.getInstance( @@ -139,7 +139,7 @@ public class ResolverRankerServiceResolverComparator extends AbstractResolverCom mAction = intent.getAction(); mRankerServiceName = new ComponentName(mContext, this.getClass()); setCallBack(afterCompute); - setChooserActivityLogger(chooserActivityLogger); + setEventLog(eventLog); mComparatorModel = buildUpdatedModel(); } diff --git a/java/tests/src/com/android/intentresolver/ChooserActionFactoryTest.kt b/java/tests/src/com/android/intentresolver/ChooserActionFactoryTest.kt index 8d994f08..af6e5f16 100644 --- a/java/tests/src/com/android/intentresolver/ChooserActionFactoryTest.kt +++ b/java/tests/src/com/android/intentresolver/ChooserActionFactoryTest.kt @@ -27,6 +27,7 @@ import android.graphics.drawable.Icon import android.service.chooser.ChooserAction import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.platform.app.InstrumentationRegistry +import com.android.intentresolver.logging.EventLog import com.google.common.collect.ImmutableList import com.google.common.truth.Truth.assertThat import java.util.concurrent.CountDownLatch @@ -43,7 +44,7 @@ import org.mockito.Mockito class ChooserActionFactoryTest { private val context = InstrumentationRegistry.getInstrumentation().getContext() - private val logger = mock() + private val logger = mock() private val actionLabel = "Action label" private val modifyShareLabel = "Modify share" private val testAction = "com.android.intentresolver.testaction" @@ -107,7 +108,7 @@ class ChooserActionFactoryTest { action.onClicked.run() Mockito.verify(logger) - .logActionSelected(eq(ChooserActivityLogger.SELECTION_TYPE_MODIFY_SHARE)) + .logActionSelected(eq(EventLog.SELECTION_TYPE_MODIFY_SHARE)) assertEquals(Activity.RESULT_OK, resultConsumer.latestReturn) // Verify the pending intent has been called countdown.await(500, TimeUnit.MILLISECONDS) diff --git a/java/tests/src/com/android/intentresolver/ChooserActivityLoggerTest.java b/java/tests/src/com/android/intentresolver/ChooserActivityLoggerTest.java deleted file mode 100644 index aa42c24c..00000000 --- a/java/tests/src/com/android/intentresolver/ChooserActivityLoggerTest.java +++ /dev/null @@ -1,422 +0,0 @@ -/* - * Copyright (C) 2020 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.intentresolver; - -import static com.google.common.truth.Truth.assertThat; - -import static org.mockito.AdditionalMatchers.gt; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyBoolean; -import static org.mockito.ArgumentMatchers.anyInt; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.ArgumentMatchers.isNull; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; - -import android.content.Intent; -import android.metrics.LogMaker; - -import com.android.intentresolver.ChooserActivityLogger.FrameworkStatsLogger; -import com.android.intentresolver.ChooserActivityLogger.SharesheetStandardEvent; -import com.android.intentresolver.ChooserActivityLogger.SharesheetStartedEvent; -import com.android.intentresolver.ChooserActivityLogger.SharesheetTargetSelectedEvent; -import com.android.intentresolver.contentpreview.ContentPreviewType; -import com.android.internal.logging.InstanceId; -import com.android.internal.logging.MetricsLogger; -import com.android.internal.logging.UiEventLogger; -import com.android.internal.logging.UiEventLogger.UiEventEnum; -import com.android.internal.logging.nano.MetricsProto.MetricsEvent; -import com.android.internal.util.FrameworkStatsLog; - -import org.junit.After; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.ArgumentCaptor; -import org.mockito.Mock; -import org.mockito.junit.MockitoJUnitRunner; - -@RunWith(MockitoJUnitRunner.class) -public final class ChooserActivityLoggerTest { - @Mock private UiEventLogger mUiEventLog; - @Mock private FrameworkStatsLogger mFrameworkLog; - @Mock private MetricsLogger mMetricsLogger; - - private ChooserActivityLogger mChooserLogger; - - @Before - public void setUp() { - //Mockito.reset(mUiEventLog, mFrameworkLog, mMetricsLogger); - mChooserLogger = new ChooserActivityLogger(mUiEventLog, mFrameworkLog, mMetricsLogger); - } - - @After - public void tearDown() { - verifyNoMoreInteractions(mUiEventLog); - verifyNoMoreInteractions(mFrameworkLog); - verifyNoMoreInteractions(mMetricsLogger); - } - - @Test - public void testLogChooserActivityShown_personalProfile() { - final boolean isWorkProfile = false; - final String mimeType = "application/TestType"; - final long systemCost = 456; - - mChooserLogger.logChooserActivityShown(isWorkProfile, mimeType, systemCost); - - ArgumentCaptor eventCaptor = ArgumentCaptor.forClass(LogMaker.class); - verify(mMetricsLogger).write(eventCaptor.capture()); - LogMaker event = eventCaptor.getValue(); - - assertThat(event.getCategory()).isEqualTo(MetricsEvent.ACTION_ACTIVITY_CHOOSER_SHOWN); - assertThat(event.getSubtype()).isEqualTo(MetricsEvent.PARENT_PROFILE); - assertThat(event.getTaggedData(MetricsEvent.FIELD_SHARESHEET_MIMETYPE)).isEqualTo(mimeType); - assertThat(event.getTaggedData(MetricsEvent.FIELD_TIME_TO_APP_TARGETS)) - .isEqualTo(systemCost); - } - - @Test - public void testLogChooserActivityShown_workProfile() { - final boolean isWorkProfile = true; - final String mimeType = "application/TestType"; - final long systemCost = 456; - - mChooserLogger.logChooserActivityShown(isWorkProfile, mimeType, systemCost); - - ArgumentCaptor eventCaptor = ArgumentCaptor.forClass(LogMaker.class); - verify(mMetricsLogger).write(eventCaptor.capture()); - LogMaker event = eventCaptor.getValue(); - - assertThat(event.getCategory()).isEqualTo(MetricsEvent.ACTION_ACTIVITY_CHOOSER_SHOWN); - assertThat(event.getSubtype()).isEqualTo(MetricsEvent.MANAGED_PROFILE); - assertThat(event.getTaggedData(MetricsEvent.FIELD_SHARESHEET_MIMETYPE)).isEqualTo(mimeType); - assertThat(event.getTaggedData(MetricsEvent.FIELD_TIME_TO_APP_TARGETS)) - .isEqualTo(systemCost); - } - - @Test - public void testLogShareStarted() { - final String packageName = "com.test.foo"; - final String mimeType = "text/plain"; - final int appProvidedDirectTargets = 123; - final int appProvidedAppTargets = 456; - final boolean workProfile = true; - final int previewType = ContentPreviewType.CONTENT_PREVIEW_FILE; - final String intentAction = Intent.ACTION_SENDTO; - final int numCustomActions = 3; - final boolean modifyShareProvided = true; - - mChooserLogger.logShareStarted( - packageName, - mimeType, - appProvidedDirectTargets, - appProvidedAppTargets, - workProfile, - previewType, - intentAction, - numCustomActions, - modifyShareProvided); - - verify(mFrameworkLog).write( - eq(FrameworkStatsLog.SHARESHEET_STARTED), - eq(SharesheetStartedEvent.SHARE_STARTED.getId()), - eq(packageName), - /* instanceId=*/ gt(0), - eq(mimeType), - eq(appProvidedDirectTargets), - eq(appProvidedAppTargets), - eq(workProfile), - eq(FrameworkStatsLog.SHARESHEET_STARTED__PREVIEW_TYPE__CONTENT_PREVIEW_FILE), - eq(FrameworkStatsLog.SHARESHEET_STARTED__INTENT_TYPE__INTENT_ACTION_SENDTO), - /* custom actions provided */ eq(numCustomActions), - /* reselection action provided */ eq(modifyShareProvided)); - } - - @Test - public void testLogShareTargetSelected() { - final int targetType = ChooserActivityLogger.SELECTION_TYPE_SERVICE; - final String packageName = "com.test.foo"; - final int positionPicked = 123; - final int directTargetAlsoRanked = -1; - final int callerTargetCount = 0; - final boolean isPinned = true; - final boolean isSuccessfullySelected = true; - final long selectionCost = 456; - - mChooserLogger.logShareTargetSelected( - targetType, - packageName, - positionPicked, - directTargetAlsoRanked, - callerTargetCount, - /* directTargetHashed= */ null, - isPinned, - isSuccessfullySelected, - selectionCost); - - verify(mFrameworkLog).write( - eq(FrameworkStatsLog.RANKING_SELECTED), - eq(SharesheetTargetSelectedEvent.SHARESHEET_SERVICE_TARGET_SELECTED.getId()), - eq(packageName), - /* instanceId=*/ gt(0), - eq(positionPicked), - eq(isPinned)); - - ArgumentCaptor eventCaptor = ArgumentCaptor.forClass(LogMaker.class); - verify(mMetricsLogger).write(eventCaptor.capture()); - LogMaker event = eventCaptor.getValue(); - assertThat(event.getCategory()).isEqualTo( - MetricsEvent.ACTION_ACTIVITY_CHOOSER_PICKED_SERVICE_TARGET); - assertThat(event.getSubtype()).isEqualTo(positionPicked); - } - - @Test - public void testLogActionSelected() { - mChooserLogger.logActionSelected(ChooserActivityLogger.SELECTION_TYPE_COPY); - - verify(mFrameworkLog).write( - eq(FrameworkStatsLog.RANKING_SELECTED), - eq(SharesheetTargetSelectedEvent.SHARESHEET_COPY_TARGET_SELECTED.getId()), - eq(""), - /* instanceId=*/ gt(0), - eq(-1), - eq(false)); - - ArgumentCaptor eventCaptor = ArgumentCaptor.forClass(LogMaker.class); - verify(mMetricsLogger).write(eventCaptor.capture()); - LogMaker event = eventCaptor.getValue(); - assertThat(event.getCategory()).isEqualTo( - MetricsEvent.ACTION_ACTIVITY_CHOOSER_PICKED_SYSTEM_TARGET); - assertThat(event.getSubtype()).isEqualTo(1); - } - - @Test - public void testLogCustomActionSelected() { - final int position = 4; - mChooserLogger.logCustomActionSelected(position); - - verify(mFrameworkLog).write( - eq(FrameworkStatsLog.RANKING_SELECTED), - eq(SharesheetTargetSelectedEvent.SHARESHEET_CUSTOM_ACTION_SELECTED.getId()), - any(), anyInt(), eq(position), eq(false)); - } - - @Test - public void testLogDirectShareTargetReceived() { - final int category = MetricsEvent.ACTION_DIRECT_SHARE_TARGETS_LOADED_SHORTCUT_MANAGER; - final int latency = 123; - - mChooserLogger.logDirectShareTargetReceived(category, latency); - - ArgumentCaptor eventCaptor = ArgumentCaptor.forClass(LogMaker.class); - verify(mMetricsLogger).write(eventCaptor.capture()); - LogMaker event = eventCaptor.getValue(); - assertThat(event.getCategory()).isEqualTo(category); - assertThat(event.getSubtype()).isEqualTo(latency); - } - - @Test - public void testLogActionShareWithPreview() { - final int previewType = ContentPreviewType.CONTENT_PREVIEW_TEXT; - - mChooserLogger.logActionShareWithPreview(previewType); - - ArgumentCaptor eventCaptor = ArgumentCaptor.forClass(LogMaker.class); - verify(mMetricsLogger).write(eventCaptor.capture()); - LogMaker event = eventCaptor.getValue(); - assertThat(event.getCategory()).isEqualTo(MetricsEvent.ACTION_SHARE_WITH_PREVIEW); - assertThat(event.getSubtype()).isEqualTo(previewType); - } - - @Test - public void testLogSharesheetTriggered() { - mChooserLogger.logSharesheetTriggered(); - verify(mUiEventLog).logWithInstanceId( - eq(SharesheetStandardEvent.SHARESHEET_TRIGGERED), eq(0), isNull(), any()); - } - - @Test - public void testLogSharesheetAppLoadComplete() { - mChooserLogger.logSharesheetAppLoadComplete(); - verify(mUiEventLog).logWithInstanceId( - eq(SharesheetStandardEvent.SHARESHEET_APP_LOAD_COMPLETE), eq(0), isNull(), any()); - } - - @Test - public void testLogSharesheetDirectLoadComplete() { - mChooserLogger.logSharesheetDirectLoadComplete(); - verify(mUiEventLog).logWithInstanceId( - eq(SharesheetStandardEvent.SHARESHEET_DIRECT_LOAD_COMPLETE), - eq(0), - isNull(), - any()); - } - - @Test - public void testLogSharesheetDirectLoadTimeout() { - mChooserLogger.logSharesheetDirectLoadTimeout(); - verify(mUiEventLog).logWithInstanceId( - eq(SharesheetStandardEvent.SHARESHEET_DIRECT_LOAD_TIMEOUT), eq(0), isNull(), any()); - } - - @Test - public void testLogSharesheetProfileChanged() { - mChooserLogger.logSharesheetProfileChanged(); - verify(mUiEventLog).logWithInstanceId( - eq(SharesheetStandardEvent.SHARESHEET_PROFILE_CHANGED), eq(0), isNull(), any()); - } - - @Test - public void testLogSharesheetExpansionChanged_collapsed() { - mChooserLogger.logSharesheetExpansionChanged(/* isCollapsed=*/ true); - verify(mUiEventLog).logWithInstanceId( - eq(SharesheetStandardEvent.SHARESHEET_COLLAPSED), eq(0), isNull(), any()); - } - - @Test - public void testLogSharesheetExpansionChanged_expanded() { - mChooserLogger.logSharesheetExpansionChanged(/* isCollapsed=*/ false); - verify(mUiEventLog).logWithInstanceId( - eq(SharesheetStandardEvent.SHARESHEET_EXPANDED), eq(0), isNull(), any()); - } - - @Test - public void testLogSharesheetAppShareRankingTimeout() { - mChooserLogger.logSharesheetAppShareRankingTimeout(); - verify(mUiEventLog).logWithInstanceId( - eq(SharesheetStandardEvent.SHARESHEET_APP_SHARE_RANKING_TIMEOUT), - eq(0), - isNull(), - any()); - } - - @Test - public void testLogSharesheetEmptyDirectShareRow() { - mChooserLogger.logSharesheetEmptyDirectShareRow(); - verify(mUiEventLog).logWithInstanceId( - eq(SharesheetStandardEvent.SHARESHEET_EMPTY_DIRECT_SHARE_ROW), - eq(0), - isNull(), - any()); - } - - @Test - public void testDifferentLoggerInstancesUseDifferentInstanceIds() { - ArgumentCaptor idIntCaptor = ArgumentCaptor.forClass(Integer.class); - ChooserActivityLogger chooserLogger2 = - new ChooserActivityLogger(mUiEventLog, mFrameworkLog, mMetricsLogger); - - final int targetType = ChooserActivityLogger.SELECTION_TYPE_COPY; - final String packageName = "com.test.foo"; - final int positionPicked = 123; - final int directTargetAlsoRanked = -1; - final int callerTargetCount = 0; - final boolean isPinned = true; - final boolean isSuccessfullySelected = true; - final long selectionCost = 456; - - mChooserLogger.logShareTargetSelected( - targetType, - packageName, - positionPicked, - directTargetAlsoRanked, - callerTargetCount, - /* directTargetHashed= */ null, - isPinned, - isSuccessfullySelected, - selectionCost); - - chooserLogger2.logShareTargetSelected( - targetType, - packageName, - positionPicked, - directTargetAlsoRanked, - callerTargetCount, - /* directTargetHashed= */ null, - isPinned, - isSuccessfullySelected, - selectionCost); - - verify(mFrameworkLog, times(2)).write( - anyInt(), anyInt(), anyString(), idIntCaptor.capture(), anyInt(), anyBoolean()); - - int id1 = idIntCaptor.getAllValues().get(0); - int id2 = idIntCaptor.getAllValues().get(1); - - assertThat(id1).isGreaterThan(0); - assertThat(id2).isGreaterThan(0); - assertThat(id1).isNotEqualTo(id2); - } - - @Test - public void testUiAndFrameworkEventsUseSameInstanceIdForSameLoggerInstance() { - ArgumentCaptor idIntCaptor = ArgumentCaptor.forClass(Integer.class); - ArgumentCaptor idObjectCaptor = ArgumentCaptor.forClass(InstanceId.class); - - final int targetType = ChooserActivityLogger.SELECTION_TYPE_COPY; - final String packageName = "com.test.foo"; - final int positionPicked = 123; - final int directTargetAlsoRanked = -1; - final int callerTargetCount = 0; - final boolean isPinned = true; - final boolean isSuccessfullySelected = true; - final long selectionCost = 456; - - mChooserLogger.logShareTargetSelected( - targetType, - packageName, - positionPicked, - directTargetAlsoRanked, - callerTargetCount, - /* directTargetHashed= */ null, - isPinned, - isSuccessfullySelected, - selectionCost); - - verify(mFrameworkLog).write( - anyInt(), anyInt(), anyString(), idIntCaptor.capture(), anyInt(), anyBoolean()); - - mChooserLogger.logSharesheetTriggered(); - verify(mUiEventLog).logWithInstanceId( - any(UiEventEnum.class), anyInt(), any(), idObjectCaptor.capture()); - - assertThat(idIntCaptor.getValue()).isGreaterThan(0); - assertThat(idObjectCaptor.getValue().getId()).isEqualTo(idIntCaptor.getValue()); - } - - @Test - public void testTargetSelectionCategories() { - assertThat(ChooserActivityLogger.getTargetSelectionCategory( - ChooserActivityLogger.SELECTION_TYPE_SERVICE)) - .isEqualTo(MetricsEvent.ACTION_ACTIVITY_CHOOSER_PICKED_SERVICE_TARGET); - assertThat(ChooserActivityLogger.getTargetSelectionCategory( - ChooserActivityLogger.SELECTION_TYPE_APP)) - .isEqualTo(MetricsEvent.ACTION_ACTIVITY_CHOOSER_PICKED_APP_TARGET); - assertThat(ChooserActivityLogger.getTargetSelectionCategory( - ChooserActivityLogger.SELECTION_TYPE_STANDARD)) - .isEqualTo(MetricsEvent.ACTION_ACTIVITY_CHOOSER_PICKED_STANDARD_TARGET); - assertThat(ChooserActivityLogger.getTargetSelectionCategory( - ChooserActivityLogger.SELECTION_TYPE_COPY)).isEqualTo(0); - assertThat(ChooserActivityLogger.getTargetSelectionCategory( - ChooserActivityLogger.SELECTION_TYPE_NEARBY)).isEqualTo(0); - assertThat(ChooserActivityLogger.getTargetSelectionCategory( - ChooserActivityLogger.SELECTION_TYPE_EDIT)).isEqualTo(0); - } -} diff --git a/java/tests/src/com/android/intentresolver/ChooserActivityOverrideData.java b/java/tests/src/com/android/intentresolver/ChooserActivityOverrideData.java index ce96ef63..84f5124c 100644 --- a/java/tests/src/com/android/intentresolver/ChooserActivityOverrideData.java +++ b/java/tests/src/com/android/intentresolver/ChooserActivityOverrideData.java @@ -30,6 +30,7 @@ import com.android.intentresolver.AbstractMultiProfilePagerAdapter.CrossProfileI import com.android.intentresolver.chooser.TargetInfo; import com.android.intentresolver.contentpreview.ImageLoader; import com.android.intentresolver.flags.FeatureFlagRepository; +import com.android.intentresolver.logging.EventLog; import com.android.intentresolver.shortcuts.ShortcutLoader; import java.util.function.Consumer; @@ -64,7 +65,7 @@ public class ChooserActivityOverrideData { public Cursor resolverCursor; public boolean resolverForceException; public ImageLoader imageLoader; - public ChooserActivityLogger chooserActivityLogger; + public EventLog mEventLog; public int alternateProfileSetting; public Resources resources; public UserHandle workProfileUserHandle; @@ -87,7 +88,7 @@ public class ChooserActivityOverrideData { resolverForceException = false; resolverListController = mock(ChooserActivity.ChooserListController.class); workResolverListController = mock(ChooserActivity.ChooserListController.class); - chooserActivityLogger = mock(ChooserActivityLogger.class); + mEventLog = mock(EventLog.class); alternateProfileSetting = 0; resources = null; workProfileUserHandle = null; diff --git a/java/tests/src/com/android/intentresolver/ChooserListAdapterTest.kt b/java/tests/src/com/android/intentresolver/ChooserListAdapterTest.kt index 4612b430..c8cb4b9b 100644 --- a/java/tests/src/com/android/intentresolver/ChooserListAdapterTest.kt +++ b/java/tests/src/com/android/intentresolver/ChooserListAdapterTest.kt @@ -31,6 +31,7 @@ import com.android.intentresolver.chooser.DisplayResolveInfo import com.android.intentresolver.chooser.SelectableTargetInfo import com.android.intentresolver.chooser.TargetInfo import com.android.intentresolver.icons.TargetDataLoader +import com.android.intentresolver.logging.EventLog import com.android.internal.R import org.junit.Before import org.junit.Test @@ -49,7 +50,7 @@ class ChooserListAdapterTest { } private val context = InstrumentationRegistry.getInstrumentation().context private val resolverListController = mock() - private val chooserActivityLogger = mock() + private val mEventLog = mock() private val mTargetDataLoader = mock() private val testSubject by lazy { @@ -64,7 +65,7 @@ class ChooserListAdapterTest { Intent(), mock(), packageManager, - chooserActivityLogger, + mEventLog, mock(), 0, null, diff --git a/java/tests/src/com/android/intentresolver/ChooserWrapperActivity.java b/java/tests/src/com/android/intentresolver/ChooserWrapperActivity.java index 6ac6b6d3..8608cf72 100644 --- a/java/tests/src/com/android/intentresolver/ChooserWrapperActivity.java +++ b/java/tests/src/com/android/intentresolver/ChooserWrapperActivity.java @@ -40,6 +40,7 @@ import com.android.intentresolver.chooser.TargetInfo; import com.android.intentresolver.flags.FeatureFlagRepository; import com.android.intentresolver.grid.ChooserGridAdapter; import com.android.intentresolver.icons.TargetDataLoader; +import com.android.intentresolver.logging.EventLog; import com.android.intentresolver.shortcuts.ShortcutLoader; import com.android.internal.logging.nano.MetricsProto.MetricsEvent; @@ -89,7 +90,7 @@ public class ChooserWrapperActivity targetIntent, this, packageManager, - getChooserActivityLogger(), + getEventLog(), chooserRequest, maxTargetsPerRow, userHandle, @@ -205,8 +206,8 @@ public class ChooserWrapperActivity } @Override - public ChooserActivityLogger getChooserActivityLogger() { - return sOverrides.chooserActivityLogger; + public EventLog getEventLog() { + return sOverrides.mEventLog; } @Override diff --git a/java/tests/src/com/android/intentresolver/IChooserWrapper.java b/java/tests/src/com/android/intentresolver/IChooserWrapper.java index af897a47..3326d7f2 100644 --- a/java/tests/src/com/android/intentresolver/IChooserWrapper.java +++ b/java/tests/src/com/android/intentresolver/IChooserWrapper.java @@ -23,6 +23,7 @@ import android.content.pm.ResolveInfo; import android.os.UserHandle; import com.android.intentresolver.chooser.DisplayResolveInfo; +import com.android.intentresolver.logging.EventLog; import java.util.concurrent.Executor; @@ -41,6 +42,6 @@ public interface IChooserWrapper { CharSequence pLabel, CharSequence pInfo, Intent replacementIntent, @Nullable TargetPresentationGetter resolveInfoPresentationGetter); UserHandle getCurrentUserHandle(); - ChooserActivityLogger getChooserActivityLogger(); + EventLog getEventLog(); Executor getMainExecutor(); } diff --git a/java/tests/src/com/android/intentresolver/UnbundledChooserActivityTest.java b/java/tests/src/com/android/intentresolver/UnbundledChooserActivityTest.java index 8233f0db..ecd05b46 100644 --- a/java/tests/src/com/android/intentresolver/UnbundledChooserActivityTest.java +++ b/java/tests/src/com/android/intentresolver/UnbundledChooserActivityTest.java @@ -108,6 +108,7 @@ import androidx.test.rule.ActivityTestRule; import com.android.intentresolver.chooser.DisplayResolveInfo; import com.android.intentresolver.contentpreview.ImageLoader; +import com.android.intentresolver.logging.EventLog; import com.android.intentresolver.shortcuts.ShortcutLoader; import com.android.internal.config.sysui.SystemUiDeviceConfigFlags; import com.android.internal.logging.nano.MetricsProto.MetricsEvent; @@ -900,8 +901,8 @@ public class UnbundledChooserActivityTest { onView(withId(R.id.copy)).check(matches(isDisplayed())); onView(withId(R.id.copy)).perform(click()); - ChooserActivityLogger logger = activity.getChooserActivityLogger(); - verify(logger, times(1)).logActionSelected(eq(ChooserActivityLogger.SELECTION_TYPE_COPY)); + EventLog logger = activity.getEventLog(); + verify(logger, times(1)).logActionSelected(eq(EventLog.SELECTION_TYPE_COPY)); } @Test @@ -1100,7 +1101,7 @@ public class UnbundledChooserActivityTest { final IChooserWrapper activity = (IChooserWrapper) mActivityRule.launchActivity(Intent.createChooser(sendIntent, "logger test")); - ChooserActivityLogger logger = activity.getChooserActivityLogger(); + EventLog logger = activity.getEventLog(); waitForIdle(); verify(logger).logChooserActivityShown(eq(false), eq(TEST_MIME_TYPE), anyLong()); @@ -1115,7 +1116,7 @@ public class UnbundledChooserActivityTest { final IChooserWrapper activity = (IChooserWrapper) mActivityRule.launchActivity(Intent.createChooser(sendIntent, "logger test")); - ChooserActivityLogger logger = activity.getChooserActivityLogger(); + EventLog logger = activity.getEventLog(); waitForIdle(); verify(logger).logChooserActivityShown(eq(true), eq(TEST_MIME_TYPE), anyLong()); @@ -1128,7 +1129,7 @@ public class UnbundledChooserActivityTest { final IChooserWrapper activity = (IChooserWrapper) mActivityRule.launchActivity( Intent.createChooser(sendIntent, "empty preview logger test")); - ChooserActivityLogger logger = activity.getChooserActivityLogger(); + EventLog logger = activity.getEventLog(); waitForIdle(); verify(logger).logChooserActivityShown(eq(false), eq(null), anyLong()); @@ -1147,7 +1148,7 @@ public class UnbundledChooserActivityTest { waitForIdle(); // Second invocation is from onCreate - ChooserActivityLogger logger = activity.getChooserActivityLogger(); + EventLog logger = activity.getEventLog(); Mockito.verify(logger, times(1)).logActionShareWithPreview(eq(CONTENT_PREVIEW_TEXT)); } @@ -1169,7 +1170,7 @@ public class UnbundledChooserActivityTest { final IChooserWrapper activity = (IChooserWrapper) mActivityRule.launchActivity(Intent.createChooser(sendIntent, null)); waitForIdle(); - ChooserActivityLogger logger = activity.getChooserActivityLogger(); + EventLog logger = activity.getEventLog(); Mockito.verify(logger, times(1)).logActionShareWithPreview(eq(CONTENT_PREVIEW_IMAGE)); } @@ -1371,8 +1372,8 @@ public class UnbundledChooserActivityTest { ArgumentCaptor hashCaptor = ArgumentCaptor.forClass(HashedStringCache.HashResult.class); - verify(activity.getChooserActivityLogger(), times(1)).logShareTargetSelected( - eq(ChooserActivityLogger.SELECTION_TYPE_SERVICE), + verify(activity.getEventLog(), times(1)).logShareTargetSelected( + eq(EventLog.SELECTION_TYPE_SERVICE), /* packageName= */ any(), /* positionPicked= */ anyInt(), /* directTargetAlsoRanked= */ eq(-1), @@ -1452,8 +1453,8 @@ public class UnbundledChooserActivityTest { .perform(click()); waitForIdle(); - verify(activity.getChooserActivityLogger(), times(1)).logShareTargetSelected( - eq(ChooserActivityLogger.SELECTION_TYPE_SERVICE), + verify(activity.getEventLog(), times(1)).logShareTargetSelected( + eq(EventLog.SELECTION_TYPE_SERVICE), /* packageName= */ any(), /* positionPicked= */ anyInt(), /* directTargetAlsoRanked= */ eq(0), @@ -1862,9 +1863,9 @@ public class UnbundledChooserActivityTest { .perform(click()); waitForIdle(); - ChooserActivityLogger logger = wrapper.getChooserActivityLogger(); + EventLog logger = wrapper.getEventLog(); verify(logger, times(1)).logShareTargetSelected( - eq(ChooserActivityLogger.SELECTION_TYPE_SERVICE), + eq(EventLog.SELECTION_TYPE_SERVICE), /* packageName= */ any(), /* positionPicked= */ anyInt(), // The packages sholdn't match for app target and direct target: @@ -2194,10 +2195,10 @@ public class UnbundledChooserActivityTest { .perform(click()); waitForIdle(); - ChooserActivityLogger logger = activity.getChooserActivityLogger(); + EventLog logger = activity.getEventLog(); ArgumentCaptor typeCaptor = ArgumentCaptor.forClass(Integer.class); verify(logger, times(1)).logShareTargetSelected( - eq(ChooserActivityLogger.SELECTION_TYPE_SERVICE), + eq(EventLog.SELECTION_TYPE_SERVICE), /* packageName= */ any(), /* positionPicked= */ anyInt(), /* directTargetAlsoRanked= */ anyInt(), diff --git a/java/tests/src/com/android/intentresolver/logging/EventLogTest.java b/java/tests/src/com/android/intentresolver/logging/EventLogTest.java new file mode 100644 index 00000000..17452774 --- /dev/null +++ b/java/tests/src/com/android/intentresolver/logging/EventLogTest.java @@ -0,0 +1,422 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.intentresolver.logging; + +import static com.google.common.truth.Truth.assertThat; + +import static org.mockito.AdditionalMatchers.gt; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; + +import android.content.Intent; +import android.metrics.LogMaker; + +import com.android.intentresolver.logging.EventLog.FrameworkStatsLogger; +import com.android.intentresolver.logging.EventLog.SharesheetStandardEvent; +import com.android.intentresolver.logging.EventLog.SharesheetStartedEvent; +import com.android.intentresolver.logging.EventLog.SharesheetTargetSelectedEvent; +import com.android.intentresolver.contentpreview.ContentPreviewType; +import com.android.internal.logging.InstanceId; +import com.android.internal.logging.MetricsLogger; +import com.android.internal.logging.UiEventLogger; +import com.android.internal.logging.UiEventLogger.UiEventEnum; +import com.android.internal.logging.nano.MetricsProto.MetricsEvent; +import com.android.internal.util.FrameworkStatsLog; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +@RunWith(MockitoJUnitRunner.class) +public final class EventLogTest { + @Mock private UiEventLogger mUiEventLog; + @Mock private FrameworkStatsLogger mFrameworkLog; + @Mock private MetricsLogger mMetricsLogger; + + private EventLog mChooserLogger; + + @Before + public void setUp() { + //Mockito.reset(mUiEventLog, mFrameworkLog, mMetricsLogger); + mChooserLogger = new EventLog(mUiEventLog, mFrameworkLog, mMetricsLogger); + } + + @After + public void tearDown() { + verifyNoMoreInteractions(mUiEventLog); + verifyNoMoreInteractions(mFrameworkLog); + verifyNoMoreInteractions(mMetricsLogger); + } + + @Test + public void testLogChooserActivityShown_personalProfile() { + final boolean isWorkProfile = false; + final String mimeType = "application/TestType"; + final long systemCost = 456; + + mChooserLogger.logChooserActivityShown(isWorkProfile, mimeType, systemCost); + + ArgumentCaptor eventCaptor = ArgumentCaptor.forClass(LogMaker.class); + verify(mMetricsLogger).write(eventCaptor.capture()); + LogMaker event = eventCaptor.getValue(); + + assertThat(event.getCategory()).isEqualTo(MetricsEvent.ACTION_ACTIVITY_CHOOSER_SHOWN); + assertThat(event.getSubtype()).isEqualTo(MetricsEvent.PARENT_PROFILE); + assertThat(event.getTaggedData(MetricsEvent.FIELD_SHARESHEET_MIMETYPE)).isEqualTo(mimeType); + assertThat(event.getTaggedData(MetricsEvent.FIELD_TIME_TO_APP_TARGETS)) + .isEqualTo(systemCost); + } + + @Test + public void testLogChooserActivityShown_workProfile() { + final boolean isWorkProfile = true; + final String mimeType = "application/TestType"; + final long systemCost = 456; + + mChooserLogger.logChooserActivityShown(isWorkProfile, mimeType, systemCost); + + ArgumentCaptor eventCaptor = ArgumentCaptor.forClass(LogMaker.class); + verify(mMetricsLogger).write(eventCaptor.capture()); + LogMaker event = eventCaptor.getValue(); + + assertThat(event.getCategory()).isEqualTo(MetricsEvent.ACTION_ACTIVITY_CHOOSER_SHOWN); + assertThat(event.getSubtype()).isEqualTo(MetricsEvent.MANAGED_PROFILE); + assertThat(event.getTaggedData(MetricsEvent.FIELD_SHARESHEET_MIMETYPE)).isEqualTo(mimeType); + assertThat(event.getTaggedData(MetricsEvent.FIELD_TIME_TO_APP_TARGETS)) + .isEqualTo(systemCost); + } + + @Test + public void testLogShareStarted() { + final String packageName = "com.test.foo"; + final String mimeType = "text/plain"; + final int appProvidedDirectTargets = 123; + final int appProvidedAppTargets = 456; + final boolean workProfile = true; + final int previewType = ContentPreviewType.CONTENT_PREVIEW_FILE; + final String intentAction = Intent.ACTION_SENDTO; + final int numCustomActions = 3; + final boolean modifyShareProvided = true; + + mChooserLogger.logShareStarted( + packageName, + mimeType, + appProvidedDirectTargets, + appProvidedAppTargets, + workProfile, + previewType, + intentAction, + numCustomActions, + modifyShareProvided); + + verify(mFrameworkLog).write( + eq(FrameworkStatsLog.SHARESHEET_STARTED), + eq(SharesheetStartedEvent.SHARE_STARTED.getId()), + eq(packageName), + /* instanceId=*/ gt(0), + eq(mimeType), + eq(appProvidedDirectTargets), + eq(appProvidedAppTargets), + eq(workProfile), + eq(FrameworkStatsLog.SHARESHEET_STARTED__PREVIEW_TYPE__CONTENT_PREVIEW_FILE), + eq(FrameworkStatsLog.SHARESHEET_STARTED__INTENT_TYPE__INTENT_ACTION_SENDTO), + /* custom actions provided */ eq(numCustomActions), + /* reselection action provided */ eq(modifyShareProvided)); + } + + @Test + public void testLogShareTargetSelected() { + final int targetType = EventLog.SELECTION_TYPE_SERVICE; + final String packageName = "com.test.foo"; + final int positionPicked = 123; + final int directTargetAlsoRanked = -1; + final int callerTargetCount = 0; + final boolean isPinned = true; + final boolean isSuccessfullySelected = true; + final long selectionCost = 456; + + mChooserLogger.logShareTargetSelected( + targetType, + packageName, + positionPicked, + directTargetAlsoRanked, + callerTargetCount, + /* directTargetHashed= */ null, + isPinned, + isSuccessfullySelected, + selectionCost); + + verify(mFrameworkLog).write( + eq(FrameworkStatsLog.RANKING_SELECTED), + eq(SharesheetTargetSelectedEvent.SHARESHEET_SERVICE_TARGET_SELECTED.getId()), + eq(packageName), + /* instanceId=*/ gt(0), + eq(positionPicked), + eq(isPinned)); + + ArgumentCaptor eventCaptor = ArgumentCaptor.forClass(LogMaker.class); + verify(mMetricsLogger).write(eventCaptor.capture()); + LogMaker event = eventCaptor.getValue(); + assertThat(event.getCategory()).isEqualTo( + MetricsEvent.ACTION_ACTIVITY_CHOOSER_PICKED_SERVICE_TARGET); + assertThat(event.getSubtype()).isEqualTo(positionPicked); + } + + @Test + public void testLogActionSelected() { + mChooserLogger.logActionSelected(EventLog.SELECTION_TYPE_COPY); + + verify(mFrameworkLog).write( + eq(FrameworkStatsLog.RANKING_SELECTED), + eq(SharesheetTargetSelectedEvent.SHARESHEET_COPY_TARGET_SELECTED.getId()), + eq(""), + /* instanceId=*/ gt(0), + eq(-1), + eq(false)); + + ArgumentCaptor eventCaptor = ArgumentCaptor.forClass(LogMaker.class); + verify(mMetricsLogger).write(eventCaptor.capture()); + LogMaker event = eventCaptor.getValue(); + assertThat(event.getCategory()).isEqualTo( + MetricsEvent.ACTION_ACTIVITY_CHOOSER_PICKED_SYSTEM_TARGET); + assertThat(event.getSubtype()).isEqualTo(1); + } + + @Test + public void testLogCustomActionSelected() { + final int position = 4; + mChooserLogger.logCustomActionSelected(position); + + verify(mFrameworkLog).write( + eq(FrameworkStatsLog.RANKING_SELECTED), + eq(SharesheetTargetSelectedEvent.SHARESHEET_CUSTOM_ACTION_SELECTED.getId()), + any(), anyInt(), eq(position), eq(false)); + } + + @Test + public void testLogDirectShareTargetReceived() { + final int category = MetricsEvent.ACTION_DIRECT_SHARE_TARGETS_LOADED_SHORTCUT_MANAGER; + final int latency = 123; + + mChooserLogger.logDirectShareTargetReceived(category, latency); + + ArgumentCaptor eventCaptor = ArgumentCaptor.forClass(LogMaker.class); + verify(mMetricsLogger).write(eventCaptor.capture()); + LogMaker event = eventCaptor.getValue(); + assertThat(event.getCategory()).isEqualTo(category); + assertThat(event.getSubtype()).isEqualTo(latency); + } + + @Test + public void testLogActionShareWithPreview() { + final int previewType = ContentPreviewType.CONTENT_PREVIEW_TEXT; + + mChooserLogger.logActionShareWithPreview(previewType); + + ArgumentCaptor eventCaptor = ArgumentCaptor.forClass(LogMaker.class); + verify(mMetricsLogger).write(eventCaptor.capture()); + LogMaker event = eventCaptor.getValue(); + assertThat(event.getCategory()).isEqualTo(MetricsEvent.ACTION_SHARE_WITH_PREVIEW); + assertThat(event.getSubtype()).isEqualTo(previewType); + } + + @Test + public void testLogSharesheetTriggered() { + mChooserLogger.logSharesheetTriggered(); + verify(mUiEventLog).logWithInstanceId( + eq(SharesheetStandardEvent.SHARESHEET_TRIGGERED), eq(0), isNull(), any()); + } + + @Test + public void testLogSharesheetAppLoadComplete() { + mChooserLogger.logSharesheetAppLoadComplete(); + verify(mUiEventLog).logWithInstanceId( + eq(SharesheetStandardEvent.SHARESHEET_APP_LOAD_COMPLETE), eq(0), isNull(), any()); + } + + @Test + public void testLogSharesheetDirectLoadComplete() { + mChooserLogger.logSharesheetDirectLoadComplete(); + verify(mUiEventLog).logWithInstanceId( + eq(SharesheetStandardEvent.SHARESHEET_DIRECT_LOAD_COMPLETE), + eq(0), + isNull(), + any()); + } + + @Test + public void testLogSharesheetDirectLoadTimeout() { + mChooserLogger.logSharesheetDirectLoadTimeout(); + verify(mUiEventLog).logWithInstanceId( + eq(SharesheetStandardEvent.SHARESHEET_DIRECT_LOAD_TIMEOUT), eq(0), isNull(), any()); + } + + @Test + public void testLogSharesheetProfileChanged() { + mChooserLogger.logSharesheetProfileChanged(); + verify(mUiEventLog).logWithInstanceId( + eq(SharesheetStandardEvent.SHARESHEET_PROFILE_CHANGED), eq(0), isNull(), any()); + } + + @Test + public void testLogSharesheetExpansionChanged_collapsed() { + mChooserLogger.logSharesheetExpansionChanged(/* isCollapsed=*/ true); + verify(mUiEventLog).logWithInstanceId( + eq(SharesheetStandardEvent.SHARESHEET_COLLAPSED), eq(0), isNull(), any()); + } + + @Test + public void testLogSharesheetExpansionChanged_expanded() { + mChooserLogger.logSharesheetExpansionChanged(/* isCollapsed=*/ false); + verify(mUiEventLog).logWithInstanceId( + eq(SharesheetStandardEvent.SHARESHEET_EXPANDED), eq(0), isNull(), any()); + } + + @Test + public void testLogSharesheetAppShareRankingTimeout() { + mChooserLogger.logSharesheetAppShareRankingTimeout(); + verify(mUiEventLog).logWithInstanceId( + eq(SharesheetStandardEvent.SHARESHEET_APP_SHARE_RANKING_TIMEOUT), + eq(0), + isNull(), + any()); + } + + @Test + public void testLogSharesheetEmptyDirectShareRow() { + mChooserLogger.logSharesheetEmptyDirectShareRow(); + verify(mUiEventLog).logWithInstanceId( + eq(SharesheetStandardEvent.SHARESHEET_EMPTY_DIRECT_SHARE_ROW), + eq(0), + isNull(), + any()); + } + + @Test + public void testDifferentLoggerInstancesUseDifferentInstanceIds() { + ArgumentCaptor idIntCaptor = ArgumentCaptor.forClass(Integer.class); + EventLog chooserLogger2 = + new EventLog(mUiEventLog, mFrameworkLog, mMetricsLogger); + + final int targetType = EventLog.SELECTION_TYPE_COPY; + final String packageName = "com.test.foo"; + final int positionPicked = 123; + final int directTargetAlsoRanked = -1; + final int callerTargetCount = 0; + final boolean isPinned = true; + final boolean isSuccessfullySelected = true; + final long selectionCost = 456; + + mChooserLogger.logShareTargetSelected( + targetType, + packageName, + positionPicked, + directTargetAlsoRanked, + callerTargetCount, + /* directTargetHashed= */ null, + isPinned, + isSuccessfullySelected, + selectionCost); + + chooserLogger2.logShareTargetSelected( + targetType, + packageName, + positionPicked, + directTargetAlsoRanked, + callerTargetCount, + /* directTargetHashed= */ null, + isPinned, + isSuccessfullySelected, + selectionCost); + + verify(mFrameworkLog, times(2)).write( + anyInt(), anyInt(), anyString(), idIntCaptor.capture(), anyInt(), anyBoolean()); + + int id1 = idIntCaptor.getAllValues().get(0); + int id2 = idIntCaptor.getAllValues().get(1); + + assertThat(id1).isGreaterThan(0); + assertThat(id2).isGreaterThan(0); + assertThat(id1).isNotEqualTo(id2); + } + + @Test + public void testUiAndFrameworkEventsUseSameInstanceIdForSameLoggerInstance() { + ArgumentCaptor idIntCaptor = ArgumentCaptor.forClass(Integer.class); + ArgumentCaptor idObjectCaptor = ArgumentCaptor.forClass(InstanceId.class); + + final int targetType = EventLog.SELECTION_TYPE_COPY; + final String packageName = "com.test.foo"; + final int positionPicked = 123; + final int directTargetAlsoRanked = -1; + final int callerTargetCount = 0; + final boolean isPinned = true; + final boolean isSuccessfullySelected = true; + final long selectionCost = 456; + + mChooserLogger.logShareTargetSelected( + targetType, + packageName, + positionPicked, + directTargetAlsoRanked, + callerTargetCount, + /* directTargetHashed= */ null, + isPinned, + isSuccessfullySelected, + selectionCost); + + verify(mFrameworkLog).write( + anyInt(), anyInt(), anyString(), idIntCaptor.capture(), anyInt(), anyBoolean()); + + mChooserLogger.logSharesheetTriggered(); + verify(mUiEventLog).logWithInstanceId( + any(UiEventEnum.class), anyInt(), any(), idObjectCaptor.capture()); + + assertThat(idIntCaptor.getValue()).isGreaterThan(0); + assertThat(idObjectCaptor.getValue().getId()).isEqualTo(idIntCaptor.getValue()); + } + + @Test + public void testTargetSelectionCategories() { + assertThat(EventLog.getTargetSelectionCategory( + EventLog.SELECTION_TYPE_SERVICE)) + .isEqualTo(MetricsEvent.ACTION_ACTIVITY_CHOOSER_PICKED_SERVICE_TARGET); + assertThat(EventLog.getTargetSelectionCategory( + EventLog.SELECTION_TYPE_APP)) + .isEqualTo(MetricsEvent.ACTION_ACTIVITY_CHOOSER_PICKED_APP_TARGET); + assertThat(EventLog.getTargetSelectionCategory( + EventLog.SELECTION_TYPE_STANDARD)) + .isEqualTo(MetricsEvent.ACTION_ACTIVITY_CHOOSER_PICKED_STANDARD_TARGET); + assertThat(EventLog.getTargetSelectionCategory( + EventLog.SELECTION_TYPE_COPY)).isEqualTo(0); + assertThat(EventLog.getTargetSelectionCategory( + EventLog.SELECTION_TYPE_NEARBY)).isEqualTo(0); + assertThat(EventLog.getTargetSelectionCategory( + EventLog.SELECTION_TYPE_EDIT)).isEqualTo(0); + } +} -- cgit v1.2.3-59-g8ed1b From cf0a09f6c54ee6060469bcd85acbefd382eba439 Mon Sep 17 00:00:00 2001 From: Govinda Wasserman Date: Mon, 17 Jul 2023 10:27:43 -0400 Subject: Creates ChooserActivity alias This alias allows us to avoid a bug that caused Share Sheet to crash any app that tried to use it. It also removes the previous solution as it had the potential to not take effect if too many receivers tried to act on the BOOT_COMPLETE broadcast. Test: $ adb root Test: $ adb shell pm disable com.android.intentresolver/.ChooserActivity Test: $ adb shell pm resolve-activity -a android.intent.action.CHOOSER Test: Verify that the activity resolves BUG: 283722356 FIX: 283722356 Change-Id: I820c5f88204c83e635cb877b222bdf6cbe53aaff --- AndroidManifest-app.xml | 32 ++++++++--------- .../intentresolver/ChooserActivityReEnabler.kt | 40 ---------------------- .../intentresolver/dagger/ReceiverBinderModule.kt | 10 ++---- 3 files changed, 18 insertions(+), 64 deletions(-) delete mode 100644 java/src/com/android/intentresolver/ChooserActivityReEnabler.kt (limited to 'java/src') diff --git a/AndroidManifest-app.xml b/AndroidManifest-app.xml index d542e627..482c9f38 100644 --- a/AndroidManifest-app.xml +++ b/AndroidManifest-app.xml @@ -34,15 +34,12 @@ tools:replace="android:appComponentFactory" android:appComponentFactory=".IntentResolverAppComponentFactory"> - + + + + + + + + diff --git a/java/src/com/android/intentresolver/contentpreview/ChooserContentPreviewUi.java b/java/src/com/android/intentresolver/contentpreview/ChooserContentPreviewUi.java index e8367c4e..18cbb8af 100644 --- a/java/src/com/android/intentresolver/contentpreview/ChooserContentPreviewUi.java +++ b/java/src/com/android/intentresolver/contentpreview/ChooserContentPreviewUi.java @@ -163,10 +163,12 @@ public final class ChooserContentPreviewUi { UnifiedContentPreviewUi unifiedContentPreviewUi = new UnifiedContentPreviewUi( isSingleImageShare, + targetIntent.getType(), actionFactory, imageLoader, typeClassifier, transitionElementStatusCallback, + previewData.getUriCount(), headlineGenerator); previewData.getFileMetadataForImagePreview(mLifecycle, unifiedContentPreviewUi::setFiles); return unifiedContentPreviewUi; diff --git a/java/src/com/android/intentresolver/contentpreview/UnifiedContentPreviewUi.java b/java/src/com/android/intentresolver/contentpreview/UnifiedContentPreviewUi.java index 6385f2b6..5db5020e 100644 --- a/java/src/com/android/intentresolver/contentpreview/UnifiedContentPreviewUi.java +++ b/java/src/com/android/intentresolver/contentpreview/UnifiedContentPreviewUi.java @@ -37,11 +37,14 @@ import java.util.Objects; class UnifiedContentPreviewUi extends ContentPreviewUi { private final boolean mShowEditAction; + @Nullable + private final String mIntentMimeType; private final ChooserContentPreviewUi.ActionFactory mActionFactory; private final ImageLoader mImageLoader; private final MimeTypeClassifier mTypeClassifier; private final TransitionElementStatusCallback mTransitionElementStatusCallback; private final HeadlineGenerator mHeadlineGenerator; + private final int mItemCount; @Nullable private List mFiles; @Nullable @@ -49,16 +52,20 @@ class UnifiedContentPreviewUi extends ContentPreviewUi { UnifiedContentPreviewUi( boolean isSingleImage, + @Nullable String intentMimeType, ChooserContentPreviewUi.ActionFactory actionFactory, ImageLoader imageLoader, MimeTypeClassifier typeClassifier, TransitionElementStatusCallback transitionElementStatusCallback, + int itemCount, HeadlineGenerator headlineGenerator) { mShowEditAction = isSingleImage; + mIntentMimeType = intentMimeType; mActionFactory = actionFactory; mImageLoader = imageLoader; mTypeClassifier = typeClassifier; mTransitionElementStatusCallback = transitionElementStatusCallback; + mItemCount = itemCount; mHeadlineGenerator = headlineGenerator; } @@ -96,11 +103,19 @@ class UnifiedContentPreviewUi extends ContentPreviewUi { ScrollableImagePreviewView imagePreview = mContentPreviewView.requireViewById(R.id.scrollable_image_preview); + imagePreview.setImageLoader(mImageLoader); imagePreview.setOnNoPreviewCallback(() -> imagePreview.setVisibility(View.GONE)); imagePreview.setTransitionElementStatusCallback(mTransitionElementStatusCallback); if (mFiles != null) { updatePreviewWithFiles(mContentPreviewView, mFiles); + } else { + displayHeadline( + mContentPreviewView, + mItemCount, + mTypeClassifier.isImageType(mIntentMimeType), + mTypeClassifier.isVideoType(mIntentMimeType)); + imagePreview.setLoading(mItemCount); } return mContentPreviewView; @@ -138,14 +153,18 @@ class UnifiedContentPreviewUi extends ContentPreviewUi { } } - imagePreview.setPreviews(previews, count - previews.size(), mImageLoader); + imagePreview.setPreviews(previews, count - previews.size()); + displayHeadline(contentPreviewView, count, allImages, allVideos); + } + private void displayHeadline( + ViewGroup layout, int count, boolean allImages, boolean allVideos) { if (allImages) { - displayHeadline(contentPreviewView, mHeadlineGenerator.getImagesHeadline(count)); + displayHeadline(layout, mHeadlineGenerator.getImagesHeadline(count)); } else if (allVideos) { - displayHeadline(contentPreviewView, mHeadlineGenerator.getVideosHeadline(count)); + displayHeadline(layout, mHeadlineGenerator.getVideosHeadline(count)); } else { - displayHeadline(contentPreviewView, mHeadlineGenerator.getFilesHeadline(count)); + displayHeadline(layout, mHeadlineGenerator.getFilesHeadline(count)); } } } diff --git a/java/src/com/android/intentresolver/widget/ScrollableImagePreviewView.kt b/java/src/com/android/intentresolver/widget/ScrollableImagePreviewView.kt index 583a2887..d9844d7b 100644 --- a/java/src/com/android/intentresolver/widget/ScrollableImagePreviewView.kt +++ b/java/src/com/android/intentresolver/widget/ScrollableImagePreviewView.kt @@ -158,22 +158,28 @@ class ScrollableImagePreviewView : RecyclerView, ImagePreviewView { return null } - fun setPreviews(previews: List, otherItemCount: Int, imageLoader: CachingImageLoader) { - previewAdapter.reset(0, imageLoader) + fun setImageLoader(imageLoader: CachingImageLoader) { + previewAdapter.imageLoader = imageLoader + } + + fun setLoading(totalItemCount: Int) { + previewAdapter.reset(totalItemCount) + } + + fun setPreviews(previews: List, otherItemCount: Int) { + previewAdapter.reset(previews.size + otherItemCount) batchLoader?.cancel() batchLoader = BatchPreviewLoader( - imageLoader, + previewAdapter.imageLoader ?: error("Image loader is not set"), previews, otherItemCount, - onReset = { totalItemCount -> - previewAdapter.reset(totalItemCount, imageLoader) - }, onUpdate = previewAdapter::addPreviews, onCompletion = { if (!previewAdapter.hasPreviews) { onNoPreviewCallback?.run() } + previewAdapter.markLoaded() } ) .apply { @@ -262,10 +268,11 @@ class ScrollableImagePreviewView : RecyclerView, ImagePreviewView { context.resources.getString(R.string.video_preview_a11y_description) private val filePreviewDescription = context.resources.getString(R.string.file_preview_a11y_description) - private var imageLoader: CachingImageLoader? = null + var imageLoader: CachingImageLoader? = null private var firstImagePos = -1 private var totalItemCount: Int = 0 + private var isLoading = false private val hasOtherItem get() = previews.size < totalItemCount val hasPreviews: Boolean @@ -273,61 +280,78 @@ class ScrollableImagePreviewView : RecyclerView, ImagePreviewView { var transitionStatusElementCallback: TransitionElementStatusCallback? = null - fun reset(totalItemCount: Int, imageLoader: CachingImageLoader) { - this.imageLoader = imageLoader + fun reset(totalItemCount: Int) { firstImagePos = -1 previews.clear() this.totalItemCount = maxOf(0, totalItemCount) + isLoading = this.totalItemCount > 0 notifyDataSetChanged() } + fun markLoaded() { + if (!isLoading) return + isLoading = false + if (hasOtherItem) { + notifyItemChanged(previews.size) + } else { + notifyItemRemoved(previews.size) + } + } + fun addPreviews(newPreviews: Collection) { if (newPreviews.isEmpty()) return val insertPos = previews.size val hadOtherItem = hasOtherItem + val wasEmpty = previews.isEmpty() previews.addAll(newPreviews) if (firstImagePos < 0) { val pos = newPreviews.indexOfFirst { it.type == PreviewType.Image } if (pos >= 0) firstImagePos = insertPos + pos } - notifyItemRangeInserted(insertPos, newPreviews.size) - when { - hadOtherItem && previews.size >= totalItemCount -> { - notifyItemRemoved(previews.size) - } - !hadOtherItem && previews.size < totalItemCount -> { - notifyItemInserted(previews.size) + if (wasEmpty) { + // we don't want any item animation in that case + notifyDataSetChanged() + } else { + notifyItemRangeInserted(insertPos, newPreviews.size) + when { + hadOtherItem && !hasOtherItem -> { + notifyItemRemoved(previews.size) + } + !hadOtherItem && hasOtherItem -> { + notifyItemInserted(previews.size) + } } } } override fun onCreateViewHolder(parent: ViewGroup, itemType: Int): ViewHolder { val view = LayoutInflater.from(context).inflate(itemType, parent, false) - return if (itemType == R.layout.image_preview_other_item) { - OtherItemViewHolder(view) - } else { - PreviewViewHolder( - view, - imagePreviewDescription, - videoPreviewDescription, - filePreviewDescription, - ) + return when (itemType) { + R.layout.image_preview_other_item -> OtherItemViewHolder(view) + R.layout.image_preview_loading_item -> LoadingItemViewHolder(view) + else -> + PreviewViewHolder( + view, + imagePreviewDescription, + videoPreviewDescription, + filePreviewDescription, + ) } } - override fun getItemCount(): Int = previews.size + if (hasOtherItem) 1 else 0 + override fun getItemCount(): Int = previews.size + if (isLoading || hasOtherItem) 1 else 0 - override fun getItemViewType(position: Int): Int { - return if (position == previews.size) { - R.layout.image_preview_other_item - } else { - R.layout.image_preview_image_item + override fun getItemViewType(position: Int): Int = + when { + position == previews.size && isLoading -> R.layout.image_preview_loading_item + position == previews.size -> R.layout.image_preview_other_item + else -> R.layout.image_preview_image_item } - } override fun onBindViewHolder(vh: ViewHolder, position: Int) { when (vh) { is OtherItemViewHolder -> vh.bind(totalItemCount - previews.size) + is LoadingItemViewHolder -> vh.bind() is PreviewViewHolder -> vh.bind( previews[position], @@ -466,6 +490,11 @@ class ScrollableImagePreviewView : RecyclerView, ImagePreviewView { override fun unbind() = Unit } + private class LoadingItemViewHolder(view: View) : ViewHolder(view) { + fun bind() = Unit + override fun unbind() = Unit + } + private class SpacingDecoration(private val innerSpacing: Int, private val outerSpacing: Int) : ItemDecoration() { override fun getItemOffsets(outRect: Rect, view: View, parent: RecyclerView, state: State) { @@ -487,7 +516,6 @@ class ScrollableImagePreviewView : RecyclerView, ImagePreviewView { private val imageLoader: CachingImageLoader, previews: List, otherItemCount: Int, - private val onReset: (Int) -> Unit, private val onUpdate: (List) -> Unit, private val onCompletion: () -> Unit, ) { @@ -527,9 +555,6 @@ class ScrollableImagePreviewView : RecyclerView, ImagePreviewView { } } .collect { - if (blockStart == 0) { - onReset(totalItemCount) - } val updates = ArrayList(blockEnd - blockStart) while (blockStart < blockEnd) { if (previewWidths[blockStart] > 0) { diff --git a/java/tests/src/com/android/intentresolver/contentpreview/UnifiedContentPreviewUiTest.kt b/java/tests/src/com/android/intentresolver/contentpreview/UnifiedContentPreviewUiTest.kt new file mode 100644 index 00000000..08331209 --- /dev/null +++ b/java/tests/src/com/android/intentresolver/contentpreview/UnifiedContentPreviewUiTest.kt @@ -0,0 +1,152 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.intentresolver.contentpreview + +import android.net.Uri +import android.view.LayoutInflater +import android.view.ViewGroup +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry.getInstrumentation +import com.android.intentresolver.R.layout.chooser_grid +import com.android.intentresolver.mock +import com.android.intentresolver.whenever +import com.android.intentresolver.widget.ImagePreviewView.TransitionElementStatusCallback +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mockito.anyInt +import org.mockito.Mockito.times +import org.mockito.Mockito.verify + +@RunWith(AndroidJUnit4::class) +class UnifiedContentPreviewUiTest { + private val actionFactory = + mock { + whenever(createCustomActions()).thenReturn(emptyList()) + } + private val imageLoader = mock() + private val headlineGenerator = + mock { + whenever(getImagesHeadline(anyInt())).thenReturn("Image Headline") + whenever(getVideosHeadline(anyInt())).thenReturn("Video Headline") + whenever(getFilesHeadline(anyInt())).thenReturn("Files Headline") + } + + private val context + get() = getInstrumentation().getContext() + + @Test + fun test_displayImagesWithoutUriMetadata_showImagesHeadline() { + testLoadingHeadline("image/*", files = null) + + verify(headlineGenerator, times(1)).getImagesHeadline(2) + } + + @Test + fun test_displayVideosWithoutUriMetadata_showImagesHeadline() { + testLoadingHeadline("video/*", files = null) + + verify(headlineGenerator, times(1)).getVideosHeadline(2) + } + + @Test + fun test_displayDocumentsWithoutUriMetadata_showImagesHeadline() { + testLoadingHeadline("application/pdf", files = null) + + verify(headlineGenerator, times(1)).getFilesHeadline(2) + } + + @Test + fun test_displayMixedContentWithoutUriMetadata_showImagesHeadline() { + testLoadingHeadline("*/*", files = null) + + verify(headlineGenerator, times(1)).getFilesHeadline(2) + } + + @Test + fun test_displayImagesWithUriMetadataSet_showImagesHeadline() { + val uri = Uri.parse("content://pkg.app/image.png") + val files = + listOf( + FileInfo.Builder(uri).withMimeType("image/png").build(), + FileInfo.Builder(uri).withMimeType("image/jpeg").build(), + ) + testLoadingHeadline("image/*", files) + + verify(headlineGenerator, times(1)).getImagesHeadline(2) + } + + @Test + fun test_displayVideosWithUriMetadataSet_showImagesHeadline() { + val uri = Uri.parse("content://pkg.app/image.png") + val files = + listOf( + FileInfo.Builder(uri).withMimeType("video/mp4").build(), + FileInfo.Builder(uri).withMimeType("video/mp4").build(), + ) + testLoadingHeadline("video/*", files) + + verify(headlineGenerator, times(1)).getVideosHeadline(2) + } + + @Test + fun test_displayImagesAndVideosWithUriMetadataSet_showImagesHeadline() { + val uri = Uri.parse("content://pkg.app/image.png") + val files = + listOf( + FileInfo.Builder(uri).withMimeType("image/png").build(), + FileInfo.Builder(uri).withMimeType("video/mp4").build(), + ) + testLoadingHeadline("*/*", files) + + verify(headlineGenerator, times(1)).getFilesHeadline(2) + } + + @Test + fun test_displayDocumentsWithUriMetadataSet_showImagesHeadline() { + val uri = Uri.parse("content://pkg.app/image.png") + val files = + listOf( + FileInfo.Builder(uri).withMimeType("application/pdf").build(), + FileInfo.Builder(uri).withMimeType("application/pdf").build(), + ) + testLoadingHeadline("application/pdf", files) + + verify(headlineGenerator, times(1)).getFilesHeadline(2) + } + + private fun testLoadingHeadline(intentMimeType: String, files: List?) { + val testSubject = + UnifiedContentPreviewUi( + /*isSingleImage=*/ false, + intentMimeType, + actionFactory, + imageLoader, + DefaultMimeTypeClassifier, + object : TransitionElementStatusCallback { + override fun onTransitionElementReady(name: String) = Unit + override fun onAllTransitionElementsReady() = Unit + }, + /*itemCount=*/ 2, + headlineGenerator + ) + val layoutInflater = LayoutInflater.from(context) + val gridLayout = layoutInflater.inflate(chooser_grid, null, false) as ViewGroup + + files?.let(testSubject::setFiles) + testSubject.display(context.resources, LayoutInflater.from(context), gridLayout) + } +} diff --git a/java/tests/src/com/android/intentresolver/widget/BatchPreviewLoaderTest.kt b/java/tests/src/com/android/intentresolver/widget/BatchPreviewLoaderTest.kt index e65cba5f..a0211308 100644 --- a/java/tests/src/com/android/intentresolver/widget/BatchPreviewLoaderTest.kt +++ b/java/tests/src/com/android/intentresolver/widget/BatchPreviewLoaderTest.kt @@ -47,7 +47,6 @@ class BatchPreviewLoaderTest { private val dispatcher = UnconfinedTestDispatcher() private val testScope = CoroutineScope(dispatcher) private val onCompletion = mock<() -> Unit>() - private val onReset = mock<(Int) -> Unit>() private val onUpdate = mock<(List) -> Unit>() @Before @@ -68,19 +67,11 @@ class BatchPreviewLoaderTest { val uriTwo = createUri(2) imageLoader.setUriLoadingOrder(succeed(uriTwo), succeed(uriOne)) val testSubject = - BatchPreviewLoader( - imageLoader, - previews(uriOne, uriTwo), - 0, - onReset, - onUpdate, - onCompletion - ) + BatchPreviewLoader(imageLoader, previews(uriOne, uriTwo), 0, onUpdate, onCompletion) testSubject.loadAspectRatios(200) { _, _, _ -> 100 } dispatcher.scheduler.advanceUntilIdle() verify(onCompletion, times(1)).invoke() - verify(onReset, times(1)).invoke(2) val list = withArgCaptor { verify(onUpdate, times(1)).invoke(capture()) }.map { it.uri } assertThat(list).containsExactly(uriOne, uriTwo).inOrder() } @@ -97,7 +88,6 @@ class BatchPreviewLoaderTest { imageLoader, previews(uriOne, uriTwo, uriThree), 0, - onReset, onUpdate, onCompletion ) @@ -105,7 +95,6 @@ class BatchPreviewLoaderTest { dispatcher.scheduler.advanceUntilIdle() verify(onCompletion, times(1)).invoke() - verify(onReset, times(1)).invoke(3) val list = withArgCaptor { verify(onUpdate, times(1)).invoke(capture()) }.map { it.uri } assertThat(list).containsExactly(uriOne, uriThree).inOrder() } @@ -126,12 +115,11 @@ class BatchPreviewLoaderTest { } imageLoader.setUriLoadingOrder(*loadingOrder) val testSubject = - BatchPreviewLoader(imageLoader, previews(*uris), 0, onReset, onUpdate, onCompletion) + BatchPreviewLoader(imageLoader, previews(*uris), 0, onUpdate, onCompletion) testSubject.loadAspectRatios(200) { _, _, _ -> 100 } dispatcher.scheduler.advanceUntilIdle() verify(onCompletion, times(1)).invoke() - verify(onReset, times(1)).invoke(uris.size) val list = captureMany { verify(onUpdate, atLeast(1)).invoke(capture()) } .fold(ArrayList()) { acc, update -> acc.apply { addAll(update) } } @@ -156,12 +144,11 @@ class BatchPreviewLoaderTest { val expectedUris = Array(uris.size / 2) { createUri(it * 2 + 1) } imageLoader.setUriLoadingOrder(*loadingOrder) val testSubject = - BatchPreviewLoader(imageLoader, previews(*uris), 0, onReset, onUpdate, onCompletion) + BatchPreviewLoader(imageLoader, previews(*uris), 0, onUpdate, onCompletion) testSubject.loadAspectRatios(200) { _, _, _ -> 100 } dispatcher.scheduler.advanceUntilIdle() verify(onCompletion, times(1)).invoke() - verify(onReset, times(1)).invoke(uris.size) val list = captureMany { verify(onUpdate, atLeast(1)).invoke(capture()) } .fold(ArrayList()) { acc, update -> acc.apply { addAll(update) } } -- cgit v1.2.3-59-g8ed1b From ec63bbe163af26cf64e8b122dfb923d1a70a62f7 Mon Sep 17 00:00:00 2001 From: Andrey Epin Date: Tue, 15 Aug 2023 23:27:30 -0700 Subject: Do not wait for URI metadata to display text in Img+Text UI Image plus text content preview UI can display shared text right away without waiting for the shared URI metadata. Headline can be derived from the target intent's mime type and refined when the metadata is loaded. Bug: 292157413 Test: manual testing with ShareTest app Test: unit tests Change-Id: Icbdc1cccc16643e7e49be6dbfe9499db0278d9ac --- .../contentpreview/ChooserContentPreviewUi.java | 1 + .../FilesPlusTextContentPreviewUi.java | 46 +++-- .../FilesPlusTextContentPreviewUiTest.kt | 225 +++++++++++++++++++++ 3 files changed, 255 insertions(+), 17 deletions(-) create mode 100644 java/tests/src/com/android/intentresolver/contentpreview/FilesPlusTextContentPreviewUiTest.kt (limited to 'java/src') diff --git a/java/src/com/android/intentresolver/contentpreview/ChooserContentPreviewUi.java b/java/src/com/android/intentresolver/contentpreview/ChooserContentPreviewUi.java index 18cbb8af..d4874cac 100644 --- a/java/src/com/android/intentresolver/contentpreview/ChooserContentPreviewUi.java +++ b/java/src/com/android/intentresolver/contentpreview/ChooserContentPreviewUi.java @@ -150,6 +150,7 @@ public final class ChooserContentPreviewUi { isSingleImageShare, previewData.getUriCount(), targetIntent.getCharSequenceExtra(Intent.EXTRA_TEXT), + targetIntent.getType(), actionFactory, imageLoader, typeClassifier, diff --git a/java/src/com/android/intentresolver/contentpreview/FilesPlusTextContentPreviewUi.java b/java/src/com/android/intentresolver/contentpreview/FilesPlusTextContentPreviewUi.java index 35990990..6e1212e9 100644 --- a/java/src/com/android/intentresolver/contentpreview/FilesPlusTextContentPreviewUi.java +++ b/java/src/com/android/intentresolver/contentpreview/FilesPlusTextContentPreviewUi.java @@ -49,6 +49,8 @@ import java.util.function.Consumer; */ class FilesPlusTextContentPreviewUi extends ContentPreviewUi { private final Lifecycle mLifecycle; + @Nullable + private final String mIntentMimeType; private final CharSequence mText; private final ChooserContentPreviewUi.ActionFactory mActionFactory; private final ImageLoader mImageLoader; @@ -70,15 +72,17 @@ class FilesPlusTextContentPreviewUi extends ContentPreviewUi { boolean isSingleImage, int fileCount, CharSequence text, + @Nullable String intentMimeType, ChooserContentPreviewUi.ActionFactory actionFactory, ImageLoader imageLoader, MimeTypeClassifier typeClassifier, HeadlineGenerator headlineGenerator) { - mLifecycle = lifecycle; if (isSingleImage && fileCount != 1) { throw new IllegalArgumentException( "fileCount = " + fileCount + " and isSingleImage = true"); } + mLifecycle = lifecycle; + mIntentMimeType = intentMimeType; mFileCount = fileCount; mIsSingleImage = isSingleImage; mText = text; @@ -127,18 +131,25 @@ class FilesPlusTextContentPreviewUi extends ContentPreviewUi { List actions = mActionFactory.createCustomActions(); actionRow.setActions(actions); + if (!mIsSingleImage) { + mContentPreviewView.requireViewById(R.id.image_view).setVisibility(View.GONE); + } + prepareTextPreview(mContentPreviewView, mActionFactory); if (mIsMetadataUpdated) { updateUiWithMetadata(mContentPreviewView); - } else if (!mIsSingleImage) { - mContentPreviewView.requireViewById(R.id.image_view).setVisibility(View.GONE); + } else { + updateHeadline( + mContentPreviewView, + mFileCount, + mTypeClassifier.isImageType(mIntentMimeType), + mTypeClassifier.isVideoType(mIntentMimeType)); } return mContentPreviewView; } private void updateUiWithMetadata(ViewGroup contentPreviewView) { - prepareTextPreview(contentPreviewView, mActionFactory); - updateHeadline(contentPreviewView); + updateHeadline(contentPreviewView, mFileCount, mAllImages, mAllVideos); ImageView imagePreview = mContentPreviewView.requireViewById(R.id.image_view); if (mIsSingleImage && mFirstFilePreviewUri != null) { @@ -157,24 +168,25 @@ class FilesPlusTextContentPreviewUi extends ContentPreviewUi { } } - private void updateHeadline(ViewGroup contentPreview) { + private void updateHeadline( + ViewGroup contentPreview, int fileCount, boolean allImages, boolean allVideos) { CheckBox includeText = contentPreview.requireViewById(R.id.include_text_action); String headline; if (includeText.getVisibility() == View.VISIBLE && includeText.isChecked()) { - if (mAllImages) { - headline = mHeadlineGenerator.getImagesWithTextHeadline(mText, mFileCount); - } else if (mAllVideos) { - headline = mHeadlineGenerator.getVideosWithTextHeadline(mText, mFileCount); + if (allImages) { + headline = mHeadlineGenerator.getImagesWithTextHeadline(mText, fileCount); + } else if (allVideos) { + headline = mHeadlineGenerator.getVideosWithTextHeadline(mText, fileCount); } else { - headline = mHeadlineGenerator.getFilesWithTextHeadline(mText, mFileCount); + headline = mHeadlineGenerator.getFilesWithTextHeadline(mText, fileCount); } } else { - if (mAllImages) { - headline = mHeadlineGenerator.getImagesHeadline(mFileCount); - } else if (mAllVideos) { - headline = mHeadlineGenerator.getVideosHeadline(mFileCount); + if (allImages) { + headline = mHeadlineGenerator.getImagesHeadline(fileCount); + } else if (allVideos) { + headline = mHeadlineGenerator.getVideosHeadline(fileCount); } else { - headline = mHeadlineGenerator.getFilesHeadline(mFileCount); + headline = mHeadlineGenerator.getFilesHeadline(fileCount); } } @@ -201,7 +213,7 @@ class FilesPlusTextContentPreviewUi extends ContentPreviewUi { textView.setText(getNoTextString(contentPreview.getResources())); } shareTextAction.accept(!isChecked); - updateHeadline(contentPreview); + updateHeadline(contentPreview, mFileCount, mAllImages, mAllVideos); }); if (SHOW_TOGGLE_CHECKMARK) { includeText.setVisibility(View.VISIBLE); diff --git a/java/tests/src/com/android/intentresolver/contentpreview/FilesPlusTextContentPreviewUiTest.kt b/java/tests/src/com/android/intentresolver/contentpreview/FilesPlusTextContentPreviewUiTest.kt new file mode 100644 index 00000000..06ade1ce --- /dev/null +++ b/java/tests/src/com/android/intentresolver/contentpreview/FilesPlusTextContentPreviewUiTest.kt @@ -0,0 +1,225 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.intentresolver.contentpreview + +import android.net.Uri +import android.view.LayoutInflater +import android.view.ViewGroup +import android.widget.TextView +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry.getInstrumentation +import com.android.intentresolver.R +import com.android.intentresolver.TestLifecycleOwner +import com.android.intentresolver.mock +import com.android.intentresolver.whenever +import com.android.intentresolver.widget.ActionRow +import com.google.common.truth.Truth.assertThat +import java.util.function.Consumer +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mockito.anyInt +import org.mockito.Mockito.never +import org.mockito.Mockito.times +import org.mockito.Mockito.verify + +private const val HEADLINE_IMAGES = "Image Headline" +private const val HEADLINE_VIDEOS = "Video Headline" +private const val HEADLINE_FILES = "Files Headline" +private const val SHARED_TEXT = "Some text to share" + +@RunWith(AndroidJUnit4::class) +class FilesPlusTextContentPreviewUiTest { + private val lifecycleOwner = TestLifecycleOwner() + private val actionFactory = + object : ChooserContentPreviewUi.ActionFactory { + override fun getEditButtonRunnable(): Runnable? = null + override fun getCopyButtonRunnable(): Runnable? = null + override fun createCustomActions(): List = emptyList() + override fun getModifyShareAction(): ActionRow.Action? = null + override fun getExcludeSharedTextAction(): Consumer = Consumer {} + } + private val imageLoader = mock() + private val headlineGenerator = + mock { + whenever(getImagesHeadline(anyInt())).thenReturn(HEADLINE_IMAGES) + whenever(getVideosHeadline(anyInt())).thenReturn(HEADLINE_VIDEOS) + whenever(getFilesHeadline(anyInt())).thenReturn(HEADLINE_FILES) + } + + private val context + get() = getInstrumentation().getContext() + + @Test + fun test_displayImagesPlusTextWithoutUriMetadata_showImagesHeadline() { + val sharedFileCount = 2 + val previewView = testLoadingHeadline("image/*", sharedFileCount) + + verify(headlineGenerator, times(1)).getImagesHeadline(sharedFileCount) + verifyPreviewHeadline(previewView, HEADLINE_IMAGES) + verifySharedText(previewView) + } + + @Test + fun test_displayVideosPlusTextWithoutUriMetadata_showVideosHeadline() { + val sharedFileCount = 2 + val previewView = testLoadingHeadline("video/*", sharedFileCount) + + verify(headlineGenerator, times(1)).getVideosHeadline(sharedFileCount) + verifyPreviewHeadline(previewView, HEADLINE_VIDEOS) + verifySharedText(previewView) + } + + @Test + fun test_displayDocsPlusTextWithoutUriMetadata_showFilesHeadline() { + val sharedFileCount = 2 + val previewView = testLoadingHeadline("application/pdf", sharedFileCount) + + verify(headlineGenerator, times(1)).getFilesHeadline(sharedFileCount) + verifyPreviewHeadline(previewView, HEADLINE_FILES) + verifySharedText(previewView) + } + + @Test + fun test_displayMixedContentPlusTextWithoutUriMetadata_showFilesHeadline() { + val sharedFileCount = 2 + val previewView = testLoadingHeadline("*/*", sharedFileCount) + + verify(headlineGenerator, times(1)).getFilesHeadline(sharedFileCount) + verifyPreviewHeadline(previewView, HEADLINE_FILES) + verifySharedText(previewView) + } + + @Test + fun test_displayImagesPlusTextWithUriMetadataSet_showImagesHeadline() { + val loadedFileMetadata = createFileInfosWithMimeTypes("image/png", "image/jpeg") + val sharedFileCount = loadedFileMetadata.size + val previewView = testLoadingHeadline("image/*", sharedFileCount, loadedFileMetadata) + + verify(headlineGenerator, times(1)).getImagesHeadline(sharedFileCount) + verifyPreviewHeadline(previewView, HEADLINE_IMAGES) + verifySharedText(previewView) + } + + @Test + fun test_displayVideosPlusTextWithUriMetadataSet_showVideosHeadline() { + val loadedFileMetadata = createFileInfosWithMimeTypes("video/mp4", "video/mp4") + val sharedFileCount = loadedFileMetadata.size + val previewView = testLoadingHeadline("video/*", sharedFileCount, loadedFileMetadata) + + verify(headlineGenerator, times(1)).getVideosHeadline(sharedFileCount) + verifyPreviewHeadline(previewView, HEADLINE_VIDEOS) + verifySharedText(previewView) + } + + @Test + fun test_displayImagesAndVideosPlusTextWithUriMetadataSet_showFilesHeadline() { + val loadedFileMetadata = createFileInfosWithMimeTypes("image/png", "video/mp4") + val sharedFileCount = loadedFileMetadata.size + val previewView = testLoadingHeadline("*/*", sharedFileCount, loadedFileMetadata) + + verify(headlineGenerator, times(1)).getFilesHeadline(sharedFileCount) + verifyPreviewHeadline(previewView, HEADLINE_FILES) + verifySharedText(previewView) + } + + @Test + fun test_displayDocsPlusTextWithUriMetadataSet_showFilesHeadline() { + val loadedFileMetadata = createFileInfosWithMimeTypes("application/pdf", "application/pdf") + val sharedFileCount = loadedFileMetadata.size + val previewView = + testLoadingHeadline("application/pdf", sharedFileCount, loadedFileMetadata) + + verify(headlineGenerator, times(1)).getFilesHeadline(sharedFileCount) + verifyPreviewHeadline(previewView, HEADLINE_FILES) + verifySharedText(previewView) + } + + @Test + fun test_uriMetadataIsMoreSpecificThanIntentMimeType_headlineGetsUpdated() { + val sharedFileCount = 2 + val testSubject = + FilesPlusTextContentPreviewUi( + lifecycleOwner.lifecycle, + /*isSingleImage=*/ false, + sharedFileCount, + SHARED_TEXT, + /*intentMimeType=*/ "*/*", + actionFactory, + imageLoader, + DefaultMimeTypeClassifier, + headlineGenerator + ) + val layoutInflater = LayoutInflater.from(context) + val gridLayout = layoutInflater.inflate(R.layout.chooser_grid, null, false) as ViewGroup + + val previewView = + testSubject.display(context.resources, LayoutInflater.from(context), gridLayout) + + verify(headlineGenerator, times(1)).getFilesHeadline(sharedFileCount) + verify(headlineGenerator, never()).getImagesHeadline(sharedFileCount) + verifyPreviewHeadline(previewView, HEADLINE_FILES) + + testSubject.updatePreviewMetadata(createFileInfosWithMimeTypes("image/png", "image/jpg")) + + verify(headlineGenerator, times(1)).getFilesHeadline(sharedFileCount) + verify(headlineGenerator, times(1)).getImagesHeadline(sharedFileCount) + verifyPreviewHeadline(previewView, HEADLINE_IMAGES) + } + + private fun testLoadingHeadline( + intentMimeType: String, + sharedFileCount: Int, + loadedFileMetadata: List? = null + ): ViewGroup? { + val testSubject = + FilesPlusTextContentPreviewUi( + lifecycleOwner.lifecycle, + /*isSingleImage=*/ false, + sharedFileCount, + SHARED_TEXT, + intentMimeType, + actionFactory, + imageLoader, + DefaultMimeTypeClassifier, + headlineGenerator + ) + val layoutInflater = LayoutInflater.from(context) + val gridLayout = layoutInflater.inflate(R.layout.chooser_grid, null, false) as ViewGroup + + loadedFileMetadata?.let(testSubject::updatePreviewMetadata) + return testSubject.display(context.resources, LayoutInflater.from(context), gridLayout) + } + + private fun createFileInfosWithMimeTypes(vararg mimeTypes: String): List { + val uri = Uri.parse("content://pkg.app/file") + return mimeTypes.map { mimeType -> FileInfo.Builder(uri).withMimeType(mimeType).build() } + } + + private fun verifyPreviewHeadline(previewView: ViewGroup?, expectedText: String) { + assertThat(previewView).isNotNull() + val headlineView = previewView?.findViewById(R.id.headline) + assertThat(headlineView).isNotNull() + assertThat(headlineView?.text).isEqualTo(expectedText) + } + + private fun verifySharedText(previewView: ViewGroup?) { + assertThat(previewView).isNotNull() + val textContentView = previewView?.findViewById(R.id.content_preview_text) + assertThat(textContentView).isNotNull() + assertThat(textContentView?.text).isEqualTo(SHARED_TEXT) + } +} -- cgit v1.2.3-59-g8ed1b From ea59592a728c2fb00070f15873e85939b806a3d9 Mon Sep 17 00:00:00 2001 From: Andrey Epin Date: Fri, 11 Aug 2023 10:45:36 -0700 Subject: Load preivews eagerly Currently, content preview is not started until the metadata for all the shared URIs is loaded. This CL changes it to eagerly loaded previews i.e. previews are getting loaded as soon as the corresponding metadata becomes available. This is achieved by: * Adding support for a Flow as a preview source into ScrollableImgePreviewView (patchset #1); Specifically, as a Flow may never complete, the flow collection is cancelled in `onDetachFromWindow` to avoid possible memory leaks. The view is used inside a RecyclerView (the single-profile case) and can be attached and detached multiple times per one `setPreviews` call. Thus BatchPreviewLoader is made relaunchable and captures a notion of a pending loading job; `#batchLoader` value gets updated accordingly. (patchset #8) * Make PreviewDataProvider expose a Flow of shared URIs metadata, FileInfo (patchset #2, #3); * Make content preview classes to pass the Flow from PreviewDataProvider to ScrollableImagePreviewView (patchset #4). Bug: 292157413 Test: manual testing with ShareTest app: with and without artificial image loading delays, orientation changes and on single- and multi-profile cases. Test: unit tests Test: integration test Change-Id: Ib663bab8917624493a9ba619e64e4cb81fa35a93 --- .../contentpreview/ChooserContentPreviewUi.java | 14 +- .../contentpreview/ContentPreviewUi.java | 2 +- .../contentpreview/JavaFlowHelper.kt | 51 +++++ .../contentpreview/PreviewDataProvider.kt | 124 ++++++------ .../contentpreview/PreviewViewModel.kt | 17 +- .../contentpreview/UnifiedContentPreviewUi.java | 28 +-- .../widget/ScrollableImagePreviewView.kt | 172 +++++++++-------- .../android/intentresolver/TestContentProvider.kt | 32 +++- .../UnbundledChooserActivityTest.java | 116 ++++++++++- .../contentpreview/ChooserContentPreviewUiTest.kt | 19 +- .../contentpreview/PreviewDataProviderTest.kt | 212 ++++++++++----------- .../contentpreview/UnifiedContentPreviewUiTest.kt | 52 +++-- .../widget/BatchPreviewLoaderTest.kt | 23 ++- 13 files changed, 539 insertions(+), 323 deletions(-) create mode 100644 java/src/com/android/intentresolver/contentpreview/JavaFlowHelper.kt (limited to 'java/src') diff --git a/java/src/com/android/intentresolver/contentpreview/ChooserContentPreviewUi.java b/java/src/com/android/intentresolver/contentpreview/ChooserContentPreviewUi.java index d4874cac..d279f11f 100644 --- a/java/src/com/android/intentresolver/contentpreview/ChooserContentPreviewUi.java +++ b/java/src/com/android/intentresolver/contentpreview/ChooserContentPreviewUi.java @@ -16,6 +16,8 @@ package com.android.intentresolver.contentpreview; +import static androidx.lifecycle.LifecycleKt.getCoroutineScope; + import static com.android.intentresolver.contentpreview.ContentPreviewType.CONTENT_PREVIEW_FILE; import static com.android.intentresolver.contentpreview.ContentPreviewType.CONTENT_PREVIEW_IMAGE; import static com.android.intentresolver.contentpreview.ContentPreviewType.CONTENT_PREVIEW_TEXT; @@ -156,23 +158,25 @@ public final class ChooserContentPreviewUi { typeClassifier, headlineGenerator); if (previewData.getUriCount() > 0) { - previewData.getFileMetadataForImagePreview( - mLifecycle, previewUi::updatePreviewMetadata); + JavaFlowHelper.collectToList( + getCoroutineScope(mLifecycle), + previewData.getImagePreviewFileInfoFlow(), + previewUi::updatePreviewMetadata); } return previewUi; } - UnifiedContentPreviewUi unifiedContentPreviewUi = new UnifiedContentPreviewUi( + return new UnifiedContentPreviewUi( + getCoroutineScope(mLifecycle), isSingleImageShare, targetIntent.getType(), actionFactory, imageLoader, typeClassifier, transitionElementStatusCallback, + previewData.getImagePreviewFileInfoFlow(), previewData.getUriCount(), headlineGenerator); - previewData.getFileMetadataForImagePreview(mLifecycle, unifiedContentPreviewUi::setFiles); - return unifiedContentPreviewUi; } public int getPreferredContentPreview() { diff --git a/java/src/com/android/intentresolver/contentpreview/ContentPreviewUi.java b/java/src/com/android/intentresolver/contentpreview/ContentPreviewUi.java index 07071236..2d81794e 100644 --- a/java/src/com/android/intentresolver/contentpreview/ContentPreviewUi.java +++ b/java/src/com/android/intentresolver/contentpreview/ContentPreviewUi.java @@ -85,7 +85,7 @@ abstract class ContentPreviewUi { } } - protected static ScrollableImagePreviewView.PreviewType getPreviewType( + static ScrollableImagePreviewView.PreviewType getPreviewType( MimeTypeClassifier typeClassifier, String mimeType) { if (mimeType == null) { return ScrollableImagePreviewView.PreviewType.File; diff --git a/java/src/com/android/intentresolver/contentpreview/JavaFlowHelper.kt b/java/src/com/android/intentresolver/contentpreview/JavaFlowHelper.kt new file mode 100644 index 00000000..b29c5774 --- /dev/null +++ b/java/src/com/android/intentresolver/contentpreview/JavaFlowHelper.kt @@ -0,0 +1,51 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +@file:JvmName("JavaFlowHelper") + +package com.android.intentresolver.contentpreview + +import com.android.intentresolver.widget.ScrollableImagePreviewView.Preview +import java.util.function.Consumer +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.filter +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.toList +import kotlinx.coroutines.launch + +internal fun mapFileIntoToPreview( + flow: Flow, + typeClassifier: MimeTypeClassifier, + editAction: Runnable? +): Flow = + flow + .filter { it.previewUri != null } + .map { fileInfo -> + Preview( + ContentPreviewUi.getPreviewType(typeClassifier, fileInfo.mimeType), + requireNotNull(fileInfo.previewUri), + editAction + ) + } + +internal fun collectToList( + clientScope: CoroutineScope, + flow: Flow, + callback: Consumer> +) { + clientScope.launch { callback.accept(flow.toList()) } +} diff --git a/java/src/com/android/intentresolver/contentpreview/PreviewDataProvider.kt b/java/src/com/android/intentresolver/contentpreview/PreviewDataProvider.kt index 8ab3a272..fd5ce3f8 100644 --- a/java/src/com/android/intentresolver/contentpreview/PreviewDataProvider.kt +++ b/java/src/com/android/intentresolver/contentpreview/PreviewDataProvider.kt @@ -38,14 +38,18 @@ import com.android.intentresolver.measurements.runTracing import com.android.intentresolver.util.ownedByCurrentUser import java.util.concurrent.atomic.AtomicInteger import java.util.function.Consumer +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CompletableDeferred -import kotlinx.coroutines.CoroutineDispatcher -import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.async import kotlinx.coroutines.coroutineScope +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.SharedFlow +import kotlinx.coroutines.flow.take import kotlinx.coroutines.isActive import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking -import kotlinx.coroutines.withContext import kotlinx.coroutines.withTimeoutOrNull /** @@ -69,29 +73,44 @@ private const val TIMEOUT_MS = 1_000L @OpenForTesting open class PreviewDataProvider @VisibleForTesting +@JvmOverloads constructor( + private val scope: CoroutineScope, private val targetIntent: Intent, private val contentResolver: ContentInterface, - private val typeClassifier: MimeTypeClassifier, - private val dispatcher: CoroutineDispatcher, + private val typeClassifier: MimeTypeClassifier = DefaultMimeTypeClassifier, ) { - constructor( - targetIntent: Intent, - contentResolver: ContentInterface, - ) : this( - targetIntent, - contentResolver, - DefaultMimeTypeClassifier, - Dispatchers.IO, - ) private val records = targetIntent.contentUris.map { UriRecord(it) } + private val fileInfoSharedFlow: SharedFlow by lazy { + // Alternatively, we could just use [shareIn()] on a [flow] -- and it would be, arguably, + // cleaner -- but we'd lost the ability to trace the traverse as [runTracing] does not + // generally work over suspend function invocations. + MutableSharedFlow(replay = records.size).apply { + scope.launch { + runTracing("image-preview-metadata") { + for (record in records) { + tryEmit(FileInfo.Builder(record.uri).readFromRecord(record).build()) + } + } + } + } + } + /** returns number of shared URIs, see [Intent.EXTRA_STREAM] */ @get:OpenForTesting open val uriCount: Int get() = records.size + /** + * Returns a [Flow] of [FileInfo], for each shared URI in order, with [FileInfo.mimeType] and + * [FileInfo.previewUri] set (a data projection tailored for the image preview UI). + */ + @get:OpenForTesting + open val imagePreviewFileInfoFlow: Flow + get() = fileInfoSharedFlow.take(records.size) + /** * Preview type to use. The type is determined asynchronously with a timeout; the fall-back * values is [ContentPreviewType.CONTENT_PREVIEW_FILE] @@ -107,10 +126,17 @@ constructor( if (!targetIntent.isSend || records.isEmpty()) { CONTENT_PREVIEW_TEXT } else { - runBlocking(dispatcher) { - withTimeoutOrNull(TIMEOUT_MS) { - loadPreviewType() - } ?: CONTENT_PREVIEW_FILE + try { + runBlocking(scope.coroutineContext) { + withTimeoutOrNull(TIMEOUT_MS) { loadPreviewType() } ?: CONTENT_PREVIEW_FILE + } + } catch (e: CancellationException) { + Log.w( + ContentPreviewUi.TAG, + "An attempt to read preview type from a cancelled scope", + e + ) + CONTENT_PREVIEW_FILE } } } @@ -123,46 +149,22 @@ constructor( open val firstFileInfo: FileInfo? by lazy { runTracing("first-uri-metadata") { records.firstOrNull()?.let { record -> - runBlocking(dispatcher) { - val builder = FileInfo.Builder(record.uri) - withTimeoutOrNull(TIMEOUT_MS) { - builder.readFromRecord(record) + val builder = FileInfo.Builder(record.uri) + try { + runBlocking(scope.coroutineContext) { + withTimeoutOrNull(TIMEOUT_MS) { builder.readFromRecord(record) } } - builder.build() - } - } - } - } - - /** - * Returns a collection of [FileInfo], for each shared URI in order, with [FileInfo.mimeType] - * and [FileInfo.previewUri] set (a data projection tailored for the image preview UI). - */ - @OpenForTesting - open fun getFileMetadataForImagePreview( - callerLifecycle: Lifecycle, - callback: Consumer>, - ) { - callerLifecycle.coroutineScope.launch { - val result = withContext(dispatcher) { - getFileMetadataForImagePreview() - } - callback.accept(result) - } - } - - private fun getFileMetadataForImagePreview(): List = - runTracing("image-preview-metadata") { - ArrayList(records.size).also { result -> - for (record in records) { - result.add( - FileInfo.Builder(record.uri) - .readFromRecord(record) - .build() + } catch (e: CancellationException) { + Log.w( + ContentPreviewUi.TAG, + "An attempt to read first file info from a cancelled scope", + e ) } + builder.build() } } + } private fun FileInfo.Builder.readFromRecord(record: UriRecord): FileInfo.Builder { withMimeType(record.mimeType) @@ -186,9 +188,7 @@ constructor( throw IndexOutOfBoundsException("There are no shared URIs") } callerLifecycle.coroutineScope.launch { - val result = withContext(dispatcher) { - getFirstFileName() - } + val result = scope.async { getFirstFileName() }.await() callback.accept(result) } } @@ -237,8 +237,7 @@ constructor( } resultDeferred.complete(CONTENT_PREVIEW_FILE) } - resultDeferred.await() - .also { job.cancel() } + resultDeferred.await().also { job.cancel() } } } @@ -251,8 +250,8 @@ constructor( val isImageType: Boolean get() = typeClassifier.isImageType(mimeType) val supportsImageType: Boolean by lazy { - contentResolver.getStreamTypesSafe(uri) - ?.firstOrNull(typeClassifier::isImageType) != null + contentResolver.getStreamTypesSafe(uri)?.firstOrNull(typeClassifier::isImageType) != + null } val supportsThumbnail: Boolean get() = query.supportsThumbnail @@ -264,9 +263,8 @@ constructor( private val query by lazy { readQueryResult() } private fun readQueryResult(): QueryResult { - val cursor = contentResolver.querySafe(uri) - ?.takeIf { it.moveToFirst() } - ?: return QueryResult() + val cursor = + contentResolver.querySafe(uri)?.takeIf { it.moveToFirst() } ?: return QueryResult() var flagColIdx = -1 var displayIconUriColIdx = -1 diff --git a/java/src/com/android/intentresolver/contentpreview/PreviewViewModel.kt b/java/src/com/android/intentresolver/contentpreview/PreviewViewModel.kt index 331b0cb6..6013f5a0 100644 --- a/java/src/com/android/intentresolver/contentpreview/PreviewViewModel.kt +++ b/java/src/com/android/intentresolver/contentpreview/PreviewViewModel.kt @@ -25,11 +25,15 @@ import androidx.lifecycle.viewModelScope import androidx.lifecycle.viewmodel.CreationExtras import com.android.intentresolver.ChooserRequestParameters import com.android.intentresolver.R +import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.plus /** A trivial view model to keep a [PreviewDataProvider] instance over a configuration change */ -class PreviewViewModel(private val application: Application) : BasePreviewViewModel() { +class PreviewViewModel( + private val application: Application, + private val dispatcher: CoroutineDispatcher = Dispatchers.IO, +) : BasePreviewViewModel() { private var previewDataProvider: PreviewDataProvider? = null private var imageLoader: ImagePreviewImageLoader? = null @@ -38,15 +42,18 @@ class PreviewViewModel(private val application: Application) : BasePreviewViewMo chooserRequest: ChooserRequestParameters ): PreviewDataProvider = previewDataProvider - ?: PreviewDataProvider(chooserRequest.targetIntent, application.contentResolver).also { - previewDataProvider = it - } + ?: PreviewDataProvider( + viewModelScope + dispatcher, + chooserRequest.targetIntent, + application.contentResolver + ) + .also { previewDataProvider = it } @MainThread override fun createOrReuseImageLoader(): ImageLoader = imageLoader ?: ImagePreviewImageLoader( - viewModelScope + Dispatchers.IO, + viewModelScope + dispatcher, thumbnailSize = application.resources.getDimensionPixelSize( R.dimen.chooser_preview_image_max_dimen diff --git a/java/src/com/android/intentresolver/contentpreview/UnifiedContentPreviewUi.java b/java/src/com/android/intentresolver/contentpreview/UnifiedContentPreviewUi.java index 5db5020e..8e635aba 100644 --- a/java/src/com/android/intentresolver/contentpreview/UnifiedContentPreviewUi.java +++ b/java/src/com/android/intentresolver/contentpreview/UnifiedContentPreviewUi.java @@ -31,10 +31,12 @@ import com.android.intentresolver.widget.ActionRow; import com.android.intentresolver.widget.ImagePreviewView.TransitionElementStatusCallback; import com.android.intentresolver.widget.ScrollableImagePreviewView; -import java.util.ArrayList; import java.util.List; import java.util.Objects; +import kotlinx.coroutines.CoroutineScope; +import kotlinx.coroutines.flow.Flow; + class UnifiedContentPreviewUi extends ContentPreviewUi { private final boolean mShowEditAction; @Nullable @@ -44,6 +46,7 @@ class UnifiedContentPreviewUi extends ContentPreviewUi { private final MimeTypeClassifier mTypeClassifier; private final TransitionElementStatusCallback mTransitionElementStatusCallback; private final HeadlineGenerator mHeadlineGenerator; + private final Flow mFileInfoFlow; private final int mItemCount; @Nullable private List mFiles; @@ -51,12 +54,14 @@ class UnifiedContentPreviewUi extends ContentPreviewUi { private ViewGroup mContentPreviewView; UnifiedContentPreviewUi( + CoroutineScope scope, boolean isSingleImage, @Nullable String intentMimeType, ChooserContentPreviewUi.ActionFactory actionFactory, ImageLoader imageLoader, MimeTypeClassifier typeClassifier, TransitionElementStatusCallback transitionElementStatusCallback, + Flow fileInfoFlow, int itemCount, HeadlineGenerator headlineGenerator) { mShowEditAction = isSingleImage; @@ -65,8 +70,11 @@ class UnifiedContentPreviewUi extends ContentPreviewUi { mImageLoader = imageLoader; mTypeClassifier = typeClassifier; mTransitionElementStatusCallback = transitionElementStatusCallback; + mFileInfoFlow = fileInfoFlow; mItemCount = itemCount; mHeadlineGenerator = headlineGenerator; + + JavaFlowHelper.collectToList(scope, fileInfoFlow, this::setFiles); } @Override @@ -81,7 +89,7 @@ class UnifiedContentPreviewUi extends ContentPreviewUi { return layout; } - public void setFiles(List files) { + private void setFiles(List files) { mImageLoader.prePopulate(files.stream() .map(FileInfo::getPreviewUri) .filter(Objects::nonNull) @@ -106,6 +114,12 @@ class UnifiedContentPreviewUi extends ContentPreviewUi { imagePreview.setImageLoader(mImageLoader); imagePreview.setOnNoPreviewCallback(() -> imagePreview.setVisibility(View.GONE)); imagePreview.setTransitionElementStatusCallback(mTransitionElementStatusCallback); + imagePreview.setPreviews( + JavaFlowHelper.mapFileIntoToPreview( + mFileInfoFlow, + mTypeClassifier, + mShowEditAction ? mActionFactory.getEditButtonRunnable() : null), + mItemCount); if (mFiles != null) { updatePreviewWithFiles(mContentPreviewView, mFiles); @@ -135,7 +149,6 @@ class UnifiedContentPreviewUi extends ContentPreviewUi { return; } - List previews = new ArrayList<>(); boolean allImages = true; boolean allVideos = true; for (FileInfo fileInfo : files) { @@ -143,17 +156,8 @@ class UnifiedContentPreviewUi extends ContentPreviewUi { getPreviewType(mTypeClassifier, fileInfo.getMimeType()); allImages = allImages && previewType == ScrollableImagePreviewView.PreviewType.Image; allVideos = allVideos && previewType == ScrollableImagePreviewView.PreviewType.Video; - - if (fileInfo.getPreviewUri() != null) { - Runnable editAction = - mShowEditAction ? mActionFactory.getEditButtonRunnable() : null; - previews.add( - new ScrollableImagePreviewView.Preview( - previewType, fileInfo.getPreviewUri(), editAction)); - } } - imagePreview.setPreviews(previews, count - previews.size()); displayHeadline(contentPreviewView, count, allImages, allVideos); } diff --git a/java/src/com/android/intentresolver/widget/ScrollableImagePreviewView.kt b/java/src/com/android/intentresolver/widget/ScrollableImagePreviewView.kt index d9844d7b..3bbafc40 100644 --- a/java/src/com/android/intentresolver/widget/ScrollableImagePreviewView.kt +++ b/java/src/com/android/intentresolver/widget/ScrollableImagePreviewView.kt @@ -39,14 +39,12 @@ import com.android.intentresolver.widget.ImagePreviewView.TransitionElementStatu import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job -import kotlinx.coroutines.MainScope import kotlinx.coroutines.cancel +import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow -import kotlinx.coroutines.flow.onCompletion import kotlinx.coroutines.flow.takeWhile import kotlinx.coroutines.joinAll import kotlinx.coroutines.launch -import kotlinx.coroutines.plus private const val TRANSITION_NAME = "screenshot_preview_image" private const val PLURALS_COUNT = "count" @@ -127,7 +125,7 @@ class ScrollableImagePreviewView : RecyclerView, ImagePreviewView { isMeasured = true updateMaxWidthHint(widthSpec) updateMaxAspectRatio() - batchLoader?.loadAspectRatios(getMaxWidth(), this::updatePreviewSize) + maybeLoadAspectRatios() } } @@ -145,6 +143,17 @@ class ScrollableImagePreviewView : RecyclerView, ImagePreviewView { ) } + override fun onAttachedToWindow() { + super.onAttachedToWindow() + batchLoader?.totalItemCount?.let(previewAdapter::reset) + maybeLoadAspectRatios() + } + + override fun onDetachedFromWindow() { + batchLoader?.cancel() + super.onDetachedFromWindow() + } + override fun setTransitionElementStatusCallback(callback: TransitionElementStatusCallback?) { previewAdapter.transitionStatusElementCallback = callback } @@ -166,30 +175,30 @@ class ScrollableImagePreviewView : RecyclerView, ImagePreviewView { previewAdapter.reset(totalItemCount) } - fun setPreviews(previews: List, otherItemCount: Int) { - previewAdapter.reset(previews.size + otherItemCount) + fun setPreviews(previews: Flow, totalItemCount: Int) { + previewAdapter.reset(totalItemCount) batchLoader?.cancel() batchLoader = BatchPreviewLoader( - previewAdapter.imageLoader ?: error("Image loader is not set"), - previews, - otherItemCount, - onUpdate = previewAdapter::addPreviews, - onCompletion = { - if (!previewAdapter.hasPreviews) { - onNoPreviewCallback?.run() - } - previewAdapter.markLoaded() - } - ) - .apply { - if (isMeasured) { - loadAspectRatios( - getMaxWidth(), - this@ScrollableImagePreviewView::updatePreviewSize - ) + previewAdapter.imageLoader ?: error("Image loader is not set"), + previews, + totalItemCount, + onUpdate = previewAdapter::addPreviews, + onCompletion = { + batchLoader = null + if (!previewAdapter.hasPreviews) { + onNoPreviewCallback?.run() } + previewAdapter.markLoaded() } + ) + maybeLoadAspectRatios() + } + + private fun maybeLoadAspectRatios() { + if (isMeasured && isAttachedToWindow()) { + batchLoader?.let { it.loadAspectRatios(getMaxWidth(), this::updatePreviewSize) } + } } var onNoPreviewCallback: Runnable? = null @@ -320,6 +329,7 @@ class ScrollableImagePreviewView : RecyclerView, ImagePreviewView { !hadOtherItem && hasOtherItem -> { notifyItemInserted(previews.size) } + else -> notifyItemChanged(previews.size) } } } @@ -464,7 +474,7 @@ class ScrollableImagePreviewView : RecyclerView, ImagePreviewView { } private fun resetScope(): CoroutineScope = - (MainScope() + Dispatchers.Main.immediate).also { + CoroutineScope(Dispatchers.Main.immediate).also { scope?.cancel() scope = it } @@ -514,26 +524,22 @@ class ScrollableImagePreviewView : RecyclerView, ImagePreviewView { @VisibleForTesting class BatchPreviewLoader( private val imageLoader: CachingImageLoader, - previews: List, - otherItemCount: Int, + private val previews: Flow, + val totalItemCount: Int, private val onUpdate: (List) -> Unit, private val onCompletion: () -> Unit, ) { - private val previews: List = - if (previews is RandomAccess) previews else ArrayList(previews) - private val totalItemCount = previews.size + otherItemCount - private var scope: CoroutineScope? = MainScope() + Dispatchers.Main.immediate + private var scope: CoroutineScope = createScope() + + private fun createScope() = CoroutineScope(Dispatchers.Main.immediate) fun cancel() { - scope?.cancel() - scope = null + scope.cancel() + scope = createScope() } fun loadAspectRatios(maxWidth: Int, previewSizeUpdater: (Preview, Int, Int) -> Int) { - val scope = this.scope ?: return - // -1 encodes that the preview has not been processed, - // 0 means failed, > 0 is a preview width - val previewWidths = IntArray(previews.size) { -1 } + val previewInfos = ArrayList(totalItemCount) var blockStart = 0 // inclusive var blockEnd = 0 // exclusive @@ -542,23 +548,16 @@ class ScrollableImagePreviewView : RecyclerView, ImagePreviewView { val updateEvent = Any() val completedEvent = Any() - // throttle adapter updates using flow; the flow first emits when enough preview - // elements is loaded to fill the viewport and then each time a subsequent block of - // previews is loaded + // collects updates from [reportFlow] throttling adapter updates; scope.launch(Dispatchers.Main) { reportFlow .takeWhile { it !== completedEvent } .throttle(ADAPTER_UPDATE_INTERVAL_MS) - .onCompletion { cause -> - if (cause == null) { - onCompletion() - } - } .collect { val updates = ArrayList(blockEnd - blockStart) while (blockStart < blockEnd) { - if (previewWidths[blockStart] > 0) { - updates.add(previews[blockStart]) + if (previewInfos[blockStart].width > 0) { + updates.add(previewInfos[blockStart].preview) } blockStart++ } @@ -566,57 +565,64 @@ class ScrollableImagePreviewView : RecyclerView, ImagePreviewView { onUpdate(updates) } } + onCompletion() } + // Collects [previews] flow and loads aspect ratios, emits updates into [reportFlow] + // when a next sequential block of preview aspect ratios is loaded: initially emits when + // enough preview elements is loaded to fill the viewport. scope.launch { var blockWidth = 0 var isFirstBlock = true - var nextIdx = 0 - List(4) { - launch { - while (true) { - val i = nextIdx++ - if (i >= previews.size) break - val preview = previews[i] - - previewWidths[i] = - runCatching { - // TODO: decide on adding a timeout - imageLoader(preview.uri, isFirstBlock)?.let { bitmap -> - previewSizeUpdater( - preview, - bitmap.width, - bitmap.height - ) - } - ?: 0 - } - .getOrDefault(0) - - if (blockEnd != i) continue - while ( - blockEnd < previewWidths.size && previewWidths[blockEnd] >= 0 - ) { - blockWidth += previewWidths[blockEnd] - blockEnd++ - } - if (isFirstBlock) { - if (blockWidth >= maxWidth) { - isFirstBlock = false - // notify that the preview now can be displayed - reportFlow.emit(updateEvent) + + val jobs = ArrayList() + previews.collect { preview -> + val i = previewInfos.size + val pair = PreviewWidthInfo(preview) + previewInfos.add(pair) + + val job = launch { + pair.width = + runCatching { + // TODO: decide on adding a timeout. The worst case I can + // imagine is one of the first images never loads so we never + // fill the initial viewport and does not show the previews at + // all. + imageLoader(preview.uri, isFirstBlock)?.let { bitmap -> + previewSizeUpdater(preview, bitmap.width, bitmap.height) } - } else { - reportFlow.emit(updateEvent) + ?: 0 } + .getOrDefault(0) + + if (i == blockEnd) { + while ( + blockEnd < previewInfos.size && previewInfos[blockEnd].width >= 0 + ) { + blockWidth += previewInfos[blockEnd].width + blockEnd++ + } + if (isFirstBlock && blockWidth >= maxWidth) { + isFirstBlock = false + } + if (!isFirstBlock) { + reportFlow.emit(updateEvent) } } } - .joinAll() + jobs.add(job) + } + jobs.joinAll() // in case all previews have failed to load reportFlow.emit(updateEvent) reportFlow.emit(completedEvent) } } } + + private class PreviewWidthInfo(val preview: Preview) { + // -1 encodes that the preview has not been processed, + // 0 means failed, > 0 is a preview width + var width: Int = -1 + } } diff --git a/java/tests/src/com/android/intentresolver/TestContentProvider.kt b/java/tests/src/com/android/intentresolver/TestContentProvider.kt index b3b53baa..426f9af2 100644 --- a/java/tests/src/com/android/intentresolver/TestContentProvider.kt +++ b/java/tests/src/com/android/intentresolver/TestContentProvider.kt @@ -30,15 +30,23 @@ class TestContentProvider : ContentProvider() { sortOrder: String? ): Cursor? = null - override fun getType(uri: Uri): String? - = runCatching { - uri.getQueryParameter("mimeType") - }.getOrNull() + override fun getType(uri: Uri): String? = + runCatching { uri.getQueryParameter(PARAM_MIME_TYPE) }.getOrNull() - override fun getStreamTypes(uri: Uri, mimeTypeFilter: String): Array? - = runCatching { - uri.getQueryParameter("streamType")?.let { arrayOf(it) } - }.getOrNull() + override fun getStreamTypes(uri: Uri, mimeTypeFilter: String): Array? { + val delay = + runCatching { uri.getQueryParameter(PARAM_STREAM_TYPE_TIMEOUT)?.toLong() ?: 0L } + .getOrDefault(0L) + if (delay > 0) { + try { + Thread.sleep(delay) + } catch (e: InterruptedException) { + Thread.currentThread().interrupt() + } + } + return runCatching { uri.getQueryParameter(PARAM_STREAM_TYPE)?.let { arrayOf(it) } } + .getOrNull() + } override fun insert(uri: Uri, values: ContentValues?): Uri? = null @@ -52,4 +60,10 @@ class TestContentProvider : ContentProvider() { ): Int = 0 override fun onCreate(): Boolean = true -} \ No newline at end of file + + companion object { + const val PARAM_MIME_TYPE = "mimeType" + const val PARAM_STREAM_TYPE = "streamType" + const val PARAM_STREAM_TYPE_TIMEOUT = "streamTypeTo" + } +} diff --git a/java/tests/src/com/android/intentresolver/UnbundledChooserActivityTest.java b/java/tests/src/com/android/intentresolver/UnbundledChooserActivityTest.java index 28a45051..5709c912 100644 --- a/java/tests/src/com/android/intentresolver/UnbundledChooserActivityTest.java +++ b/java/tests/src/com/android/intentresolver/UnbundledChooserActivityTest.java @@ -136,6 +136,10 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.function.Function; @@ -1041,6 +1045,63 @@ public class UnbundledChooserActivityTest { }); } + @Test + public void testPartiallyLoadedMetadata_previewIsShownForTheLoadedPart() + throws InterruptedException { + Uri imgOneUri = createTestContentProviderUri("image/png", null); + Uri imgTwoUri = createTestContentProviderUri("image/png", null) + .buildUpon() + .path("image-2.png") + .build(); + Uri docUri = createTestContentProviderUri("application/pdf", "image/png", 3_000); + ArrayList uris = new ArrayList<>(2); + // two large previews to fill the screen and be presented right away and one + // document that would be delayed by the URI metadata reading + uris.add(imgOneUri); + uris.add(imgTwoUri); + uris.add(docUri); + + Intent sendIntent = createSendUriIntentWithPreview(uris); + Map bitmaps = new HashMap<>(); + bitmaps.put(imgOneUri, createWideBitmap(Color.RED)); + bitmaps.put(imgTwoUri, createWideBitmap(Color.GREEN)); + bitmaps.put(docUri, createWideBitmap(Color.BLUE)); + ChooserActivityOverrideData.getInstance().imageLoader = + new TestPreviewImageLoader(bitmaps); + + List resolvedComponentInfos = createResolvedComponentsForTest(2); + setupResolverControllers(resolvedComponentInfos); + + assertThat(launchActivityWithTimeout(Intent.createChooser(sendIntent, null), 1_000)) + .isTrue(); + waitForIdle(); + + onView(withId(R.id.scrollable_image_preview)) + .check((view, exception) -> { + if (exception != null) { + throw exception; + } + RecyclerView recyclerView = (RecyclerView) view; + assertThat(recyclerView.getChildCount()).isAtLeast(1); + // the first view is a preview + View imageView = recyclerView.getChildAt(0).findViewById(R.id.image); + assertThat(imageView).isNotNull(); + }) + .perform(RecyclerViewActions.scrollToLastPosition()) + .check((view, exception) -> { + if (exception != null) { + throw exception; + } + RecyclerView recyclerView = (RecyclerView) view; + assertThat(recyclerView.getChildCount()).isAtLeast(1); + // check that the last view is a loading indicator + View loadingIndicator = + recyclerView.getChildAt(recyclerView.getChildCount() - 1); + assertThat(loadingIndicator).isNotNull(); + }); + waitForIdle(); + } + @Test public void testImageAndTextPreview() { final Uri uri = createTestContentProviderUri("image/png", null); @@ -2641,15 +2702,25 @@ public class UnbundledChooserActivityTest { private Uri createTestContentProviderUri( @Nullable String mimeType, @Nullable String streamType) { + return createTestContentProviderUri(mimeType, streamType, 0); + } + + private Uri createTestContentProviderUri( + @Nullable String mimeType, @Nullable String streamType, long streamTypeTimeout) { String packageName = InstrumentationRegistry.getInstrumentation().getContext().getPackageName(); Uri.Builder builder = Uri.parse("content://" + packageName + "/image.png") .buildUpon(); if (mimeType != null) { - builder.appendQueryParameter("mimeType", mimeType); + builder.appendQueryParameter(TestContentProvider.PARAM_MIME_TYPE, mimeType); } if (streamType != null) { - builder.appendQueryParameter("streamType", streamType); + builder.appendQueryParameter(TestContentProvider.PARAM_STREAM_TYPE, streamType); + } + if (streamTypeTimeout > 0) { + builder.appendQueryParameter( + TestContentProvider.PARAM_STREAM_TYPE_TIMEOUT, + Long.toString(streamTypeTimeout)); } return builder.build(); } @@ -2779,11 +2850,44 @@ public class UnbundledChooserActivityTest { InstrumentationRegistry.getInstrumentation().waitForIdleSync(); } + private boolean launchActivityWithTimeout(Intent intent, long timeout) + throws InterruptedException { + final int initialState = 0; + final int completedState = 1; + final int timeoutState = 2; + final AtomicInteger state = new AtomicInteger(initialState); + final CountDownLatch cdl = new CountDownLatch(1); + + ScheduledExecutorService executor = Executors.newScheduledThreadPool(2); + try { + executor.execute(() -> { + mActivityRule.launchActivity(intent); + state.compareAndSet(initialState, completedState); + cdl.countDown(); + }); + executor.schedule( + () -> { + state.compareAndSet(initialState, timeoutState); + cdl.countDown(); + }, + timeout, + TimeUnit.MILLISECONDS); + cdl.await(); + return state.get() == completedState; + } finally { + executor.shutdownNow(); + } + } + private Bitmap createBitmap() { return createBitmap(200, 200); } private Bitmap createWideBitmap() { + return createWideBitmap(Color.RED); + } + + private Bitmap createWideBitmap(int bgColor) { WindowManager windowManager = InstrumentationRegistry.getInstrumentation() .getTargetContext() .getSystemService(WindowManager.class); @@ -2792,15 +2896,19 @@ public class UnbundledChooserActivityTest { Rect bounds = windowManager.getMaximumWindowMetrics().getBounds(); width = bounds.width() + 200; } - return createBitmap(width, 100); + return createBitmap(width, 100, bgColor); } private Bitmap createBitmap(int width, int height) { + return createBitmap(width, height, Color.RED); + } + + private Bitmap createBitmap(int width, int height, int bgColor) { Bitmap bitmap = Bitmap.createBitmap(width, height, Bitmap.Config.ARGB_8888); Canvas canvas = new Canvas(bitmap); Paint paint = new Paint(); - paint.setColor(Color.RED); + paint.setColor(bgColor); paint.setStyle(Paint.Style.FILL); canvas.drawPaint(paint); diff --git a/java/tests/src/com/android/intentresolver/contentpreview/ChooserContentPreviewUiTest.kt b/java/tests/src/com/android/intentresolver/contentpreview/ChooserContentPreviewUiTest.kt index 9bfd2052..008cc162 100644 --- a/java/tests/src/com/android/intentresolver/contentpreview/ChooserContentPreviewUiTest.kt +++ b/java/tests/src/com/android/intentresolver/contentpreview/ChooserContentPreviewUiTest.kt @@ -20,7 +20,7 @@ import android.content.Intent import android.graphics.Bitmap import android.net.Uri import androidx.lifecycle.Lifecycle -import com.android.intentresolver.any +import com.android.intentresolver.TestLifecycleOwner import com.android.intentresolver.contentpreview.ChooserContentPreviewUi.ActionFactory import com.android.intentresolver.mock import com.android.intentresolver.whenever @@ -28,13 +28,14 @@ import com.android.intentresolver.widget.ActionRow import com.android.intentresolver.widget.ImagePreviewView import com.google.common.truth.Truth.assertThat import java.util.function.Consumer +import kotlinx.coroutines.flow.MutableSharedFlow import org.junit.Test import org.mockito.Mockito.never import org.mockito.Mockito.times import org.mockito.Mockito.verify class ChooserContentPreviewUiTest { - private val lifecycle = mock() + private val lifecycleOwner = TestLifecycleOwner() private val previewData = mock() private val headlineGenerator = mock() private val imageLoader = @@ -64,7 +65,7 @@ class ChooserContentPreviewUiTest { whenever(previewData.previewType).thenReturn(ContentPreviewType.CONTENT_PREVIEW_TEXT) val testSubject = ChooserContentPreviewUi( - lifecycle, + lifecycleOwner.lifecycle, previewData, Intent(Intent.ACTION_VIEW), imageLoader, @@ -83,7 +84,7 @@ class ChooserContentPreviewUiTest { whenever(previewData.previewType).thenReturn(ContentPreviewType.CONTENT_PREVIEW_FILE) val testSubject = ChooserContentPreviewUi( - lifecycle, + lifecycleOwner.lifecycle, previewData, Intent(Intent.ACTION_SEND), imageLoader, @@ -104,9 +105,10 @@ class ChooserContentPreviewUiTest { whenever(previewData.uriCount).thenReturn(2) whenever(previewData.firstFileInfo) .thenReturn(FileInfo.Builder(uri).withPreviewUri(uri).withMimeType("image/png").build()) + whenever(previewData.imagePreviewFileInfoFlow).thenReturn(MutableSharedFlow()) val testSubject = ChooserContentPreviewUi( - lifecycle, + lifecycleOwner.lifecycle, previewData, Intent(Intent.ACTION_SEND).apply { putExtra(Intent.EXTRA_TEXT, "Shared text") }, imageLoader, @@ -116,7 +118,7 @@ class ChooserContentPreviewUiTest { ) assertThat(testSubject.mContentPreviewUi) .isInstanceOf(FilesPlusTextContentPreviewUi::class.java) - verify(previewData, times(1)).getFileMetadataForImagePreview(any(), any()) + verify(previewData, times(1)).imagePreviewFileInfoFlow verify(transitionCallback, times(1)).onAllTransitionElementsReady() } @@ -127,9 +129,10 @@ class ChooserContentPreviewUiTest { whenever(previewData.uriCount).thenReturn(2) whenever(previewData.firstFileInfo) .thenReturn(FileInfo.Builder(uri).withPreviewUri(uri).withMimeType("image/png").build()) + whenever(previewData.imagePreviewFileInfoFlow).thenReturn(MutableSharedFlow()) val testSubject = ChooserContentPreviewUi( - lifecycle, + lifecycleOwner.lifecycle, previewData, Intent(Intent.ACTION_SEND), imageLoader, @@ -140,7 +143,7 @@ class ChooserContentPreviewUiTest { assertThat(testSubject.preferredContentPreview) .isEqualTo(ContentPreviewType.CONTENT_PREVIEW_IMAGE) assertThat(testSubject.mContentPreviewUi).isInstanceOf(UnifiedContentPreviewUi::class.java) - verify(previewData, times(1)).getFileMetadataForImagePreview(any(), any()) + verify(previewData, times(1)).imagePreviewFileInfoFlow verify(transitionCallback, never()).onAllTransitionElementsReady() } } diff --git a/java/tests/src/com/android/intentresolver/contentpreview/PreviewDataProviderTest.kt b/java/tests/src/com/android/intentresolver/contentpreview/PreviewDataProviderTest.kt index 145b89ad..6599baa9 100644 --- a/java/tests/src/com/android/intentresolver/contentpreview/PreviewDataProviderTest.kt +++ b/java/tests/src/com/android/intentresolver/contentpreview/PreviewDataProviderTest.kt @@ -22,18 +22,15 @@ import android.database.MatrixCursor import android.media.MediaMetadata import android.net.Uri import android.provider.DocumentsContract -import androidx.lifecycle.Lifecycle -import com.android.intentresolver.TestLifecycleOwner import com.android.intentresolver.mock import com.android.intentresolver.whenever import com.google.common.truth.Truth.assertThat -import kotlinx.coroutines.Dispatchers +import kotlin.coroutines.EmptyCoroutineContext import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.toList +import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.UnconfinedTestDispatcher -import kotlinx.coroutines.test.resetMain -import kotlinx.coroutines.test.setMain -import org.junit.After -import org.junit.Before +import kotlinx.coroutines.test.runTest import org.junit.Test import org.mockito.Mockito.any import org.mockito.Mockito.never @@ -44,27 +41,13 @@ import org.mockito.Mockito.verify class PreviewDataProviderTest { private val contentResolver = mock() private val mimeTypeClassifier = DefaultMimeTypeClassifier - - private val lifecycleOwner = TestLifecycleOwner() - private val dispatcher = UnconfinedTestDispatcher() - - @Before - fun setup() { - Dispatchers.setMain(dispatcher) - lifecycleOwner.state = Lifecycle.State.CREATED - } - - @After - fun cleanup() { - lifecycleOwner.state = Lifecycle.State.DESTROYED - Dispatchers.resetMain() - } + private val testScope = TestScope(EmptyCoroutineContext + UnconfinedTestDispatcher()) @Test fun test_nonSendIntentAction_resolvesToTextPreviewUiSynchronously() { val targetIntent = Intent(Intent.ACTION_VIEW) val testSubject = - PreviewDataProvider(targetIntent, contentResolver, mimeTypeClassifier, dispatcher) + PreviewDataProvider(testScope, targetIntent, contentResolver, mimeTypeClassifier) assertThat(testSubject.previewType).isEqualTo(ContentPreviewType.CONTENT_PREVIEW_TEXT) verify(contentResolver, never()).getType(any()) @@ -73,14 +56,14 @@ class PreviewDataProviderTest { @Test fun test_sendSingleTextFileWithoutPreview_resolvesToFilePreviewUi() { val uri = Uri.parse("content://org.pkg.app/notes.txt") - val targetIntent = Intent(Intent.ACTION_SEND) - .apply { + val targetIntent = + Intent(Intent.ACTION_SEND).apply { putExtra(Intent.EXTRA_STREAM, uri) type = "text/plain" } whenever(contentResolver.getType(uri)).thenReturn("text/plain") val testSubject = - PreviewDataProvider(targetIntent, contentResolver, mimeTypeClassifier, dispatcher) + PreviewDataProvider(testScope, targetIntent, contentResolver, mimeTypeClassifier) assertThat(testSubject.previewType).isEqualTo(ContentPreviewType.CONTENT_PREVIEW_FILE) assertThat(testSubject.uriCount).isEqualTo(1) @@ -90,12 +73,9 @@ class PreviewDataProviderTest { @Test fun test_sendIntentWithoutUris_resolvesToTextPreviewUiSynchronously() { - val targetIntent = Intent(Intent.ACTION_SEND) - .apply { - type = "image/png" - } + val targetIntent = Intent(Intent.ACTION_SEND).apply { type = "image/png" } val testSubject = - PreviewDataProvider(targetIntent, contentResolver, mimeTypeClassifier, dispatcher) + PreviewDataProvider(testScope, targetIntent, contentResolver, mimeTypeClassifier) assertThat(testSubject.previewType).isEqualTo(ContentPreviewType.CONTENT_PREVIEW_TEXT) verify(contentResolver, never()).getType(any()) @@ -104,13 +84,10 @@ class PreviewDataProviderTest { @Test fun test_sendSingleImage_resolvesToImagePreviewUi() { val uri = Uri.parse("content://org.pkg.app/image.png") - val targetIntent = Intent(Intent.ACTION_SEND) - .apply { - putExtra(Intent.EXTRA_STREAM, uri) - } + val targetIntent = Intent(Intent.ACTION_SEND).apply { putExtra(Intent.EXTRA_STREAM, uri) } whenever(contentResolver.getType(uri)).thenReturn("image/png") val testSubject = - PreviewDataProvider(targetIntent, contentResolver, mimeTypeClassifier, dispatcher) + PreviewDataProvider(testScope, targetIntent, contentResolver, mimeTypeClassifier) assertThat(testSubject.previewType).isEqualTo(ContentPreviewType.CONTENT_PREVIEW_IMAGE) assertThat(testSubject.uriCount).isEqualTo(1) @@ -122,13 +99,10 @@ class PreviewDataProviderTest { @Test fun test_sendSingleNonImage_resolvesToFilePreviewUi() { val uri = Uri.parse("content://org.pkg.app/paper.pdf") - val targetIntent = Intent(Intent.ACTION_SEND) - .apply { - putExtra(Intent.EXTRA_STREAM, uri) - } + val targetIntent = Intent(Intent.ACTION_SEND).apply { putExtra(Intent.EXTRA_STREAM, uri) } whenever(contentResolver.getType(uri)).thenReturn("application/pdf") val testSubject = - PreviewDataProvider(targetIntent, contentResolver, mimeTypeClassifier, dispatcher) + PreviewDataProvider(testScope, targetIntent, contentResolver, mimeTypeClassifier) assertThat(testSubject.previewType).isEqualTo(ContentPreviewType.CONTENT_PREVIEW_FILE) assertThat(testSubject.uriCount).isEqualTo(1) @@ -141,14 +115,13 @@ class PreviewDataProviderTest { fun test_sendSingleImageWithFailingGetType_resolvesToFilePreviewUi() { val uri = Uri.parse("content://org.pkg.app/image.png") val targetIntent = - Intent(Intent.ACTION_SEND) - .apply { - type = "image/png" - putExtra(Intent.EXTRA_STREAM, uri) - } + Intent(Intent.ACTION_SEND).apply { + type = "image/png" + putExtra(Intent.EXTRA_STREAM, uri) + } whenever(contentResolver.getType(uri)).thenThrow(SecurityException("test failure")) val testSubject = - PreviewDataProvider(targetIntent, contentResolver, mimeTypeClassifier, dispatcher) + PreviewDataProvider(testScope, targetIntent, contentResolver, mimeTypeClassifier) assertThat(testSubject.previewType).isEqualTo(ContentPreviewType.CONTENT_PREVIEW_FILE) assertThat(testSubject.uriCount).isEqualTo(1) @@ -161,17 +134,16 @@ class PreviewDataProviderTest { fun test_sendSingleImageWithFailingMetadata_resolvesToFilePreviewUi() { val uri = Uri.parse("content://org.pkg.app/image.png") val targetIntent = - Intent(Intent.ACTION_SEND) - .apply { - type = "image/png" - putExtra(Intent.EXTRA_STREAM, uri) - } + Intent(Intent.ACTION_SEND).apply { + type = "image/png" + putExtra(Intent.EXTRA_STREAM, uri) + } whenever(contentResolver.getStreamTypes(uri, "*/*")) .thenThrow(SecurityException("test failure")) whenever(contentResolver.query(uri, METADATA_COLUMNS, null, null)) .thenThrow(SecurityException("test failure")) val testSubject = - PreviewDataProvider(targetIntent, contentResolver, mimeTypeClassifier, dispatcher) + PreviewDataProvider(testScope, targetIntent, contentResolver, mimeTypeClassifier) assertThat(testSubject.previewType).isEqualTo(ContentPreviewType.CONTENT_PREVIEW_FILE) assertThat(testSubject.uriCount).isEqualTo(1) @@ -183,14 +155,11 @@ class PreviewDataProviderTest { @Test fun test_SingleNonImageUriWithImageTypeInGetStreamTypes_useImagePreviewUi() { val uri = Uri.parse("content://org.pkg.app/paper.pdf") - val targetIntent = Intent(Intent.ACTION_SEND) - .apply { - putExtra(Intent.EXTRA_STREAM, uri) - } + val targetIntent = Intent(Intent.ACTION_SEND).apply { putExtra(Intent.EXTRA_STREAM, uri) } whenever(contentResolver.getStreamTypes(uri, "*/*")) .thenReturn(arrayOf("application/pdf", "image/png")) val testSubject = - PreviewDataProvider(targetIntent, contentResolver, mimeTypeClassifier, dispatcher) + PreviewDataProvider(testScope, targetIntent, contentResolver, mimeTypeClassifier) assertThat(testSubject.previewType).isEqualTo(ContentPreviewType.CONTENT_PREVIEW_IMAGE) assertThat(testSubject.uriCount).isEqualTo(1) @@ -221,15 +190,12 @@ class PreviewDataProviderTest { private fun testMetadataToImagePreview(columns: Array, values: Array) { val uri = Uri.parse("content://org.pkg.app/test.pdf") - val targetIntent = Intent(Intent.ACTION_SEND) - .apply { - putExtra(Intent.EXTRA_STREAM, uri) - } + val targetIntent = Intent(Intent.ACTION_SEND).apply { putExtra(Intent.EXTRA_STREAM, uri) } whenever(contentResolver.getType(uri)).thenReturn("application/pdf") whenever(contentResolver.query(uri, METADATA_COLUMNS, null, null)) .thenReturn(MatrixCursor(columns).apply { addRow(values) }) val testSubject = - PreviewDataProvider(targetIntent, contentResolver, mimeTypeClassifier, dispatcher) + PreviewDataProvider(testScope, targetIntent, contentResolver, mimeTypeClassifier) assertThat(testSubject.previewType).isEqualTo(ContentPreviewType.CONTENT_PREVIEW_IMAGE) assertThat(testSubject.uriCount).isEqualTo(1) @@ -243,20 +209,19 @@ class PreviewDataProviderTest { val uri1 = Uri.parse("content://org.pkg.app/test.png") val uri2 = Uri.parse("content://org.pkg.app/test.jpg") val targetIntent = - Intent(Intent.ACTION_SEND_MULTIPLE) - .apply { - putExtra( - Intent.EXTRA_STREAM, - ArrayList().apply { - add(uri1) - add(uri2) - } - ) - } + Intent(Intent.ACTION_SEND_MULTIPLE).apply { + putExtra( + Intent.EXTRA_STREAM, + ArrayList().apply { + add(uri1) + add(uri2) + } + ) + } whenever(contentResolver.getType(uri1)).thenReturn("image/png") whenever(contentResolver.getType(uri2)).thenReturn("image/jpeg") val testSubject = - PreviewDataProvider(targetIntent, contentResolver, mimeTypeClassifier, dispatcher) + PreviewDataProvider(testScope, targetIntent, contentResolver, mimeTypeClassifier) assertThat(testSubject.previewType).isEqualTo(ContentPreviewType.CONTENT_PREVIEW_IMAGE) assertThat(testSubject.uriCount).isEqualTo(2) @@ -273,18 +238,17 @@ class PreviewDataProviderTest { whenever(contentResolver.getType(uri1)).thenReturn("image/png") whenever(contentResolver.getType(uri2)).thenReturn("application/pdf") val targetIntent = - Intent(Intent.ACTION_SEND_MULTIPLE) - .apply { - putExtra( - Intent.EXTRA_STREAM, - ArrayList().apply { - add(uri1) - add(uri2) - } - ) - } + Intent(Intent.ACTION_SEND_MULTIPLE).apply { + putExtra( + Intent.EXTRA_STREAM, + ArrayList().apply { + add(uri1) + add(uri2) + } + ) + } val testSubject = - PreviewDataProvider(targetIntent, contentResolver, mimeTypeClassifier, dispatcher) + PreviewDataProvider(testScope, targetIntent, contentResolver, mimeTypeClassifier) assertThat(testSubject.previewType).isEqualTo(ContentPreviewType.CONTENT_PREVIEW_IMAGE) assertThat(testSubject.uriCount).isEqualTo(2) @@ -299,21 +263,20 @@ class PreviewDataProviderTest { val uri1 = Uri.parse("content://org.pkg.app/test.mp4") val uri2 = Uri.parse("content://org.pkg.app/test.pdf") val targetIntent = - Intent(Intent.ACTION_SEND_MULTIPLE) - .apply { - putExtra( - Intent.EXTRA_STREAM, - ArrayList().apply { - add(uri1) - add(uri2) - } - ) - } + Intent(Intent.ACTION_SEND_MULTIPLE).apply { + putExtra( + Intent.EXTRA_STREAM, + ArrayList().apply { + add(uri1) + add(uri2) + } + ) + } whenever(contentResolver.getType(uri1)).thenReturn("video/mpeg4") whenever(contentResolver.getStreamTypes(uri1, "*/*")).thenReturn(arrayOf("image/png")) whenever(contentResolver.getType(uri2)).thenReturn("application/pdf") val testSubject = - PreviewDataProvider(targetIntent, contentResolver, mimeTypeClassifier, dispatcher) + PreviewDataProvider(testScope, targetIntent, contentResolver, mimeTypeClassifier) assertThat(testSubject.previewType).isEqualTo(ContentPreviewType.CONTENT_PREVIEW_IMAGE) assertThat(testSubject.uriCount).isEqualTo(2) @@ -327,20 +290,19 @@ class PreviewDataProviderTest { val uri1 = Uri.parse("content://org.pkg.app/test.html") val uri2 = Uri.parse("content://org.pkg.app/test.pdf") val targetIntent = - Intent(Intent.ACTION_SEND_MULTIPLE) - .apply { - putExtra( - Intent.EXTRA_STREAM, - ArrayList().apply { - add(uri1) - add(uri2) - } - ) - } + Intent(Intent.ACTION_SEND_MULTIPLE).apply { + putExtra( + Intent.EXTRA_STREAM, + ArrayList().apply { + add(uri1) + add(uri2) + } + ) + } whenever(contentResolver.getType(uri1)).thenReturn("text/html") whenever(contentResolver.getType(uri2)).thenReturn("application/pdf") val testSubject = - PreviewDataProvider(targetIntent, contentResolver, mimeTypeClassifier, dispatcher) + PreviewDataProvider(testScope, targetIntent, contentResolver, mimeTypeClassifier) assertThat(testSubject.previewType).isEqualTo(ContentPreviewType.CONTENT_PREVIEW_FILE) assertThat(testSubject.uriCount).isEqualTo(2) @@ -348,4 +310,40 @@ class PreviewDataProviderTest { assertThat(testSubject.firstFileInfo?.previewUri).isNull() verify(contentResolver, times(2)).getType(any()) } + + @Test + fun test_imagePreviewFileInfoFlow_dataLoadedOnce() = + testScope.runTest { + val uri1 = Uri.parse("content://org.pkg.app/test.html") + val uri2 = Uri.parse("content://org.pkg.app/test.pdf") + val targetIntent = + Intent(Intent.ACTION_SEND_MULTIPLE).apply { + putExtra( + Intent.EXTRA_STREAM, + ArrayList().apply { + add(uri1) + add(uri2) + } + ) + } + whenever(contentResolver.getType(uri1)).thenReturn("text/html") + whenever(contentResolver.getType(uri2)).thenReturn("application/pdf") + whenever(contentResolver.getStreamTypes(uri1, "*/*")) + .thenReturn(arrayOf("text/html", "image/jpeg")) + whenever(contentResolver.getStreamTypes(uri2, "*/*")) + .thenReturn(arrayOf("application/pdf", "image/png")) + val testSubject = + PreviewDataProvider(testScope, targetIntent, contentResolver, mimeTypeClassifier) + + val fileInfoListOne = testSubject.imagePreviewFileInfoFlow.toList() + val fileInfoListTwo = testSubject.imagePreviewFileInfoFlow.toList() + + assertThat(fileInfoListOne).hasSize(2) + assertThat(fileInfoListOne).containsAtLeastElementsIn(fileInfoListTwo).inOrder() + + verify(contentResolver, times(1)).getType(uri1) + verify(contentResolver, times(1)).getStreamTypes(uri1, "*/*") + verify(contentResolver, times(1)).getType(uri2) + verify(contentResolver, times(1)).getStreamTypes(uri2, "*/*") + } } diff --git a/java/tests/src/com/android/intentresolver/contentpreview/UnifiedContentPreviewUiTest.kt b/java/tests/src/com/android/intentresolver/contentpreview/UnifiedContentPreviewUiTest.kt index 08331209..e7de0b7b 100644 --- a/java/tests/src/com/android/intentresolver/contentpreview/UnifiedContentPreviewUiTest.kt +++ b/java/tests/src/com/android/intentresolver/contentpreview/UnifiedContentPreviewUiTest.kt @@ -25,6 +25,13 @@ import com.android.intentresolver.R.layout.chooser_grid import com.android.intentresolver.mock import com.android.intentresolver.whenever import com.android.intentresolver.widget.ImagePreviewView.TransitionElementStatusCallback +import kotlin.coroutines.EmptyCoroutineContext +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.asFlow +import kotlinx.coroutines.flow.takeWhile +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.runTest import org.junit.Test import org.junit.runner.RunWith import org.mockito.Mockito.anyInt @@ -33,6 +40,7 @@ import org.mockito.Mockito.verify @RunWith(AndroidJUnit4::class) class UnifiedContentPreviewUiTest { + private val testScope = TestScope(EmptyCoroutineContext + UnconfinedTestDispatcher()) private val actionFactory = mock { whenever(createCustomActions()).thenReturn(emptyList()) @@ -129,24 +137,30 @@ class UnifiedContentPreviewUiTest { } private fun testLoadingHeadline(intentMimeType: String, files: List?) { - val testSubject = - UnifiedContentPreviewUi( - /*isSingleImage=*/ false, - intentMimeType, - actionFactory, - imageLoader, - DefaultMimeTypeClassifier, - object : TransitionElementStatusCallback { - override fun onTransitionElementReady(name: String) = Unit - override fun onAllTransitionElementsReady() = Unit - }, - /*itemCount=*/ 2, - headlineGenerator - ) - val layoutInflater = LayoutInflater.from(context) - val gridLayout = layoutInflater.inflate(chooser_grid, null, false) as ViewGroup - - files?.let(testSubject::setFiles) - testSubject.display(context.resources, LayoutInflater.from(context), gridLayout) + testScope.runTest { + val endMarker = FileInfo.Builder(Uri.EMPTY).build() + val emptySourceFlow = MutableSharedFlow(replay = 1) + val testSubject = + UnifiedContentPreviewUi( + testScope, + /*isSingleImage=*/ false, + intentMimeType, + actionFactory, + imageLoader, + DefaultMimeTypeClassifier, + object : TransitionElementStatusCallback { + override fun onTransitionElementReady(name: String) = Unit + override fun onAllTransitionElementsReady() = Unit + }, + files?.let { it.asFlow() } ?: emptySourceFlow.takeWhile { it !== endMarker }, + /*itemCount=*/ 2, + headlineGenerator + ) + val layoutInflater = LayoutInflater.from(context) + val gridLayout = layoutInflater.inflate(chooser_grid, null, false) as ViewGroup + + testSubject.display(context.resources, LayoutInflater.from(context), gridLayout) + emptySourceFlow.tryEmit(endMarker) + } } } diff --git a/java/tests/src/com/android/intentresolver/widget/BatchPreviewLoaderTest.kt b/java/tests/src/com/android/intentresolver/widget/BatchPreviewLoaderTest.kt index a0211308..4f4223c0 100644 --- a/java/tests/src/com/android/intentresolver/widget/BatchPreviewLoaderTest.kt +++ b/java/tests/src/com/android/intentresolver/widget/BatchPreviewLoaderTest.kt @@ -31,6 +31,7 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.cancel import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.asFlow import kotlinx.coroutines.launch import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.resetMain @@ -67,7 +68,13 @@ class BatchPreviewLoaderTest { val uriTwo = createUri(2) imageLoader.setUriLoadingOrder(succeed(uriTwo), succeed(uriOne)) val testSubject = - BatchPreviewLoader(imageLoader, previews(uriOne, uriTwo), 0, onUpdate, onCompletion) + BatchPreviewLoader( + imageLoader, + previews(uriOne, uriTwo), + totalItemCount = 2, + onUpdate, + onCompletion + ) testSubject.loadAspectRatios(200) { _, _, _ -> 100 } dispatcher.scheduler.advanceUntilIdle() @@ -87,7 +94,7 @@ class BatchPreviewLoaderTest { BatchPreviewLoader( imageLoader, previews(uriOne, uriTwo, uriThree), - 0, + totalItemCount = 3, onUpdate, onCompletion ) @@ -115,7 +122,7 @@ class BatchPreviewLoaderTest { } imageLoader.setUriLoadingOrder(*loadingOrder) val testSubject = - BatchPreviewLoader(imageLoader, previews(*uris), 0, onUpdate, onCompletion) + BatchPreviewLoader(imageLoader, previews(*uris), uris.size, onUpdate, onCompletion) testSubject.loadAspectRatios(200) { _, _, _ -> 100 } dispatcher.scheduler.advanceUntilIdle() @@ -144,7 +151,7 @@ class BatchPreviewLoaderTest { val expectedUris = Array(uris.size / 2) { createUri(it * 2 + 1) } imageLoader.setUriLoadingOrder(*loadingOrder) val testSubject = - BatchPreviewLoader(imageLoader, previews(*uris), 0, onUpdate, onCompletion) + BatchPreviewLoader(imageLoader, previews(*uris), uris.size, onUpdate, onCompletion) testSubject.loadAspectRatios(200) { _, _, _ -> 100 } dispatcher.scheduler.advanceUntilIdle() @@ -161,9 +168,11 @@ class BatchPreviewLoaderTest { private fun fail(uri: Uri) = uri to false private fun succeed(uri: Uri) = uri to true private fun previews(vararg uris: Uri) = - uris.fold(ArrayList(uris.size)) { acc, uri -> - acc.apply { add(Preview(PreviewType.Image, uri, editAction = null)) } - } + uris + .fold(ArrayList(uris.size)) { acc, uri -> + acc.apply { add(Preview(PreviewType.Image, uri, editAction = null)) } + } + .asFlow() } private class TestImageLoader(scope: CoroutineScope) : suspend (Uri, Boolean) -> Bitmap? { -- cgit v1.2.3-59-g8ed1b From 8ac875364c8d77b6f4a0ff2a10f9b4b5976618f6 Mon Sep 17 00:00:00 2001 From: Andrey Epin Date: Tue, 15 Aug 2023 00:53:21 -0700 Subject: Fix PreviewDataProvider previewType and firstFileInfo timeout logic Execute time-critical operations within `withTimeoutOrNull` function on a different task launched from a separate scope. The separate scope is need as, according to structural concurrency principals, `withTimeout`, upon timeout, canceling all its child jobs and completes only when all of them complete. The core of the logic we're wrapping with the timeout are ContentProvider calls which are not cancellable (and the whole operation can be canceled in some intermediate points between the calls). Calling them from another scope allows us just abandon them on timeout which is OK as we run the logic once anyway (and cache the results). Bug: 295985906 Test: Use the ShareTest app with metadata timeout and media type disguise option. Test: Integration tests. Change-Id: I922142385804abd78c2ad880d88d416d78155800 --- .../contentpreview/PreviewDataProvider.kt | 8 ++-- .../UnbundledChooserActivityTest.java | 49 ++++++++++++++++++++++ 2 files changed, 54 insertions(+), 3 deletions(-) (limited to 'java/src') diff --git a/java/src/com/android/intentresolver/contentpreview/PreviewDataProvider.kt b/java/src/com/android/intentresolver/contentpreview/PreviewDataProvider.kt index fd5ce3f8..9f1cc6c1 100644 --- a/java/src/com/android/intentresolver/contentpreview/PreviewDataProvider.kt +++ b/java/src/com/android/intentresolver/contentpreview/PreviewDataProvider.kt @@ -72,7 +72,6 @@ private const val TIMEOUT_MS = 1_000L */ @OpenForTesting open class PreviewDataProvider -@VisibleForTesting @JvmOverloads constructor( private val scope: CoroutineScope, @@ -128,7 +127,8 @@ constructor( } else { try { runBlocking(scope.coroutineContext) { - withTimeoutOrNull(TIMEOUT_MS) { loadPreviewType() } ?: CONTENT_PREVIEW_FILE + withTimeoutOrNull(TIMEOUT_MS) { scope.async { loadPreviewType() }.await() } + ?: CONTENT_PREVIEW_FILE } } catch (e: CancellationException) { Log.w( @@ -152,7 +152,9 @@ constructor( val builder = FileInfo.Builder(record.uri) try { runBlocking(scope.coroutineContext) { - withTimeoutOrNull(TIMEOUT_MS) { builder.readFromRecord(record) } + withTimeoutOrNull(TIMEOUT_MS) { + scope.async { builder.readFromRecord(record) }.await() + } } } catch (e: CancellationException) { Log.w( diff --git a/java/tests/src/com/android/intentresolver/UnbundledChooserActivityTest.java b/java/tests/src/com/android/intentresolver/UnbundledChooserActivityTest.java index 5709c912..b8b57403 100644 --- a/java/tests/src/com/android/intentresolver/UnbundledChooserActivityTest.java +++ b/java/tests/src/com/android/intentresolver/UnbundledChooserActivityTest.java @@ -1009,6 +1009,55 @@ public class UnbundledChooserActivityTest { .check(matches(withEffectiveVisibility(ViewMatchers.Visibility.GONE))); } + @Test + public void testSlowUriMetadata_fallbackToFilePreview() throws InterruptedException { + Uri uri = createTestContentProviderUri( + "application/pdf", "image/png", /*streamTypeTimeout=*/4_000); + ArrayList uris = new ArrayList<>(1); + uris.add(uri); + Intent sendIntent = createSendUriIntentWithPreview(uris); + ChooserActivityOverrideData.getInstance().imageLoader = + createImageLoader(uri, createBitmap()); + + List resolvedComponentInfos = createResolvedComponentsForTest(2); + + setupResolverControllers(resolvedComponentInfos); + assertThat(launchActivityWithTimeout(Intent.createChooser(sendIntent, null), 2_000)) + .isTrue(); + waitForIdle(); + + onView(withId(R.id.content_preview_filename)).check(matches(isDisplayed())); + onView(withId(R.id.content_preview_filename)).check(matches(withText("image.png"))); + onView(withId(R.id.content_preview_file_icon)).check(matches(isDisplayed())); + } + + @Test + public void testSendManyFilesWithSmallMetadataDelayAndOneImage_fallbackToFilePreviewUi() + throws InterruptedException { + Uri fileUri = createTestContentProviderUri( + "application/pdf", "application/pdf", /*streamTypeTimeout=*/150); + Uri imageUri = createTestContentProviderUri("application/pdf", "image/png"); + ArrayList uris = new ArrayList<>(50); + for (int i = 0; i < 49; i++) { + uris.add(fileUri); + } + uris.add(imageUri); + Intent sendIntent = createSendUriIntentWithPreview(uris); + ChooserActivityOverrideData.getInstance().imageLoader = + createImageLoader(imageUri, createBitmap()); + + List resolvedComponentInfos = createResolvedComponentsForTest(2); + setupResolverControllers(resolvedComponentInfos); + assertThat(launchActivityWithTimeout(Intent.createChooser(sendIntent, null), 2_000)) + .isTrue(); + + waitForIdle(); + + onView(withId(R.id.content_preview_filename)).check(matches(isDisplayed())); + onView(withId(R.id.content_preview_filename)).check(matches(withText("image.png"))); + onView(withId(R.id.content_preview_file_icon)).check(matches(isDisplayed())); + } + @Test public void testManyVisibleImagePreview_ScrollableImagePreview() { Uri uri = createTestContentProviderUri("image/png", null); -- cgit v1.2.3-59-g8ed1b From 2408b1e62c0facac74545ad97705ca4386323956 Mon Sep 17 00:00:00 2001 From: Matt Casey Date: Tue, 22 Aug 2023 19:58:56 +0000 Subject: Defer finish() to onStop() when doing shared transition We used to immediately finish() after starting the shared element transition. In high-latency situations, this transition took a while to start and resulted in some broken UI in window manager. Instead of finishing immediately, we finish onStop() such that we can wait for the transition to actually begin. Bug: 292158050 Test: Share an image that has high latency to load. Validate that the UI looks normal (albeit slow) when the edit transition happens (see bug). Change-Id: I66686398781aeea630c500c48198c5febc1e8fc2 --- .../android/intentresolver/ChooserActivity.java | 106 +++------------------ 1 file changed, 14 insertions(+), 92 deletions(-) (limited to 'java/src') diff --git a/java/src/com/android/intentresolver/ChooserActivity.java b/java/src/com/android/intentresolver/ChooserActivity.java index 8edbba08..b5899e5c 100644 --- a/java/src/com/android/intentresolver/ChooserActivity.java +++ b/java/src/com/android/intentresolver/ChooserActivity.java @@ -27,7 +27,6 @@ import static android.stats.devicepolicy.nano.DevicePolicyEnums.RESOLVER_EMPTY_S import static com.android.internal.util.LatencyTracker.ACTION_LOAD_SHARE_SHEET; import android.annotation.IntDef; -import android.annotation.NonNull; import android.annotation.Nullable; import android.app.Activity; import android.app.ActivityManager; @@ -66,9 +65,6 @@ import android.view.ViewGroup; import android.view.ViewGroup.LayoutParams; import android.view.ViewTreeObserver; import android.view.WindowInsets; -import android.view.animation.AlphaAnimation; -import android.view.animation.Animation; -import android.view.animation.LinearInterpolator; import android.widget.TextView; import androidx.annotation.MainThread; @@ -238,6 +234,13 @@ public class ChooserActivity extends ResolverActivity implements private final SparseArray mProfileRecords = new SparseArray<>(); private boolean mExcludeSharedText = false; + /** + * When we intend to finish the activity with a shared element transition, we can't immediately + * finish() when the transition is invoked, as the receiving end may not be able to start the + * animation and the UI breaks if this takes too long. Instead we defer finishing until onStop + * in order to wait for the transition to begin. + */ + private boolean mFinishWhenStopped = false; @Inject public ChooserActivity(ViewModelComponent.Builder builder) { @@ -645,8 +648,7 @@ public class ChooserActivity extends ResolverActivity implements protected void onResume() { super.onResume(); Log.d(TAG, "onResume: " + getComponentName().flattenToShortString()); - maybeCancelFinishAnimation(); - + mFinishWhenStopped = false; mRefinementManager.onActivityResume(); } @@ -747,7 +749,8 @@ public class ChooserActivity extends ResolverActivity implements super.onStop(); mRefinementManager.onActivityStop(isChangingConfigurations()); - if (maybeCancelFinishAnimation()) { + if (mFinishWhenStopped) { + mFinishWhenStopped = false; finish(); } } @@ -1363,7 +1366,10 @@ public class ChooserActivity extends ResolverActivity implements ChooserActivity.this, sharedElement, sharedElementName); safelyStartActivityAsUser( targetInfo, getPersonalProfileUserHandle(), options.toBundle()); - startFinishAnimation(); + // Can't finish right away because the shared element transition may not + // be ready to start. + mFinishWhenStopped = true; + } }, (status) -> { @@ -1748,25 +1754,6 @@ public class ChooserActivity extends ResolverActivity implements contentPreviewContainer.setVisibility(View.GONE); } - private void startFinishAnimation() { - View rootView = findRootView(); - if (rootView != null) { - rootView.startAnimation(new FinishAnimation(this, rootView)); - } - } - - private boolean maybeCancelFinishAnimation() { - View rootView = findRootView(); - Animation animation = (rootView == null) ? null : rootView.getAnimation(); - if (animation instanceof FinishAnimation) { - boolean hasEnded = animation.hasEnded(); - animation.cancel(); - rootView.clearAnimation(); - return !hasEnded; - } - return false; - } - private View findRootView() { if (mContentView == null) { mContentView = findViewById(android.R.id.content); @@ -1847,71 +1834,6 @@ public class ChooserActivity extends ResolverActivity implements } } - /** - * Used in combination with the scene transition when launching the image editor - */ - private static class FinishAnimation extends AlphaAnimation implements - Animation.AnimationListener { - @Nullable - private Activity mActivity; - @Nullable - private View mRootView; - private final float mFromAlpha; - - FinishAnimation(@NonNull Activity activity, @NonNull View rootView) { - super(rootView.getAlpha(), 0.0f); - mActivity = activity; - mRootView = rootView; - mFromAlpha = rootView.getAlpha(); - setInterpolator(new LinearInterpolator()); - long duration = activity.getWindow().getTransitionBackgroundFadeDuration(); - setDuration(duration); - // The scene transition animation looks better when it's not overlapped with this - // fade-out animation thus the delay. - // It is most likely that the image editor will cause this activity to stop and this - // animation will be cancelled in the background without running (i.e. we'll animate - // only when this activity remains partially visible after the image editor launch). - setStartOffset(duration); - super.setAnimationListener(this); - } - - @Override - public void setAnimationListener(AnimationListener listener) { - throw new UnsupportedOperationException(); - } - - @Override - public void cancel() { - if (mRootView != null) { - mRootView.setAlpha(mFromAlpha); - } - cleanup(); - super.cancel(); - } - - @Override - public void onAnimationStart(Animation animation) { - } - - @Override - public void onAnimationEnd(Animation animation) { - Activity activity = mActivity; - cleanup(); - if (activity != null) { - activity.finish(); - } - } - - @Override - public void onAnimationRepeat(Animation animation) { - } - - private void cleanup() { - mActivity = null; - mRootView = null; - } - } - @Override protected void maybeLogProfileChange() { getEventLog().logSharesheetProfileChanged(); -- cgit v1.2.3-59-g8ed1b From b45704b2e6810a85fe594855a9e9a307b5fd43fe Mon Sep 17 00:00:00 2001 From: Mark Renouf Date: Mon, 28 Aug 2023 15:12:19 -0400 Subject: Revert "Creates application-level and activity-level dagger graphs" This reverts commit 114b886b4aaf3341b4072f184c96cc71d4763f76. and dependent commit 35a34bcaf236597703208cf0472cb268b42abb1c. Change-Id: Ied8c43ed7c596624e045cb8e7b4e7f1c9ba66c58 --- Android.bp | 5 -- AndroidManifest-app.xml | 14 ++-- .../intentresolver/ApplicationComponentOwner.kt | 15 ---- .../android/intentresolver/ChooserActivity.java | 40 +--------- .../intentresolver/IntentForwarderActivity.java | 7 -- .../intentresolver/IntentResolverApplication.kt | 30 ------- .../android/intentresolver/ResolverActivity.java | 3 - .../intentresolver/dagger/ActivityBinderModule.kt | 33 -------- .../intentresolver/dagger/ActivityComponent.kt | 21 ----- .../intentresolver/dagger/ActivityModule.kt | 6 -- .../intentresolver/dagger/ApplicationComponent.kt | 21 ----- .../intentresolver/dagger/ApplicationModule.kt | 29 ------- .../intentresolver/dagger/CoroutinesModule.kt | 51 ------------ .../dagger/InjectedAppComponentFactory.kt | 92 ---------------------- .../dagger/InjectedViewModelFactory.kt | 84 -------------------- .../intentresolver/dagger/ReceiverBinderModule.kt | 12 --- .../intentresolver/dagger/ViewModelBinderModule.kt | 34 -------- .../intentresolver/dagger/ViewModelComponent.kt | 57 -------------- .../intentresolver/dagger/ViewModelModule.kt | 6 -- .../intentresolver/dagger/qualifiers/Qualifiers.kt | 37 --------- .../android/intentresolver/ui/ChooserViewModel.kt | 34 -------- java/tests/Android.bp | 1 + java/tests/AndroidManifest.xml | 8 +- .../intentresolver/ChooserWrapperActivity.java | 8 -- .../intentresolver/ResolverWrapperActivity.java | 3 - .../com/android/intentresolver/TestApplication.kt | 7 +- .../dagger/TestActivityBinderModule.kt | 40 ---------- .../intentresolver/dagger/TestActivityComponent.kt | 30 ------- .../dagger/TestApplicationComponent.kt | 34 -------- .../intentresolver/dagger/TestApplicationModule.kt | 38 --------- .../dagger/TestViewModelComponent.kt | 29 ------- .../intentresolver/dagger/TestViewModelModule.kt | 6 -- 32 files changed, 15 insertions(+), 820 deletions(-) delete mode 100644 java/src/com/android/intentresolver/ApplicationComponentOwner.kt delete mode 100644 java/src/com/android/intentresolver/IntentResolverApplication.kt delete mode 100644 java/src/com/android/intentresolver/dagger/ActivityBinderModule.kt delete mode 100644 java/src/com/android/intentresolver/dagger/ActivityComponent.kt delete mode 100644 java/src/com/android/intentresolver/dagger/ActivityModule.kt delete mode 100644 java/src/com/android/intentresolver/dagger/ApplicationComponent.kt delete mode 100644 java/src/com/android/intentresolver/dagger/ApplicationModule.kt delete mode 100644 java/src/com/android/intentresolver/dagger/CoroutinesModule.kt delete mode 100644 java/src/com/android/intentresolver/dagger/InjectedAppComponentFactory.kt delete mode 100644 java/src/com/android/intentresolver/dagger/InjectedViewModelFactory.kt delete mode 100644 java/src/com/android/intentresolver/dagger/ReceiverBinderModule.kt delete mode 100644 java/src/com/android/intentresolver/dagger/ViewModelBinderModule.kt delete mode 100644 java/src/com/android/intentresolver/dagger/ViewModelComponent.kt delete mode 100644 java/src/com/android/intentresolver/dagger/ViewModelModule.kt delete mode 100644 java/src/com/android/intentresolver/dagger/qualifiers/Qualifiers.kt delete mode 100644 java/src/com/android/intentresolver/ui/ChooserViewModel.kt delete mode 100644 java/tests/src/com/android/intentresolver/dagger/TestActivityBinderModule.kt delete mode 100644 java/tests/src/com/android/intentresolver/dagger/TestActivityComponent.kt delete mode 100644 java/tests/src/com/android/intentresolver/dagger/TestApplicationComponent.kt delete mode 100644 java/tests/src/com/android/intentresolver/dagger/TestApplicationModule.kt delete mode 100644 java/tests/src/com/android/intentresolver/dagger/TestViewModelComponent.kt delete mode 100644 java/tests/src/com/android/intentresolver/dagger/TestViewModelModule.kt (limited to 'java/src') diff --git a/Android.bp b/Android.bp index 6aad4419..9d0a8ee6 100644 --- a/Android.bp +++ b/Android.bp @@ -75,9 +75,6 @@ android_library { "androidx.lifecycle_lifecycle-extensions", "androidx.lifecycle_lifecycle-runtime-ktx", "androidx.lifecycle_lifecycle-viewmodel-ktx", - "androidx.savedstate_savedstate-ktx", - "dagger2", - "jsr330", "kotlin-stdlib", "kotlinx_coroutines", "kotlinx-coroutines-android", @@ -86,8 +83,6 @@ android_library { "SystemUIFlagsLib", ], - plugins: ["dagger2-compiler"], - lint: { strict_updatability_linting: false, }, diff --git a/AndroidManifest-app.xml b/AndroidManifest-app.xml index ba9afe28..57ea497b 100644 --- a/AndroidManifest-app.xml +++ b/AndroidManifest-app.xml @@ -17,22 +17,18 @@ */ --> + package="com.android.intentresolver" + android:versionCode="0" + android:versionName="2021-11" + coreApp="true"> + android:supportsRtl="true"> diff --git a/java/src/com/android/intentresolver/ApplicationComponentOwner.kt b/java/src/com/android/intentresolver/ApplicationComponentOwner.kt deleted file mode 100644 index fb39814c..00000000 --- a/java/src/com/android/intentresolver/ApplicationComponentOwner.kt +++ /dev/null @@ -1,15 +0,0 @@ -package com.android.intentresolver - -import com.android.intentresolver.dagger.ApplicationComponent - -/** - * Interface that should be implemented by the [Application][android.app.Application] object as the - * owner of the [ApplicationComponent]. - */ -interface ApplicationComponentOwner { - /** - * Invokes the given [action] when the [ApplicationComponent] has been created. If it has - * already been created, then it invokes [action] immediately. - */ - fun doWhenApplicationComponentReady(action: (ApplicationComponent) -> Unit) -} diff --git a/java/src/com/android/intentresolver/ChooserActivity.java b/java/src/com/android/intentresolver/ChooserActivity.java index 8edbba08..d302233f 100644 --- a/java/src/com/android/intentresolver/ChooserActivity.java +++ b/java/src/com/android/intentresolver/ChooserActivity.java @@ -72,7 +72,6 @@ import android.view.animation.LinearInterpolator; import android.widget.TextView; import androidx.annotation.MainThread; -import androidx.lifecycle.HasDefaultViewModelProviderFactory; import androidx.lifecycle.ViewModelProvider; import androidx.recyclerview.widget.GridLayoutManager; import androidx.recyclerview.widget.RecyclerView; @@ -88,8 +87,6 @@ import com.android.intentresolver.contentpreview.BasePreviewViewModel; import com.android.intentresolver.contentpreview.ChooserContentPreviewUi; import com.android.intentresolver.contentpreview.HeadlineGeneratorImpl; import com.android.intentresolver.contentpreview.PreviewViewModel; -import com.android.intentresolver.dagger.InjectedViewModelFactory; -import com.android.intentresolver.dagger.ViewModelComponent; import com.android.intentresolver.flags.FeatureFlagRepository; import com.android.intentresolver.flags.FeatureFlagRepositoryFactory; import com.android.intentresolver.grid.ChooserGridAdapter; @@ -102,14 +99,11 @@ import com.android.intentresolver.model.AppPredictionServiceResolverComparator; import com.android.intentresolver.model.ResolverRankerServiceResolverComparator; import com.android.intentresolver.shortcuts.AppPredictorFactory; import com.android.intentresolver.shortcuts.ShortcutLoader; -import com.android.intentresolver.ui.ChooserViewModel; import com.android.intentresolver.widget.ImagePreviewView; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.content.PackageMonitor; import com.android.internal.logging.nano.MetricsProto.MetricsEvent; -import org.jetbrains.annotations.NotNull; - import java.io.File; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; @@ -125,15 +119,13 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.function.Consumer; -import javax.inject.Inject; - /** * The Chooser Activity handles intent resolution specifically for sharing intents - * for example, as generated by {@see android.content.Intent#createChooser(Intent, CharSequence)}. * */ public class ChooserActivity extends ResolverActivity implements - ResolverListAdapter.ResolverListCommunicator, HasDefaultViewModelProviderFactory { + ResolverListAdapter.ResolverListCommunicator { private static final String TAG = "ChooserActivity"; /** @@ -173,11 +165,6 @@ public class ChooserActivity extends ResolverActivity implements private static final int SCROLL_STATUS_SCROLLING_VERTICAL = 1; private static final int SCROLL_STATUS_SCROLLING_HORIZONTAL = 2; - private ViewModelProvider.Factory mViewModelFactory; - private final ViewModelComponent.Builder mViewModelComponentBuilder; - - private ChooserViewModel mViewModel; - @IntDef(flag = false, prefix = { "TARGET_TYPE_" }, value = { TARGET_TYPE_DEFAULT, TARGET_TYPE_CHOOSER_TARGET, @@ -239,33 +226,16 @@ public class ChooserActivity extends ResolverActivity implements private boolean mExcludeSharedText = false; - @Inject - public ChooserActivity(ViewModelComponent.Builder builder) { - mViewModelComponentBuilder = builder; - } - - @NotNull - @Override - public final ViewModelProvider.Factory getDefaultViewModelProviderFactory() { - if (mViewModelFactory == null) { - mViewModelFactory = new InjectedViewModelFactory(mViewModelComponentBuilder, - getDefaultViewModelCreationExtras(), - getReferrer()); - } - return mViewModelFactory; - } + public ChooserActivity() {} @Override protected void onCreate(Bundle savedInstanceState) { - Log.d(TAG, "onCreate"); Tracer.INSTANCE.markLaunched(); final long intentReceivedTime = System.currentTimeMillis(); mLatencyTracker.onActionStart(ACTION_LOAD_SHARE_SHEET); getEventLog().logSharesheetTriggered(); - mViewModel = new ViewModelProvider(this).get(ChooserViewModel.class); - mFeatureFlagRepository = createFeatureFlagRepository(); mIntegratedDeviceComponents = getIntegratedDeviceComponents(); @@ -282,9 +252,7 @@ public class ChooserActivity extends ResolverActivity implements return; } - // Note: Uses parent ViewModelProvider.Factory because RefinementManager is not injectable - mRefinementManager = new ViewModelProvider(this, super.getDefaultViewModelProviderFactory()) - .get(ChooserRefinementManager.class); + mRefinementManager = new ViewModelProvider(this).get(ChooserRefinementManager.class); mRefinementManager.getRefinementCompletion().observe(this, completion -> { if (completion.consume()) { @@ -306,7 +274,7 @@ public class ChooserActivity extends ResolverActivity implements BasePreviewViewModel previewViewModel = new ViewModelProvider(this, createPreviewViewModelFactory()) - .get(PreviewViewModel.class); + .get(BasePreviewViewModel.class); mChooserContentPreviewUi = new ChooserContentPreviewUi( getLifecycle(), previewViewModel.createOrReuseProvider(mChooserRequest), diff --git a/java/src/com/android/intentresolver/IntentForwarderActivity.java b/java/src/com/android/intentresolver/IntentForwarderActivity.java index d69a6c71..5e8945f1 100644 --- a/java/src/com/android/intentresolver/IntentForwarderActivity.java +++ b/java/src/com/android/intentresolver/IntentForwarderActivity.java @@ -57,8 +57,6 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; -import javax.inject.Inject; - /** * This is used in conjunction with * {@link DevicePolicyManager#addCrossProfileIntentFilter} to enable intents to @@ -86,11 +84,6 @@ public class IntentForwarderActivity extends Activity { private MetricsLogger mMetricsLogger; protected ExecutorService mExecutorService; - @Inject - public IntentForwarderActivity() { - super(); - } - @Override protected void onDestroy() { super.onDestroy(); diff --git a/java/src/com/android/intentresolver/IntentResolverApplication.kt b/java/src/com/android/intentresolver/IntentResolverApplication.kt deleted file mode 100644 index 61df7fff..00000000 --- a/java/src/com/android/intentresolver/IntentResolverApplication.kt +++ /dev/null @@ -1,30 +0,0 @@ -package com.android.intentresolver - -import android.app.Application -import com.android.intentresolver.dagger.ApplicationComponent -import com.android.intentresolver.dagger.DaggerApplicationComponent - -/** [Application] that maintains the [ApplicationComponent]. */ -open class IntentResolverApplication : Application(), ApplicationComponentOwner { - - private lateinit var applicationComponent: ApplicationComponent - - private val pendingDaggerActions = mutableSetOf<(ApplicationComponent) -> Unit>() - - open fun createApplicationComponentBuilder() = DaggerApplicationComponent.builder() - - override fun onCreate() { - super.onCreate() - applicationComponent = createApplicationComponentBuilder().application(this).build() - pendingDaggerActions.forEach { it.invoke(applicationComponent) } - pendingDaggerActions.clear() - } - - override fun doWhenApplicationComponentReady(action: (ApplicationComponent) -> Unit) { - if (this::applicationComponent.isInitialized) { - action.invoke(applicationComponent) - } else { - pendingDaggerActions.add(action) - } - } -} diff --git a/java/src/com/android/intentresolver/ResolverActivity.java b/java/src/com/android/intentresolver/ResolverActivity.java index 252d0a41..35c7e897 100644 --- a/java/src/com/android/intentresolver/ResolverActivity.java +++ b/java/src/com/android/intentresolver/ResolverActivity.java @@ -127,8 +127,6 @@ import java.util.Objects; import java.util.Set; import java.util.function.Supplier; -import javax.inject.Inject; - /** * This is a copy of ResolverActivity to support IntentResolver's ChooserActivity. This code is * *not* the resolver that is actually triggered by the system right now (you want @@ -139,7 +137,6 @@ import javax.inject.Inject; public class ResolverActivity extends FragmentActivity implements ResolverListAdapter.ResolverListCommunicator { - @Inject public ResolverActivity() { mIsIntentPicker = getClass().equals(ResolverActivity.class); } diff --git a/java/src/com/android/intentresolver/dagger/ActivityBinderModule.kt b/java/src/com/android/intentresolver/dagger/ActivityBinderModule.kt deleted file mode 100644 index 7c997ef7..00000000 --- a/java/src/com/android/intentresolver/dagger/ActivityBinderModule.kt +++ /dev/null @@ -1,33 +0,0 @@ -package com.android.intentresolver.dagger - -import android.app.Activity -import com.android.intentresolver.ChooserActivity -import com.android.intentresolver.IntentForwarderActivity -import com.android.intentresolver.ResolverActivity -import dagger.Binds -import dagger.Module -import dagger.multibindings.ClassKey -import dagger.multibindings.IntoMap - -/** Injection instructions for injectable [Activities][Activity]. */ -@Module -interface ActivityBinderModule { - - @Binds - @IntoMap - @ClassKey(ChooserActivity::class) - @ActivityScope - fun bindChooserActivity(activity: ChooserActivity): Activity - - @Binds - @IntoMap - @ClassKey(ResolverActivity::class) - @ActivityScope - fun bindResolverActivity(activity: ResolverActivity): Activity - - @Binds - @IntoMap - @ClassKey(IntentForwarderActivity::class) - @ActivityScope - fun bindIntentForwarderActivity(activity: IntentForwarderActivity): Activity -} diff --git a/java/src/com/android/intentresolver/dagger/ActivityComponent.kt b/java/src/com/android/intentresolver/dagger/ActivityComponent.kt deleted file mode 100644 index bf5ff761..00000000 --- a/java/src/com/android/intentresolver/dagger/ActivityComponent.kt +++ /dev/null @@ -1,21 +0,0 @@ -package com.android.intentresolver.dagger - -import android.app.Activity -import dagger.Subcomponent -import javax.inject.Provider -import javax.inject.Scope - -@MustBeDocumented @Retention(AnnotationRetention.RUNTIME) @Scope annotation class ActivityScope - -/** Subcomponent for injections across the life of an Activity. */ -@ActivityScope -@Subcomponent(modules = [ActivityModule::class, ActivityBinderModule::class]) -interface ActivityComponent { - - @Subcomponent.Factory - interface Factory { - fun create(): ActivityComponent - } - - fun activities(): Map, @JvmSuppressWildcards Provider> -} diff --git a/java/src/com/android/intentresolver/dagger/ActivityModule.kt b/java/src/com/android/intentresolver/dagger/ActivityModule.kt deleted file mode 100644 index f6a2229d..00000000 --- a/java/src/com/android/intentresolver/dagger/ActivityModule.kt +++ /dev/null @@ -1,6 +0,0 @@ -package com.android.intentresolver.dagger - -import dagger.Module - -/** Bindings provided to [@ActivityScope][ActivityScope]. */ -@Module interface ActivityModule diff --git a/java/src/com/android/intentresolver/dagger/ApplicationComponent.kt b/java/src/com/android/intentresolver/dagger/ApplicationComponent.kt deleted file mode 100644 index 9fc57712..00000000 --- a/java/src/com/android/intentresolver/dagger/ApplicationComponent.kt +++ /dev/null @@ -1,21 +0,0 @@ -package com.android.intentresolver.dagger - -import android.app.Application -import dagger.BindsInstance -import dagger.Component -import javax.inject.Singleton - -/** Top level component for injections across the life of the process. */ -@Singleton -@Component(modules = [ApplicationModule::class]) -interface ApplicationComponent { - - @Component.Builder - interface Builder { - @BindsInstance fun application(application: Application): Builder - - fun build(): ApplicationComponent - } - - fun inject(appComponentFactory: InjectedAppComponentFactory) -} diff --git a/java/src/com/android/intentresolver/dagger/ApplicationModule.kt b/java/src/com/android/intentresolver/dagger/ApplicationModule.kt deleted file mode 100644 index 4986d7e1..00000000 --- a/java/src/com/android/intentresolver/dagger/ApplicationModule.kt +++ /dev/null @@ -1,29 +0,0 @@ -package com.android.intentresolver.dagger - -import android.app.Application -import android.content.Context -import com.android.intentresolver.dagger.qualifiers.App -import dagger.Module -import dagger.Provides -import javax.inject.Singleton - -/** - * Bindings provided to [ApplicationComponent] and children. - * - * These are all @Singleton scope, one for the duration of the process. - */ -@Module( - subcomponents = [ActivityComponent::class, ViewModelComponent::class], - includes = [ReceiverBinderModule::class, CoroutinesModule::class], -) -interface ApplicationModule { - - companion object { - - @JvmStatic - @Provides - @Singleton - @App - fun applicationContext(app: Application): Context = app.applicationContext - } -} diff --git a/java/src/com/android/intentresolver/dagger/CoroutinesModule.kt b/java/src/com/android/intentresolver/dagger/CoroutinesModule.kt deleted file mode 100644 index 5fda2c30..00000000 --- a/java/src/com/android/intentresolver/dagger/CoroutinesModule.kt +++ /dev/null @@ -1,51 +0,0 @@ -/* - * Copyright (C) 2023 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.intentresolver.dagger - -import com.android.intentresolver.dagger.qualifiers.Background -import com.android.intentresolver.dagger.qualifiers.Main -import dagger.Module -import dagger.Provides -import javax.inject.Singleton -import kotlinx.coroutines.CoroutineDispatcher -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.SupervisorJob - -@Module -interface CoroutinesModule { - companion object { - @JvmStatic - @Provides - @Singleton - @Main - fun mainDispatcher(): CoroutineDispatcher = Dispatchers.Main.immediate - - @JvmStatic - @Provides - @Singleton - @Main - fun mainCoroutineScope(@Main mainDispatcher: CoroutineDispatcher) = - CoroutineScope(SupervisorJob() + mainDispatcher) - - @JvmStatic - @Provides - @Singleton - @Background - fun backgroundDispatcher(): CoroutineDispatcher = Dispatchers.IO - } -} diff --git a/java/src/com/android/intentresolver/dagger/InjectedAppComponentFactory.kt b/java/src/com/android/intentresolver/dagger/InjectedAppComponentFactory.kt deleted file mode 100644 index db209ef0..00000000 --- a/java/src/com/android/intentresolver/dagger/InjectedAppComponentFactory.kt +++ /dev/null @@ -1,92 +0,0 @@ -/* - * Copyright (C) 2023 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.intentresolver.dagger - -import android.app.Activity -import android.app.Application -import android.content.BroadcastReceiver -import android.content.Intent -import android.util.Log -import androidx.core.app.AppComponentFactory -import com.android.intentresolver.ApplicationComponentOwner -import javax.inject.Inject -import javax.inject.Provider - -/** Provides instances of application components, delegates construction to Dagger. */ -class InjectedAppComponentFactory : AppComponentFactory() { - - @set:Inject lateinit var activityComponentBuilder: ActivityComponent.Factory - - @set:Inject - lateinit var receivers: Map, @JvmSuppressWildcards Provider> - - override fun instantiateApplicationCompat(cl: ClassLoader, className: String): Application { - val app = super.instantiateApplicationCompat(cl, className) - if (app !is ApplicationComponentOwner) { - throw RuntimeException("App must be ApplicationComponentOwner") - } - app.doWhenApplicationComponentReady { it.inject(this) } - return app - } - - override fun instantiateActivityCompat( - cl: ClassLoader, - className: String, - intent: Intent?, - ): Activity { - return runCatching { - val activities = activityComponentBuilder.create().activities() - instantiate(className, activities) - } - .onFailure { - if (it is UninitializedPropertyAccessException) { - // This should never happen but if it did it would cause errors that could - // be very difficult to identify, so we log it out of an abundance of - // caution. - Log.e(TAG, "Tried to instantiate $className before AppComponent", it) - } - } - .getOrNull() - ?: super.instantiateActivityCompat(cl, className, intent) - } - - override fun instantiateReceiverCompat( - cl: ClassLoader, - className: String, - intent: Intent?, - ): BroadcastReceiver { - return instantiate(className, receivers) - ?: super.instantiateReceiverCompat(cl, className, intent) - } - - private fun instantiate(className: String, providers: Map, Provider>): T? { - return runCatching { providers[Class.forName(className)]?.get() } - .onFailure { - if (it is UninitializedPropertyAccessException) { - // This should never happen but if it did it would cause errors that could - // be very difficult to identify, so we log it out of an abundance of - // caution. - Log.e(TAG, "Tried to instantiate $className before AppComponent", it) - } - } - .getOrNull() - } - - companion object { - private const val TAG = "AppComponentFactory" - } -} diff --git a/java/src/com/android/intentresolver/dagger/InjectedViewModelFactory.kt b/java/src/com/android/intentresolver/dagger/InjectedViewModelFactory.kt deleted file mode 100644 index f0906d3e..00000000 --- a/java/src/com/android/intentresolver/dagger/InjectedViewModelFactory.kt +++ /dev/null @@ -1,84 +0,0 @@ -/* - * Copyright (C) 2023 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.intentresolver.dagger - -import android.net.Uri -import android.os.Bundle -import androidx.lifecycle.DEFAULT_ARGS_KEY -import androidx.lifecycle.ViewModel -import androidx.lifecycle.ViewModelProvider -import androidx.lifecycle.viewmodel.CreationExtras -import java.io.Closeable -import javax.inject.Provider -import kotlin.coroutines.CoroutineContext -import kotlinx.coroutines.CoroutineName -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.SupervisorJob -import kotlinx.coroutines.cancel -import kotlinx.coroutines.isActive - -/** Instantiates new ViewModel instances using Dagger. */ -class InjectedViewModelFactory( - private val viewModelComponentBuilder: ViewModelComponent.Builder, - creationExtras: CreationExtras, - private val referrer: Uri, -) : ViewModelProvider.Factory { - - private val defaultArgs = creationExtras[DEFAULT_ARGS_KEY] ?: Bundle() - - private fun viewModelScope(viewModelClass: Class<*>) = - CloseableCoroutineScope( - SupervisorJob() + CoroutineName(viewModelClass.simpleName) + Dispatchers.Main.immediate - ) - - private fun newViewModel( - providerMap: Map, Provider>, - modelClass: Class - ): T { - val provider = - providerMap[modelClass] - ?: error( - "Unable to create an instance of $modelClass. " + - "Does the class have a binding in ViewModelComponent?" - ) - return modelClass.cast(provider.get()) - } - - override fun create(modelClass: Class): T { - val viewModelScope = viewModelScope(modelClass) - val viewModelComponent = - viewModelComponentBuilder - .coroutineScope(viewModelScope) - .intentExtras(defaultArgs) - .referrer(referrer) - .build() - val viewModel = newViewModel(viewModelComponent.viewModels(), modelClass) - viewModel.addCloseable(viewModelScope) - return viewModel - } -} - -internal class CloseableCoroutineScope(context: CoroutineContext) : Closeable, CoroutineScope { - override val coroutineContext: CoroutineContext = context - - override fun close() { - if (isActive) { - coroutineContext.cancel() - } - } -} diff --git a/java/src/com/android/intentresolver/dagger/ReceiverBinderModule.kt b/java/src/com/android/intentresolver/dagger/ReceiverBinderModule.kt deleted file mode 100644 index 32ce2f45..00000000 --- a/java/src/com/android/intentresolver/dagger/ReceiverBinderModule.kt +++ /dev/null @@ -1,12 +0,0 @@ -package com.android.intentresolver.dagger - -import android.content.BroadcastReceiver -import dagger.Module -import dagger.multibindings.Multibinds - -/** Injection instructions for injectable [BroadcastReceivers][BroadcastReceiver] */ -@Module -interface ReceiverBinderModule { - - @Multibinds fun bindReceivers(): Map, @JvmSuppressWildcards BroadcastReceiver> -} diff --git a/java/src/com/android/intentresolver/dagger/ViewModelBinderModule.kt b/java/src/com/android/intentresolver/dagger/ViewModelBinderModule.kt deleted file mode 100644 index 91ba039c..00000000 --- a/java/src/com/android/intentresolver/dagger/ViewModelBinderModule.kt +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright (C) 2023 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.intentresolver.dagger - -import androidx.lifecycle.ViewModel -import com.android.intentresolver.ui.ChooserViewModel -import dagger.Binds -import dagger.Module -import dagger.multibindings.ClassKey -import dagger.multibindings.IntoMap - -/** Defines a map of injectable ViewModel classes. */ -@Module -interface ViewModelBinderModule { - @Binds - @IntoMap - @ClassKey(ChooserViewModel::class) - @ViewModelScope - fun chooserViewModel(viewModel: ChooserViewModel): ViewModel -} diff --git a/java/src/com/android/intentresolver/dagger/ViewModelComponent.kt b/java/src/com/android/intentresolver/dagger/ViewModelComponent.kt deleted file mode 100644 index 3e2e2681..00000000 --- a/java/src/com/android/intentresolver/dagger/ViewModelComponent.kt +++ /dev/null @@ -1,57 +0,0 @@ -/* - * Copyright (C) 2023 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.intentresolver.dagger - -import android.net.Uri -import android.os.Bundle -import com.android.intentresolver.dagger.qualifiers.Referrer -import com.android.intentresolver.dagger.qualifiers.ViewModel -import dagger.BindsInstance -import dagger.Subcomponent -import javax.inject.Provider -import javax.inject.Scope -import kotlin.annotation.AnnotationRetention.RUNTIME -import kotlinx.coroutines.CoroutineScope - -@Scope @Retention(RUNTIME) @MustBeDocumented annotation class ViewModelScope - -/** - * Provides dependencies within [ViewModelScope] within a [ViewModel]. - * - * @see InjectedViewModelFactory - */ -@ViewModelScope -@Subcomponent(modules = [ViewModelModule::class, ViewModelBinderModule::class]) -interface ViewModelComponent { - - /** - * Binds instance values from the creating Activity to make them available for injection within - * [ViewModelScope]. - */ - @Subcomponent.Builder - interface Builder { - @BindsInstance fun intentExtras(@ViewModel intentExtras: Bundle): Builder - - @BindsInstance fun referrer(@Referrer uri: Uri): Builder - - @BindsInstance fun coroutineScope(@ViewModel scope: CoroutineScope): Builder - - fun build(): ViewModelComponent - } - - fun viewModels(): Map, @JvmSuppressWildcards Provider> -} diff --git a/java/src/com/android/intentresolver/dagger/ViewModelModule.kt b/java/src/com/android/intentresolver/dagger/ViewModelModule.kt deleted file mode 100644 index 23320311..00000000 --- a/java/src/com/android/intentresolver/dagger/ViewModelModule.kt +++ /dev/null @@ -1,6 +0,0 @@ -package com.android.intentresolver.dagger - -import dagger.Module - -/** Provides bindings shared among components within [@ViewModelScope][ViewModelScope]. */ -@Module abstract class ViewModelModule diff --git a/java/src/com/android/intentresolver/dagger/qualifiers/Qualifiers.kt b/java/src/com/android/intentresolver/dagger/qualifiers/Qualifiers.kt deleted file mode 100644 index fa50170e..00000000 --- a/java/src/com/android/intentresolver/dagger/qualifiers/Qualifiers.kt +++ /dev/null @@ -1,37 +0,0 @@ -/* - * Copyright (C) 2023 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.intentresolver.dagger.qualifiers - -import javax.inject.Qualifier - -// Note: 'qualifiers' package avoids name collisions in Dagger code. - -@Qualifier @MustBeDocumented @Retention(AnnotationRetention.RUNTIME) annotation class App - -@Qualifier @MustBeDocumented @Retention(AnnotationRetention.RUNTIME) annotation class Activity - -@Qualifier @MustBeDocumented @Retention(AnnotationRetention.RUNTIME) annotation class ViewModel - -@Qualifier @MustBeDocumented @Retention(AnnotationRetention.RUNTIME) annotation class Main - -@Qualifier @MustBeDocumented @Retention(AnnotationRetention.RUNTIME) annotation class Background - -@Qualifier @MustBeDocumented @Retention(AnnotationRetention.RUNTIME) annotation class Delegate - -@Qualifier @MustBeDocumented @Retention(AnnotationRetention.RUNTIME) annotation class Default - -@Qualifier @MustBeDocumented @Retention(AnnotationRetention.RUNTIME) annotation class Referrer diff --git a/java/src/com/android/intentresolver/ui/ChooserViewModel.kt b/java/src/com/android/intentresolver/ui/ChooserViewModel.kt deleted file mode 100644 index 817f0b6c..00000000 --- a/java/src/com/android/intentresolver/ui/ChooserViewModel.kt +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright (C) 2023 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.intentresolver.ui - -import android.util.Log -import androidx.lifecycle.ViewModel -import com.android.intentresolver.dagger.qualifiers.ViewModel as ViewModelQualifier -import javax.inject.Inject -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.awaitCancellation -import kotlinx.coroutines.launch - -const val TAG = "ChooserViewModel" - -/** The primary container for ViewModelScope dependencies. */ -class ChooserViewModel -@Inject -constructor( - @ViewModelQualifier val viewModelScope: CoroutineScope, -) : ViewModel() \ No newline at end of file diff --git a/java/tests/Android.bp b/java/tests/Android.bp index 3936b38e..90c7fb7a 100644 --- a/java/tests/Android.bp +++ b/java/tests/Android.bp @@ -20,6 +20,7 @@ android_test { static_libs: [ "IntentResolver-core", "androidx.test.core", + "androidx.test.rules", "androidx.test.ext.junit", "androidx.test.ext.truth", "androidx.test.espresso.contrib", diff --git a/java/tests/AndroidManifest.xml b/java/tests/AndroidManifest.xml index 9f8dd41c..05830c4c 100644 --- a/java/tests/AndroidManifest.xml +++ b/java/tests/AndroidManifest.xml @@ -15,8 +15,7 @@ --> + package="com.android.intentresolver.tests"> @@ -26,10 +25,7 @@ - + diff --git a/java/tests/src/com/android/intentresolver/ChooserWrapperActivity.java b/java/tests/src/com/android/intentresolver/ChooserWrapperActivity.java index 49305a6c..8608cf72 100644 --- a/java/tests/src/com/android/intentresolver/ChooserWrapperActivity.java +++ b/java/tests/src/com/android/intentresolver/ChooserWrapperActivity.java @@ -37,7 +37,6 @@ import androidx.lifecycle.ViewModelProvider; import com.android.intentresolver.AbstractMultiProfilePagerAdapter.CrossProfileIntentsChecker; import com.android.intentresolver.chooser.DisplayResolveInfo; import com.android.intentresolver.chooser.TargetInfo; -import com.android.intentresolver.dagger.TestViewModelComponent; import com.android.intentresolver.flags.FeatureFlagRepository; import com.android.intentresolver.grid.ChooserGridAdapter; import com.android.intentresolver.icons.TargetDataLoader; @@ -48,8 +47,6 @@ import com.android.internal.logging.nano.MetricsProto.MetricsEvent; import java.util.List; import java.util.function.Consumer; -import javax.inject.Inject; - /** * Simple wrapper around chooser activity to be able to initiate it under test. For more * information, see {@code com.android.internal.app.ChooserWrapperActivity}. @@ -59,11 +56,6 @@ public class ChooserWrapperActivity static final ChooserActivityOverrideData sOverrides = ChooserActivityOverrideData.getInstance(); private UsageStatsManager mUsm; - @Inject - public ChooserWrapperActivity(TestViewModelComponent.Builder builder) { - super(builder); - } - // ResolverActivity (the base class of ChooserActivity) inspects the launched-from UID at // onCreate and needs to see some non-negative value in the test. @Override diff --git a/java/tests/src/com/android/intentresolver/ResolverWrapperActivity.java b/java/tests/src/com/android/intentresolver/ResolverWrapperActivity.java index 11e7dffd..401ede26 100644 --- a/java/tests/src/com/android/intentresolver/ResolverWrapperActivity.java +++ b/java/tests/src/com/android/intentresolver/ResolverWrapperActivity.java @@ -44,8 +44,6 @@ import java.util.List; import java.util.function.Consumer; import java.util.function.Function; -import javax.inject.Inject; - /* * Simple wrapper around chooser activity to be able to initiate it under test */ @@ -55,7 +53,6 @@ public class ResolverWrapperActivity extends ResolverActivity { private final CountingIdlingResource mLabelIdlingResource = new CountingIdlingResource("LoadLabelTask"); - @Inject public ResolverWrapperActivity() { super(/* isIntentPicker= */ true); } diff --git a/java/tests/src/com/android/intentresolver/TestApplication.kt b/java/tests/src/com/android/intentresolver/TestApplication.kt index 4f5aefb9..849cfbab 100644 --- a/java/tests/src/com/android/intentresolver/TestApplication.kt +++ b/java/tests/src/com/android/intentresolver/TestApplication.kt @@ -16,13 +16,12 @@ package com.android.intentresolver +import android.app.Application import android.content.Context import android.os.UserHandle -import com.android.intentresolver.dagger.DaggerTestApplicationComponent -class TestApplication : IntentResolverApplication() { - override fun createApplicationComponentBuilder() = DaggerTestApplicationComponent.builder() +class TestApplication : Application() { // return the current context as a work profile doesn't really exist in these tests override fun createContextAsUser(user: UserHandle, flags: Int): Context = this -} +} \ No newline at end of file diff --git a/java/tests/src/com/android/intentresolver/dagger/TestActivityBinderModule.kt b/java/tests/src/com/android/intentresolver/dagger/TestActivityBinderModule.kt deleted file mode 100644 index c08bc3b2..00000000 --- a/java/tests/src/com/android/intentresolver/dagger/TestActivityBinderModule.kt +++ /dev/null @@ -1,40 +0,0 @@ -/* - * Copyright (C) 2023 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.intentresolver.dagger - -import android.app.Activity -import com.android.intentresolver.ChooserWrapperActivity -import com.android.intentresolver.ResolverWrapperActivity -import dagger.Binds -import dagger.Module -import dagger.multibindings.ClassKey -import dagger.multibindings.IntoMap - -@Module -interface TestActivityBinderModule { - @Binds - @IntoMap - @ClassKey(ResolverWrapperActivity::class) - @ActivityScope - fun resolverWrapperActivity(activity: ResolverWrapperActivity): Activity - - @Binds - @IntoMap - @ClassKey(ChooserWrapperActivity::class) - @ActivityScope - fun chooserWrapperActivity(activity: ChooserWrapperActivity): Activity -} diff --git a/java/tests/src/com/android/intentresolver/dagger/TestActivityComponent.kt b/java/tests/src/com/android/intentresolver/dagger/TestActivityComponent.kt deleted file mode 100644 index 4416c852..00000000 --- a/java/tests/src/com/android/intentresolver/dagger/TestActivityComponent.kt +++ /dev/null @@ -1,30 +0,0 @@ -/* - * Copyright (C) 2023 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.intentresolver.dagger - -import dagger.Subcomponent - -@ActivityScope -@Subcomponent( - modules = [ActivityModule::class, ActivityBinderModule::class, TestActivityBinderModule::class] -) -interface TestActivityComponent : ActivityComponent { - @Subcomponent.Factory - interface Factory : ActivityComponent.Factory { - override fun create(): TestActivityComponent - } -} diff --git a/java/tests/src/com/android/intentresolver/dagger/TestApplicationComponent.kt b/java/tests/src/com/android/intentresolver/dagger/TestApplicationComponent.kt deleted file mode 100644 index 224c46c6..00000000 --- a/java/tests/src/com/android/intentresolver/dagger/TestApplicationComponent.kt +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright (C) 2023 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.intentresolver.dagger - -import android.app.Application -import dagger.BindsInstance -import dagger.Component -import javax.inject.Singleton - -@Singleton -@Component(modules = [TestApplicationModule::class]) -interface TestApplicationComponent : ApplicationComponent { - @Component.Builder - interface Builder : ApplicationComponent.Builder { - @BindsInstance - override fun application(application: Application): TestApplicationComponent.Builder - - override fun build(): TestApplicationComponent - } -} diff --git a/java/tests/src/com/android/intentresolver/dagger/TestApplicationModule.kt b/java/tests/src/com/android/intentresolver/dagger/TestApplicationModule.kt deleted file mode 100644 index 714748d2..00000000 --- a/java/tests/src/com/android/intentresolver/dagger/TestApplicationModule.kt +++ /dev/null @@ -1,38 +0,0 @@ -/* - * Copyright (C) 2023 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.intentresolver.dagger - -import dagger.Binds -import dagger.Module -import javax.inject.Singleton - -@Module( - subcomponents = [TestActivityComponent::class, TestViewModelComponent::class], - includes = [ReceiverBinderModule::class, CoroutinesModule::class] -) -interface TestApplicationModule : ApplicationModule { - - /** Required to support field injection of [InjectedAppComponentFactory] */ - @Binds - @Singleton - fun activityComponent(component: TestActivityComponent.Factory): ActivityComponent.Factory - - /** Required to support injection into Activity instances */ - @Binds - @Singleton - fun viewModelComponent(component: TestViewModelComponent.Builder): ViewModelComponent.Builder -} diff --git a/java/tests/src/com/android/intentresolver/dagger/TestViewModelComponent.kt b/java/tests/src/com/android/intentresolver/dagger/TestViewModelComponent.kt deleted file mode 100644 index 539b3f36..00000000 --- a/java/tests/src/com/android/intentresolver/dagger/TestViewModelComponent.kt +++ /dev/null @@ -1,29 +0,0 @@ -/* - * Copyright (C) 2023 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.intentresolver.dagger - -import dagger.Subcomponent - -/** A ViewModelComponent for tests which replaces ViewModelModule -> TestViewModelModule */ -@ViewModelScope -@Subcomponent(modules = [TestViewModelModule::class, ViewModelBinderModule::class]) -interface TestViewModelComponent : ViewModelComponent { - @Subcomponent.Builder - interface Builder : ViewModelComponent.Builder { - override fun build(): TestViewModelComponent - } -} diff --git a/java/tests/src/com/android/intentresolver/dagger/TestViewModelModule.kt b/java/tests/src/com/android/intentresolver/dagger/TestViewModelModule.kt deleted file mode 100644 index 28f4fa73..00000000 --- a/java/tests/src/com/android/intentresolver/dagger/TestViewModelModule.kt +++ /dev/null @@ -1,6 +0,0 @@ -package com.android.intentresolver.dagger - -import dagger.Module - -/** Provides bindings shared among components within [@ViewModelScope][ViewModelScope]. */ -@Module abstract class TestViewModelModule -- cgit v1.2.3-59-g8ed1b From a509ca0b4f1f4621394f7c951842f11bdfc7e3bf Mon Sep 17 00:00:00 2001 From: Andrey Epin Date: Thu, 14 Sep 2023 16:25:18 -0700 Subject: Make chooser full-width in phone-portrait orientation Max Chooser width for phones increased based on Pixel 6/7 pro dimensions and max configurable display density (System Settings -> Display -> Dispaly size and text). Fix: 297980420 Test: visual testing on a phone and tablet with various display settings Merged-In: I6111fd214075646832e667a84d2ea9ada7626536 Change-Id: I6111fd214075646832e667a84d2ea9ada7626536 (cherry picked from commit 369287f575566fda8d2173f3dc76c3388fea18a5) --- java/res/values-port/dimens.xml | 18 ++++++++++++++++++ java/res/values/dimens.xml | 2 +- .../intentresolver/grid/ChooserGridAdapter.java | 6 ++++-- 3 files changed, 23 insertions(+), 3 deletions(-) create mode 100644 java/res/values-port/dimens.xml (limited to 'java/src') diff --git a/java/res/values-port/dimens.xml b/java/res/values-port/dimens.xml new file mode 100644 index 00000000..100a7e17 --- /dev/null +++ b/java/res/values-port/dimens.xml @@ -0,0 +1,18 @@ + + + -1px + diff --git a/java/res/values/dimens.xml b/java/res/values/dimens.xml index 6590d70e..ae80815b 100644 --- a/java/res/values/dimens.xml +++ b/java/res/values/dimens.xml @@ -20,7 +20,7 @@ 28dp 2dp 200dp - 412dp + 450dp 28dp 14dp 25dp diff --git a/java/src/com/android/intentresolver/grid/ChooserGridAdapter.java b/java/src/com/android/intentresolver/grid/ChooserGridAdapter.java index 77ae20f5..fadea934 100644 --- a/java/src/com/android/intentresolver/grid/ChooserGridAdapter.java +++ b/java/src/com/android/intentresolver/grid/ChooserGridAdapter.java @@ -164,8 +164,10 @@ public final class ChooserGridAdapter extends RecyclerView.Adapter= 0) { + width = Math.min(mChooserWidthPixels, width); + } int newWidth = width / mMaxTargetsPerRow; if (newWidth != mChooserTargetWidth) { -- cgit v1.2.3-59-g8ed1b