Commit 61f46b7d authored by Lijin Shen's avatar Lijin Shen Committed by Commit Bot

Exclude actions from other browsers in PROCESS_TEXT handling

This CL excludes actions that are provided by other browsers and system
launchers, because it is reported as a really odd experience.

Bug: 850195
Change-Id: I70bc4482a03eb57e28ac46da6130b575ff532cdd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2082995Reviewed-by: default avatarYaron Friedman <yfriedman@chromium.org>
Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Commit-Queue: Lijin Shen <lazzzis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748900}
parent fa8f73f7
...@@ -7,6 +7,7 @@ package org.chromium.base; ...@@ -7,6 +7,7 @@ package org.chromium.base;
import android.content.Intent; import android.content.Intent;
import android.content.pm.PackageManager; import android.content.pm.PackageManager;
import android.content.pm.ResolveInfo; import android.content.pm.ResolveInfo;
import android.net.Uri;
import android.os.TransactionTooLargeException; import android.os.TransactionTooLargeException;
import java.util.Collections; import java.util.Collections;
...@@ -17,6 +18,7 @@ import java.util.List; ...@@ -17,6 +18,7 @@ import java.util.List;
*/ */
public class PackageManagerUtils { public class PackageManagerUtils {
private static final String TAG = "PackageManagerUtils"; private static final String TAG = "PackageManagerUtils";
private static final String SAMPLE_URL = "http://";
/** /**
* Retrieve information about the Activity that will handle the given Intent. * Retrieve information about the Activity that will handle the given Intent.
...@@ -58,6 +60,44 @@ public class PackageManagerUtils { ...@@ -58,6 +60,44 @@ public class PackageManagerUtils {
return Collections.emptyList(); return Collections.emptyList();
} }
/**
* @return Intent to query a list of installed web browsers.
*/
public static Intent getQueryInstalledBrowsersIntent() {
return new Intent(Intent.ACTION_VIEW, Uri.parse(SAMPLE_URL))
.addCategory(Intent.CATEGORY_BROWSABLE);
}
/**
* @return Intent to query a list of installed home launchers.
*/
public static Intent getQueryInstalledHomeLaunchersIntent() {
return new Intent(Intent.ACTION_MAIN).addCategory(Intent.CATEGORY_HOME);
}
/**
* @return Default ResolveInfo to handle a VIEW intent for a url.
*/
public static ResolveInfo resolveDefaultWebBrowserActivity() {
return resolveActivity(getQueryInstalledBrowsersIntent(), 0);
}
/**
* @return The list of names of web browser applications available in the system. A browser
* may appear twice if it has multiple intent handlers.
*/
public static List<ResolveInfo> queryAllWebBrowsersInfo() {
return queryIntentActivities(getQueryInstalledBrowsersIntent(), PackageManager.MATCH_ALL);
}
/**
* @return The list of names of system launcher applications available in the system.
*/
public static List<ResolveInfo> queryAllLaunchersInfo() {
return queryIntentActivities(
getQueryInstalledHomeLaunchersIntent(), PackageManager.MATCH_ALL);
}
// See https://crbug.com/700505 and https://crbug.com/369574. // See https://crbug.com/700505 and https://crbug.com/369574.
private static void handleExpectedExceptionsOrRethrow(RuntimeException e, Intent intent) { private static void handleExpectedExceptionsOrRethrow(RuntimeException e, Intent intent) {
if (e instanceof NullPointerException if (e instanceof NullPointerException
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
package org.chromium.chrome.browser; package org.chromium.chrome.browser;
import android.content.pm.ResolveInfo;
import android.text.TextUtils; import android.text.TextUtils;
import android.view.ActionMode; import android.view.ActionMode;
import android.view.Menu; import android.view.Menu;
...@@ -12,6 +13,8 @@ import android.view.MenuItem; ...@@ -12,6 +13,8 @@ import android.view.MenuItem;
import androidx.annotation.VisibleForTesting; import androidx.annotation.VisibleForTesting;
import org.chromium.base.Callback; import org.chromium.base.Callback;
import org.chromium.base.CollectionUtil;
import org.chromium.base.PackageManagerUtils;
import org.chromium.base.metrics.RecordUserAction; import org.chromium.base.metrics.RecordUserAction;
import org.chromium.chrome.browser.feature_engagement.TrackerFactory; import org.chromium.chrome.browser.feature_engagement.TrackerFactory;
import org.chromium.chrome.browser.firstrun.FirstRunStatus; import org.chromium.chrome.browser.firstrun.FirstRunStatus;
...@@ -30,6 +33,10 @@ import org.chromium.content_public.browser.SelectionPopupController; ...@@ -30,6 +33,10 @@ import org.chromium.content_public.browser.SelectionPopupController;
import org.chromium.content_public.browser.WebContents; import org.chromium.content_public.browser.WebContents;
import org.chromium.ui.base.PageTransition; import org.chromium.ui.base.PageTransition;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
/** /**
* A class that handles selection action mode for an associated {@link Tab}. * A class that handles selection action mode for an associated {@link Tab}.
*/ */
...@@ -67,7 +74,22 @@ public class ChromeActionModeCallback implements ActionMode.Callback { ...@@ -67,7 +74,22 @@ public class ChromeActionModeCallback implements ActionMode.Callback {
@Override @Override
public boolean onPrepareActionMode(ActionMode mode, Menu menu) { public boolean onPrepareActionMode(ActionMode mode, Menu menu) {
notifyContextualActionBarVisibilityChanged(true); notifyContextualActionBarVisibilityChanged(true);
return mHelper.onPrepareActionMode(mode, menu); boolean res = mHelper.onPrepareActionMode(mode, menu);
Set<String> browsers = getPackageNames(PackageManagerUtils.queryAllWebBrowsersInfo());
Set<String> launchers = getPackageNames(PackageManagerUtils.queryAllLaunchersInfo());
for (int i = 0; i < menu.size(); i++) {
MenuItem item = menu.getItem(i);
if (item.getGroupId() != R.id.select_action_menu_text_processing_menus
|| item.getIntent() == null || item.getIntent().getComponent() == null) {
continue;
}
String packageName = item.getIntent().getComponent().getPackageName();
// Exclude actions from browsers and system launchers. https://crbug.com/850195
if (browsers.contains(packageName) || launchers.contains(packageName)) {
item.setVisible(false);
}
}
return res;
} }
@Override @Override
...@@ -100,6 +122,12 @@ public class ChromeActionModeCallback implements ActionMode.Callback { ...@@ -100,6 +122,12 @@ public class ChromeActionModeCallback implements ActionMode.Callback {
} }
} }
private Set<String> getPackageNames(List<ResolveInfo> list) {
Set<String> set = new HashSet<>();
CollectionUtil.forEach(list, (info) -> set.add(info.activityInfo.packageName));
return set;
}
/** /**
* Generate the LoadUrlParams necessary to load the specified search query. * Generate the LoadUrlParams necessary to load the specified search query.
*/ */
......
...@@ -5,11 +5,9 @@ ...@@ -5,11 +5,9 @@
package org.chromium.chrome.browser; package org.chromium.chrome.browser;
import android.content.Context; import android.content.Context;
import android.content.Intent;
import android.content.pm.ApplicationInfo; import android.content.pm.ApplicationInfo;
import android.content.pm.PackageManager; import android.content.pm.PackageManager;
import android.content.pm.ResolveInfo; import android.content.pm.ResolveInfo;
import android.net.Uri;
import android.text.TextUtils; import android.text.TextUtils;
import androidx.annotation.IntDef; import androidx.annotation.IntDef;
...@@ -38,8 +36,6 @@ import java.util.concurrent.RejectedExecutionException; ...@@ -38,8 +36,6 @@ import java.util.concurrent.RejectedExecutionException;
* A utility class for querying information about the default browser setting. * A utility class for querying information about the default browser setting.
*/ */
public final class DefaultBrowserInfo { public final class DefaultBrowserInfo {
private static final String SAMPLE_URL = "https://www.madeupdomainforcheck123.chrome/";
/** /**
* A list of potential default browser states. To add a type to this list please update * A list of potential default browser states. To add a type to this list please update
* MobileDefaultBrowserState in histograms.xml and make sure to keep this list in sync. * MobileDefaultBrowserState in histograms.xml and make sure to keep this list in sync.
...@@ -97,7 +93,7 @@ public final class DefaultBrowserInfo { ...@@ -97,7 +93,7 @@ public final class DefaultBrowserInfo {
context, BuildInfo.getInstance().hostPackageLabel)); context, BuildInfo.getInstance().hostPackageLabel));
PackageManager pm = context.getPackageManager(); PackageManager pm = context.getPackageManager();
ResolveInfo info = getResolveInfoForViewIntent(); ResolveInfo info = PackageManagerUtils.resolveDefaultWebBrowserActivity();
// Caches whether Chrome is set as a default browser on the device. // Caches whether Chrome is set as a default browser on the device.
boolean isDefault = info != null && info.match != 0 boolean isDefault = info != null && info.match != 0
...@@ -127,19 +123,6 @@ public final class DefaultBrowserInfo { ...@@ -127,19 +123,6 @@ public final class DefaultBrowserInfo {
: context.getString(R.string.menu_open_in_product, packageLabel); : context.getString(R.string.menu_open_in_product, packageLabel);
} }
/**
* @return Default ResolveInfo to handle a VIEW intent for a url.
*/
private static ResolveInfo getResolveInfoForViewIntent() {
Intent intent = new Intent(Intent.ACTION_VIEW, Uri.parse(SAMPLE_URL));
return PackageManagerUtils.resolveActivity(intent, 0);
}
private static List<ResolveInfo> getResolveInfoListForViewIntent() {
Intent intent = new Intent(Intent.ACTION_VIEW, Uri.parse(SAMPLE_URL));
return PackageManagerUtils.queryIntentActivities(intent, PackageManager.MATCH_ALL);
}
/** /**
* @return Title of the menu item for opening a link in the default browser. * @return Title of the menu item for opening a link in the default browser.
* @param forceChromeAsDefault Whether the Custom Tab is created by Chrome. * @param forceChromeAsDefault Whether the Custom Tab is created by Chrome.
...@@ -174,7 +157,7 @@ public final class DefaultBrowserInfo { ...@@ -174,7 +157,7 @@ public final class DefaultBrowserInfo {
DefaultInfo info = new DefaultInfo(); DefaultInfo info = new DefaultInfo();
// Query the default handler first. // Query the default handler first.
ResolveInfo defaultRi = getResolveInfoForViewIntent(); ResolveInfo defaultRi = PackageManagerUtils.resolveDefaultWebBrowserActivity();
if (defaultRi != null && defaultRi.match != 0) { if (defaultRi != null && defaultRi.match != 0) {
info.hasDefault = true; info.hasDefault = true;
info.isChromeDefault = isSamePackage(context, defaultRi); info.isChromeDefault = isSamePackage(context, defaultRi);
...@@ -183,7 +166,7 @@ public final class DefaultBrowserInfo { ...@@ -183,7 +166,7 @@ public final class DefaultBrowserInfo {
// Query all other intent handlers. // Query all other intent handlers.
Set<String> uniquePackages = new HashSet<>(); Set<String> uniquePackages = new HashSet<>();
List<ResolveInfo> ris = getResolveInfoListForViewIntent(); List<ResolveInfo> ris = PackageManagerUtils.queryAllWebBrowsersInfo();
if (ris != null) { if (ris != null) {
for (ResolveInfo ri : ris) { for (ResolveInfo ri : ris) {
String packageName = ri.activityInfo.applicationInfo.packageName; String packageName = ri.activityInfo.applicationInfo.packageName;
......
...@@ -5,20 +5,29 @@ ...@@ -5,20 +5,29 @@
package org.chromium.chrome.browser; package org.chromium.chrome.browser;
import android.app.Activity; import android.app.Activity;
import android.content.Intent;
import android.content.pm.ActivityInfo;
import android.content.pm.ResolveInfo;
import android.view.ActionMode; import android.view.ActionMode;
import android.view.Menu; import android.view.Menu;
import android.view.MenuItem; import android.view.MenuItem;
import org.junit.After; import org.junit.After;
import org.junit.Assert;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.mockito.Mock; import org.mockito.Mock;
import org.mockito.Mockito; import org.mockito.Mockito;
import org.mockito.MockitoAnnotations; import org.mockito.MockitoAnnotations;
import org.robolectric.RuntimeEnvironment;
import org.robolectric.Shadows;
import org.robolectric.annotation.Config; import org.robolectric.annotation.Config;
import org.robolectric.fakes.RoboMenu;
import org.robolectric.shadows.ShadowPackageManager;
import org.chromium.base.Callback; import org.chromium.base.Callback;
import org.chromium.base.PackageManagerUtils;
import org.chromium.base.metrics.RecordUserAction; import org.chromium.base.metrics.RecordUserAction;
import org.chromium.base.test.BaseRobolectricTestRunner; import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.chrome.browser.firstrun.FirstRunStatus; import org.chromium.chrome.browser.firstrun.FirstRunStatus;
...@@ -29,6 +38,12 @@ import org.chromium.content.R; ...@@ -29,6 +38,12 @@ import org.chromium.content.R;
import org.chromium.content_public.browser.ActionModeCallbackHelper; import org.chromium.content_public.browser.ActionModeCallbackHelper;
import org.chromium.content_public.browser.WebContents; import org.chromium.content_public.browser.WebContents;
import java.util.ArrayList;
import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
import java.util.Random;
/** /**
* Unit tests for the {@link ChromeActionModeCallback}. * Unit tests for the {@link ChromeActionModeCallback}.
*/ */
...@@ -117,4 +132,77 @@ public class ChromeActionModeCallbackTest { ...@@ -117,4 +132,77 @@ public class ChromeActionModeCallbackTest {
Mockito.verify(localeManager).showSearchEnginePromoIfNeeded(Mockito.any(), Mockito.any()); Mockito.verify(localeManager).showSearchEnginePromoIfNeeded(Mockito.any(), Mockito.any());
} }
@Test
public void testSelectActionMenuTextProcessingMenus() {
ShadowPackageManager packageManager =
Shadows.shadowOf(RuntimeEnvironment.application.getPackageManager());
List<String> browserPackageNames = new ArrayList<>();
List<String> launcherPackageNames = new ArrayList<>();
List<String> otherPackageNames = new ArrayList<>();
List<ResolveInfo> browsersList = new LinkedList<>();
List<ResolveInfo> launchersList = new LinkedList<>();
for (int i = 0; i < 5; i++) {
browserPackageNames.add("foo " + i);
browsersList.add(createResolveInfo(browserPackageNames.get(i)));
launcherPackageNames.add("bar " + i);
launchersList.add(createResolveInfo(launcherPackageNames.get(i)));
otherPackageNames.add("baz " + i);
}
// Mock intent for querying web browsers.
packageManager.addResolveInfoForIntent(
PackageManagerUtils.getQueryInstalledBrowsersIntent(), browsersList);
// Mock intent for querying home launchers.
packageManager.addResolveInfoForIntent(
PackageManagerUtils.getQueryInstalledHomeLaunchersIntent(), launchersList);
RoboMenu menu = new RoboMenu(RuntimeEnvironment.application);
List<String> allNames = new LinkedList<>();
allNames.addAll(browserPackageNames);
allNames.addAll(launcherPackageNames);
allNames.addAll(otherPackageNames);
// Shuffle the list to get it closer to the reality.
Collections.shuffle(allNames, new Random(42));
for (int i = 0; i < allNames.size(); i++) {
addMenuItem(menu, i, allNames.get(i));
}
mActionModeCallback.onPrepareActionMode(mActionMode, menu);
// Verify that some menu items have been made invisible.
for (int i = 0; i < menu.size(); i++) {
MenuItem item = menu.getItem(i);
if (item.getIntent() == null || item.getIntent().getComponent() == null) continue;
String packageName = item.getIntent().getComponent().getPackageName();
if (browserPackageNames.contains(packageName)
|| launcherPackageNames.contains(packageName)) {
Assert.assertFalse("Browser or home launcher application should be filtered out",
item.isVisible());
} else {
Assert.assertTrue(
"Actions other than browsers or home launchers should not be filtered out",
item.isVisible());
}
}
}
private ResolveInfo createResolveInfo(String packageName) {
ResolveInfo resolveInfo = new ResolveInfo();
ActivityInfo activityInfo = new ActivityInfo();
activityInfo.packageName = packageName;
resolveInfo.activityInfo = activityInfo;
return resolveInfo;
}
private void addMenuItem(Menu menu, int order, String packageName) {
menu.add(R.id.select_action_menu_text_processing_menus, Menu.NONE, order, "title")
.setIntent(new Intent()
.setAction(Intent.ACTION_PROCESS_TEXT)
.setType("text/plain")
.putExtra(Intent.EXTRA_PROCESS_TEXT_READONLY, true)
.setClassName(packageName, "foo"));
}
} }
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