Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Feat: Filter allowed CustomTabs browsers #353

Merged
merged 9 commits into from
Sep 25, 2020
Merged

Feat: Filter allowed CustomTabs browsers #353

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
056038f
Adding support to override the browser packages to be used during Log…
nelsonmaia Sep 8, 2020
7236981
Adding support to search all the browsers in the device
nelsonmaia Sep 11, 2020
f3b3b64
refactor to use extensible BrowserPicker filtering
lbalmaceda Sep 22, 2020
57badc1
Fix tests for changed logic
lbalmaceda Sep 23, 2020
1df97f9
Replace deprecated import with the alternative
lbalmaceda Sep 23, 2020
2070eb9
add tests for the BrowserPicker class
lbalmaceda Sep 23, 2020
e1692ff
add coverage for introduced use cases and javadocs
lbalmaceda Sep 23, 2020
4c25864
add CompatibleBrowser tests
lbalmaceda Sep 23, 2020
7985131
fix flag passed when querying browser apps for post marshmallow
lbalmaceda Sep 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
fix flag passed when querying browser apps for post marshmallow
  • Loading branch information
@lbalmaceda
lbalmaceda committed Sep 24, 2020
commit 7985131ae65585d67e918898164aca1f51be2e30
5 changes: 2 additions & 3 deletions auth0/src/main/java/com/auth0/android/provider/BrowserPicker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,14 +110,13 @@ public BrowserPicker build() {
@Nullable
String getBestBrowserPackage(@NonNull PackageManager pm) {
Intent browserIntent = new Intent(Intent.ACTION_VIEW, Uri.parse("http:https://www.example.com"));
ResolveInfo webHandler = pm.resolveActivity(browserIntent,
Build.VERSION.SDK_INT >= Build.VERSION_CODES.M ? PackageManager.MATCH_ALL : PackageManager.MATCH_DEFAULT_ONLY);
ResolveInfo webHandler = pm.resolveActivity(browserIntent, PackageManager.MATCH_DEFAULT_ONLY);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix: this line's purpose is to find the "default browser app" in the device. "Matching all" would return all the browsers and pick one from there, which is not always the default app.

String defaultBrowser = null;
if (webHandler != null) {
defaultBrowser = webHandler.activityInfo.packageName;
}

final List<ResolveInfo> availableBrowsers = pm.queryIntentActivities(browserIntent, 0);
final List<ResolveInfo> availableBrowsers = pm.queryIntentActivities(browserIntent, Build.VERSION.SDK_INT >= Build.VERSION_CODES.M ? PackageManager.MATCH_ALL : 0);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix: when listing the installed browsers on marshmallow and above, the ALL flag should be used. Otherwise, the returned list has only one result.

final List<String> regularBrowsers = new ArrayList<>();
final List<String> customTabsBrowsers = new ArrayList<>();
final boolean isFilterEnabled = allowedPackages != null;
Expand Down
7 changes: 4 additions & 3 deletions auth0/src/test/java/com/auth0/android/provider/BrowserPickerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,12 @@

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.isOneOf;
import static org.hamcrest.Matchers.nullValue;
import static org.hamcrest.core.IsNull.notNullValue;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyInt;
import static org.mockito.Matchers.eq;
import static org.mockito.Matchers.intThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.when;
Expand Down Expand Up @@ -327,7 +328,7 @@ static void setupBrowserContext(@NonNull Context context, @NonNull List<String>
PackageManager pm = mock(PackageManager.class);
when(context.getPackageManager()).thenReturn(pm);
ResolveInfo defaultPackage = resolveInfoForPackageName(defaultBrowserPackage);
when(pm.resolveActivity(any(Intent.class), anyInt())).thenReturn(defaultPackage);
when(pm.resolveActivity(any(Intent.class), eq(PackageManager.MATCH_DEFAULT_ONLY))).thenReturn(defaultPackage);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better matchers, only by the values that we know we use. Will help catch changes like changing the prod code flags by mistake in the future 👍


List<ResolveInfo> allBrowsers = new ArrayList<>();
for (String browser : browserPackages) {
Expand All @@ -337,7 +338,7 @@ static void setupBrowserContext(@NonNull Context context, @NonNull List<String>
}
allBrowsers.add(info);
}
when(pm.queryIntentActivities(any(Intent.class), eq(0))).thenReturn(allBrowsers);
when(pm.queryIntentActivities(any(Intent.class), intThat(isOneOf(0, PackageManager.MATCH_ALL)))).thenReturn(allBrowsers);
}

private static ResolveInfo resolveInfoForPackageName(@Nullable String packageName) {
Expand Down