[url_launcher_android] Add support for Custom Tabs

[rajveermalviya]

Implement support for Android Custom Tabs.

Custom Tabs will only be used if all of the following conditions are true:

• launchMode == LaunchMode.inAppWebView (or LaunchMode.platformDefault; only if url is web url)
• WebViewConfiguration.headers == {} (or if it only contains CORS-safelisted headers)

Fixes flutter/flutter#18589

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
• I signed the CLA.
• The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
• I listed at least one issue that this PR fixes in the description above.
• I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
• I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
• I updated/added relevant documentation (doc comments with ///).
• I added new tests to check the change I am making, or this PR is test-exempt.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[flutter-dashboard]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

[gnprice]

Welcome @rajveermalviya, and thanks for the contribution!

This version introduces Chrome custom tabs to replace launching the URL in an external app, for LaunchMode.externalApplication. The thread flutter/flutter#18589 is a bit confusing, because that is the approach requested in the original issue description. But I think what we actually would like to do is to instead have Chrome custom tabs replace the custom WebView-based screen, for LaunchMode.inAppWebView.

See @stuartmorgan's comment at flutter/flutter#18589 (comment) suggesting that, and my two comments starting at flutter/flutter#18589 (comment) about why the existing custom WebView screen isn't a good UX.

Conversely, I think it wouldn't be appropriate for LaunchMode.externalApplication to open the URL in a browser that stays within the app, which is what a Chrome custom tab means from a user perspective.

The FLAG_ACTIVITY_REQUIRE_NON_BROWSER part is also useful but should probably be left for a separate PR; see below.

[gnprice]

This part is useful functionality, but should be controlled by the launch mode — this is implementing LaunchMode.externalNonBrowserApplication.

An implementation of that would resolve flutter/flutter#66721 and would also be quite welcome. It's probably simplest to handle that as a separate PR, though. In order to make it properly conditional on the launch mode, I think it may also require changes in url_launcher_platform_interface.

[rajveermalviya]

Hello @gnprice 👋

I think what we actually would like to do is to instead have Chrome custom tabs replace the custom WebView-based screen, for LaunchMode.inAppWebView.

Yeah, I went with the updating LaunchMode.externalApplication mode because that's what Custom Tabs actually feels like - links are still opened by an external application (a browser in this case) but with a compact look, a custom tab.
If the WebView mode is replaced then that will indeed be a breaking change. Also Custom Tabs won't be able to completely replace them anyway, because WebView mode allows passing headers not present in CORS-safelisted headers list; Custom Tabs can't support that.
One way would be to break the users and tell them to migrate to using webview package for their exotic needs.

Also I wouldn't call them Chrome Custom Tabs, since it is supported by many browsers on Android, and the doc website also calls them Android Custom Tabs now. It's API is also part of androidx:browser jetpack library - nothing chrome specific anymore ^^.

Firefox	Chrome

The FLAG_ACTIVITY_REQUIRE_NON_BROWSER part is also useful but should probably be left for a separate PR; see below.

That was added to match the behavior of legacy launch (Intent(Intent.ACTION_VIEW).setData(uri)). So that there is no breaking change, only changed behavior is that if the link is open-able in a browser it will now open in a Custom Tab and if not it will be handed over to the corresponding native app, if any, the latter part remains the same.

[stuartmorgan-g]

Thanks for the contribution, but along the lines of @gnprice's comments above this is not a change we would accept as written.

Yeah, I went with the updating LaunchMode.externalApplication mode because that's what Custom Tabs actually feels like - links are still opened by an external application (a browser in this case) but with a compact look, a custom tab.

That is not the way the custom tabs feature is described:

Custom Tabs are a feature in Android browsers that gives app developers a way to add a customized browser experience directly within their app.

Custom Tabs offer a better user experience than simply opening an external browser. They allow users to remain within the app while browsing

And importantly, it's also not the way they actually behave from a system perspective. If you open a custom tab and then pull up the app switching UI you will be shown as in the same app, not in your browser. It is closely analogous to the Safari View Controller used for launch-in-app mode on iOS.

If the WebView mode is replaced then that will indeed be a breaking change.

As currently written, this PR removes the ability to launch in a the user's browser, which is absolutely a breaking change. And one that violates the documented and intended behavior of that mode.

WebView mode allows passing headers not present in CORS-safelisted headers list; Custom Tabs can't support that.

We need to keep the webview for the foreseeable future anyway, to handle devices that don't have Custom Tab support; we can fall back to that codepath if any unsupported headers are passed (and add a comment to that effect on the header property) so that nothing regresses.

I'm going to mark this as a draft for now; if you are interested in reworking this to match the behavior we want for the plugin please mark it as ready for review once those changes are made.

[rajveermalviya]

Thanks for clearing the doubts @stuartmorgan - I'll update the PR to use Custom Tabs when LaunchMode.inAppWebView
is used and fallback to actual WebView implementation if Custom Tabs are unavailable or if any headers passed are not present in CORS-safelisted headers list.

[rajveermalviya]

(also updated the PR description to reflect current implementation)

[stuartmorgan-g]

Custom Tabs will only be used if all of the following conditions are true:
[...]

• WebViewConfiguration.enableJavaScript == true
• WebViewConfiguration.enableDomStorage == true

We don't need these checks; the docs already say they may not be supported on all platforms. It's fine for us to silently ignore the false setting on the Custom Tab codepath the same way they are already ignored on iOS.

[rajveermalviya]

Thanks @stuartmorgan, updated the description & implementation to ignore them.

[gmackall]

Thanks for the contribution, and for the fast iteration on reviews! I'll take a look this afternoon 🙂

[gmackall]

Re running the checks, as it looks like the failures are due to infra

[gmackall]

This mostly LGTM, I just have one question I want to clarify before approving!

[gmackall]

There are additional restrictions listed in the link - my understanding is that if we try to launch with the safelisted headers, but in a case that fails the additional restrictions, we will simply fail and fall back to the webview mode.

Do you have a sense of how frequently we would end up in this case of mismatch between containsRestrictedHeader, and the source of truth link? Is it worth complicating this check to make it line up fully?

[rajveermalviya]

Digging into this a bit more, Chromium has a lot more exhaustive list of headers and checks for the values. Should the same be done here?
(Disclaimer; BSD licensed) - https://source.chromium.org/chromium/chromium/src/+/main:services/network/public/cpp/cors/cors.cc;l=278-425;drc=86339296c23927c8e8abb6cd81c8cb8d4bb3700e

Problem with not doing this correctly, that is check everything header+values, chrome & firefox will just ignore the restricted headers and the page will be opened in the custom tabs without those headers, it doesn't fail to open custom tab. So we can't fallback to webview if that happens, unless we do the check ourselves.

[stuartmorgan-g]

My preference would be to not add a bunch of complexity without knowing if anyone actually needs it. I would vote for keeping the current basic check, and see if we get feedback about it. Anyone affected can trivially work around it in the meantime by just adding an arbitrary other header to the request (e.g., add 'do-not-use-custom-tabs': true to the header map).

[gmackall]

Anyone affected can trivially work around it in the meantime by just adding an arbitrary other header to the request (e.g., add 'do-not-use-custom-tabs': true to the header map).

Good point, waiting and seeing if we get feedback SGTM with that in mind

Obsolete

[stuartmorgan-g]

This is now published. Thanks for the great contribution!