From 8b32f4ae7ac518233fa876a7ed84c93caea11535 Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Sun, 7 Jan 2024 10:35:45 -0500 Subject: [PATCH] LibWebView+WebContent: Let the WebView client broadcast when it painted When the WebContent process has painted to its shared bitmaps, it sends a synchronous IPC to the browser process to let the chrome paint. It is synchronous to ensure the WC process doesn't paint onto the backing bitmap again while it is being displayed. However, this can cause a crash at exit if the browser process quits while the WC process is waiting for a response to this IPC. This patch makes the painting logic asynchronous by letting the browser process broadcast when it has finished handling the paint IPC. The WC process will not paint anything again until it receives that message. If it had tried to repaint while waiting for that message, that paint will be deferred until it arrives. --- .../Libraries/LibWebView/ViewImplementation.cpp | 2 ++ .../WebContent/ConnectionFromClient.cpp | 5 +++++ .../Services/WebContent/ConnectionFromClient.h | 1 + Userland/Services/WebContent/PageClient.cpp | 17 ++++++++++++++++- Userland/Services/WebContent/PageClient.h | 9 +++++++++ .../Services/WebContent/WebContentClient.ipc | 2 +- .../Services/WebContent/WebContentServer.ipc | 1 + 7 files changed, 35 insertions(+), 2 deletions(-) diff --git a/Userland/Libraries/LibWebView/ViewImplementation.cpp b/Userland/Libraries/LibWebView/ViewImplementation.cpp index 620440f920..2fd611d0e1 100644 --- a/Userland/Libraries/LibWebView/ViewImplementation.cpp +++ b/Userland/Libraries/LibWebView/ViewImplementation.cpp @@ -58,6 +58,8 @@ void ViewImplementation::server_did_paint(Badge, i32 bitmap_id if (on_ready_to_paint) on_ready_to_paint(); } + + client().async_ready_to_paint(); } void ViewImplementation::load(AK::URL const& url) diff --git a/Userland/Services/WebContent/ConnectionFromClient.cpp b/Userland/Services/WebContent/ConnectionFromClient.cpp index 10ab012e7e..177ce89314 100644 --- a/Userland/Services/WebContent/ConnectionFromClient.cpp +++ b/Userland/Services/WebContent/ConnectionFromClient.cpp @@ -148,6 +148,11 @@ void ConnectionFromClient::add_backing_store(i32 front_bitmap_id, Gfx::Shareable m_backing_stores.back_bitmap = *const_cast(back_bitmap).bitmap(); } +void ConnectionFromClient::ready_to_paint() +{ + page().ready_to_paint(); +} + void ConnectionFromClient::process_next_input_event() { if (m_input_event_queue.is_empty()) diff --git a/Userland/Services/WebContent/ConnectionFromClient.h b/Userland/Services/WebContent/ConnectionFromClient.h index be4ebb8123..7b84d94171 100644 --- a/Userland/Services/WebContent/ConnectionFromClient.h +++ b/Userland/Services/WebContent/ConnectionFromClient.h @@ -69,6 +69,7 @@ private: virtual void key_down(i32, unsigned, u32) override; virtual void key_up(i32, unsigned, u32) override; virtual void add_backing_store(i32 front_bitmap_id, Gfx::ShareableBitmap const& front_bitmap, i32 back_bitmap_id, Gfx::ShareableBitmap const& back_bitmap) override; + virtual void ready_to_paint() override; virtual void debug_request(ByteString const&, ByteString const&) override; virtual void get_source() override; virtual void inspect_dom_tree() override; diff --git a/Userland/Services/WebContent/PageClient.cpp b/Userland/Services/WebContent/PageClient.cpp index d57a6c78ef..95e2af6ecd 100644 --- a/Userland/Services/WebContent/PageClient.cpp +++ b/Userland/Services/WebContent/PageClient.cpp @@ -64,7 +64,9 @@ PageClient::PageClient(PageHost& owner, u64 id) auto& backing_stores = client().backing_stores(); swap(backing_stores.front_bitmap, backing_stores.back_bitmap); swap(backing_stores.front_bitmap_id, backing_stores.back_bitmap_id); - client().did_paint(viewport_rect.to_type(), backing_stores.front_bitmap_id); + + m_paint_state = PaintState::WaitingForClient; + client().async_did_paint(viewport_rect.to_type(), backing_stores.front_bitmap_id); }); #ifdef HAS_ACCELERATED_GRAPHICS @@ -76,10 +78,23 @@ PageClient::PageClient(PageHost& owner, u64 id) void PageClient::schedule_repaint() { + if (m_paint_state != PaintState::Ready) { + m_paint_state = PaintState::PaintWhenReady; + return; + } + if (!m_repaint_timer->is_active()) m_repaint_timer->start(); } +void PageClient::ready_to_paint() +{ + auto old_paint_state = exchange(m_paint_state, PaintState::Ready); + + if (old_paint_state == PaintState::PaintWhenReady) + schedule_repaint(); +} + void PageClient::visit_edges(JS::Cell::Visitor& visitor) { Base::visit_edges(visitor); diff --git a/Userland/Services/WebContent/PageClient.h b/Userland/Services/WebContent/PageClient.h index 1a394fc44e..e6d2350453 100644 --- a/Userland/Services/WebContent/PageClient.h +++ b/Userland/Services/WebContent/PageClient.h @@ -65,6 +65,8 @@ public: void set_user_style(String source); + void ready_to_paint(); + private: PageClient(PageHost&, u64 id); @@ -149,6 +151,13 @@ private: bool m_should_show_line_box_borders { false }; bool m_has_focus { false }; + enum class PaintState { + Ready, + WaitingForClient, + PaintWhenReady, + }; + + PaintState m_paint_state { PaintState::Ready }; RefPtr m_repaint_timer; Web::CSS::PreferredColorScheme m_preferred_color_scheme { Web::CSS::PreferredColorScheme::Auto }; diff --git a/Userland/Services/WebContent/WebContentClient.ipc b/Userland/Services/WebContent/WebContentClient.ipc index eb7cdf9b05..a547283777 100644 --- a/Userland/Services/WebContent/WebContentClient.ipc +++ b/Userland/Services/WebContent/WebContentClient.ipc @@ -17,7 +17,7 @@ endpoint WebContentClient did_request_navigate_back() =| did_request_navigate_forward() =| did_request_refresh() =| - did_paint(Gfx::IntRect content_rect, i32 bitmap_id) => () + did_paint(Gfx::IntRect content_rect, i32 bitmap_id) =| did_request_cursor_change(i32 cursor_type) =| did_layout(Gfx::IntSize content_size) =| did_change_title(ByteString title) =| diff --git a/Userland/Services/WebContent/WebContentServer.ipc b/Userland/Services/WebContent/WebContentServer.ipc index 0e35e1d5cb..d76b5c698b 100644 --- a/Userland/Services/WebContent/WebContentServer.ipc +++ b/Userland/Services/WebContent/WebContentServer.ipc @@ -24,6 +24,7 @@ endpoint WebContentServer load_html(ByteString html) =| add_backing_store(i32 front_bitmap_id, Gfx::ShareableBitmap front_bitmap, i32 back_bitmap_id, Gfx::ShareableBitmap back_bitmap) =| + ready_to_paint() =| set_viewport_rect(Web::DevicePixelRect rect) =|