Browse Source

refactor(core): handle dialog threading internally, closes #2223 (#2429)

* refactor(core): handle dialog threading internally, closes #2223

* thread spawn
Lucas Fernandes Nogueira 4 years ago
parent
commit
2088cd0f24

+ 6 - 0
.changes/dialog-thread-handling.md

@@ -0,0 +1,6 @@
+---
+"tauri": patch
+---
+
+Allow the `tauri::api::dialog` APIs to be executed on any thread.
+**Breaking change:** All dialog APIs now takes a closure instead of returning the response on the function call.

+ 61 - 36
core/tauri/src/api/dialog.rs

@@ -5,6 +5,29 @@
 #[cfg(any(dialog_open, dialog_save))]
 use std::path::{Path, PathBuf};
 
+#[cfg(not(target_os = "linux"))]
+macro_rules! run_dialog {
+  ($e:expr, $h: ident) => {{
+    std::thread::spawn(move || {
+      let response = $e;
+      $h(response);
+    });
+  }};
+}
+
+#[cfg(target_os = "linux")]
+macro_rules! run_dialog {
+  ($e:expr, $h: ident) => {{
+    std::thread::spawn(move || {
+      let context = glib::MainContext::default();
+      context.invoke_with_priority(glib::PRIORITY_HIGH, move || {
+        let response = $e;
+        $h(response);
+      });
+    });
+  }};
+}
+
 /// The file dialog builder.
 /// Constructs file picker dialogs that can select single/multiple files or directories.
 #[cfg(any(dialog_open, dialog_save))]
@@ -44,55 +67,57 @@ impl FileDialogBuilder {
   }
 
   /// Pick one file.
-  pub fn pick_file(self) -> Option<PathBuf> {
-    self.0.pick_file()
+  pub fn pick_file<F: FnOnce(Option<PathBuf>) + Send + 'static>(self, f: F) {
+    run_dialog!(self.0.pick_file(), f)
   }
 
   /// Pick multiple files.
-  pub fn pick_files(self) -> Option<Vec<PathBuf>> {
-    self.0.pick_files()
+  pub fn pick_files<F: FnOnce(Option<Vec<PathBuf>>) + Send + 'static>(self, f: F) {
+    run_dialog!(self.0.pick_files(), f)
   }
 
   /// Pick one folder.
-  pub fn pick_folder(self) -> Option<PathBuf> {
-    self.0.pick_folder()
+  pub fn pick_folder<F: FnOnce(Option<PathBuf>) + Send + 'static>(self, f: F) {
+    run_dialog!(self.0.pick_folder(), f)
   }
 
   /// Opens save file dialog.
-  pub fn save_file(self) -> Option<PathBuf> {
-    self.0.save_file()
+  pub fn save_file<F: FnOnce(Option<PathBuf>) + Send + 'static>(self, f: F) {
+    run_dialog!(self.0.save_file(), f)
   }
 }
 
-/// Response for the ask dialog
-#[derive(Debug, Clone, PartialEq, Eq)]
-pub enum AskResponse {
-  /// User confirmed.
-  Yes,
-  /// User denied.
-  No,
-}
-
-/// Displays a dialog with a message and an optional title with a "yes" and a "no" button
-pub fn ask(title: impl AsRef<str>, message: impl AsRef<str>) -> AskResponse {
-  match rfd::MessageDialog::new()
-    .set_title(title.as_ref())
-    .set_description(message.as_ref())
-    .set_buttons(rfd::MessageButtons::YesNo)
-    .set_level(rfd::MessageLevel::Info)
-    .show()
-  {
-    true => AskResponse::Yes,
-    false => AskResponse::No,
-  }
+/// Displays a dialog with a message and an optional title with a "yes" and a "no" button.
+pub fn ask<F: FnOnce(bool) + Send + 'static>(
+  title: impl AsRef<str>,
+  message: impl AsRef<str>,
+  f: F,
+) {
+  let title = title.as_ref().to_string();
+  let message = message.as_ref().to_string();
+  run_dialog!(
+    rfd::MessageDialog::new()
+      .set_title(&title)
+      .set_description(&message)
+      .set_buttons(rfd::MessageButtons::YesNo)
+      .set_level(rfd::MessageLevel::Info)
+      .show(),
+    f
+  )
 }
 
