LibWeb: Make Agent's MutationObserver list weak

Before this change, Agent held on to all of the live MutationObserver
objects via GC::Root. This prevented them from ever getting
garbage-collected.

Instead of roots, we now use a simple IntrusiveList and remove them
from it in the finalizer for MutationObserver.

This fixes a massive GC leak on Speedometer.
This commit is contained in:
Andreas Kling 2025-02-07 11:22:11 +01:00 committed by Andreas Kling
commit 53c9c6f3ee
Notes: github-actions[bot] 2025-02-07 15:54:21 +00:00
4 changed files with 14 additions and 6 deletions

View file

@ -675,7 +675,7 @@ void queue_mutation_observer_microtask(DOM::Document const& document)
// 2. Let notifySet be a clone of the surrounding agents mutation observers. // 2. Let notifySet be a clone of the surrounding agents mutation observers.
GC::RootVector<DOM::MutationObserver*> notify_set(heap); GC::RootVector<DOM::MutationObserver*> notify_set(heap);
for (auto& observer : surrounding_agent.mutation_observers) for (auto& observer : surrounding_agent.mutation_observers)
notify_set.append(observer); notify_set.append(&observer);
// FIXME: 3. Let signalSet be a clone of the surrounding agents signal slots. // FIXME: 3. Let signalSet be a clone of the surrounding agents signal slots.

View file

@ -35,9 +35,11 @@ MutationObserver::MutationObserver(JS::Realm& realm, GC::Ptr<WebIDL::CallbackTyp
MutationObserver::~MutationObserver() MutationObserver::~MutationObserver()
{ {
HTML::relevant_agent(*this).mutation_observers.remove_all_matching([this](auto& observer) { }
return observer.ptr() == this;
}); void MutationObserver::finalize()
{
HTML::relevant_agent(*this).mutation_observers.remove(*this);
} }
void MutationObserver::initialize(JS::Realm& realm) void MutationObserver::initialize(JS::Realm& realm)

View file

@ -53,6 +53,7 @@ private:
virtual void initialize(JS::Realm&) override; virtual void initialize(JS::Realm&) override;
virtual void visit_edges(Cell::Visitor&) override; virtual void visit_edges(Cell::Visitor&) override;
virtual void finalize() override;
// https://dom.spec.whatwg.org/#concept-mo-callback // https://dom.spec.whatwg.org/#concept-mo-callback
GC::Ptr<WebIDL::CallbackType> m_callback; GC::Ptr<WebIDL::CallbackType> m_callback;
@ -64,6 +65,11 @@ private:
// https://dom.spec.whatwg.org/#concept-mo-queue // https://dom.spec.whatwg.org/#concept-mo-queue
Vector<GC::Ref<MutationRecord>> m_record_queue; Vector<GC::Ref<MutationRecord>> m_record_queue;
IntrusiveListNode<MutationObserver> m_list_node;
public:
using List = IntrusiveList<&MutationObserver::m_list_node>;
}; };
// https://dom.spec.whatwg.org/#registered-observer // https://dom.spec.whatwg.org/#registered-observer

View file

@ -10,6 +10,7 @@
#include <AK/Vector.h> #include <AK/Vector.h>
#include <LibGC/Root.h> #include <LibGC/Root.h>
#include <LibJS/Forward.h> #include <LibJS/Forward.h>
#include <LibWeb/DOM/MutationObserver.h>
#include <LibWeb/Forward.h> #include <LibWeb/Forward.h>
#include <LibWeb/HTML/CustomElements/CustomElementReactionsStack.h> #include <LibWeb/HTML/CustomElements/CustomElementReactionsStack.h>
@ -25,8 +26,7 @@ struct Agent {
bool mutation_observer_microtask_queued { false }; bool mutation_observer_microtask_queued { false };
// https://dom.spec.whatwg.org/#mutation-observer-list // https://dom.spec.whatwg.org/#mutation-observer-list
// FIXME: This should be a set. DOM::MutationObserver::List mutation_observers;
Vector<GC::Root<DOM::MutationObserver>> mutation_observers;
// https://html.spec.whatwg.org/multipage/custom-elements.html#custom-element-reactions-stack // https://html.spec.whatwg.org/multipage/custom-elements.html#custom-element-reactions-stack
// Each similar-origin window agent has a custom element reactions stack, which is initially empty. // Each similar-origin window agent has a custom element reactions stack, which is initially empty.