From 904a931cfc5f2ffd6fd0c0fb03718abca37b5ee5 Mon Sep 17 00:00:00 2001 From: Abodunrinwa Toki Date: Wed, 18 Apr 2018 21:21:27 +0100 Subject: [PATCH] Fix non-unique PendingIntent issue with TCImpl. As per the referenced bug, we're running into issues where apps are being fired with stale intents. The reason is because we need intents we fire to be unique by Intent.filterEquals. Some of the intents we generate put unique data in the intent extra which is not considered by filterEquals. The solution here is to create PendingIntents with unique request codes (using classifiedText.hashCode()). See more info about this in https://developer.android.com/reference/android/app/PendingIntent.html Bug: 77930684 Test: manually tested broken scenarios. See referenced bug Test: bit FrameworksCoreTests:android.view.textclassifier.TextClassificationManagerTest Test: bit CtsViewTestCases:android.view.textclassifier.cts.TextClassificationManagerTest Test: bit FrameworksCoreTests:android.view.textclassifier.TextClassificationTest Test: bit CtsWidgetTestCases:android.widget.cts.TextViewTest Test: bit FrameworksCoreTests:android.widget.TextViewActivityTest Change-Id: Ib7275f94ca5ada51e4ba191742d4b614df12e1ea --- .../textclassifier/TextClassification.java | 6 +- .../textclassifier/TextClassifierImpl.java | 66 ++++++++++++++----- core/java/android/widget/Editor.java | 15 ++++- 3 files changed, 64 insertions(+), 23 deletions(-) diff --git a/core/java/android/view/textclassifier/TextClassification.java b/core/java/android/view/textclassifier/TextClassification.java index 96016b44c0819..ad50dc0272790 100644 --- a/core/java/android/view/textclassifier/TextClassification.java +++ b/core/java/android/view/textclassifier/TextClassification.java @@ -277,12 +277,12 @@ public final class TextClassification implements Parcelable { */ @Nullable public static PendingIntent createPendingIntent( - @NonNull final Context context, @NonNull final Intent intent) { + @NonNull final Context context, @NonNull final Intent intent, int requestCode) { switch (getIntentType(intent, context)) { case IntentType.ACTIVITY: - return PendingIntent.getActivity(context, 0, intent, 0); + return PendingIntent.getActivity(context, requestCode, intent, 0); case IntentType.SERVICE: - return PendingIntent.getService(context, 0, intent, 0); + return PendingIntent.getService(context, requestCode, intent, 0); default: return null; } diff --git a/core/java/android/view/textclassifier/TextClassifierImpl.java b/core/java/android/view/textclassifier/TextClassifierImpl.java index 2213355821d43..910fcaace159f 100644 --- a/core/java/android/view/textclassifier/TextClassifierImpl.java +++ b/core/java/android/view/textclassifier/TextClassifierImpl.java @@ -412,7 +412,7 @@ public final class TextClassifierImpl implements TextClassifier { boolean isPrimaryAction = true; for (LabeledIntent labeledIntent : IntentFactory.create( mContext, referenceTime, highestScoringResult, classifiedText)) { - RemoteAction action = labeledIntent.asRemoteAction(mContext); + final RemoteAction action = labeledIntent.asRemoteAction(mContext); if (isPrimaryAction) { // For O backwards compatibility, the first RemoteAction is also written to the // legacy API fields. @@ -421,7 +421,7 @@ public final class TextClassifierImpl implements TextClassifier { builder.setIntent(labeledIntent.getIntent()); builder.setOnClickListener(TextClassification.createIntentOnClickListener( TextClassification.createPendingIntent(mContext, - labeledIntent.getIntent()))); + labeledIntent.getIntent(), labeledIntent.getRequestCode()))); isPrimaryAction = false; } builder.addAction(action); @@ -559,14 +559,30 @@ public final class TextClassifierImpl implements TextClassifier { * Helper class to store the information from which RemoteActions are built. */ private static final class LabeledIntent { - private String mTitle; - private String mDescription; - private Intent mIntent; - LabeledIntent(String title, String description, Intent intent) { + static final int DEFAULT_REQUEST_CODE = 0; + + private final String mTitle; + private final String mDescription; + private final Intent mIntent; + private final int mRequestCode; + + /** + * Initializes a LabeledIntent. + * + *

