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.

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

Support multiple webviews on a single WebContext (webkit2gtk) #359

Merged
merged 15 commits into from
Jul 29, 2021
5 changes: 5 additions & 0 deletions .changes/webcontext-multiple-webviews.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"wry": patch
---

Support having multiple webkit2gtk `WebView`s on a single `WebContext`.
6 changes: 3 additions & 3 deletions examples/multi_window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ fn main() -> wry::Result<()> {
};

let event_loop = EventLoop::new();
let web_context = WebContext::default();
let mut web_context = WebContext::default();
let window1 = WindowBuilder::new().build(&event_loop).unwrap();

let (window_tx, window_rx) = std::sync::mpsc::channel::<String>();
Expand All @@ -51,7 +51,7 @@ fn main() -> wry::Result<()> {
}"#,
)
.with_rpc_handler(handler)
.with_web_context(&web_context)
.with_web_context(&mut web_context)
.build()?;
let mut webviews = HashMap::new();
webviews.insert(id, webview1);
Expand All @@ -73,7 +73,7 @@ fn main() -> wry::Result<()> {
.unwrap()
.with_url(&url)
.unwrap()
.with_web_context(&web_context)
.with_web_context(&mut web_context)
.build()
.unwrap();
webviews.insert(id, webview2);
Expand Down
7 changes: 0 additions & 7 deletions examples/system_tray.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ fn main() -> wry::Result<()> {
#[cfg(target_os = "linux")]
use wry::application::platform::unix::WindowBuilderExtUnix;
#[cfg(target_os = "windows")]
use wry::application::platform::windows::SystemTrayExtWindows;
#[cfg(target_os = "windows")]
use wry::application::platform::windows::WindowBuilderExtWindows;
use wry::{
application::{
Expand Down Expand Up @@ -201,11 +199,6 @@ fn main() -> wry::Result<()> {
}
// click on `quit` item
if menu_id == quit_item.clone().id() {
// on windows, we make sure to remove the icon from the tray
// it require the `SystemTrayExtWindows`
#[cfg(target_os = "windows")]
system_tray.remove();

// tell our app to close at the end of the loop.
*control_flow = ControlFlow::Exit;
}
Expand Down
2 changes: 2 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,4 +153,6 @@ pub enum Error {
#[cfg(target_os = "windows")]
#[error(transparent)]
WebView2Error(#[from] webview2::Error),
#[error("Duplicate custom protocol registered: {0}")]
DuplicateCustomProtocol(String),
}
4 changes: 2 additions & 2 deletions src/webview/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ impl Default for WebViewAttributes {
/// [`WebViewBuilder`] privides ability to setup initialization before web engine starts.
pub struct WebViewBuilder<'a> {
pub webview: WebViewAttributes,
web_context: Option<&'a WebContext>,
web_context: Option<&'a mut WebContext>,
window: Window,
}

Expand Down Expand Up @@ -205,7 +205,7 @@ impl<'a> WebViewBuilder<'a> {
}

/// Set the web context that can share with multiple [`WebView`]s.
pub fn with_web_context(mut self, web_context: &'a WebContext) -> Self {
pub fn with_web_context(mut self, web_context: &'a mut WebContext) -> Self {
self.web_context = Some(web_context);
self
}
Expand Down
251 changes: 237 additions & 14 deletions src/webview/web_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ use std::path::{Path, PathBuf};
/// private/incognito tabs.
///
/// [`WebView`]: crate::webview::WebView
#[derive(Debug)]
pub struct WebContext {
data: WebContextData,
#[allow(dead_code)] // It's no need on Windows and macOS.
#[allow(dead_code)] // It's not needed on Windows and macOS.
os: WebContextImpl,
}

Expand Down Expand Up @@ -47,7 +48,7 @@ impl Default for WebContext {
}

/// Data that all [`WebContext`] share regardless of platform.
#[derive(Default)]
#[derive(Debug, Default)]
struct WebContextData {
data_directory: Option<PathBuf>,
}
Expand All @@ -66,6 +67,7 @@ impl WebContextData {
target_os = "netbsd",
target_os = "openbsd"
)))]
#[derive(Debug)]
struct WebContextImpl;

#[cfg(not(any(
Expand Down Expand Up @@ -102,14 +104,29 @@ use self::unix::WebContextImpl;
pub mod unix {
//! Unix platform extensions for [`WebContext`](super::WebContext).

use crate::{application::window::Window, Error};
use glib::FileError;
use std::{
collections::{HashSet, VecDeque},
rc::Rc,
sync::{
atomic::{AtomicBool, Ordering::SeqCst},
Mutex,
},
};
use url::Url;
use webkit2gtk::{
ApplicationInfo, WebContext, WebContextBuilder, WebContextExt as WebkitWebContextExt,
WebsiteDataManagerBuilder,
ApplicationInfo, AutomationSessionExt, LoadEvent, SecurityManagerExt, URISchemeRequestExt,
UserContentManager, WebContext, WebContextBuilder, WebContextExt as WebkitWebContextExt,
WebView, WebViewExt, WebsiteDataManagerBuilder,
};

#[derive(Debug)]
pub(super) struct WebContextImpl {
app_info: ApplicationInfo,
context: WebContext,
manager: UserContentManager,
webview_uri_loader: Rc<WebviewUriLoader>,
registered_protocols: HashSet<String>,
automation: bool,
}

Expand Down Expand Up @@ -155,10 +172,23 @@ pub mod unix {
.expect("invalid wry version patch"),
);

context.connect_automation_started(move |_, auto| {
auto.set_application_info(&app_info);

// We do **NOT** support arbitrarily creating new webviews.
// To support this in the future, we would need a way to specify the
// default WindowBuilder to use to create the window it will use, and
// possibly "default" webview attributes. Difficulty comes in for controlling
// the owned Window that would need to be used.
auto.connect_create_web_view(move |_| unimplemented!());
});

Self {
app_info,
context,
automation,
manager: UserContentManager::new(),
registered_protocols: Default::default(),
webview_uri_loader: Rc::default(),
}
}

Expand All @@ -170,29 +200,222 @@ pub mod unix {

/// [`WebContext`](super::WebContext) items that only matter on unix.
pub trait WebContextExt {
/// The application info shared between webviews.
fn app_info(&self) -> &ApplicationInfo;

/// The context of all webviews opened.
/// The GTK [`WebContext`] of all webviews in the context.
fn context(&self) -> &WebContext;

/// The GTK [`UserContentManager`] of all webviews in the context.
fn manager(&self) -> &UserContentManager;

/// Register a custom protocol to the web context.
///
/// When duplicate schemes are registered, the duplicate handler will still be submitted and the
/// `Err(Error::DuplicateCustomProtocol)` will be returned. It is safe to ignore if you are
/// relying on the platform's implementation to properly handle duplicated scheme handlers.
fn register_uri_scheme<F>(
&mut self,
name: &str,
handler: F,
window: Rc<Window>,
) -> crate::Result<()>
where
F: Fn(&Window, &str) -> crate::Result<(Vec<u8>, String)> + 'static;

/// Register a custom protocol to the web context, only if it is not a duplicate scheme.
///
/// If a duplicate scheme has been passed, its handler will **NOT** be registered and the
/// function will return `Err(Error::DuplicateCustomProtocol)`.
fn try_register_uri_scheme<F>(
&mut self,
name: &str,
handler: F,
window: Rc<Window>,
) -> crate::Result<()>
where
F: Fn(&Window, &str) -> crate::Result<(Vec<u8>, String)> + 'static;

/// Add a [`WebView`] to the queue waiting to be opened.
///
/// See the `WebviewUriLoader` for more information.
fn queue_load_uri(&self, webview: Rc<WebView>, url: Url);

/// Flush all queued [`WebView`]s waiting to load a uri.
///
/// See the `WebviewUriLoader` for more information.
fn flush_queue_loader(&self);

/// If the context allows automation.
///
/// **Note:** `libwebkit2gtk` only allows 1 automation context at a time.
fn allows_automation(&self) -> bool;
}

impl WebContextExt for super::WebContext {
fn app_info(&self) -> &ApplicationInfo {
&self.os.app_info
}

fn context(&self) -> &WebContext {
&self.os.context
}

fn manager(&self) -> &UserContentManager {
&self.os.manager
}

fn register_uri_scheme<F>(
&mut self,
name: &str,
handler: F,
window: Rc<Window>,
) -> crate::Result<()>
where
F: Fn(&Window, &str) -> crate::Result<(Vec<u8>, String)> + 'static,
{
actually_register_uri_scheme(self, name, handler, window)?;
if self.os.registered_protocols.insert(name.to_string()) {
Ok(())
} else {
Err(Error::DuplicateCustomProtocol(name.to_string()))
Copy link
Member

Choose a reason for hiding this comment

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

The conflict of duplocate custom protocol only happen if they share same web context.
I think this should let users to decide and we could add some warnings on the behaviour of this.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, in my item #2 the with_custom_protocol returns a Result like some of the other builder items so that the user can choose to ignore it instead of it popping off in the InnerWebView constructor. I think we can support custom protocols on both the WebviewBuilder and the WebContext, where using WebviewBuilder it will just swallow this result inside the webview constructor, and from WebContext it allows the user to handle it how they want

Copy link
Member Author

@chippers chippers Jul 28, 2021

Choose a reason for hiding this comment

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

I went with register_uri_scheme and try_register_uri_scheme. Both will return an Err(DuplicateCustomProtocol) when a duplicate is passed, but the non-try one will still pass it to the underlying platform (only webkit2gtk in this instance). The code in InnerWebview has been updated to keep the old behavior. I'll leave this conversation open if you think we should resolve it another way.

I decided to keep it like this because I think there should be another PR after this for adding the register URI functions to the WebContext itself, which will expose both. The current WebviewBuilder method if we keep it will continue to use the existing way

Copy link
Member

Choose a reason for hiding this comment

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

I think it's fine to add both methods. But for adding URI method to WebContext should be exclusive on Linux imho.
Mac and Windows handle them differently (and it's even a hack on Window for now). They probably don't need it.

}
}

fn try_register_uri_scheme<F>(
&mut self,
name: &str,
handler: F,
window: Rc<Window>,
) -> crate::Result<()>
where
F: Fn(&Window, &str) -> crate::Result<(Vec<u8>, String)> + 'static,
{
if self.os.registered_protocols.insert(name.to_string()) {
actually_register_uri_scheme(self, name, handler, window)
} else {
Err(Error::DuplicateCustomProtocol(name.to_string()))
}
}

fn queue_load_uri(&self, webview: Rc<WebView>, url: Url) {
self.os.webview_uri_loader.push(webview, url)
}

fn flush_queue_loader(&self) {
Rc::clone(&self.os.webview_uri_loader).flush()
}

fn allows_automation(&self) -> bool {
self.os.automation
}
}

fn actually_register_uri_scheme<F>(
context: &mut super::WebContext,
name: &str,
handler: F,
window: Rc<Window>,
) -> crate::Result<()>
where
F: Fn(&Window, &str) -> crate::Result<(Vec<u8>, String)> + 'static,
{
let context = &context.os.context;
context
.get_security_manager()
.ok_or(Error::MissingManager)?
.register_uri_scheme_as_secure(name);

context.register_uri_scheme(name, move |request| {
if let Some(uri) = request.get_uri() {
let uri = uri.as_str();

match handler(&window, uri) {
Ok((buffer, mime)) => {
let input = gio::MemoryInputStream::from_bytes(&glib::Bytes::from(&buffer));
request.finish(&input, buffer.len() as i64, Some(&mime))
}
Err(_) => request.finish_error(&mut glib::Error::new(
FileError::Exist,
"Could not get requested file.",
)),
}
} else {
request.finish_error(&mut glib::Error::new(
FileError::Exist,
"Could not get uri.",
));
}
});

Ok(())
}

/// Prevents an unknown concurrency bug with loading multiple URIs at the same time on webkit2gtk.
///
/// Using the queue prevents data race issues with loading uris for multiple [`WebView`]s in the
/// same context at the same time. Occasionally, the one of the [`WebView`]s will be clobbered
/// and it's content will be injected into a different [`WebView`].
///
/// Example of `webview-c` clobbering `webview-b` while `webview-a` is okay:
/// ```text
/// webview-a triggers load-change::started
/// URISchemeRequestCallback triggered with webview-a
/// webview-a triggers load-change::committed
/// webview-a triggers load-change::finished
/// webview-b triggers load-change::started
/// webview-c triggers load-change::started
/// URISchemeRequestCallback triggered with webview-c
/// URISchemeRequestCallback triggered with webview-c
/// webview-c triggers load-change::committed
/// webview-c triggers load-change::finished
/// ```
///
/// In that example, `webview-a` will load fine. `webview-b` will remain empty as the uri was
/// never loaded. `webview-c` will contain the content of both `webview-b` and `webview-c`
/// because it was triggered twice even through only started once. The content injected will not
/// be sequential, and often is interjected in the middle of one of the other contents.
///
/// FIXME: We think this may be an underlying concurrency bug in webkit2gtk as the usual ways of
/// fixing threading issues are not working. Ideally, the locks are not needed if we can understand
/// the true cause of the bug.
#[derive(Debug, Default)]
struct WebviewUriLoader {
lock: AtomicBool,
queue: Mutex<VecDeque<(Rc<WebView>, Url)>>,
}

impl WebviewUriLoader {
/// Check if the lock is in use.
fn is_locked(&self) -> bool {
self.lock.swap(true, SeqCst)
}

/// Unlock the lock.
fn unlock(&self) {
self.lock.store(false, SeqCst)
}

/// Add a [`WebView`] to the queue.
fn push(&self, webview: Rc<WebView>, url: Url) {
let mut queue = self.queue.lock().expect("poisoned load queue");
queue.push_back((webview, url))
}

/// Remove a [`WebView`] from the queue and return it.
fn pop(&self) -> Option<(Rc<WebView>, Url)> {
let mut queue = self.queue.lock().expect("poisoned load queue");
queue.pop_front()
}

/// Load the next uri to load if the lock is not engaged.
fn flush(self: Rc<Self>) {
if !self.is_locked() {
if let Some((webview, url)) = self.pop() {
// we do not need to listen to failed events because those will finish the change event anyways
webview.connect_load_changed(move |_, event| {
if let LoadEvent::Finished = event {
self.unlock();
Rc::clone(&self).flush();
};
});

webview.load_uri(url.as_str());
}
}
}
}
}
Loading