From 67cfb64d070b2f19bc04dd5967d2fdb8c0b7b1e8 Mon Sep 17 00:00:00 2001 From: Luke Wilde Date: Wed, 26 Feb 2025 14:58:17 +0000 Subject: [PATCH] LibWeb: Separate adding to performance buffers and queueing to observers The User Timing, Resource Timing and Navigation Timing specifications have not been updated to account for the queue method to also append to the underlying performance buffer and it's method of checking it's full. This separates the functionality into a different function, with a flag to indicate whether to use the custom full buffer logic or not. --- .../LibWeb/HTML/WindowOrWorkerGlobalScope.cpp | 46 +++++++++++++------ .../LibWeb/HTML/WindowOrWorkerGlobalScope.h | 6 +++ .../LibWeb/HighResolutionTime/Performance.cpp | 4 +- 3 files changed, 41 insertions(+), 15 deletions(-) diff --git a/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.cpp b/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.cpp index e334051634f..ce192ee89fe 100644 --- a/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.cpp +++ b/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.cpp @@ -435,24 +435,44 @@ void WindowOrWorkerGlobalScopeMixin::queue_performance_entry(GC::Refappend_to_observer_buffer({}, new_entry); } - // 6. Let tuple be the relevant performance entry tuple of entryType and relevantGlobal. - auto& tuple = relevant_performance_entry_tuple(entry_type); - - // 7. Let isBufferFull be the return value of the determine if a performance entry buffer is full algorithm with tuple - // as input. - bool is_buffer_full = tuple.is_full(); - - // 8. Let shouldAdd be the result of should add entry with newEntry as input. - auto should_add = new_entry->should_add_entry(); - - // 9. If isBufferFull is false and shouldAdd is true, append newEntry to tuple's performance entry buffer. - if (!is_buffer_full && should_add == PerformanceTimeline::ShouldAddEntry::Yes) - tuple.performance_entry_buffer.append(new_entry); + // AD-HOC: Steps 6-9 are not here because other engines do not add to the performance entry buffer when queuing + // the performance observer task. The users of the Performance Timeline specification also do not expect + // this function to add to the entry buffer, instead queuing the observer task, then adding to the entry + // buffer separately. // 10. Queue the PerformanceObserver task with relevantGlobal as input. queue_the_performance_observer_task(); } +// https://www.w3.org/TR/performance-timeline/#dfn-queue-a-performanceentry +// AD-HOC: This is a separate function because the users of this specification queues PerformanceObserver tasks and add +// to the entry buffer separately. +void WindowOrWorkerGlobalScopeMixin::add_performance_entry(GC::Ref new_entry, CheckIfPerformanceBufferIsFull check_if_performance_buffer_is_full) +{ + // 6. Let tuple be the relevant performance entry tuple of entryType and relevantGlobal. + auto& tuple = relevant_performance_entry_tuple(new_entry->entry_type()); + + // AD-HOC: We have a custom flag to always append to the buffer by default, as other performance specs do this by default + // (either they don't have a limit, or they check the limit themselves). This flag allows compatibility for specs + // that rely do and don't rely on this. + bool is_buffer_full = false; + auto should_add = PerformanceTimeline::ShouldAddEntry::Yes; + + if (check_if_performance_buffer_is_full == CheckIfPerformanceBufferIsFull::Yes) { + // 7. Let isBufferFull be the return value of the determine if a performance entry buffer is full algorithm with tuple + // as input. + is_buffer_full = tuple.is_full(); + + // 8. Let shouldAdd be the result of should add entry with newEntry as input. + should_add = new_entry->should_add_entry(); + } + + // 9. If isBufferFull is false and shouldAdd is true, append newEntry to tuple's performance entry buffer. + if (!is_buffer_full && should_add == PerformanceTimeline::ShouldAddEntry::Yes) + tuple.performance_entry_buffer.append(new_entry); +} + + void WindowOrWorkerGlobalScopeMixin::clear_performance_entry_buffer(Badge, FlyString const& entry_type) { auto& tuple = relevant_performance_entry_tuple(entry_type); diff --git a/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.h b/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.h index a593755a55f..154574f992c 100644 --- a/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.h +++ b/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.h @@ -47,8 +47,14 @@ public: void clear_interval(i32); void clear_map_of_active_timers(); + enum class CheckIfPerformanceBufferIsFull { + No, + Yes, + }; + PerformanceTimeline::PerformanceEntryTuple& relevant_performance_entry_tuple(FlyString const& entry_type); void queue_performance_entry(GC::Ref new_entry); + void add_performance_entry(GC::Ref new_entry, CheckIfPerformanceBufferIsFull check_if_performance_buffer_is_full = CheckIfPerformanceBufferIsFull::No); void clear_performance_entry_buffer(Badge, FlyString const& entry_type); void remove_entries_from_performance_entry_buffer(Badge, FlyString const& entry_type, String entry_name); diff --git a/Libraries/LibWeb/HighResolutionTime/Performance.cpp b/Libraries/LibWeb/HighResolutionTime/Performance.cpp index 08aa5b947d9..d9be79f28e0 100644 --- a/Libraries/LibWeb/HighResolutionTime/Performance.cpp +++ b/Libraries/LibWeb/HighResolutionTime/Performance.cpp @@ -89,7 +89,7 @@ WebIDL::ExceptionOr> Performance::mark(Stri window_or_worker().queue_performance_entry(entry); // 3. Add entry to the performance entry buffer. - // FIXME: This seems to be a holdover from moving to the `queue` structure for PerformanceObserver, as this would cause a double append. + window_or_worker().add_performance_entry(entry); // 4. Return entry. return entry; @@ -309,7 +309,7 @@ WebIDL::ExceptionOr> Performance::measur window_or_worker().queue_performance_entry(entry); // 11. Add entry to the performance entry buffer. - // FIXME: This seems to be a holdover from moving to the `queue` structure for PerformanceObserver, as this would cause a double append. + window_or_worker().add_performance_entry(entry); // 12. Return entry. return entry;