From 7323a54e59ccb23e82d9629c123f88d717bfe460 Mon Sep 17 00:00:00 2001
From: thankyouverycool <66646555+thankyouverycool@users.noreply.github.com>
Date: Sat, 13 May 2023 05:07:41 -0400
Subject: LibGUI+FileSystemAccessServer: Avoid using dummy windows

Creates two new gatekept helpers for FilePicker and MessageBox to be
used by FSAS to replace the "dummy window" approach to centering
Dialogs. There was a slight delay in creating two windows, one a
transparent intermediary hidden behind the second, to display FSAS
Dialogs. Now we only need to make the window we actually see.
---
 Userland/Libraries/LibGUI/FilePicker.cpp           | 21 +++++++++++++
 Userland/Libraries/LibGUI/FilePicker.h             |  8 +++++
 Userland/Libraries/LibGUI/MessageBox.cpp           | 13 +++++++++
 Userland/Libraries/LibGUI/MessageBox.h             |  7 +++++
 .../ConnectionFromClient.cpp                       | 34 ++++++----------------
 .../FileSystemAccessServer/ConnectionFromClient.h  |  1 -
 6 files changed, 58 insertions(+), 26 deletions(-)

diff --git a/Userland/Libraries/LibGUI/FilePicker.cpp b/Userland/Libraries/LibGUI/FilePicker.cpp
index 59927b525d..3a099939e6 100644
--- a/Userland/Libraries/LibGUI/FilePicker.cpp
+++ b/Userland/Libraries/LibGUI/FilePicker.cpp
@@ -13,6 +13,7 @@
 #include <LibGUI/BoxLayout.h>
 #include <LibGUI/Button.h>
 #include <LibGUI/CommonLocationsProvider.h>
+#include <LibGUI/ConnectionToWindowServer.h>
 #include <LibGUI/FileIconProvider.h>
 #include <LibGUI/FilePicker.h>
 #include <LibGUI/FilePickerDialogGML.h>
