Skip to content

Commit

Permalink
[Android] Componentize and dedupe getSpecializedHandlersWithFilter
Browse files Browse the repository at this point in the history 
This CL dedupes the //chrome and //weblayer implementations of
getSpecializedHandlersWithFilter() into a static function in
ExternalNavigationHandler.java. There is a slight difference between
the two impls: //chrome's does not consider the InstantApp launcher as
a specialized handler because it handles launching of Instant Apps
internally. WebLayer, however, does not handle launching of Instant Apps
itself and instead lets the system do so as appropriate. We handle this
by adding a parameter to the new //components-level version of the
function.

We also move the tests of this function to
ExternalNavigationHandlerTest.java, including testing the behavior in
response to the value of this new parameter.

A followup CL will dedupe and eliminate the ExternalNavigationDelegate
methods that can be implemented within ExternalNavigationHandler
following this change.

Bug: 1071390
Change-Id: I701c4aa8c46260de3357b3d52e536a237d8c7b60
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2184277
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#766447}
  • Loading branch information
@colinblundell
colinblundell authored and Commit Bot committed May 7, 2020
1 parent 3e11650 commit f5fb7c8
Show file tree
Hide file tree
Showing 5 changed files with 191 additions and 225 deletions.
38 changes: 5 additions & 33 deletions ...roid/java/src/org/chromium/chrome/browser/externalnav/ExternalNavigationDelegateImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
import android.text.TextUtils;
import android.view.WindowManager.BadTokenException;

import androidx.annotation.VisibleForTesting;

