diff --git a/library/java/net/openid/appauth/BrowserPackageHelper.java b/library/java/net/openid/appauth/BrowserPackageHelper.java index 5fd9ed4d..087d185f 100644 --- a/library/java/net/openid/appauth/BrowserPackageHelper.java +++ b/library/java/net/openid/appauth/BrowserPackageHelper.java @@ -90,38 +90,26 @@ public String getPackageNameToUse(Context context) { return mPackageNameToUse; } - // Get default VIEW intent handler for web URIs PackageManager pm = context.getPackageManager(); - ResolveInfo defaultViewHandlerInfo = - pm.resolveActivity(BROWSER_INTENT, PackageManager.GET_RESOLVED_FILTER); - // if the default is not a full browser, ignore it - String defaultViewHandlerPackageName = null; - if (defaultViewHandlerInfo != null && isFullBrowser(defaultViewHandlerInfo)) { - defaultViewHandlerPackageName = defaultViewHandlerInfo.activityInfo.packageName; - - // check whether the default handler has a warmup service and return if it does - if (hasWarmupService(pm, defaultViewHandlerPackageName)) { - mPackageNameToUse = defaultViewHandlerPackageName; - return mPackageNameToUse; - } - } - - // If the default handler is not set / eligible, or does not have a warmup service, return - // the first handler eligible handler found which supports a warmup service (if available). - ResolveInfo alternateBrowser = null; + // retrieve a list of all the matching handlers for the browser intent. + // queryIntentActivities will ensure that these are priority ordered, with the default + // (if set) as the first entry. Ignoring any matches which are not "full" browsers, + // pick the first that supports custom tabs, or the first full browser otherwise. + ResolveInfo firstMatch = null; List resolvedActivityList = pm.queryIntentActivities(BROWSER_INTENT, PackageManager.GET_RESOLVED_FILTER); + for (ResolveInfo info : resolvedActivityList) { // ignore handlers which are not browers if (!isFullBrowser(info)) { continue; } - // we hold the first non-default browser as the alternate browser to use, if we do - // not find any that support a warmup service - if (alternateBrowser == null) { - alternateBrowser = info; + // we hold the first non-default browser as the default browser to use, if we do + // not find any that support a warmup service. + if (firstMatch == null) { + firstMatch = info; } if (hasWarmupService(pm, info.activityInfo.packageName)) { @@ -131,12 +119,10 @@ public String getPackageNameToUse(Context context) { } } - // No handlers have a warmup service, so we return default browser, or an arbitrary - // browser if the default is not set / not eligible. - if (!TextUtils.isEmpty(defaultViewHandlerPackageName)) { - mPackageNameToUse = defaultViewHandlerPackageName; - } else if (alternateBrowser != null) { - mPackageNameToUse = alternateBrowser.activityInfo.packageName; + // No handlers have a warmup service, so we return the first match (typically the + // default browser), or null if there are no identifiable browsers. + if (firstMatch != null) { + mPackageNameToUse = firstMatch.activityInfo.packageName; } else { mPackageNameToUse = null; } diff --git a/library/javatests/net/openid/appauth/BrowserPackageHelperTest.java b/library/javatests/net/openid/appauth/BrowserPackageHelperTest.java index 000be9e0..8056160f 100644 --- a/library/javatests/net/openid/appauth/BrowserPackageHelperTest.java +++ b/library/javatests/net/openid/appauth/BrowserPackageHelperTest.java @@ -86,73 +86,36 @@ public void tearDown() { } @Test - public void testGetPackageNameToUse_withDefaultBrowser_warmUpSupportOnDefault() { - setDefaultBrowser(CHROME); - setBrowserList(FIREFOX, DOLPHIN, CHROME); + public void testGetPackageNameToUse_warmUpSupportOnFirstMatch() { + setBrowserList(CHROME, FIREFOX, DOLPHIN); setBrowsersWithWarmupSupport(CHROME, FIREFOX); checkPackageNameToUse(CHROME); } @Test - public void testGetPackageNameToUse_withDefaultBrowser_warmUpSupportOnAlternateBrowser() { - setDefaultBrowser(DOLPHIN); - setBrowserList(FIREFOX, DOLPHIN); + public void testGetPackageNameToUse_warmUpSupportOnAlternateBrowser() { + setBrowserList(DOLPHIN, FIREFOX); setBrowsersWithWarmupSupport(FIREFOX); checkPackageNameToUse(FIREFOX); } @Test - public void testGetPackageNameToUse_withDefaultBrowser_warmUpSupportOnAlternateBrowsers() { - setDefaultBrowser(DOLPHIN); - setBrowserList(CHROME, DOLPHIN, FIREFOX); + public void testGetPackageNameToUse_warmUpSupportOnAlternateBrowsers() { + setBrowserList(DOLPHIN, CHROME, FIREFOX); setBrowsersWithWarmupSupport(CHROME, FIREFOX); - checkPackageNameToUseIsOneOf(FIREFOX, CHROME); - } - - @Test - public void testGetPackageNameToUse_withDefaultBrowser_noWarmUpSupportOnAnyBrowser() { - setDefaultBrowser(CHROME); - setBrowserList(DOLPHIN, CHROME); - setBrowsersWithWarmupSupport(NO_BROWSERS); + // first in priority list always wins checkPackageNameToUse(CHROME); } - - @Test - public void testGetPackageNameToUse_withNoDefaultBrowser_warmupSupportOnOneBrowser() { - setDefaultBrowser(null); - setBrowserList(FIREFOX, DOLPHIN); - setBrowsersWithWarmupSupport(FIREFOX); - checkPackageNameToUse(FIREFOX); - } - - @Test - public void testGetPackageNameToUse_withNoDefaultBrowser_warmUpSupportOnMultipleBrowsers() { - setDefaultBrowser(null); - setBrowserList(CHROME, FIREFOX); - setBrowsersWithWarmupSupport(CHROME, FIREFOX); - checkPackageNameToUseIsOneOf(CHROME, FIREFOX); - } - - @Test - public void testGetPackageNameToUse_withNoDefaultBrowser_singleBrowser_noWarmUpSupport() { - setDefaultBrowser(null); - setBrowserList(DOLPHIN); - setBrowsersWithWarmupSupport(NO_BROWSERS); - checkPackageNameToUseIsOneOf(DOLPHIN); - } - @Test - public void testGetPackageNameToUse_withNoDefaultBrowser_noWarmUpSupportOnAnyBrowser() { - setDefaultBrowser(null); - setBrowserList(DOLPHIN, FIREFOX); + public void testGetPackageNameToUse_noWarmUpSupportOnAnyBrowser() { + setBrowserList(CHROME, DOLPHIN); setBrowsersWithWarmupSupport(NO_BROWSERS); - checkPackageNameToUseIsOneOf(DOLPHIN, FIREFOX); + checkPackageNameToUse(CHROME); } @Test public void testGetPackageNameToUse_noBrowsers() { - setDefaultBrowser(null); setBrowserList(NO_BROWSERS); setBrowsersWithWarmupSupport(NO_BROWSERS); assertNull(mHelper.getPackageNameToUse(mContext)); @@ -165,7 +128,6 @@ public void testGetPackageNameToUse_ignoreAuthorityRestrictedBrowsers() { .withBrowserDefaults() .addAuthority("www.example.com") .build(); - setDefaultBrowser(authorityRestrictedBrowser); setBrowserList(authorityRestrictedBrowser, CHROME); setBrowsersWithWarmupSupport(authorityRestrictedBrowser, CHROME); checkPackageNameToUse(CHROME); @@ -180,7 +142,6 @@ public void testGetPackageNameToUse_ignoreBrowsersWithoutBrowseableCategory() { .addScheme(SCHEME_HTTP) .addScheme(SCHEME_HTTPS) .build(); - setDefaultBrowser(misconfiguredBrowser); setBrowserList(misconfiguredBrowser, CHROME); setBrowsersWithWarmupSupport(misconfiguredBrowser, CHROME); checkPackageNameToUse(CHROME); @@ -194,19 +155,14 @@ public void testGetPackageNameToUse_ignoreBrowsersWithoutHttpsSupport() { .addCategory(Intent.CATEGORY_BROWSABLE) .addScheme(SCHEME_HTTP) .build(); - setDefaultBrowser(DOLPHIN); setBrowserList(DOLPHIN, noHttpsBrowser); setBrowsersWithWarmupSupport(noHttpsBrowser); checkPackageNameToUse(DOLPHIN); } - private void setDefaultBrowser(ResolveInfo defaultBrowser) { - when(mPackageManager.resolveActivity( - BrowserPackageHelper.BROWSER_INTENT, - PackageManager.GET_RESOLVED_FILTER)) - .thenReturn(defaultBrowser); - } - + /** + * Browsers are expected to be in priority order, such that the default would be first. + */ private void setBrowserList(ResolveInfo... browsers) { if (browsers == null) { return; @@ -235,16 +191,6 @@ private void checkPackageNameToUse(ResolveInfo expected) { expected.activityInfo.packageName, result); } - private void checkPackageNameToUseIsOneOf(ResolveInfo... possibleResults) { - String result = mHelper.getPackageNameToUse(mContext); - boolean matched = false; - for (ResolveInfo possibleResult : possibleResults) { - matched |= (possibleResult.activityInfo.packageName.equals(result)); - } - - assertTrue("returned package does not match any in possible set", matched); - } - private static class ResolveInfoBuilder { private final String mPackageName; private final List mActions = new ArrayList<>();