NOTE: {@code reqestCode} is required to not be {@link #DEFAULT_REQUEST_CODE} + * if distinguishing info (e.g. the classified text) is represented in intent extras only. + * In such circumstances, the request code should represent the distinguishing info + * (e.g. by generating a hashcode) so that the generated PendingIntent is (somewhat) + * unique. To be correct, the PendingIntent should be definitely unique but we try a + * best effort approach that avoids spamming the system with PendingIntents. + */ + // TODO: Fix the issue mentioned above so the behaviour is correct. + LabeledIntent(String title, String description, Intent intent, int requestCode) { mTitle = title; mDescription = description; mIntent = intent; + mRequestCode = requestCode; } String getTitle() { @@ -581,6 +597,10 @@ public final class TextClassifierImpl implements TextClassifier { return mIntent; } + int getRequestCode() { + return mRequestCode; + } + RemoteAction asRemoteAction(Context context) { final PackageManager pm = context.getPackageManager(); final ResolveInfo resolveInfo = pm.resolveActivity(mIntent, 0); @@ -602,8 +622,8 @@ public final class TextClassifierImpl implements TextClassifier { icon = Icon.createWithResource("android", com.android.internal.R.drawable.ic_more_items); } - RemoteAction action = new RemoteAction(icon, mTitle, mDescription, - TextClassification.createPendingIntent(context, mIntent)); + final RemoteAction action = new RemoteAction(icon, mTitle, mDescription, + TextClassification.createPendingIntent(context, mIntent, mRequestCode)); action.setShouldShowIcon(shouldShowIcon); return action; } @@ -659,13 +679,15 @@ public final class TextClassifierImpl implements TextClassifier { context.getString(com.android.internal.R.string.email), context.getString(com.android.internal.R.string.email_desc), new Intent(Intent.ACTION_SENDTO) - .setData(Uri.parse(String.format("mailto:%s", text)))), + .setData(Uri.parse(String.format("mailto:%s", text))), + LabeledIntent.DEFAULT_REQUEST_CODE), new LabeledIntent( context.getString(com.android.internal.R.string.add_contact), context.getString(com.android.internal.R.string.add_contact_desc), new Intent(Intent.ACTION_INSERT_OR_EDIT) .setType(ContactsContract.Contacts.CONTENT_ITEM_TYPE) - .putExtra(ContactsContract.Intents.Insert.EMAIL, text))); + .putExtra(ContactsContract.Intents.Insert.EMAIL, text), + text.hashCode())); } @NonNull @@ -679,20 +701,23 @@ public final class TextClassifierImpl implements TextClassifier { context.getString(com.android.internal.R.string.dial), context.getString(com.android.internal.R.string.dial_desc), new Intent(Intent.ACTION_DIAL).setData( - Uri.parse(String.format("tel:%s", text))))); + Uri.parse(String.format("tel:%s", text))), + LabeledIntent.DEFAULT_REQUEST_CODE)); } actions.add(new LabeledIntent( context.getString(com.android.internal.R.string.add_contact), context.getString(com.android.internal.R.string.add_contact_desc), new Intent(Intent.ACTION_INSERT_OR_EDIT) .setType(ContactsContract.Contacts.CONTENT_ITEM_TYPE) - .putExtra(ContactsContract.Intents.Insert.PHONE, text))); + .putExtra(ContactsContract.Intents.Insert.PHONE, text), + text.hashCode())); if (!userRestrictions.getBoolean(UserManager.DISALLOW_SMS, false)) { actions.add(new LabeledIntent( context.getString(com.android.internal.R.string.sms), context.getString(com.android.internal.R.string.sms_desc), new Intent(Intent.ACTION_SENDTO) - .setData(Uri.parse(String.format("smsto:%s", text))))); + .setData(Uri.parse(String.format("smsto:%s", text))), + LabeledIntent.DEFAULT_REQUEST_CODE)); } return actions; } @@ -706,7 +731,8 @@ public final class TextClassifierImpl implements TextClassifier { context.getString(com.android.internal.R.string.map), context.getString(com.android.internal.R.string.map_desc), new Intent(Intent.ACTION_VIEW) - .setData(Uri.parse(String.format("geo:0,0?q=%s", encText))))); + .setData(Uri.parse(String.format("geo:0,0?q=%s", encText))), + LabeledIntent.DEFAULT_REQUEST_CODE)); } catch (UnsupportedEncodingException e) { Log.e(LOG_TAG, "Could not encode address", e); } @@ -728,7 +754,8 @@ public final class TextClassifierImpl implements TextClassifier { context.getString(com.android.internal.R.string.browse), context.getString(com.android.internal.R.string.browse_desc), new Intent(Intent.ACTION_VIEW, Uri.parse(text)) - .putExtra(Browser.EXTRA_APPLICATION_ID, context.getPackageName()))); + .putExtra(Browser.EXTRA_APPLICATION_ID, context.getPackageName()), + LabeledIntent.DEFAULT_REQUEST_CODE)); } @NonNull @@ -754,7 +781,8 @@ public final class TextClassifierImpl implements TextClassifier { context.getString(com.android.internal.R.string.view_flight), context.getString(com.android.internal.R.string.view_flight_desc), new Intent(Intent.ACTION_WEB_SEARCH) - .putExtra(SearchManager.QUERY, text))); + .putExtra(SearchManager.QUERY, text), + text.hashCode())); } @NonNull @@ -765,7 +793,8 @@ public final class TextClassifierImpl implements TextClassifier { return new LabeledIntent( context.getString(com.android.internal.R.string.view_calendar), context.getString(com.android.internal.R.string.view_calendar_desc), - new Intent(Intent.ACTION_VIEW).setData(builder.build())); + new Intent(Intent.ACTION_VIEW).setData(builder.build()), + LabeledIntent.DEFAULT_REQUEST_CODE); } @NonNull @@ -781,7 +810,8 @@ public final class TextClassifierImpl implements TextClassifier { .putExtra(CalendarContract.EXTRA_EVENT_BEGIN_TIME, parsedTime.toEpochMilli()) .putExtra(CalendarContract.EXTRA_EVENT_END_TIME, - parsedTime.toEpochMilli() + DEFAULT_EVENT_DURATION)); + parsedTime.toEpochMilli() + DEFAULT_EVENT_DURATION), + parsedTime.hashCode()); } } } diff --git a/core/java/android/widget/Editor.java b/core/java/android/widget/Editor.java index 6af678b1053d1..80b6f909f89fd 100644 --- a/core/java/android/widget/Editor.java +++ b/core/java/android/widget/Editor.java @@ -4062,7 +4062,8 @@ public class Editor { item.setShowAsAction(MenuItem.SHOW_AS_ACTION_ALWAYS); mAssistClickHandlers.put(item, TextClassification.createIntentOnClickListener( TextClassification.createPendingIntent(mTextView.getContext(), - textClassification.getIntent()))); + textClassification.getIntent(), + createAssistMenuItemPendingIntentRequestCode()))); } final int count = textClassification.getActions().size(); for (int i = 1; i < count; i++) { @@ -4120,7 +4121,9 @@ public class Editor { final Intent intent = assistMenuItem.getIntent(); if (intent != null) { onClickListener = TextClassification.createIntentOnClickListener( - TextClassification.createPendingIntent(mTextView.getContext(), intent)); + TextClassification.createPendingIntent( + mTextView.getContext(), intent, + createAssistMenuItemPendingIntentRequestCode())); } } if (onClickListener != null) { @@ -4131,6 +4134,14 @@ public class Editor { return true; } + private int createAssistMenuItemPendingIntentRequestCode() { + return mTextView.hasSelection() + ? mTextView.getText().subSequence( + mTextView.getSelectionStart(), mTextView.getSelectionEnd()) + .hashCode() + : 0; + } + private boolean shouldEnableAssistMenuItems() { return mTextView.isDeviceProvisioned() && TextClassificationManager.getSettings(mTextView.getContext())