-/// Displays a message dialog
+/// Displays a message dialog.
 pub fn message(title: impl AsRef<str>, message: impl AsRef<str>) {
-  rfd::MessageDialog::new()
-    .set_title(title.as_ref())
-    .set_description(message.as_ref())
-    .set_buttons(rfd::MessageButtons::Ok)
-    .set_level(rfd::MessageLevel::Info)
-    .show();
+  let title = title.as_ref().to_string();
+  let message = message.as_ref().to_string();
+  let cb = |_| {};
+  run_dialog!(
+    rfd::MessageDialog::new()
+      .set_title(&title)
+      .set_description(&message)
+      .set_buttons(rfd::MessageButtons::Ok)
+      .set_level(rfd::MessageLevel::Info)
+      .show(),
+    cb
+  )
 }

+ 0 - 9
core/tauri/src/app.rs

@@ -483,18 +483,9 @@ impl<R: Runtime> App<R> {
     let updater_config = self.manager.config().tauri.updater.clone();
     let package_info = self.manager.package_info().clone();
 
-    #[cfg(not(target_os = "linux"))]
     crate::async_runtime::spawn(async move {
       updater::check_update_with_dialog(updater_config, package_info, window).await
     });
-
-    #[cfg(target_os = "linux")]
-    {
-      let context = glib::MainContext::default();
-      context.spawn_with_priority(glib::PRIORITY_HIGH, async move {
-        updater::check_update_with_dialog(updater_config, package_info, window).await
-      });
-    }
   }
 
   /// Listen updater events when dialog are disabled.

+ 0 - 13
core/tauri/src/endpoints.rs

@@ -111,25 +111,12 @@ impl Module {
           .and_then(|r| r.json)
           .map_err(InvokeError::from)
       }),
-      // on macOS, the dialog must run on another thread: https://github.com/rust-windowing/winit/issues/1779
-      // we do the same on Windows just to stay consistent with `tao` (and it also improves UX because of the event loop)
-      #[cfg(not(target_os = "linux"))]
       Self::Dialog(cmd) => resolver.respond_async(async move {
         cmd
           .run(window)
           .and_then(|r| r.json)
           .map_err(InvokeError::from)
       }),
