From 4c1d4aa678fc8a7099f5cab468a88496eea47cc9 Mon Sep 17 00:00:00 2001 From: Johan Dahlin Date: Tue, 19 Nov 2024 16:12:29 +0100 Subject: [PATCH] Tests: Wait for fonts before completing tests (WIP) --- Libraries/LibWeb/CSS/FontFaceSet.cpp | 15 ++++-- Libraries/LibWeb/CSS/FontFaceSet.h | 9 ++-- Libraries/LibWeb/DOM/Document.cpp | 2 +- Libraries/LibWeb/HTML/WorkerGlobalScope.cpp | 2 +- Libraries/LibWeb/Page/Page.cpp | 8 +++ Libraries/LibWeb/Page/Page.h | 3 ++ Libraries/LibWebView/ViewImplementation.h | 4 +- Libraries/LibWebView/WebContentClient.cpp | 9 ++++ Libraries/LibWebView/WebContentClient.h | 1 + Services/WebContent/ConnectionFromClient.cpp | 1 + Services/WebContent/PageClient.cpp | 6 +++ Services/WebContent/PageClient.h | 1 + Services/WebContent/WebContentClient.ipc | 2 +- .../css-fonts/font-variant-numeric-ref.html | 7 ++- .../css/css-fonts/font-variant-numeric.html | 6 ++- UI/Headless/Test.cpp | 49 ++++++++++++++++++- UI/Headless/Test.h | 6 ++- 17 files changed, 112 insertions(+), 19 deletions(-) diff --git a/Libraries/LibWeb/CSS/FontFaceSet.cpp b/Libraries/LibWeb/CSS/FontFaceSet.cpp index 254cb3dfa3d..a68cd2774de 100644 --- a/Libraries/LibWeb/CSS/FontFaceSet.cpp +++ b/Libraries/LibWeb/CSS/FontFaceSet.cpp @@ -28,7 +28,7 @@ namespace Web::CSS { GC_DEFINE_ALLOCATOR(FontFaceSet); // https://drafts.csswg.org/css-font-loading/#dom-fontfaceset-fontfaceset -GC::Ref FontFaceSet::construct_impl(JS::Realm& realm, Vector> const& initial_faces) +GC::Ref FontFaceSet::construct_impl(Web::Page& page, JS::Realm& realm, Vector> const& initial_faces) { auto ready_promise = WebIDL::create_promise(realm); auto set_entries = JS::Set::create(realm); @@ -37,16 +37,17 @@ GC::Ref FontFaceSet::construct_impl(JS::Realm& realm, Vectorset_add(face); - return realm.create(realm, ready_promise, set_entries); + return realm.create(page, realm, ready_promise, set_entries); } -GC::Ref FontFaceSet::create(JS::Realm& realm) +GC::Ref FontFaceSet::create(Web::Page& page, JS::Realm& realm) { - return construct_impl(realm, {}); + return construct_impl(page, realm, {}); } -FontFaceSet::FontFaceSet(JS::Realm& realm, GC::Ref ready_promise, GC::Ref set_entries) +FontFaceSet::FontFaceSet(Web::Page& page, JS::Realm& realm, GC::Ref ready_promise, GC::Ref set_entries) : DOM::EventTarget(realm) + , m_page(page) , m_set_entries(set_entries) , m_ready_promise(ready_promise) , m_status(Bindings::FontFaceSetLoadStatus::Loaded) @@ -63,6 +64,7 @@ void FontFaceSet::initialize(JS::Realm& realm) void FontFaceSet::visit_edges(Cell::Visitor& visitor) { Base::visit_edges(visitor); + visitor.visit(m_page); visitor.visit(m_set_entries); visitor.visit(m_ready_promise); visitor.visit(m_loading_fonts); @@ -289,7 +291,10 @@ GC::Ref FontFaceSet::ready() const void FontFaceSet::resolve_ready_promise() { + dbgln("FontFaceSet::resolve_ready_promise(): {}", m_page->url()); WebIDL::resolve_promise(realm(), m_ready_promise); + + m_page->client().page_did_finish_loading_page_and_fonts(m_page->url()); } } diff --git a/Libraries/LibWeb/CSS/FontFaceSet.h b/Libraries/LibWeb/CSS/FontFaceSet.h index 1c84ed10f91..f1f19dc3f84 100644 --- a/Libraries/LibWeb/CSS/FontFaceSet.h +++ b/Libraries/LibWeb/CSS/FontFaceSet.h @@ -13,6 +13,8 @@ #include #include #include +#include +#include namespace Web::CSS { @@ -21,8 +23,8 @@ class FontFaceSet final : public DOM::EventTarget { GC_DECLARE_ALLOCATOR(FontFaceSet); public: - [[nodiscard]] static GC::Ref construct_impl(JS::Realm&, Vector> const& initial_faces); - [[nodiscard]] static GC::Ref create(JS::Realm&); + [[nodiscard]] static GC::Ref construct_impl(Web::Page&, JS::Realm&, Vector> const& initial_faces); + [[nodiscard]] static GC::Ref create(Web::Page&, JS::Realm&); virtual ~FontFaceSet() override = default; GC::Ref set_entries() const { return m_set_entries; } @@ -46,11 +48,12 @@ public: void resolve_ready_promise(); private: - FontFaceSet(JS::Realm&, GC::Ref ready_promise, GC::Ref set_entries); + FontFaceSet(Web::Page&, JS::Realm&, GC::Ref ready_promise, GC::Ref set_entries); virtual void initialize(JS::Realm&) override; virtual void visit_edges(Cell::Visitor&) override; + GC::Ref m_page; GC::Ref m_set_entries; GC::Ref m_ready_promise; // [[ReadyPromise]] diff --git a/Libraries/LibWeb/DOM/Document.cpp b/Libraries/LibWeb/DOM/Document.cpp index eefd4d6ab00..2bec4f097ff 100644 --- a/Libraries/LibWeb/DOM/Document.cpp +++ b/Libraries/LibWeb/DOM/Document.cpp @@ -1606,7 +1606,7 @@ GC::Ref Document::all() GC::Ref Document::fonts() { if (!m_fonts) - m_fonts = CSS::FontFaceSet::create(realm()); + m_fonts = CSS::FontFaceSet::create(page(), realm()); return *m_fonts; } diff --git a/Libraries/LibWeb/HTML/WorkerGlobalScope.cpp b/Libraries/LibWeb/HTML/WorkerGlobalScope.cpp index 198830a9072..9bdc6154e87 100644 --- a/Libraries/LibWeb/HTML/WorkerGlobalScope.cpp +++ b/Libraries/LibWeb/HTML/WorkerGlobalScope.cpp @@ -160,7 +160,7 @@ ENUMERATE_WORKER_GLOBAL_SCOPE_EVENT_HANDLERS(__ENUMERATE) GC::Ref WorkerGlobalScope::fonts() { if (!m_fonts) - m_fonts = CSS::FontFaceSet::create(realm()); + m_fonts = CSS::FontFaceSet::create(*page(), realm()); return *m_fonts; } diff --git a/Libraries/LibWeb/Page/Page.cpp b/Libraries/LibWeb/Page/Page.cpp index 768475e4bc3..c9795cdbb60 100644 --- a/Libraries/LibWeb/Page/Page.cpp +++ b/Libraries/LibWeb/Page/Page.cpp @@ -578,6 +578,14 @@ Vector> Page::documents_in_active_window() const return documents; } +URL::URL Page::url() const +{ + if (!top_level_traversable_is_initialized()) + return {}; + + return top_level_traversable()->active_document()->url(); +} + void Page::clear_selection() { for (auto const& document : documents_in_active_window()) { diff --git a/Libraries/LibWeb/Page/Page.h b/Libraries/LibWeb/Page/Page.h index 5892ec90981..54459f61215 100644 --- a/Libraries/LibWeb/Page/Page.h +++ b/Libraries/LibWeb/Page/Page.h @@ -212,6 +212,8 @@ public: FindInPageResult find_in_page_previous_match(); Optional last_find_in_page_query() const { return m_last_find_in_page_query; } + URL::URL url() const; + private: explicit Page(GC::Ref); virtual void visit_edges(Visitor&) override; @@ -368,6 +370,7 @@ public: virtual void page_did_request_select_dropdown([[maybe_unused]] Web::CSSPixelPoint content_position, [[maybe_unused]] Web::CSSPixels minimum_width, [[maybe_unused]] Vector items) { } virtual void page_did_finish_text_test([[maybe_unused]] String const& text) { } + virtual void page_did_finish_loading_page_and_fonts([[maybe_unused]] URL::URL const& url) { } virtual void page_did_change_theme_color(Gfx::Color) { } diff --git a/Libraries/LibWebView/ViewImplementation.h b/Libraries/LibWebView/ViewImplementation.h index fd3979cfc4a..9a8bbfaa10e 100644 --- a/Libraries/LibWebView/ViewImplementation.h +++ b/Libraries/LibWebView/ViewImplementation.h @@ -231,11 +231,14 @@ public: Function on_inspector_executed_console_script; Function on_inspector_exported_inspector_html; Function on_web_content_crashed; + Function on_loading_page_and_fonts_finish; virtual Web::DevicePixelSize viewport_size() const = 0; virtual Gfx::IntPoint to_content_position(Gfx::IntPoint widget_position) const = 0; virtual Gfx::IntPoint to_widget_position(Gfx::IntPoint content_position) const = 0; + u64 page_id() const; + protected: static constexpr auto ZOOM_MIN_LEVEL = 0.3f; static constexpr auto ZOOM_MAX_LEVEL = 5.0f; @@ -245,7 +248,6 @@ protected: WebContentClient& client(); WebContentClient const& client() const; - u64 page_id() const; virtual void update_zoom() = 0; void handle_resize(); diff --git a/Libraries/LibWebView/WebContentClient.cpp b/Libraries/LibWebView/WebContentClient.cpp index 81dc1c554e6..31245f0e80c 100644 --- a/Libraries/LibWebView/WebContentClient.cpp +++ b/Libraries/LibWebView/WebContentClient.cpp @@ -93,6 +93,15 @@ void WebContentClient::did_finish_text_test(u64 page_id, String const& text) } } +void WebContentClient::did_finish_loading_page_and_fonts(u64 page_id, URL::URL const& url) +{ + if (auto view = view_for_page_id(page_id); view.has_value()) { + if (view->on_loading_page_and_fonts_finish) + view->on_loading_page_and_fonts_finish(url); + } +} + + void WebContentClient::did_find_in_page(u64 page_id, size_t current_match_index, Optional const& total_match_count) { if (auto view = view_for_page_id(page_id); view.has_value()) { diff --git a/Libraries/LibWebView/WebContentClient.h b/Libraries/LibWebView/WebContentClient.h index b03c38553c9..26c51fd6eda 100644 --- a/Libraries/LibWebView/WebContentClient.h +++ b/Libraries/LibWebView/WebContentClient.h @@ -108,6 +108,7 @@ private: virtual void did_request_select_dropdown(u64 page_id, Gfx::IntPoint content_position, i32 minimum_width, Vector const& items) override; virtual void did_finish_handling_input_event(u64 page_id, Web::EventResult event_result) override; virtual void did_finish_text_test(u64 page_id, String const& text) override; + virtual void did_finish_loading_page_and_fonts(u64 page_id, URL::URL const& url) override; virtual void did_find_in_page(u64 page_id, size_t current_match_index, Optional const& total_match_count) override; virtual void did_change_theme_color(u64 page_id, Gfx::Color color) override; virtual void did_insert_clipboard_entry(u64 page_id, String const& data, String const& presentation_style, String const& mime_type) override; diff --git a/Services/WebContent/ConnectionFromClient.cpp b/Services/WebContent/ConnectionFromClient.cpp index 9c36a255982..c351e85aed0 100644 --- a/Services/WebContent/ConnectionFromClient.cpp +++ b/Services/WebContent/ConnectionFromClient.cpp @@ -140,6 +140,7 @@ void ConnectionFromClient::update_screen_rects(u64 page_id, Vectorpage(page_id); if (!page.has_value()) return; diff --git a/Services/WebContent/PageClient.cpp b/Services/WebContent/PageClient.cpp index f0dfccedd1e..cc2e49ebafb 100644 --- a/Services/WebContent/PageClient.cpp +++ b/Services/WebContent/PageClient.cpp @@ -355,6 +355,12 @@ void PageClient::page_did_finish_text_test(String const& text) client().async_did_finish_text_test(m_id, text); } +void PageClient::page_did_finish_loading_page_and_fonts(URL::URL const& url) +{ + dbgln("PageClient::page_did_finish_loading_page_and_fonts: {}", url); + client().async_did_finish_loading_page_and_fonts(m_id, url); +} + void PageClient::page_did_request_context_menu(Web::CSSPixelPoint content_position) { client().async_did_request_context_menu(m_id, page().css_to_device_point(content_position).to_type()); diff --git a/Services/WebContent/PageClient.h b/Services/WebContent/PageClient.h index f8680b11d5b..477f7b3a312 100644 --- a/Services/WebContent/PageClient.h +++ b/Services/WebContent/PageClient.h @@ -155,6 +155,7 @@ private: virtual void page_did_request_file_picker(Web::HTML::FileFilter accepted_file_types, Web::HTML::AllowMultipleFiles) override; virtual void page_did_request_select_dropdown(Web::CSSPixelPoint content_position, Web::CSSPixels minimum_width, Vector items) override; virtual void page_did_finish_text_test(String const& text) override; + virtual void page_did_finish_loading_page_and_fonts(URL::URL const& url) override; virtual void page_did_change_theme_color(Gfx::Color color) override; virtual void page_did_insert_clipboard_entry(String data, String presentation_style, String mime_type) override; virtual void page_did_change_audio_play_state(Web::HTML::AudioPlayState) override; diff --git a/Services/WebContent/WebContentClient.ipc b/Services/WebContent/WebContentClient.ipc index 548b3b633ab..76b66b0ad18 100644 --- a/Services/WebContent/WebContentClient.ipc +++ b/Services/WebContent/WebContentClient.ipc @@ -95,7 +95,7 @@ endpoint WebContentClient did_get_js_console_messages(u64 page_id, i32 start_index, Vector message_types, Vector messages) =| did_finish_text_test(u64 page_id, String text) =| - + did_finish_loading_page_and_fonts(u64 page_id, URL::URL url) =| did_find_in_page(u64 page_id, size_t current_match_index, Optional total_match_count) =| request_worker_agent(u64 page_id) => (IPC::File socket) // FIXME: Add required attributes to select a SharedWorker Agent diff --git a/Tests/LibWeb/Ref/expected/wpt-import/css/css-fonts/font-variant-numeric-ref.html b/Tests/LibWeb/Ref/expected/wpt-import/css/css-fonts/font-variant-numeric-ref.html index 52f17eee451..360cb20f0f7 100644 --- a/Tests/LibWeb/Ref/expected/wpt-import/css/css-fonts/font-variant-numeric-ref.html +++ b/Tests/LibWeb/Ref/expected/wpt-import/css/css-fonts/font-variant-numeric-ref.html @@ -11,10 +11,13 @@
diff --git a/Tests/LibWeb/Ref/input/wpt-import/css/css-fonts/font-variant-numeric.html b/Tests/LibWeb/Ref/input/wpt-import/css/css-fonts/font-variant-numeric.html index eaae3b29dec..fbd23297af9 100644 --- a/Tests/LibWeb/Ref/input/wpt-import/css/css-fonts/font-variant-numeric.html +++ b/Tests/LibWeb/Ref/input/wpt-import/css/css-fonts/font-variant-numeric.html @@ -14,10 +14,12 @@
diff --git a/UI/Headless/Test.cpp b/UI/Headless/Test.cpp index 8d3b529207c..f5e3f29b260 100644 --- a/UI/Headless/Test.cpp +++ b/UI/Headless/Test.cpp @@ -102,6 +102,7 @@ static ErrorOr collect_ref_tests(Vector& tests, StringView path, Str static void clear_test_callbacks(HeadlessWebView& view) { view.on_load_finish = {}; + view.on_loading_page_and_fonts_finish = {}; view.on_text_test_finish = {}; view.on_web_content_crashed = {}; } @@ -240,6 +241,7 @@ static void run_ref_test(HeadlessWebView& view, Test& test, URL::URL const& url, { auto timer = Core::Timer::create_single_shot(timeout_in_milliseconds, [&view, &test]() { view.on_load_finish = {}; + view.on_loading_page_and_fonts_finish = {}; view.on_text_test_finish = {}; view.on_test_complete({ test, TestResult::Timeout }); @@ -288,20 +290,63 @@ static void run_ref_test(HeadlessWebView& view, Test& test, URL::URL const& url, view.on_test_complete({ test, TestResult::Crashed }); }; - view.on_load_finish = [&view, &test, on_test_complete = move(on_test_complete)](auto const&) { + view.on_load_finish = [&view, &test, &on_test_complete](auto const&) { + dbgln("on loading page: page={}, url={}", test.did_load_page, view.url().serialize_path()); + test.did_load_page = true; + if (!test.did_load_fonts) + return; + //dbgln("DONE on loading page: fonts={}, {}", test.did_load_fonts, view.url()); + test.did_load_page = true; if (test.actual_screenshot) { - view.take_screenshot()->when_resolved([&test, on_test_complete = move(on_test_complete)](RefPtr screenshot) { + //dbgln("Take expectation screenshot <- this happens too early"); + view.take_screenshot()->when_resolved([&test, &on_test_complete](RefPtr screenshot) { test.expectation_screenshot = move(screenshot); + test.did_load_fonts = false; + test.did_load_page = false; on_test_complete(); }); } else { + //dbgln("\nTake actual screenshot"); view.take_screenshot()->when_resolved([&view, &test](RefPtr screenshot) { test.actual_screenshot = move(screenshot); + test.did_load_fonts = false; + test.did_load_page = false; + test.loading_reference_page = true; + dbgln("load-reference-page"); view.debug_request("load-reference-page"); }); } }; + // FIXME: rename to on_fonts_ready + view.on_loading_page_and_fonts_finish = [&view, &test, &on_test_complete](auto const&) { + dbgln("on fonts finish: load={}, url={}", test.did_load_fonts, view.url().serialize_path()); + // This callback will be called multiple times, one for normal page load and one for the reference page load. + // We only want to take the screenshot once the reference page has loaded its fonts + if (test.loading_reference_page && !view.url().serialize_path().ends_with_bytes("-ref.html"sv)) + return; + if (test.did_load_fonts) + return; + test.did_load_fonts = true; + if (!test.did_load_page) + return; + if (test.actual_screenshot) { + view.take_screenshot()->when_resolved([&test, &on_test_complete](RefPtr screenshot) { + test.expectation_screenshot = move(screenshot); + on_test_complete(); + }); + } else { + dbgln("\nTake actual screenshot"); + view.take_screenshot()->when_resolved([&view, &test](RefPtr screenshot) { + test.actual_screenshot = move(screenshot); + test.did_load_fonts = false; + test.did_load_page = false; + test.loading_reference_page = true; + dbgln("load-reference-page"); + view.debug_request("load-reference-page"); + }); + } + }; view.on_text_test_finish = [&](auto const&) { dbgln("Unexpected text test finished during ref test for {}", url); }; diff --git a/UI/Headless/Test.h b/UI/Headless/Test.h index 46b25dad707..68358c66ba9 100644 --- a/UI/Headless/Test.h +++ b/UI/Headless/Test.h @@ -66,9 +66,13 @@ struct Test { String text {}; bool did_finish_test { false }; bool did_finish_loading { false }; - + bool did_load_page { false }; + bool did_load_fonts { false }; + bool loading_reference_page { false }; RefPtr actual_screenshot {}; RefPtr expectation_screenshot {}; + + bool is_ref() const { return mode == TestMode::Ref; }; }; struct TestCompletion {