-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[url_launcher_android] Add support for Custom Tabs #4739
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,7 @@ | ||
## 6.1.0 | ||
|
||
* Adds support for Android Custom Tabs. | ||
|
||
## 6.0.39 | ||
|
||
* Adds pub topics to package metadata. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,8 +16,10 @@ | |
import androidx.annotation.NonNull; | ||
import androidx.annotation.Nullable; | ||
import androidx.annotation.VisibleForTesting; | ||
import androidx.browser.customtabs.CustomTabsIntent; | ||
import io.flutter.plugins.urllauncher.Messages.UrlLauncherApi; | ||
import io.flutter.plugins.urllauncher.Messages.WebViewOptions; | ||
import java.util.Locale; | ||
import java.util.Map; | ||
|
||
/** Implements the Pigeon-defined interface for calls from Dart. */ | ||
|
@@ -97,13 +99,24 @@ void setActivity(@Nullable Activity activity) { | |
ensureActivity(); | ||
assert activity != null; | ||
|
||
Bundle headersBundle = extractBundle(options.getHeaders()); | ||
|
||
// Try to launch using Custom Tabs if they have the necessary functionality. | ||
if (!containsRestrictedHeader(options.getHeaders())) { | ||
Uri uri = Uri.parse(url); | ||
if (openCustomTab(activity, uri, headersBundle)) { | ||
return true; | ||
} | ||
} | ||
|
||
// Fall back to a web view if necessary. | ||
Intent launchIntent = | ||
WebViewActivity.createIntent( | ||
activity, | ||
url, | ||
options.getEnableJavaScript(), | ||
options.getEnableDomStorage(), | ||
extractBundle(options.getHeaders())); | ||
headersBundle); | ||
try { | ||
activity.startActivity(launchIntent); | ||
} catch (ActivityNotFoundException e) { | ||
|
@@ -118,6 +131,35 @@ public void closeWebView() { | |
applicationContext.sendBroadcast(new Intent(WebViewActivity.ACTION_CLOSE)); | ||
} | ||
|
||
private static boolean openCustomTab( | ||
@NonNull Context context, @NonNull Uri uri, @NonNull Bundle headersBundle) { | ||
CustomTabsIntent customTabsIntent = new CustomTabsIntent.Builder().build(); | ||
customTabsIntent.intent.putExtra(Browser.EXTRA_HEADERS, headersBundle); | ||
try { | ||
customTabsIntent.launchUrl(context, uri); | ||
} catch (ActivityNotFoundException ex) { | ||
return false; | ||
} | ||
return true; | ||
} | ||
|
||
// Checks if headers contains a CORS restricted header. | ||
// https://developer.mozilla.org/en-US/docs/Glossary/CORS-safelisted_request_header | ||
private static boolean containsRestrictedHeader(Map<String, String> headersMap) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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? 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Good point, waiting and seeing if we get feedback SGTM with that in mind |
||
for (String key : headersMap.keySet()) { | ||
switch (key.toLowerCase(Locale.US)) { | ||
case "accept": | ||
case "accept-language": | ||
case "content-language": | ||
case "content-type": | ||
continue; | ||
default: | ||
return true; | ||
} | ||
} | ||
return false; | ||
} | ||
|
||
private static @NonNull Bundle extractBundle(Map<String, String> headersMap) { | ||
final Bundle headersBundle = new Bundle(); | ||
for (String key : headersMap.keySet()) { | ||
|
Uh oh!
There was an error while loading. Please reload this page.