Commit 4a6ccde5 authored by Shimi Zhang's avatar Shimi Zhang Committed by Chromium LUCI CQ

Android: Fix StrictMode violation on Smart Selection icon loading

We are calling Icon#loadDrawable on the UI thread previously, based on
the Android doc, we should use Icon#loadDrawableAsync instead.

Previously, this is not enforced, but now we encountered a StrictMode
violation for this.

Since now we are calling Icon#loadDrawableAsync, we need to pre-warm the
Drawable before we show the popup menu, otherwise the icon will be set
after the popup menu showed for the first time.

Bug: 1129211
Change-Id: Iac5127a307d1d2d124a4b2debb7b021a2be2da75
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2562059Reviewed-by: default avatarBo <boliu@chromium.org>
Reviewed-by: default avatarRichard Coles <torne@chromium.org>
Commit-Queue: Shimi Zhang <ctzsm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#831948}
parent 1370fa55
...@@ -5,13 +5,19 @@ ...@@ -5,13 +5,19 @@
package org.chromium.base.compat; package org.chromium.base.compat;
import android.annotation.TargetApi; import android.annotation.TargetApi;
import android.app.RemoteAction;
import android.content.pm.PackageInfo; import android.content.pm.PackageInfo;
import android.location.LocationManager; import android.location.LocationManager;
import android.net.LinkProperties; import android.net.LinkProperties;
import android.os.Build; import android.os.Build;
import android.view.textclassifier.TextClassification;
import androidx.annotation.NonNull;
import org.chromium.base.annotations.VerifiesOnP; import org.chromium.base.annotations.VerifiesOnP;
import java.util.List;
/** /**
* Utility class to use new APIs that were added in P (API level 28). These need to exist in a * Utility class to use new APIs that were added in P (API level 28). These need to exist in a
* separate class so that Android framework can successfully verify classes without * separate class so that Android framework can successfully verify classes without
...@@ -41,4 +47,9 @@ public final class ApiHelperForP { ...@@ -41,4 +47,9 @@ public final class ApiHelperForP {
public static boolean isLocationEnabled(LocationManager locationManager) { public static boolean isLocationEnabled(LocationManager locationManager) {
return locationManager.isLocationEnabled(); return locationManager.isLocationEnabled();
} }
/** See {@link TextClassification#getActions() } */
public static @NonNull List<RemoteAction> getActions(TextClassification classification) {
return classification.getActions();
}
} }
...@@ -9,6 +9,7 @@ import android.app.PendingIntent; ...@@ -9,6 +9,7 @@ import android.app.PendingIntent;
import android.app.RemoteAction; import android.app.RemoteAction;
import android.content.Context; import android.content.Context;
import android.os.Build; import android.os.Build;
import android.os.Handler;
import android.text.TextUtils; import android.text.TextUtils;
import android.view.Menu; import android.view.Menu;
import android.view.MenuItem; import android.view.MenuItem;
...@@ -53,7 +54,7 @@ public class AdditionalMenuItemProviderImpl implements AdditionalMenuItemProvide ...@@ -53,7 +54,7 @@ public class AdditionalMenuItemProviderImpl implements AdditionalMenuItemProvide
MenuItem item = menu.findItem(android.R.id.textAssist); MenuItem item = menu.findItem(android.R.id.textAssist);
if (primaryAction.shouldShowIcon()) { if (primaryAction.shouldShowIcon()) {
item.setIcon(primaryAction.getIcon().loadDrawable(context)); setItemIconFromRemoteAction(context, primaryAction, item);
} else { } else {
item.setIcon(null); item.setIcon(null);
} }
...@@ -72,7 +73,7 @@ public class AdditionalMenuItemProviderImpl implements AdditionalMenuItemProvide ...@@ -72,7 +73,7 @@ public class AdditionalMenuItemProviderImpl implements AdditionalMenuItemProvide
MENU_ITEM_ORDER_SECONDARY_ASSIST_ACTIONS_START + i, action.getTitle()); MENU_ITEM_ORDER_SECONDARY_ASSIST_ACTIONS_START + i, action.getTitle());
item.setContentDescription(action.getContentDescription()); item.setContentDescription(action.getContentDescription());
if (action.shouldShowIcon()) { if (action.shouldShowIcon()) {
item.setIcon(action.getIcon().loadDrawable(context)); setItemIconFromRemoteAction(context, action, item);
} }
// Set this flag to SHOW_AS_ACTION_IF_ROOM to match text processing menu items. So // Set this flag to SHOW_AS_ACTION_IF_ROOM to match text processing menu items. So
// Android could put them to the same level and then consider their actual order. // Android could put them to the same level and then consider their actual order.
...@@ -105,4 +106,9 @@ public class AdditionalMenuItemProviderImpl implements AdditionalMenuItemProvide ...@@ -105,4 +106,9 @@ public class AdditionalMenuItemProviderImpl implements AdditionalMenuItemProvide
} }
}; };
} }
private static void setItemIconFromRemoteAction(
Context context, RemoteAction action, MenuItem item) {
action.getIcon().loadDrawableAsync(context, item::setIcon, new Handler());
}
} }
...@@ -6,6 +6,7 @@ package org.chromium.content.browser.selection; ...@@ -6,6 +6,7 @@ package org.chromium.content.browser.selection;
import android.annotation.SuppressLint; import android.annotation.SuppressLint;
import android.annotation.TargetApi; import android.annotation.TargetApi;
import android.app.RemoteAction;
import android.content.Context; import android.content.Context;
import android.os.Build; import android.os.Build;
import android.os.Handler; import android.os.Handler;
...@@ -17,6 +18,7 @@ import android.view.textclassifier.TextSelection; ...@@ -17,6 +18,7 @@ import android.view.textclassifier.TextSelection;
import androidx.annotation.IntDef; import androidx.annotation.IntDef;
import org.chromium.base.compat.ApiHelperForP;
import org.chromium.base.task.AsyncTask; import org.chromium.base.task.AsyncTask;
import org.chromium.content.browser.WindowEventObserver; import org.chromium.content.browser.WindowEventObserver;
import org.chromium.content.browser.WindowEventObserverManager; import org.chromium.content.browser.WindowEventObserverManager;
...@@ -26,6 +28,7 @@ import org.chromium.ui.base.WindowAndroid; ...@@ -26,6 +28,7 @@ import org.chromium.ui.base.WindowAndroid;
import java.lang.annotation.Retention; import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy; import java.lang.annotation.RetentionPolicy;
import java.util.List;
/** /**
* Controls Smart Text selection. Talks to the Android TextClassificationManager API. * Controls Smart Text selection. Talks to the Android TextClassificationManager API.
...@@ -189,7 +192,27 @@ public class SmartSelectionProvider { ...@@ -189,7 +192,27 @@ public class SmartSelectionProvider {
@Override @Override
protected void onPostExecute(SelectionClient.Result result) { protected void onPostExecute(SelectionClient.Result result) {
mResultCallback.onClassified(result); if (Build.VERSION.SDK_INT < Build.VERSION_CODES.P) {
mResultCallback.onClassified(result);
return;
}
Context context = mWindowAndroid.getContext().get();
if (context == null || result.textClassification == null) {
mResultCallback.onClassified(result);
return;
}
List<RemoteAction> actions = ApiHelperForP.getActions(result.textClassification);
if (actions == null || actions.size() == 0) {
mResultCallback.onClassified(result);
return;
}
RemoteAction primaryAction = actions.get(0);
// Wait until the drawable for the primary action is loaded.
primaryAction.getIcon().loadDrawableAsync(
context, (drawable) -> mResultCallback.onClassified(result), new Handler());
} }
} }
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment