From 8df7b4207888fcba94bfbc4a62c38442dcebb7bd Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Fri, 26 May 2023 19:48:45 +0200 Subject: [PATCH] FileSystemAccessServer: Don't transmit unused data The server cannot use these values anywhere, because this method always sets 'prompt = ShouldPrompt::No'. This saves a bunch of roundtrips for all clients that use FSAS to read abritrary files. --- Userland/Applications/ThemeEditor/main.cpp | 2 +- .../Libraries/LibFileSystemAccessClient/Client.cpp | 14 ++------------ .../ConnectionFromClient.cpp | 5 +++-- .../FileSystemAccessServer/ConnectionFromClient.h | 2 +- .../FileSystemAccessServer.ipc | 2 +- 5 files changed, 8 insertions(+), 17 deletions(-) diff --git a/Userland/Applications/ThemeEditor/main.cpp b/Userland/Applications/ThemeEditor/main.cpp index 63ea7b39d4..c6cdd086da 100644 --- a/Userland/Applications/ThemeEditor/main.cpp +++ b/Userland/Applications/ThemeEditor/main.cpp @@ -49,7 +49,7 @@ ErrorOr serenity_main(Main::Arguments arguments) auto main_widget = TRY(window->set_main_widget()); if (path.has_value()) { - // Note: This is deferred to ensure that the window has already popped and thus proper window stealing can be performed. + // Note: This is deferred to ensure that the window has already popped and any error dialog boxes would show up correctly. app->event_loop().deferred_invoke( [&window, &path, &main_widget]() { auto response = FileSystemAccessClient::Client::the().request_file_read_only_approved(window, path.value().to_deprecated_string()); diff --git a/Userland/Libraries/LibFileSystemAccessClient/Client.cpp b/Userland/Libraries/LibFileSystemAccessClient/Client.cpp index 4ddf2aa2f2..5987e4f8f2 100644 --- a/Userland/Libraries/LibFileSystemAccessClient/Client.cpp +++ b/Userland/Libraries/LibFileSystemAccessClient/Client.cpp @@ -28,21 +28,11 @@ Result Client::request_file_read_only_approved(GUI::Window* parent_window, Depre auto const id = get_new_id(); m_promises.set(id, RequestData { { Core::Promise::construct() }, parent_window, Core::File::OpenMode::Read }); - auto parent_window_server_client_id = GUI::ConnectionToWindowServer::the().expose_client_id(); - auto child_window_server_client_id = expose_window_server_client_id(); - auto parent_window_id = parent_window->window_id(); - - GUI::ConnectionToWindowServer::the().add_window_stealing_for_client(child_window_server_client_id, parent_window_id); - - ScopeGuard guard([parent_window_id, child_window_server_client_id] { - GUI::ConnectionToWindowServer::the().remove_window_stealing_for_client(child_window_server_client_id, parent_window_id); - }); - if (path.starts_with('/')) { - async_request_file_read_only_approved(id, parent_window_server_client_id, parent_window_id, path); + async_request_file_read_only_approved(id, path); } else { auto full_path = LexicalPath::join(TRY(FileSystem::current_working_directory()), path).string(); - async_request_file_read_only_approved(id, parent_window_server_client_id, parent_window_id, full_path); + async_request_file_read_only_approved(id, full_path); } return handle_promise(id); diff --git a/Userland/Services/FileSystemAccessServer/ConnectionFromClient.cpp b/Userland/Services/FileSystemAccessServer/ConnectionFromClient.cpp index 16e5872f7c..61c937e458 100644 --- a/Userland/Services/FileSystemAccessServer/ConnectionFromClient.cpp +++ b/Userland/Services/FileSystemAccessServer/ConnectionFromClient.cpp @@ -55,6 +55,7 @@ void ConnectionFromClient::request_file_handler(i32 request_id, i32 window_serve auto exe_path = FileSystem::real_path(exe_link).release_value_but_fixme_should_propagate_errors(); if (prompt == ShouldPrompt::Yes) { + VERIFY(window_server_client_id != -1 && parent_window_id != -1); auto exe_name = LexicalPath::basename(exe_path.to_deprecated_string()); auto text = String::formatted("Allow {} ({}) to {} \"{}\"?", exe_name, pid, access_string, path).release_value_but_fixme_should_propagate_errors(); auto result = GUI::MessageBox::try_show({}, window_server_client_id, parent_window_id, text, "File Permissions Requested"sv).release_value_but_fixme_should_propagate_errors(); @@ -87,9 +88,9 @@ void ConnectionFromClient::request_file_handler(i32 request_id, i32 window_serve } } -void ConnectionFromClient::request_file_read_only_approved(i32 request_id, i32 window_server_client_id, i32 parent_window_id, DeprecatedString const& path) +void ConnectionFromClient::request_file_read_only_approved(i32 request_id, DeprecatedString const& path) { - request_file_handler(request_id, window_server_client_id, parent_window_id, path, Core::File::OpenMode::Read, ShouldPrompt::No); + request_file_handler(request_id, -1, -1, path, Core::File::OpenMode::Read, ShouldPrompt::No); } void ConnectionFromClient::request_file(i32 request_id, i32 window_server_client_id, i32 parent_window_id, DeprecatedString const& path, Core::File::OpenMode requested_access) diff --git a/Userland/Services/FileSystemAccessServer/ConnectionFromClient.h b/Userland/Services/FileSystemAccessServer/ConnectionFromClient.h index 762b15c848..b8664eb1dc 100644 --- a/Userland/Services/FileSystemAccessServer/ConnectionFromClient.h +++ b/Userland/Services/FileSystemAccessServer/ConnectionFromClient.h @@ -28,7 +28,7 @@ public: private: explicit ConnectionFromClient(NonnullOwnPtr); - virtual void request_file_read_only_approved(i32, i32, i32, DeprecatedString const&) override; + virtual void request_file_read_only_approved(i32, DeprecatedString const&) override; virtual void request_file(i32, i32, i32, DeprecatedString const&, Core::File::OpenMode) override; virtual void prompt_open_file(i32, i32, i32, DeprecatedString const&, DeprecatedString const&, Core::File::OpenMode, Optional> const&) override; virtual void prompt_save_file(i32, i32, i32, DeprecatedString const&, DeprecatedString const&, DeprecatedString const&, Core::File::OpenMode) override; diff --git a/Userland/Services/FileSystemAccessServer/FileSystemAccessServer.ipc b/Userland/Services/FileSystemAccessServer/FileSystemAccessServer.ipc index 17a8863140..75b517d199 100644 --- a/Userland/Services/FileSystemAccessServer/FileSystemAccessServer.ipc +++ b/Userland/Services/FileSystemAccessServer/FileSystemAccessServer.ipc @@ -3,7 +3,7 @@ endpoint FileSystemAccessServer { - request_file_read_only_approved(i32 request_id, i32 window_server_client_id, i32 window_id, DeprecatedString path) =| + request_file_read_only_approved(i32 request_id, DeprecatedString path) =| request_file(i32 request_id, i32 window_server_client_id, i32 window_id, DeprecatedString path, Core::File::OpenMode requested_access) =| prompt_open_file(i32 request_id, i32 window_server_client_id, i32 window_id, DeprecatedString window_title, DeprecatedString path_to_view, Core::File::OpenMode requested_access, Optional> allowed_file_types) =| prompt_save_file(i32 request_id, i32 window_server_client_id, i32 window_id, DeprecatedString title, DeprecatedString ext, DeprecatedString path_to_view, Core::File::OpenMode requested_access) =|