-      // on Linux, the dialog must run on the rpc task.
-      #[cfg(target_os = "linux")]
-      Self::Dialog(cmd) => {
-        resolver.respond_closure(move || {
-          cmd
-            .run(window)
-            .and_then(|r| r.json)
-            .map_err(InvokeError::from)
-        });
-      }
       Self::Cli(cmd) => {
         if let Some(cli_config) = config.tauri.cli.clone() {
           resolver.respond_async(async move {

+ 18 - 13
core/tauri/src/endpoints/dialog.rs

@@ -6,13 +6,13 @@ use super::InvokeResponse;
 #[cfg(any(dialog_open, dialog_save))]
 use crate::api::dialog::FileDialogBuilder;
 use crate::{
-  api::dialog::{ask as ask_dialog, message as message_dialog, AskResponse},
+  api::dialog::{ask as ask_dialog, message as message_dialog},
   runtime::Runtime,
   Window,
 };
 use serde::Deserialize;
 
-use std::path::PathBuf;
+use std::{path::PathBuf, sync::mpsc::channel};
 
 #[allow(dead_code)]
 #[derive(Deserialize)]
@@ -175,14 +175,18 @@ pub fn open<R: Runtime>(
     let extensions: Vec<&str> = filter.extensions.iter().map(|s| &**s).collect();
     dialog_builder = dialog_builder.add_filter(filter.name, &extensions);
   }
-  let response = if options.directory {
-    dialog_builder.pick_folder().into()
+
+  let (tx, rx) = channel();
+
+  if options.directory {
+    dialog_builder.pick_folder(move |p| tx.send(p.into()).unwrap());
   } else if options.multiple {
-    dialog_builder.pick_files().into()
+    dialog_builder.pick_files(move |p| tx.send(p.into()).unwrap());
   } else {
-    dialog_builder.pick_file().into()
-  };
-  Ok(response)
+    dialog_builder.pick_file(move |p| tx.send(p.into()).unwrap());
+  }
+
+  Ok(rx.recv().unwrap())
 }
 
 /// Shows a save dialog.
@@ -204,13 +208,14 @@ pub fn save<R: Runtime>(
     let extensions: Vec<&str> = filter.extensions.iter().map(|s| &**s).collect();
     dialog_builder = dialog_builder.add_filter(filter.name, &extensions);
   }
-  Ok(dialog_builder.save_file().into())
+  let (tx, rx) = channel();
+  dialog_builder.save_file(move |p| tx.send(p).unwrap());
+  Ok(rx.recv().unwrap().into())
 }
 
 /// Shows a dialog with a yes/no question.
 pub fn ask(title: String, message: String) -> crate::Result<InvokeResponse> {
-  match ask_dialog(title, message) {
-    AskResponse::Yes => Ok(true.into()),
-    _ => Ok(false.into()),
-  }
+  let (tx, rx) = channel();
+  ask_dialog(title, message, move |m| tx.send(m).unwrap());
+  Ok(rx.recv().unwrap().into())
 }

+ 15 - 12
core/tauri/src/endpoints/notification.rs

@@ -105,20 +105,23 @@ pub fn request_permission(config: &Config, package_info: &PackageInfo) -> crate:
       PERMISSION_DENIED.to_string()
     });
   }
