Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .changes/validate-environment-options.md
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.
54 changes: 53 additions & 1 deletion crates/tauri-runtime-wry/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down Expand Up @@ -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,
Comment on lines +165 to +166
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe?

Suggested change
#[cfg(windows)]
environment_options: WebView2EnvironmentOptions,
#[cfg(all(debug_assertions, 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 {
Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Member Author

Choose a reason for hiding this comment

The 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>;
Expand Down Expand Up @@ -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
Expand All @@ -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) => {
Expand All @@ -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,
})
}
};
Expand All @@ -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());
Copy link
Contributor

Choose a reason for hiding this comment

The 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 on_new_window and tell people to pass it in with with_environment/window_features, but we also have it done automatically here

Some documentation around the related attributes would be nice

Copy link
Member Author

Choose a reason for hiding this comment

The 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

Copy link
Contributor

Choose a reason for hiding this comment

The 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..

I think forcing the same environment makes sense here, no matter the options, it's too error prune the other way around

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

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);
}
Expand Down Expand Up @@ -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();
Expand Down
Loading