From b86527727556b3f7612004aad6403babb371b64c Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Mon, 22 May 2023 07:41:34 -0400 Subject: [PATCH] LibWeb: Wait for media candidates without endlessly queueing microtasks Rather than queueing microtasks ad nauseam to check if a media element has a new source candidate, let the media element tell us when it might have a new child to inspect. This removes endless CPU churn in cases where there is never a candidate that we can play. --- .../LibWeb/HTML/HTMLMediaElement.cpp | 28 +++++++++++++++---- .../Libraries/LibWeb/HTML/HTMLMediaElement.h | 1 + 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/Userland/Libraries/LibWeb/HTML/HTMLMediaElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLMediaElement.cpp index fd4ad61a609..535aeb3b7f5 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLMediaElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLMediaElement.cpp @@ -452,6 +452,7 @@ public: Base::visit_edges(visitor); visitor.visit(m_media_element); visitor.visit(m_candidate); + visitor.visit(m_previously_failed_candidate); } WebIDL::ExceptionOr process_candidate() @@ -500,6 +501,15 @@ public: return {}; } + WebIDL::ExceptionOr process_next_candidate() + { + if (!m_previously_failed_candidate) + return {}; + + TRY(wait_for_next_candidate(*m_previously_failed_candidate)); + return {}; + } + private: WebIDL::ExceptionOr failed_with_elements() { @@ -573,16 +583,15 @@ private: WebIDL::ExceptionOr wait_for_next_candidate(JS::NonnullGCPtr previous_candidate) { - // FIXME: We implement the "waiting" by constantly queueing a microtask to check if the previous candidate now - // has a sibling. It might be nicer for the DOM tree to just tell us when the sibling becomes available. + // NOTE: If there isn't another candidate to check, we implement the "waiting" step by returning until the media + // element's children have changed. if (previous_candidate->next_sibling() == nullptr) { - queue_a_microtask(&m_media_element->document(), [this, previous_candidate]() { - wait_for_next_candidate(previous_candidate).release_value_but_fixme_should_propagate_errors(); - }); - + m_previously_failed_candidate = previous_candidate; return {}; } + m_previously_failed_candidate = nullptr; + // FIXME: 22. Await a stable state. The synchronous section consists of all the remaining steps of this algorithm until // the algorithm says the synchronous section has ended. (Steps in synchronous sections are marked with ⌛.) @@ -601,8 +610,15 @@ private: JS::NonnullGCPtr m_media_element; JS::NonnullGCPtr m_candidate; + JS::GCPtr m_previously_failed_candidate; }; +void HTMLMediaElement::children_changed() +{ + if (m_source_element_selector) + m_source_element_selector->process_next_candidate().release_value_but_fixme_should_propagate_errors(); +} + // https://html.spec.whatwg.org/multipage/media.html#concept-media-load-algorithm WebIDL::ExceptionOr HTMLMediaElement::select_resource() { diff --git a/Userland/Libraries/LibWeb/HTML/HTMLMediaElement.h b/Userland/Libraries/LibWeb/HTML/HTMLMediaElement.h index 3f5b0c7d064..a052c176741 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLMediaElement.h +++ b/Userland/Libraries/LibWeb/HTML/HTMLMediaElement.h @@ -93,6 +93,7 @@ protected: virtual void parse_attribute(DeprecatedFlyString const& name, DeprecatedString const& value) override; virtual void did_remove_attribute(DeprecatedFlyString const&) override; virtual void removed_from(DOM::Node*) override; + virtual void children_changed() override; // Override in subclasses to handle implementation-specific behavior when the element state changes // to playing or paused, e.g. to start/stop play timers.