-  let answer = crate::api::dialog::ask(
+  let (tx, rx) = std::sync::mpsc::channel();
+  crate::api::dialog::ask(
     "Permissions",
     "This app wants to show notifications. Do you allow?",
+    move |answer| {
+      tx.send(answer).unwrap();
+    },
   );
-  match answer {
-    crate::api::dialog::AskResponse::Yes => {
-      settings.allow_notification = Some(true);
-      crate::settings::write_settings(config, package_info, settings)?;
-      Ok(PERMISSION_GRANTED.to_string())
-    }
-    crate::api::dialog::AskResponse::No => {
-      settings.allow_notification = Some(false);
-      crate::settings::write_settings(config, package_info, settings)?;
-      Ok(PERMISSION_DENIED.to_string())
-    }
+
+  let answer = rx.recv().unwrap();
+
+  settings.allow_notification = Some(answer);
+  crate::settings::write_settings(config, package_info, settings)?;
+
+  if answer {
+    Ok(PERMISSION_GRANTED.to_string())
+  } else {
+    Ok(PERMISSION_DENIED.to_string())
   }
 }

+ 22 - 32
core/tauri/src/updater/mod.rs

@@ -333,15 +333,13 @@ mod error;
 pub use self::error::Error;
 
 use crate::{
-  api::{
-    config::UpdaterConfig,
-    dialog::{ask, AskResponse},
-    process::restart,
-  },
+  api::{config::UpdaterConfig, dialog::ask, process::restart},
   runtime::Runtime,
   Window,
 };
 
+use std::sync::mpsc::channel;
+
 /// Check for new updates
 pub const EVENT_CHECK_UPDATE: &str = "tauri://update";
 /// New update available
@@ -526,9 +524,11 @@ async fn prompt_for_install(
   // remove single & double quote
   let escaped_body = body.replace(&['\"', '\''][..], "");
 
+  let (tx, rx) = channel();
+
   // todo(lemarier): We should review this and make sure we have
   // something more conventional.
-  let should_install = ask(
+  ask(
     format!(r#"A new version of {} is available! "#, app_name),
     format!(
       r#"{} {} is now available -- you have {}.
@@ -539,36 +539,26 @@ Release Notes:
 {}"#,
       app_name, updater.version, updater.current_version, escaped_body,
     ),
+    move |should_install| tx.send(should_install).unwrap(),
   );
 
-  match should_install {
-    AskResponse::Yes => {
-      // Launch updater download process
-      // macOS we display the `Ready to restart dialog` asking to restart
-      // Windows is closing the current App and launch the downloaded MSI when ready (the process stop here)
-      // Linux we replace the AppImage by launching a new install, it start a new AppImage instance, so we're closing the previous. (the process stop here)
-      updater.download_and_install(pubkey.clone()).await?;
-
-      // Ask user if we need to restart the application
-      let should_exit = ask(
-        "Ready to Restart",
-        "The installation was successful, do you want to restart the application now?",
-      );
-      match should_exit {
-        AskResponse::Yes => {
+  if rx.recv().unwrap() {
+    // Launch updater download process
+    // macOS we display the `Ready to restart dialog` asking to restart
+    // Windows is closing the current App and launch the downloaded MSI when ready (the process stop here)
+    // Linux we replace the AppImage by launching a new install, it start a new AppImage instance, so we're closing the previous. (the process stop here)
+    updater.download_and_install(pubkey.clone()).await?;
+
+    // Ask user if we need to restart the application
+    ask(
+      "Ready to Restart",
+      "The installation was successful, do you want to restart the application now?",
+      |should_exit| {
+        if should_exit {
           restart();
-          // safely exit even if the process
-          // should be killed
-          return Ok(());
-        }
-        AskResponse::No => {
-          // Do nothing -- maybe we can emit some event here
         }
-      }
-    }
-    AskResponse::No => {
-      // Do nothing -- maybe we can emit some event here
-    }
+      },
+    );
   }
 
   Ok(())

File diff suppressed because it is too large
+ 0 - 0
examples/api/public/build/bundle.js


File diff suppressed because it is too large
+ 0 - 0
examples/api/public/build/bundle.js.map


+ 8 - 4
examples/api/src-tauri/src/main.rs

@@ -15,8 +15,8 @@ use std::path::PathBuf;
 
 use serde::Serialize;
 use tauri::{
-  CustomMenuItem, Event, Manager, SystemTray, SystemTrayEvent, SystemTrayMenu, WindowBuilder,
-  WindowUrl,
+  api::dialog::ask, CustomMenuItem, Event, Manager, SystemTray, SystemTrayEvent, SystemTrayMenu,
+  WindowBuilder, WindowUrl,
 };
 
 #[derive(Serialize)]
@@ -160,8 +160,12 @@ fn main() {
   app.run(|app_handle, e| {
     if let Event::CloseRequested { label, api, .. } = e {
       api.prevent_close();
-      let window = app_handle.get_window(&label).unwrap();
-      window.emit("close-requested", ()).unwrap();
+      let app_handle = app_handle.clone();
+      ask("Tauri API", "Are you sure?", move |answer| {
+        if answer {
+          app_handle.get_window(&label).unwrap().close().unwrap();
+        }
+      });
     }
   })
 }

+ 0 - 7
examples/api/src/App.svelte

@@ -4,7 +4,6 @@
   import hotkeys from "hotkeys-js";
   import { open } from "@tauri-apps/api/shell";
   import { invoke } from "@tauri-apps/api/tauri";
-  import { appWindow, getCurrent } from "@tauri-apps/api/window";
 
   import Welcome from "./components/Welcome.svelte";
   import Cli from "./components/Cli.svelte";
@@ -25,12 +24,6 @@
     hotkeys(MENU_TOGGLE_HOTKEY, () => {
       invoke('menu_toggle');
     });
-
-    getCurrent().listen('close-requested', async () => {
-      if (await confirm('Are you sure?')) {
-        await appWindow.close()
-      }
-    })
   });
 
   const views = [

Some files were not shown because too many files changed in this diff