gpui(windows): Move jumplist creation to background thread (#47956)

Lukas Wirth created

This can block the main thread too much otherwise

Release Notes:

- N/A *or* Added/Fixed/Improved ...

Change summary

crates/gpui/src/app.rs                               |  2 
crates/gpui/src/platform.rs                          |  4 
crates/gpui/src/platform/windows/destination_list.rs | 38 +++++----
crates/gpui/src/platform/windows/platform.rs         | 31 ++++++-
crates/workspace/src/history_manager.rs              | 53 +++++++------
5 files changed, 77 insertions(+), 51 deletions(-)

Detailed changes

crates/gpui/src/app.rs 🔗

@@ -2010,7 +2010,7 @@ impl App {
         &self,
         menus: Vec<MenuItem>,
         entries: Vec<SmallVec<[PathBuf; 2]>>,
-    ) -> Vec<SmallVec<[PathBuf; 2]>> {
+    ) -> Task<Vec<SmallVec<[PathBuf; 2]>>> {
         self.platform.update_jump_list(menus, entries)
     }
 

crates/gpui/src/platform.rs 🔗

@@ -254,8 +254,8 @@ pub(crate) trait Platform: 'static {
         &self,
         _menus: Vec<MenuItem>,
         _entries: Vec<SmallVec<[PathBuf; 2]>>,
-    ) -> Vec<SmallVec<[PathBuf; 2]>> {
-        Vec::new()
+    ) -> Task<Vec<SmallVec<[PathBuf; 2]>>> {
+        Task::ready(Vec::new())
     }
     fn on_app_menu_action(&self, callback: Box<dyn FnMut(&dyn Action)>);
     fn on_will_open_app_menu(&self, callback: Box<dyn FnMut()>);

crates/gpui/src/platform/windows/destination_list.rs 🔗

@@ -1,4 +1,4 @@
-use std::path::PathBuf;
+use std::{path::PathBuf, sync::Arc};
 
 use itertools::Itertools;
 use smallvec::SmallVec;
@@ -20,25 +20,25 @@ use windows::{
     core::{GUID, HSTRING, Interface},
 };
 
-use crate::{Action, MenuItem};
+use crate::{Action, MenuItem, SharedString};
 
 pub(crate) struct JumpList {
     pub(crate) dock_menus: Vec<DockMenuItem>,
-    pub(crate) recent_workspaces: Vec<SmallVec<[PathBuf; 2]>>,
+    pub(crate) recent_workspaces: Arc<[SmallVec<[PathBuf; 2]>]>,
 }
 
 impl JumpList {
     pub(crate) fn new() -> Self {
         Self {
-            dock_menus: Vec::new(),
-            recent_workspaces: Vec::new(),
+            dock_menus: Vec::default(),
+            recent_workspaces: Arc::default(),
         }
     }
 }
 
 pub(crate) struct DockMenuItem {
-    pub(crate) name: String,
-    pub(crate) description: String,
+    pub(crate) name: SharedString,
+    pub(crate) description: SharedString,
     pub(crate) action: Box<dyn Action>,
 }
 