@@ -36,6 +37,26 @@
 
 namespace GUI {
 
+ErrorOr<Optional<String>> FilePicker::get_filepath(Badge<FileSystemAccessServer::ConnectionFromClient>, i32 window_server_client_id, i32 parent_window_id, Mode mode, StringView window_title, StringView file_basename, StringView path, Optional<Vector<FileTypeFilter>> allowed_file_types)
+{
+    auto picker = TRY(FilePicker::try_create(nullptr, mode, file_basename, path, ScreenPosition::DoNotPosition, move(allowed_file_types)));
+    auto parent_rect = ConnectionToWindowServer::the().get_window_rect_from_client(window_server_client_id, parent_window_id);
+    picker->center_within(parent_rect);
+    picker->constrain_to_desktop();
+    if (!window_title.is_empty())
+        picker->set_title(window_title);
+    picker->show();
+    ConnectionToWindowServer::the().set_window_parent_from_client(window_server_client_id, parent_window_id, picker->window_id());
+
+    if (picker->exec() == ExecResult::OK) {
+        auto file_path = TRY(String::from_deprecated_string(picker->selected_file()));
+        if (file_path.is_empty())
+            return Optional<String> {};
+        return file_path;
+    }
+    return Optional<String> {};
+}
+
 Optional<DeprecatedString> FilePicker::get_open_filepath(Window* parent_window, DeprecatedString const& window_title, StringView path, bool folder, ScreenPosition screen_position, Optional<Vector<FileTypeFilter>> allowed_file_types)
 {
     auto picker = FilePicker::construct(parent_window, folder ? Mode::OpenFolder : Mode::Open, ""sv, path, screen_position, move(allowed_file_types));
diff --git a/Userland/Libraries/LibGUI/FilePicker.h b/Userland/Libraries/LibGUI/FilePicker.h
index 0096877844..7a958a4e7f 100644
--- a/Userland/Libraries/LibGUI/FilePicker.h
+++ b/Userland/Libraries/LibGUI/FilePicker.h
@@ -16,6 +16,12 @@
 #include <LibGUI/ImageWidget.h>
 #include <LibGUI/Model.h>
 
+namespace FileSystemAccessServer {
+
+class ConnectionFromClient;
+
+}
+
 namespace GUI {
 
 class FilePicker final
@@ -34,6 +40,8 @@ public:
     static Optional<DeprecatedString> get_open_filepath(Window* parent_window, DeprecatedString const& window_title = {}, StringView path = Core::StandardPaths::home_directory(), bool folder = false, ScreenPosition screen_position = Dialog::ScreenPosition::CenterWithinParent, Optional<Vector<FileTypeFilter>> allowed_file_types = {});
     static Optional<DeprecatedString> get_save_filepath(Window* parent_window, DeprecatedString const& title, DeprecatedString const& extension, StringView path = Core::StandardPaths::home_directory(), ScreenPosition screen_position = Dialog::ScreenPosition::CenterWithinParent);
 
+    static ErrorOr<Optional<String>> get_filepath(Badge<FileSystemAccessServer::ConnectionFromClient>, i32 window_server_client_id, i32 parent_window_id, Mode, StringView window_title, StringView file_basename = {}, StringView path = Core::StandardPaths::home_directory(), Optional<Vector<FileTypeFilter>> = {});
+
     virtual ~FilePicker() override;
 
     DeprecatedString const& selected_file() const { return m_selected_file; }
diff --git a/Userland/Libraries/LibGUI/MessageBox.cpp b/Userland/Libraries/LibGUI/MessageBox.cpp
index a19f2753b2..6b40dc14c1 100644
--- a/Userland/Libraries/LibGUI/MessageBox.cpp
+++ b/Userland/Libraries/LibGUI/MessageBox.cpp
@@ -9,6 +9,7 @@
 #include <AK/NumberFormat.h>
 #include <LibGUI/BoxLayout.h>
 #include <LibGUI/Button.h>
+#include <LibGUI/ConnectionToWindowServer.h>
 #include <LibGUI/ImageWidget.h>
 #include <LibGUI/Label.h>
 #include <LibGUI/MessageBox.h>
@@ -40,6 +41,18 @@ ErrorOr<Dialog::ExecResult> MessageBox::try_show(Window* parent_window, StringVi
     return box->exec();
 }
 
+ErrorOr<Dialog::ExecResult> MessageBox::try_show(Badge<FileSystemAccessServer::ConnectionFromClient>, i32 window_server_client_id, i32 parent_window_id, StringView text, StringView title)
+{
+    auto box = TRY(MessageBox::create(nullptr, text, title, MessageBox::Type::Warning, MessageBox::InputType::YesNo));
+    auto parent_rect = ConnectionToWindowServer::the().get_window_rect_from_client(window_server_client_id, parent_window_id);
+    box->center_within(parent_rect);
+    box->constrain_to_desktop();
+    box->set_screen_position(ScreenPosition::DoNotPosition);
+    box->Dialog::show();
+    ConnectionToWindowServer::the().set_window_parent_from_client(window_server_client_id, parent_window_id, box->window_id());
+    return box->exec();
+}
+
 Dialog::ExecResult MessageBox::show_error(Window* parent_window, StringView text)
 {
     return MUST(try_show_error(parent_window, text));
diff --git a/Userland/Libraries/LibGUI/MessageBox.h b/Userland/Libraries/LibGUI/MessageBox.h
index 03a89eec9b..04e2f429db 100644
--- a/Userland/Libraries/LibGUI/MessageBox.h
+++ b/Userland/Libraries/LibGUI/MessageBox.h
@@ -10,6 +10,12 @@
 #include <AK/Time.h>
 #include <LibGUI/Dialog.h>
 
+namespace FileSystemAccessServer {
+
+class ConnectionFromClient;
+
+}
+
 namespace GUI {
 
 class MessageBox : public Dialog {
@@ -36,6 +42,7 @@ public:
     static ExecResult show_error(Window* parent_window, StringView text);
     static ExecResult ask_about_unsaved_changes(Window* parent_window, StringView path, Optional<Time> last_unmodified_timestamp = {});
 
+    static ErrorOr<ExecResult> try_show(Badge<FileSystemAccessServer::ConnectionFromClient>, i32 window_server_client_id, i32 parent_window_id, StringView text, StringView title);
     static ErrorOr<ExecResult> try_show(Window* parent_window, StringView text, StringView title, Type type = Type::None, InputType input_type = InputType::OK);
     static ErrorOr<ExecResult> try_show_error(Window* parent_window, StringView text);
     static ErrorOr<ExecResult> try_ask_about_unsaved_changes(Window* parent_window, StringView path, Optional<Time> last_unmodified_timestamp = {});
diff --git a/Userland/Services/FileSystemAccessServer/ConnectionFromClient.cpp b/Userland/Services/FileSystemAccessServer/ConnectionFromClient.cpp
index 020ea3415e..2b1710a5ea 100644
--- a/Userland/Services/FileSystemAccessServer/ConnectionFromClient.cpp
+++ b/Userland/Services/FileSystemAccessServer/ConnectionFromClient.cpp
@@ -27,20 +27,6 @@ void ConnectionFromClient::die()
     GUI::Application::the()->quit();
 }
 
-RefPtr<GUI::Window> ConnectionFromClient::create_dummy_child_window(i32 window_server_client_id, i32 parent_window_id)
-{
-    auto window = GUI::Window::construct();
-    window->set_opacity(0);
-    window->set_frameless(true);
-    window->set_window_mode(GUI::WindowMode::Passive);
-    auto rect = GUI::ConnectionToWindowServer::the().get_window_rect_from_client(window_server_client_id, parent_window_id);
-    window->set_rect(rect);
-    window->show();
-    GUI::ConnectionToWindowServer::the().set_window_parent_from_client(window_server_client_id, parent_window_id, window->window_id());
-
-    return window;
-}
-
 void ConnectionFromClient::request_file_handler(i32 request_id, i32 window_server_client_id, i32 parent_window_id, DeprecatedString const& path, Core::File::OpenMode requested_access, ShouldPrompt prompt)
 {
     VERIFY(path.starts_with("/"sv));
@@ -68,11 +54,10 @@ void ConnectionFromClient::request_file_handler(i32 request_id, i32 window_serve
         auto exe_link = LexicalPath("/proc").append(DeprecatedString::number(pid)).append("exe"sv).string();
         auto exe_path = Core::DeprecatedFile::real_path_for(exe_link);
 
-        auto main_window = create_dummy_child_window(window_server_client_id, parent_window_id);
-
         if (prompt == ShouldPrompt::Yes) {
             auto exe_name = LexicalPath::basename(exe_path);
-            auto result = GUI::MessageBox::show(main_window, DeprecatedString::formatted("Allow {} ({}) to {} \"{}\"?", exe_name, pid, access_string, path), "File Permissions Requested"sv, GUI::MessageBox::Type::Warning, GUI::MessageBox::InputType::YesNo);
+            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();
             approved = result == GUI::MessageBox::ExecResult::Yes;
         } else {
             approved = true;
@@ -117,11 +102,10 @@ void ConnectionFromClient::prompt_open_file(i32 request_id, i32 window_server_cl
     auto relevant_permissions = requested_access & (Core::File::OpenMode::Read | Core::File::OpenMode::Write);
     VERIFY(relevant_permissions != Core::File::OpenMode::NotOpen);
 
-    auto main_window = create_dummy_child_window(window_server_client_id, parent_window_id);
-
-    auto user_picked_file = GUI::FilePicker::get_open_filepath(main_window, window_title, path_to_view, false, GUI::Dialog::ScreenPosition::CenterWithinParent, allowed_file_types);
+    auto user_picked_file = GUI::FilePicker::get_filepath({}, window_server_client_id, parent_window_id, GUI::FilePicker::Mode::Open, window_title, {}, path_to_view, allowed_file_types).release_value_but_fixme_should_propagate_errors();
+    auto user_picked_file_but_fixme_should_use_string = user_picked_file.has_value() ? user_picked_file.release_value().to_deprecated_string() : Optional<DeprecatedString> {};
 
-    prompt_helper(request_id, user_picked_file, requested_access);
+    prompt_helper(request_id, user_picked_file_but_fixme_should_use_string, requested_access);
 }
 
 void ConnectionFromClient::prompt_save_file(i32 request_id, i32 window_server_client_id, i32 parent_window_id, DeprecatedString const& name, DeprecatedString const& ext, DeprecatedString const& path_to_view, Core::File::OpenMode requested_access)
@@ -129,11 +113,11 @@ void ConnectionFromClient::prompt_save_file(i32 request_id, i32 window_server_cl
     auto relevant_permissions = requested_access & (Core::File::OpenMode::Read | Core::File::OpenMode::Write);
     VERIFY(relevant_permissions != Core::File::OpenMode::NotOpen);
 
-    auto main_window = create_dummy_child_window(window_server_client_id, parent_window_id);
-
-    auto user_picked_file = GUI::FilePicker::get_save_filepath(main_window, name, ext, path_to_view);
+    auto basename = String::formatted("{}.{}", name, ext).release_value_but_fixme_should_propagate_errors();
+    auto user_picked_file = GUI::FilePicker::get_filepath({}, window_server_client_id, parent_window_id, GUI::FilePicker::Mode::Save, {}, basename, path_to_view).release_value_but_fixme_should_propagate_errors();
+    auto user_picked_file_but_fixme_should_use_string = user_picked_file.has_value() ? user_picked_file.release_value().to_deprecated_string() : Optional<DeprecatedString> {};
 
-    prompt_helper(request_id, user_picked_file, requested_access);
+    prompt_helper(request_id, user_picked_file_but_fixme_should_use_string, requested_access);
 }
 
 void ConnectionFromClient::prompt_helper(i32 request_id, Optional<DeprecatedString> const& user_picked_file, Core::File::OpenMode requested_access)
diff --git a/Userland/Services/FileSystemAccessServer/ConnectionFromClient.h b/Userland/Services/FileSystemAccessServer/ConnectionFromClient.h
index 0279d0cec9..762b15c848 100644
--- a/Userland/Services/FileSystemAccessServer/ConnectionFromClient.h
+++ b/Userland/Services/FileSystemAccessServer/ConnectionFromClient.h
@@ -34,7 +34,6 @@ private:
     virtual void prompt_save_file(i32, i32, i32, DeprecatedString const&, DeprecatedString const&, DeprecatedString const&, Core::File::OpenMode) override;
 
     void prompt_helper(i32, Optional<DeprecatedString> const&, Core::File::OpenMode);
-    RefPtr<GUI::Window> create_dummy_child_window(i32, i32);
 
     enum class ShouldPrompt {
         No,
-- 
cgit v1.2.3