import org.chromium.base.ApplicationState;
import org.chromium.base.ApplicationStatus;
import org.chromium.base.ContextUtils;
Expand Down Expand Up @@ -123,40 +121,12 @@ public boolean shouldDisableExternalIntentRequestsForUrl(String url) {

@Override
public int countSpecializedHandlers(List<ResolveInfo> infos) {
return getSpecializedHandlersWithFilter(infos, null).size();
return ExternalNavigationHandler.getSpecializedHandlersWithFilter(infos, null, true).size();
}

@Override
public ArrayList<String> getSpecializedHandlers(List<ResolveInfo> infos) {
return getSpecializedHandlersWithFilter(infos, null);
}

@VisibleForTesting
public static ArrayList<String> getSpecializedHandlersWithFilter(
List<ResolveInfo> infos, String filterPackageName) {
ArrayList<String> result = new ArrayList<>();
if (infos == null) {
return result;
}

for (ResolveInfo info : infos) {
if (!ExternalNavigationHandler.matchResolveInfoExceptWildCardHost(
info, filterPackageName)) {
continue;
}

if (info.activityInfo != null) {
if (IntentUtils.isInstantAppResolveInfo(info)) {
// Don't consider the Instant Apps resolver a specialized application.
continue;
}

result.add(info.activityInfo.packageName);
} else {
result.add("");
}
}
return result;
return ExternalNavigationHandler.getSpecializedHandlersWithFilter(infos, null, true);
}

/**
Expand All @@ -170,7 +140,9 @@ public static ArrayList<String> getSpecializedHandlersWithFilter(
public static boolean isPackageSpecializedHandler(String packageName, Intent intent) {
List<ResolveInfo> handlers = PackageManagerUtils.queryIntentActivities(
intent, PackageManager.GET_RESOLVED_FILTER);
return !getSpecializedHandlersWithFilter(handlers, packageName).isEmpty();
return !ExternalNavigationHandler
.getSpecializedHandlersWithFilter(handlers, packageName, true)
.isEmpty();
}

@Override
Expand Down
145 changes: 0 additions & 145 deletions ...tests/src/org/chromium/chrome/browser/externalnav/ExternalNavigationDelegateImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,7 @@
package org.chromium.chrome.browser.externalnav;

import android.content.Intent;
import android.content.IntentFilter;
import android.content.pm.ActivityInfo;
import android.content.pm.ResolveInfo;
import android.net.Uri;
import android.os.Build;
import android.support.test.filters.SmallTest;

import org.junit.Assert;
Expand All @@ -18,7 +14,6 @@
import org.junit.Test;
import org.junit.runner.RunWith;

import org.chromium.base.IntentUtils;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.IntentHandler;
Expand All @@ -31,10 +26,6 @@
import org.chromium.components.external_intents.ExternalNavigationHandler;
import org.chromium.components.external_intents.ExternalNavigationParams;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

/**
* Instrumentation tests for {@link ExternalNavigationHandler}.
*/
Expand Down Expand Up @@ -93,142 +84,6 @@ public boolean handleWithAutofillAssistant(
public ChromeActivityTestRule<ChromeActivity> mActivityTestRule =
new ChromeActivityTestRule<>(ChromeActivity.class);

private static List<ResolveInfo> makeResolveInfos(ResolveInfo... infos) {
return Arrays.asList(infos);
}

@Test
@SmallTest
public void testIsPackageSpecializedHandler_NoResolveInfo() {
String packageName = "";
List<ResolveInfo> resolveInfos = new ArrayList<ResolveInfo>();
Assert.assertEquals(0,
ExternalNavigationDelegateImpl
.getSpecializedHandlersWithFilter(resolveInfos, packageName)
.size());
}

@Test
@SmallTest
public void testIsPackageSpecializedHandler_NoPathOrAuthority() {
String packageName = "";
ResolveInfo info = new ResolveInfo();
info.filter = new IntentFilter();
List<ResolveInfo> resolveInfos = makeResolveInfos(info);
Assert.assertEquals(0,
ExternalNavigationDelegateImpl
.getSpecializedHandlersWithFilter(resolveInfos, packageName)
.size());
}

@Test
@SmallTest
public void testIsPackageSpecializedHandler_WithPath() {
String packageName = "";
ResolveInfo info = new ResolveInfo();
info.filter = new IntentFilter();
info.filter.addDataPath("somepath", 2);
List<ResolveInfo> resolveInfos = makeResolveInfos(info);
Assert.assertEquals(1,
ExternalNavigationDelegateImpl
.getSpecializedHandlersWithFilter(resolveInfos, packageName)
.size());
}

@Test
@SmallTest
public void testIsPackageSpecializedHandler_WithAuthority() {
String packageName = "";
ResolveInfo info = new ResolveInfo();
info.filter = new IntentFilter();
info.filter.addDataAuthority("http://www.google.com", "80");
List<ResolveInfo> resolveInfos = makeResolveInfos(info);
Assert.assertEquals(1,
ExternalNavigationDelegateImpl
.getSpecializedHandlersWithFilter(resolveInfos, packageName)
.size());
}

@Test
@SmallTest
public void testIsPackageSpecializedHandler_WithAuthority_Wildcard_Host() {
String packageName = "";
ResolveInfo info = new ResolveInfo();
info.filter = new IntentFilter();
info.filter.addDataAuthority("*", null);
List<ResolveInfo> resolveInfos = makeResolveInfos(info);
Assert.assertEquals(0,
ExternalNavigationDelegateImpl
.getSpecializedHandlersWithFilter(resolveInfos, packageName)
.size());

ResolveInfo infoWildcardSubDomain = new ResolveInfo();
infoWildcardSubDomain.filter = new IntentFilter();
infoWildcardSubDomain.filter.addDataAuthority("http://*.google.com", "80");
List<ResolveInfo> resolveInfosWildcardSubDomain = makeResolveInfos(infoWildcardSubDomain);
Assert.assertEquals(1,
ExternalNavigationDelegateImpl
.getSpecializedHandlersWithFilter(
resolveInfosWildcardSubDomain, packageName)
.size());
}

@Test
@SmallTest
public void testIsPackageSpecializedHandler_WithTargetPackage_Matching() {
String packageName = "com.android.chrome";
ResolveInfo info = new ResolveInfo();
info.filter = new IntentFilter();
info.filter.addDataAuthority("http://www.google.com", "80");
info.activityInfo = new ActivityInfo();
info.activityInfo.packageName = packageName;
List<ResolveInfo> resolveInfos = makeResolveInfos(info);
Assert.assertEquals(1,
ExternalNavigationDelegateImpl
.getSpecializedHandlersWithFilter(resolveInfos, packageName)
.size());
}

@Test
@SmallTest
public void testIsPackageSpecializedHandler_WithTargetPackage_NotMatching() {
String packageName = "com.android.chrome";
ResolveInfo info = new ResolveInfo();
info.filter = new IntentFilter();
info.filter.addDataAuthority("http://www.google.com", "80");
info.activityInfo = new ActivityInfo();
info.activityInfo.packageName = "com.foo.bar";
List<ResolveInfo> resolveInfos = makeResolveInfos(info);
Assert.assertEquals(0,
ExternalNavigationDelegateImpl
.getSpecializedHandlersWithFilter(resolveInfos, packageName)
.size());
}

@Test
@SmallTest
public void testIsPackageSpecializeHandler_withEphemeralResolver() {
String packageName = "";
ResolveInfo info = new ResolveInfo();
info.filter = new IntentFilter();
info.filter.addDataPath("somepath", 2);
info.activityInfo = new ActivityInfo();

// See IntentUtils.isInstantAppResolveInfo
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
info.isInstantAppAvailable = true;
} else {
info.activityInfo.name = IntentUtils.EPHEMERAL_INSTALLER_CLASS;
}
info.activityInfo.packageName = "com.google.android.gms";
List<ResolveInfo> resolveInfos = makeResolveInfos(info);
// Ephemeral resolver is not counted as a specialized handler.
Assert.assertEquals(0,
ExternalNavigationDelegateImpl
.getSpecializedHandlersWithFilter(resolveInfos, packageName)
.size());
}

@Test
@SmallTest
public void testMaybeSetPendingIncognitoUrl() {
Expand Down
34 changes: 30 additions & 4 deletions .../android/java/src/org/chromium/components/external_intents/ExternalNavigationHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1389,10 +1389,7 @@ public static boolean resolveIntent(Intent intent, boolean allowSelfOpen) {
return !canSelfOpen || allowSelfOpen || hasPdfViewer;
}

// TODO(crbug.com/1071390): Make this method private once its consumers have been moved into
// this class.
public static boolean matchResolveInfoExceptWildCardHost(
ResolveInfo info, String filterPackageName) {
static boolean matchResolveInfoExceptWildCardHost(ResolveInfo info, String filterPackageName) {
IntentFilter intentFilter = info.filter;
if (intentFilter == null) {
// Error on the side of classifying ResolveInfo as generic.
Expand Down Expand Up @@ -1422,6 +1419,35 @@ public static boolean matchResolveInfoExceptWildCardHost(
return true;
}

@VisibleForTesting
public static ArrayList<String> getSpecializedHandlersWithFilter(List<ResolveInfo> infos,
String filterPackageName, boolean handlesInstantAppLaunchingInternally) {
ArrayList<String> result = new ArrayList<>();
if (infos == null) {
return result;
}

for (ResolveInfo info : infos) {
if (!matchResolveInfoExceptWildCardHost(info, filterPackageName)) {
continue;
}

if (info.activityInfo != null) {
if (handlesInstantAppLaunchingInternally
&& IntentUtils.isInstantAppResolveInfo(info)) {
// Don't add the Instant Apps launcher as a specialized handler if the embedder
// handles launching of Instant Apps itself.
continue;
}

result.add(info.activityInfo.packageName);
} else {
result.add("");
}
}
return result;
}

/**
* @return Default SMS application's package name at the system level. Null if there isn't any.
*/
Expand Down
Loading

0 comments on commit f5fb7c8

Please sign in to comment.