@@ -46,11 +46,11 @@ impl DockMenuItem {
     pub(crate) fn new(item: MenuItem) -> anyhow::Result<Self> {
         match item {
             MenuItem::Action { name, action, .. } => Ok(Self {
-                name: name.clone().into(),
+                name: name.clone(),
                 description: if name == "New Window" {
-                    "Opens a new window".to_string()
+                    "Opens a new window".into()
                 } else {
-                    name.into()
+                    name
                 },
                 action,
             }),
@@ -62,11 +62,12 @@ impl DockMenuItem {
 // This code is based on the example from Microsoft:
 // https://github.com/microsoft/Windows-classic-samples/blob/main/Samples/Win7Samples/winui/shell/appshellintegration/RecipePropertyHandler/RecipePropertyHandler.cpp
 pub(crate) fn update_jump_list(
-    jump_list: &JumpList,
+    recent_workspaces: &[SmallVec<[PathBuf; 2]>],
+    dock_menus: &[(SharedString, SharedString)],
 ) -> anyhow::Result<Vec<SmallVec<[PathBuf; 2]>>> {
     let (list, removed) = create_destination_list()?;
-    add_recent_folders(&list, &jump_list.recent_workspaces, removed.as_ref())?;
-    add_dock_menu(&list, &jump_list.dock_menus)?;
+    add_recent_folders(&list, recent_workspaces, removed.as_ref())?;
+    add_dock_menu(&list, dock_menus)?;
     unsafe { list.CommitList() }?;
     Ok(removed)
 }
@@ -110,14 +111,17 @@ fn create_destination_list() -> anyhow::Result<(ICustomDestinationList, Vec<Smal
     Ok((list, removed))
 }
 
-fn add_dock_menu(list: &ICustomDestinationList, dock_menus: &[DockMenuItem]) -> anyhow::Result<()> {
+fn add_dock_menu(
+    list: &ICustomDestinationList,
+    dock_menus: &[(SharedString, SharedString)],
+) -> anyhow::Result<()> {
     unsafe {
         let tasks: IObjectCollection =
             CoCreateInstance(&EnumerableObjectCollection, None, CLSCTX_INPROC_SERVER)?;
-        for (idx, dock_menu) in dock_menus.iter().enumerate() {
+        for (idx, (name, description)) in dock_menus.iter().enumerate() {
             let argument = HSTRING::from(format!("--dock-action {}", idx));
-            let description = HSTRING::from(dock_menu.description.as_str());
-            let display = dock_menu.name.as_str();
+            let description = HSTRING::from(description.as_str());
+            let display = name.as_str();
             let task = create_shell_link(argument, description, None, display)?;
             tasks.AddObject(&task)?;
         }

crates/gpui/src/platform/windows/platform.rs 🔗

@@ -240,14 +240,25 @@ impl WindowsPlatform {
             }
         });
         self.inner.state.jump_list.borrow_mut().dock_menus = actions;
-        update_jump_list(&self.inner.state.jump_list.borrow()).log_err();
+        let borrow = self.inner.state.jump_list.borrow();
+        let dock_menus = borrow
+            .dock_menus
+            .iter()
+            .map(|menu| (menu.name.clone(), menu.description.clone()))
+            .collect::<Vec<_>>();
+        let recent_workspaces = borrow.recent_workspaces.clone();
+        self.background_executor
+            .spawn(async move {
+                update_jump_list(&recent_workspaces, &dock_menus).log_err();
+            })
+            .detach();
     }
 
     fn update_jump_list(
         &self,
         menus: Vec<MenuItem>,
         entries: Vec<SmallVec<[PathBuf; 2]>>,
-    ) -> Vec<SmallVec<[PathBuf; 2]>> {
+    ) -> Task<Vec<SmallVec<[PathBuf; 2]>>> {
         let mut actions = Vec::new();
         menus.into_iter().for_each(|menu| {
             if let Some(dock_menu) = DockMenuItem::new(menu).log_err() {
@@ -256,8 +267,18 @@ impl WindowsPlatform {
         });
         let mut jump_list = self.inner.state.jump_list.borrow_mut();
         jump_list.dock_menus = actions;
-        jump_list.recent_workspaces = entries;
-        update_jump_list(&jump_list).log_err().unwrap_or_default()
+        jump_list.recent_workspaces = entries.into();
+        let dock_menus = jump_list
+            .dock_menus
+            .iter()
+            .map(|menu| (menu.name.clone(), menu.description.clone()))
+            .collect::<Vec<_>>();
+        let recent_workspaces = jump_list.recent_workspaces.clone();
+        self.background_executor.spawn(async move {
+            update_jump_list(&recent_workspaces, &dock_menus)
+                .log_err()
+                .unwrap_or_default()
+        })
     }
 
     fn find_current_active_window(&self) -> Option<HWND> {
@@ -760,7 +781,7 @@ impl Platform for WindowsPlatform {
         &self,
         menus: Vec<MenuItem>,
         entries: Vec<SmallVec<[PathBuf; 2]>>,
-    ) -> Vec<SmallVec<[PathBuf; 2]>> {
+    ) -> Task<Vec<SmallVec<[PathBuf; 2]>>> {
         self.update_jump_list(menus, entries)
     }
 }

crates/workspace/src/history_manager.rs 🔗

@@ -2,7 +2,7 @@ use std::path::PathBuf;
 
 use gpui::{AppContext, Entity, Global, MenuItem};
 use smallvec::SmallVec;
-use ui::App;
+use ui::{App, Context};
 use util::{ResultExt, paths::PathExt};
 
 use crate::{
@@ -71,7 +71,12 @@ impl HistoryManager {
         cx.set_global(GlobalHistoryManager(history_manager));
     }
 
-    pub fn update_history(&mut self, id: WorkspaceId, entry: HistoryManagerEntry, cx: &App) {
+    pub fn update_history(
+        &mut self,
+        id: WorkspaceId,
+        entry: HistoryManagerEntry,
+        cx: &mut Context<'_, HistoryManager>,
+    ) {
         if let Some(pos) = self.history.iter().position(|e| e.id == id) {
             self.history.remove(pos);
         }
@@ -79,7 +84,7 @@ impl HistoryManager {
         self.update_jump_list(cx);
     }
 
-    pub fn delete_history(&mut self, id: WorkspaceId, cx: &App) {
+    pub fn delete_history(&mut self, id: WorkspaceId, cx: &mut Context<'_, HistoryManager>) {
         let Some(pos) = self.history.iter().position(|e| e.id == id) else {
             return;
         };
@@ -87,7 +92,7 @@ impl HistoryManager {
         self.update_jump_list(cx);
     }
 
-    fn update_jump_list(&mut self, cx: &App) {
+    fn update_jump_list(&mut self, cx: &mut Context<'_, HistoryManager>) {
         let menus = vec![MenuItem::action("New Window", NewWindow)];
         let entries = self
             .history
@@ -96,29 +101,25 @@ impl HistoryManager {
             .map(|entry| entry.path.clone())
             .collect::<Vec<_>>();
         let user_removed = cx.update_jump_list(menus, entries);
-        self.remove_user_removed_workspaces(user_removed, cx);
-    }
-
-    pub fn remove_user_removed_workspaces(
-        &mut self,
-        user_removed: Vec<SmallVec<[PathBuf; 2]>>,
-        cx: &App,
-    ) {
-        if user_removed.is_empty() {
-            return;
-        }
-        let mut deleted_ids = Vec::new();
-        for idx in (0..self.history.len()).rev() {
-            if let Some(entry) = self.history.get(idx)
-                && user_removed.contains(&entry.path)
-            {
-                deleted_ids.push(entry.id);
-                self.history.remove(idx);
+        cx.spawn(async move |this, cx| {
+            let user_removed = user_removed.await;
+            if user_removed.is_empty() {
+                return;
             }
-        }
-        cx.spawn(async move |_| {
-            for id in deleted_ids.iter() {
-                WORKSPACE_DB.delete_workspace_by_id(*id).await.log_err();
+            let mut deleted_ids = Vec::new();
+            if let Ok(()) = this.update(cx, |this, _| {
+                for idx in (0..this.history.len()).rev() {
+                    if let Some(entry) = this.history.get(idx)
+                        && user_removed.contains(&entry.path)
+                    {
+                        deleted_ids.push(entry.id);
+                        this.history.remove(idx);
+                    }
+                }
+            }) {
+                for id in deleted_ids.iter() {
+                    WORKSPACE_DB.delete_workspace_by_id(*id).await.log_err();
+                }
             }
         })
         .detach();