From 364f44d7d82af7d696d50a05d8bd33cbe9e8cea7 Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Tue, 15 Nov 2022 15:49:36 -0500 Subject: [PATCH] LibWebView+WebContent: Wait for dialog responses without blocking IPC Currently, the WebContent process is completely blocked while waiting for a response to a dialog request. This patch allows the IPC event loop to continue executing while only blocking the HTML event loop. This will allow other processes like WebDriver to continue to operate on the WebContent process while a dialog is open. --- .../LibWebView/OutOfProcessWebView.cpp | 12 ++-- .../LibWebView/OutOfProcessWebView.h | 4 +- .../Libraries/LibWebView/ViewImplementation.h | 4 +- .../Libraries/LibWebView/WebContentClient.cpp | 8 +-- .../Libraries/LibWebView/WebContentClient.h | 4 +- .../WebContent/ConnectionFromClient.cpp | 15 ++++ .../WebContent/ConnectionFromClient.h | 4 ++ Userland/Services/WebContent/PageHost.cpp | 71 +++++++++++++++---- Userland/Services/WebContent/PageHost.h | 18 +++++ .../Services/WebContent/WebContentClient.ipc | 6 +- .../Services/WebContent/WebContentServer.ipc | 4 ++ 11 files changed, 118 insertions(+), 32 deletions(-) diff --git a/Userland/Libraries/LibWebView/OutOfProcessWebView.cpp b/Userland/Libraries/LibWebView/OutOfProcessWebView.cpp index 640b4109fc..2077f283a6 100644 --- a/Userland/Libraries/LibWebView/OutOfProcessWebView.cpp +++ b/Userland/Libraries/LibWebView/OutOfProcessWebView.cpp @@ -348,20 +348,22 @@ void OutOfProcessWebView::notify_server_did_request_image_context_menu(Badge, String const& message) { GUI::MessageBox::show(window(), message, "Alert"sv, GUI::MessageBox::Type::Information); + client().async_alert_closed(); } -bool OutOfProcessWebView::notify_server_did_request_confirm(Badge, String const& message) +void OutOfProcessWebView::notify_server_did_request_confirm(Badge, String const& message) { auto confirm_result = GUI::MessageBox::show(window(), message, "Confirm"sv, GUI::MessageBox::Type::Warning, GUI::MessageBox::InputType::OKCancel); - return confirm_result == GUI::Dialog::ExecResult::OK; + client().async_confirm_closed(confirm_result == GUI::Dialog::ExecResult::OK); } -String OutOfProcessWebView::notify_server_did_request_prompt(Badge, String const& message, String const& default_) +void OutOfProcessWebView::notify_server_did_request_prompt(Badge, String const& message, String const& default_) { String response { default_ }; if (GUI::InputBox::show(window(), response, message, "Prompt"sv) == GUI::InputBox::ExecResult::OK) - return response; - return {}; + client().async_prompt_closed(move(response)); + else + client().async_prompt_closed({}); } void OutOfProcessWebView::notify_server_did_get_source(const AK::URL& url, String const& source) diff --git a/Userland/Libraries/LibWebView/OutOfProcessWebView.h b/Userland/Libraries/LibWebView/OutOfProcessWebView.h index 8ee72fcad8..9d6370bff1 100644 --- a/Userland/Libraries/LibWebView/OutOfProcessWebView.h +++ b/Userland/Libraries/LibWebView/OutOfProcessWebView.h @@ -156,8 +156,8 @@ private: virtual void notify_server_did_request_link_context_menu(Badge, Gfx::IntPoint const&, const AK::URL&, String const& target, unsigned modifiers) override; virtual void notify_server_did_request_image_context_menu(Badge, Gfx::IntPoint const&, const AK::URL&, String const& target, unsigned modifiers, Gfx::ShareableBitmap const&) override; virtual void notify_server_did_request_alert(Badge, String const& message) override; - virtual bool notify_server_did_request_confirm(Badge, String const& message) override; - virtual String notify_server_did_request_prompt(Badge, String const& message, String const& default_) override; + virtual void notify_server_did_request_confirm(Badge, String const& message) override; + virtual void notify_server_did_request_prompt(Badge, String const& message, String const& default_) override; virtual void notify_server_did_get_source(const AK::URL& url, String const& source) override; virtual void notify_server_did_get_dom_tree(String const& dom_tree) override; virtual void notify_server_did_get_dom_node_properties(i32 node_id, String const& specified_style, String const& computed_style, String const& custom_properties, String const& node_box_sizing) override; diff --git a/Userland/Libraries/LibWebView/ViewImplementation.h b/Userland/Libraries/LibWebView/ViewImplementation.h index 4d72c53af6..43a41dacf7 100644 --- a/Userland/Libraries/LibWebView/ViewImplementation.h +++ b/Userland/Libraries/LibWebView/ViewImplementation.h @@ -42,8 +42,8 @@ public: virtual void notify_server_did_request_link_context_menu(Badge, Gfx::IntPoint const&, const AK::URL&, String const& target, unsigned modifiers) = 0; virtual void notify_server_did_request_image_context_menu(Badge, Gfx::IntPoint const&, const AK::URL&, String const& target, unsigned modifiers, Gfx::ShareableBitmap const&) = 0; virtual void notify_server_did_request_alert(Badge, String const& message) = 0; - virtual bool notify_server_did_request_confirm(Badge, String const& message) = 0; - virtual String notify_server_did_request_prompt(Badge, String const& message, String const& default_) = 0; + virtual void notify_server_did_request_confirm(Badge, String const& message) = 0; + virtual void notify_server_did_request_prompt(Badge, String const& message, String const& default_) = 0; virtual void notify_server_did_get_source(const AK::URL& url, String const& source) = 0; virtual void notify_server_did_get_dom_tree(String const& dom_tree) = 0; virtual void notify_server_did_get_dom_node_properties(i32 node_id, String const& specified_style, String const& computed_style, String const& custom_properties, String const& node_box_sizing) = 0; diff --git a/Userland/Libraries/LibWebView/WebContentClient.cpp b/Userland/Libraries/LibWebView/WebContentClient.cpp index bea1d30057..ec579542c8 100644 --- a/Userland/Libraries/LibWebView/WebContentClient.cpp +++ b/Userland/Libraries/LibWebView/WebContentClient.cpp @@ -181,14 +181,14 @@ void WebContentClient::did_request_alert(String const& message) m_view.notify_server_did_request_alert({}, message); } -Messages::WebContentClient::DidRequestConfirmResponse WebContentClient::did_request_confirm(String const& message) +void WebContentClient::did_request_confirm(String const& message) { - return m_view.notify_server_did_request_confirm({}, message); + m_view.notify_server_did_request_confirm({}, message); } -Messages::WebContentClient::DidRequestPromptResponse WebContentClient::did_request_prompt(String const& message, String const& default_) +void WebContentClient::did_request_prompt(String const& message, String const& default_) { - return m_view.notify_server_did_request_prompt({}, message, default_); + m_view.notify_server_did_request_prompt({}, message, default_); } void WebContentClient::did_change_favicon(Gfx::ShareableBitmap const& favicon) diff --git a/Userland/Libraries/LibWebView/WebContentClient.h b/Userland/Libraries/LibWebView/WebContentClient.h index 5f05a31c4a..7c53edea82 100644 --- a/Userland/Libraries/LibWebView/WebContentClient.h +++ b/Userland/Libraries/LibWebView/WebContentClient.h @@ -58,8 +58,8 @@ private: virtual void did_get_js_console_messages(i32 start_index, Vector const& message_types, Vector const& messages) override; virtual void did_change_favicon(Gfx::ShareableBitmap const&) override; virtual void did_request_alert(String const&) override; - virtual Messages::WebContentClient::DidRequestConfirmResponse did_request_confirm(String const&) override; - virtual Messages::WebContentClient::DidRequestPromptResponse did_request_prompt(String const&, String const&) override; + virtual void did_request_confirm(String const&) override; + virtual void did_request_prompt(String const&, String const&) override; virtual Messages::WebContentClient::DidRequestAllCookiesResponse did_request_all_cookies(AK::URL const&) override; virtual Messages::WebContentClient::DidRequestNamedCookieResponse did_request_named_cookie(AK::URL const&, String const&) override; virtual Messages::WebContentClient::DidRequestCookieResponse did_request_cookie(AK::URL const&, u8) override; diff --git a/Userland/Services/WebContent/ConnectionFromClient.cpp b/Userland/Services/WebContent/ConnectionFromClient.cpp index 5d31679859..420c432f5b 100644 --- a/Userland/Services/WebContent/ConnectionFromClient.cpp +++ b/Userland/Services/WebContent/ConnectionFromClient.cpp @@ -576,4 +576,19 @@ void ConnectionFromClient::set_system_visibility_state(bool visible) : Web::HTML::VisibilityState::Hidden); } +void ConnectionFromClient::alert_closed() +{ + m_page_host->alert_closed(); +} + +void ConnectionFromClient::confirm_closed(bool accepted) +{ + m_page_host->confirm_closed(accepted); +} + +void ConnectionFromClient::prompt_closed(String const& response) +{ + m_page_host->prompt_closed(response); +} + } diff --git a/Userland/Services/WebContent/ConnectionFromClient.h b/Userland/Services/WebContent/ConnectionFromClient.h index 3de21bb633..d6194eebb0 100644 --- a/Userland/Services/WebContent/ConnectionFromClient.h +++ b/Userland/Services/WebContent/ConnectionFromClient.h @@ -84,6 +84,10 @@ private: virtual void run_javascript(String const&) override; virtual void js_console_request_messages(i32) override; + virtual void alert_closed() override; + virtual void confirm_closed(bool accepted) override; + virtual void prompt_closed(String const& response) override; + virtual Messages::WebContentServer::TakeDocumentScreenshotResponse take_document_screenshot() override; virtual Messages::WebContentServer::GetLocalStorageEntriesResponse get_local_storage_entries() override; diff --git a/Userland/Services/WebContent/PageHost.cpp b/Userland/Services/WebContent/PageHost.cpp index 8e48db996d..f328109226 100644 --- a/Userland/Services/WebContent/PageHost.cpp +++ b/Userland/Services/WebContent/PageHost.cpp @@ -7,13 +7,17 @@ #include "PageHost.h" #include "ConnectionFromClient.h" +#include #include #include #include #include #include +#include +#include #include #include +#include #include #include #include @@ -233,33 +237,72 @@ void PageHost::page_did_request_link_context_menu(Gfx::IntPoint const& content_p m_client.async_did_request_link_context_menu(content_position, url, target, modifiers); } +template +static ResponseType spin_event_loop_until_dialog_closed(ConnectionFromClient& client, Optional& response, SourceLocation location = SourceLocation::current()) +{ + auto& event_loop = Web::HTML::current_settings_object().responsible_event_loop(); + + ScopeGuard guard { [&] { event_loop.set_execution_paused(false); } }; + event_loop.set_execution_paused(true); + + Web::Platform::EventLoopPlugin::the().spin_until([&]() { + return response.has_value() || !client.is_open(); + }); + + if (!client.is_open()) { + dbgln("WebContent client disconnected during {}. Exiting peacefully.", location.function_name()); + exit(0); + } + + return response.release_value(); +} + void PageHost::page_did_request_alert(String const& message) { - auto response = m_client.send_sync_but_allow_failure(message); - if (!response) { - dbgln("WebContent client disconnected during DidRequestAlert. Exiting peacefully."); - exit(0); + m_pending_dialog = PendingDialog::Alert; + m_client.async_did_request_alert(message); + + spin_event_loop_until_dialog_closed(m_client, m_pending_alert_response); +} + +void PageHost::alert_closed() +{ + if (m_pending_dialog == PendingDialog::Alert) { + m_pending_dialog = PendingDialog::None; + m_pending_alert_response = Empty {}; } } bool PageHost::page_did_request_confirm(String const& message) { - auto response = m_client.send_sync_but_allow_failure(message); - if (!response) { - dbgln("WebContent client disconnected during DidRequestConfirm. Exiting peacefully."); - exit(0); + m_pending_dialog = PendingDialog::Confirm; + m_client.async_did_request_confirm(message); + + return spin_event_loop_until_dialog_closed(m_client, m_pending_confirm_response); +} + +void PageHost::confirm_closed(bool accepted) +{ + if (m_pending_dialog == PendingDialog::Confirm) { + m_pending_dialog = PendingDialog::None; + m_pending_confirm_response = accepted; } - return response->take_result(); } String PageHost::page_did_request_prompt(String const& message, String const& default_) { - auto response = m_client.send_sync_but_allow_failure(message, default_); - if (!response) { - dbgln("WebContent client disconnected during DidRequestPrompt. Exiting peacefully."); - exit(0); + m_pending_dialog = PendingDialog::Prompt; + m_client.async_did_request_prompt(message, default_); + + return spin_event_loop_until_dialog_closed(m_client, m_pending_prompt_response); +} + +void PageHost::prompt_closed(String response) +{ + if (m_pending_dialog == PendingDialog::Prompt) { + m_pending_dialog = PendingDialog::None; + m_pending_prompt_response = move(response); } - return response->take_response(); } void PageHost::page_did_change_favicon(Gfx::Bitmap const& favicon) diff --git a/Userland/Services/WebContent/PageHost.h b/Userland/Services/WebContent/PageHost.h index f40222f958..15ba8fc6fc 100644 --- a/Userland/Services/WebContent/PageHost.h +++ b/Userland/Services/WebContent/PageHost.h @@ -43,6 +43,19 @@ public: ErrorOr connect_to_webdriver(String const& webdriver_ipc_path); + void alert_closed(); + void confirm_closed(bool accepted); + void prompt_closed(String response); + + enum class PendingDialog { + None, + Alert, + Confirm, + Prompt, + }; + bool has_pending_dialog() const { return m_pending_dialog != PendingDialog::None; } + PendingDialog pending_dialog() const { return m_pending_dialog; } + private: // ^PageClient virtual Gfx::Palette palette() const override; @@ -95,6 +108,11 @@ private: Web::CSS::PreferredColorScheme m_preferred_color_scheme { Web::CSS::PreferredColorScheme::Auto }; RefPtr m_webdriver; + + PendingDialog m_pending_dialog { PendingDialog::None }; + Optional m_pending_alert_response; + Optional m_pending_confirm_response; + Optional m_pending_prompt_response; }; } diff --git a/Userland/Services/WebContent/WebContentClient.ipc b/Userland/Services/WebContent/WebContentClient.ipc index fa770e8400..6fa9c350a6 100644 --- a/Userland/Services/WebContent/WebContentClient.ipc +++ b/Userland/Services/WebContent/WebContentClient.ipc @@ -29,9 +29,9 @@ endpoint WebContentClient did_request_context_menu(Gfx::IntPoint content_position) =| did_request_link_context_menu(Gfx::IntPoint content_position, URL url, String target, unsigned modifiers) =| did_request_image_context_menu(Gfx::IntPoint content_position, URL url, String target, unsigned modifiers, Gfx::ShareableBitmap bitmap) =| - did_request_alert(String message) => () - did_request_confirm(String message) => (bool result) - did_request_prompt(String message, String default_) => (String response) + did_request_alert(String message) =| + did_request_confirm(String message) =| + did_request_prompt(String message, String default_) =| did_get_source(URL url, String source) =| did_get_dom_tree(String dom_tree) =| did_get_dom_node_properties(i32 node_id, String specified_style, String computed_style, String custom_properties, String node_box_sizing_json) =| diff --git a/Userland/Services/WebContent/WebContentServer.ipc b/Userland/Services/WebContent/WebContentServer.ipc index 4e311de7cb..45d70f5356 100644 --- a/Userland/Services/WebContent/WebContentServer.ipc +++ b/Userland/Services/WebContent/WebContentServer.ipc @@ -65,4 +65,8 @@ endpoint WebContentServer handle_file_return(i32 error, Optional file, i32 request_id) =| set_system_visibility_state(bool visible) =| + + alert_closed() =| + confirm_closed(bool accepted) =| + prompt_closed(String response) =| }