-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
feat(tauri-runtime-wry): ensure webview2 environment options matches #14109
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
base: dev
Are you sure you want to change the base?
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 |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "tauri-runtime-wry": patch:enhance | ||
| --- | ||
|
|
||
| Validate environment options match previously defined environment for the given data directory on Windows. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,7 +40,10 @@ use tao::platform::unix::{WindowBuilderExtUnix, WindowExtUnix}; | |
| #[cfg(windows)] | ||
| use tao::platform::windows::{WindowBuilderExtWindows, WindowExtWindows}; | ||
| #[cfg(windows)] | ||
| use webview2_com::{ContainsFullScreenElementChangedEventHandler, FocusChangedEventHandler}; | ||
| use webview2_com::{ | ||
| ContainsFullScreenElementChangedEventHandler, FocusChangedEventHandler, | ||
| Microsoft::Web::WebView2::Win32::ICoreWebView2Environment, | ||
| }; | ||
| #[cfg(windows)] | ||
| use windows::Win32::Foundation::HWND; | ||
| #[cfg(target_os = "ios")] | ||
|
|
@@ -157,11 +160,24 @@ use window::WindowExt as _; | |
| pub struct WebContext { | ||
| pub inner: WryWebContext, | ||
| pub referenced_by_webviews: HashSet<String>, | ||
| #[cfg(windows)] | ||
| environment: Option<ICoreWebView2Environment>, | ||
| #[cfg(windows)] | ||
| environment_options: WebView2EnvironmentOptions, | ||
| // on Linux the custom protocols are associated with the context | ||
| // and you cannot register a URI scheme more than once | ||
| pub registered_custom_protocols: HashSet<String>, | ||
| } | ||
|
|
||
| #[cfg(windows)] | ||
| #[derive(Debug, PartialEq, Eq)] | ||
| struct WebView2EnvironmentOptions { | ||
|
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. To be honest, this feels very error prune to me but I don't know if there's a better way 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. well this is just for the warning - we ALWAYS force the same environment for the same data directory |
||
| proxy_url: Option<Url>, | ||
| additional_browser_args: Option<String>, | ||
| browser_extensions_enabled: bool, | ||
| // TODO: scroll_bar_style | ||
| } | ||
|
|
||
| pub type WebContextStore = Arc<Mutex<HashMap<Option<PathBuf>, WebContext>>>; | ||
| // window | ||
| pub type WindowEventHandler = Box<dyn Fn(&WindowEvent) + Send>; | ||
|
|
@@ -4544,6 +4560,13 @@ You may have it installed on another user account, but it is not available for t | |
| .. | ||
| } = pending; | ||
|
|
||
| #[cfg(windows)] | ||
| let environment_options = WebView2EnvironmentOptions { | ||
| proxy_url: webview_attributes.proxy_url.clone(), | ||
| additional_browser_args: webview_attributes.additional_browser_args.clone(), | ||
| browser_extensions_enabled: webview_attributes.browser_extensions_enabled, | ||
| }; | ||
|
|
||
| let mut web_context = context | ||
| .main_thread | ||
| .web_context | ||
|
|
@@ -4557,7 +4580,22 @@ You may have it installed on another user account, but it is not available for t | |
| let web_context = match entry { | ||
| Occupied(occupied) => { | ||
| let occupied = occupied.into_mut(); | ||
|
|
||
| #[cfg(windows)] | ||
| if cfg!(debug_assertions) && occupied.environment_options != environment_options { | ||
| let message = format!( | ||
| "environment options {environment_options:?} for {} do not match previously defined options for the same data directory on webviews {:?}, expected {:?}. Since this results in a crash, we are going to force the previously defined environment options", | ||
| web_context_key.as_ref().map(|p| p.to_string_lossy()).unwrap_or_else(|| "".into()), | ||
| occupied.referenced_by_webviews, occupied.environment_options | ||
| ); | ||
| #[cfg(feature = "tracing")] | ||
| tracing::warn!("{message}"); | ||
| #[cfg(not(feature = "tracing"))] | ||
| eprintln!("{message}"); | ||
| } | ||
|
|
||
| occupied.referenced_by_webviews.insert(label.clone()); | ||
|
|
||
| occupied | ||
| } | ||
| Vacant(vacant) => { | ||
|
|
@@ -4571,6 +4609,10 @@ You may have it installed on another user account, but it is not available for t | |
| inner: web_context, | ||
| referenced_by_webviews: [label.clone()].into(), | ||
| registered_custom_protocols: HashSet::new(), | ||
| #[cfg(windows)] | ||
| environment: None, | ||
| #[cfg(windows)] | ||
| environment_options, | ||
| }) | ||
| } | ||
| }; | ||
|
|
@@ -4584,6 +4626,11 @@ You may have it installed on another user account, but it is not available for t | |
| .with_clipboard(webview_attributes.clipboard) | ||
| .with_hotkeys_zoom(webview_attributes.zoom_hotkeys_enabled); | ||
|
|
||
| #[cfg(windows)] | ||
| if let Some(environment) = &web_context.environment { | ||
| webview_builder = webview_builder.with_environment(environment.clone()); | ||
|
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. The whole environment thing is a bit confusing now, that we have it in Some documentation around the related attributes would be nice 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. well i'm not sure if we should be explicit or not. like only validate the environment options matches? though in this case we get to that "error prune" validation issue.. since the on_new_window environment is all handled by window_features i think it's ok to keep it as is, so it's explicitly matched instead of relying on internal details but i agree documentation should be improved anyway 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.
I think forcing the same environment makes sense here, no matter the options, it's too error prune the other way around
If we want to force the environment, keeping it feels like some extra confusions, anyways, we can't remove it in v2 anyways, let's just update documentation |
||
| } | ||
|
|
||
| if url != "about:blank" { | ||
| webview_builder = webview_builder.with_url(&url); | ||
| } | ||
|
|
@@ -5016,6 +5063,11 @@ You may have it installed on another user account, but it is not available for t | |
|
|
||
| #[cfg(windows)] | ||
| { | ||
| // set the environment on the web context | ||
| // important so the next webview with the same context can reuse the environment instance | ||
| // to ensure its options matches | ||
| web_context.environment.replace(webview.environment()); | ||
|
|
||
| let controller = webview.controller(); | ||
| let proxy_clone = context.proxy.clone(); | ||
| let window_id_ = window_id.clone(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe?