mirror of
https://github.com/LadybirdBrowser/ladybird.git
synced 2025-09-07 18:17:23 +00:00
LibWeb: Unregister ResizeObserver from Document when it has no targets
According to the spec, `ResizeObserver` needs to live for as long as it's referenced from script or has observation targets. With this change we make sure that `ResizeObserver` is unregistered from the `Document` when it has no target. Fixes GC leak that caused us to keep all resize observers alive until document they belong to is destroyed.
This commit is contained in:
parent
40fd2643cc
commit
4cbf47dcd2
Notes:
github-actions[bot]
2025-07-29 22:56:18 +00:00
Author: https://github.com/kalenikaliaksandr
Commit: 4cbf47dcd2
Pull-request: https://github.com/LadybirdBrowser/ladybird/pull/5646
4 changed files with 53 additions and 20 deletions
|
@ -584,7 +584,9 @@ void Document::visit_edges(Cell::Visitor& visitor)
|
||||||
visitor.visit(m_document_observers_being_notified);
|
visitor.visit(m_document_observers_being_notified);
|
||||||
visitor.visit(m_pending_scroll_event_targets);
|
visitor.visit(m_pending_scroll_event_targets);
|
||||||
visitor.visit(m_pending_scrollend_event_targets);
|
visitor.visit(m_pending_scrollend_event_targets);
|
||||||
visitor.visit(m_resize_observers);
|
for (auto& resize_observer : m_resize_observers) {
|
||||||
|
visitor.visit(resize_observer);
|
||||||
|
}
|
||||||
|
|
||||||
visitor.visit(m_shared_resource_requests);
|
visitor.visit(m_shared_resource_requests);
|
||||||
|
|
||||||
|
@ -4595,9 +4597,7 @@ void Document::register_resize_observer(Badge<ResizeObserver::ResizeObserver>, R
|
||||||
|
|
||||||
void Document::unregister_resize_observer(Badge<ResizeObserver::ResizeObserver>, ResizeObserver::ResizeObserver& observer)
|
void Document::unregister_resize_observer(Badge<ResizeObserver::ResizeObserver>, ResizeObserver::ResizeObserver& observer)
|
||||||
{
|
{
|
||||||
m_resize_observers.remove_first_matching([&](auto& registered_observer) {
|
m_resize_observers.remove(observer);
|
||||||
return registered_observer.ptr() == &observer;
|
|
||||||
});
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// https://www.w3.org/TR/intersection-observer/#queue-an-intersection-observer-task
|
// https://www.w3.org/TR/intersection-observer/#queue-an-intersection-observer-task
|
||||||
|
@ -5787,13 +5787,13 @@ void Document::gather_active_observations_at_depth(size_t depth)
|
||||||
// 1. Let depth be the depth passed in.
|
// 1. Let depth be the depth passed in.
|
||||||
|
|
||||||
// 2. For each observer in [[resizeObservers]] run these steps:
|
// 2. For each observer in [[resizeObservers]] run these steps:
|
||||||
for (auto const& observer : m_resize_observers) {
|
for (auto& observer : m_resize_observers) {
|
||||||
// 1. Clear observer’s [[activeTargets]], and [[skippedTargets]].
|
// 1. Clear observer’s [[activeTargets]], and [[skippedTargets]].
|
||||||
observer->active_targets().clear();
|
observer.active_targets().clear();
|
||||||
observer->skipped_targets().clear();
|
observer.skipped_targets().clear();
|
||||||
|
|
||||||
// 2. For each observation in observer.[[observationTargets]] run this step:
|
// 2. For each observation in observer.[[observationTargets]] run this step:
|
||||||
for (auto const& observation : observer->observation_targets()) {
|
for (auto& observation : observer.observation_targets()) {
|
||||||
// 1. If observation.isActive() is true
|
// 1. If observation.isActive() is true
|
||||||
if (observation->is_active()) {
|
if (observation->is_active()) {
|
||||||
// 1. Let targetDepth be result of calculate depth for node for observation.target.
|
// 1. Let targetDepth be result of calculate depth for node for observation.target.
|
||||||
|
@ -5801,10 +5801,10 @@ void Document::gather_active_observations_at_depth(size_t depth)
|
||||||
|
|
||||||
// 2. If targetDepth is greater than depth then add observation to [[activeTargets]].
|
// 2. If targetDepth is greater than depth then add observation to [[activeTargets]].
|
||||||
if (target_depth > depth) {
|
if (target_depth > depth) {
|
||||||
observer->active_targets().append(observation);
|
observer.active_targets().append(observation);
|
||||||
} else {
|
} else {
|
||||||
// 3. Else add observation to [[skippedTargets]].
|
// 3. Else add observation to [[skippedTargets]].
|
||||||
observer->skipped_targets().append(observation);
|
observer.skipped_targets().append(observation);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -5820,7 +5820,10 @@ size_t Document::broadcast_active_resize_observations()
|
||||||
// 2. For each observer in document.[[resizeObservers]] run these steps:
|
// 2. For each observer in document.[[resizeObservers]] run these steps:
|
||||||
|
|
||||||
// NOTE: We make a copy of the resize observers list to avoid modifying it while iterating.
|
// NOTE: We make a copy of the resize observers list to avoid modifying it while iterating.
|
||||||
auto resize_observers = GC::RootVector { heap(), m_resize_observers };
|
auto resize_observers = GC::RootVector<GC::Ref<ResizeObserver::ResizeObserver>> { heap() };
|
||||||
|
for (auto& observer : m_resize_observers)
|
||||||
|
resize_observers.append(observer);
|
||||||
|
|
||||||
for (auto const& observer : resize_observers) {
|
for (auto const& observer : resize_observers) {
|
||||||
// 1. If observer.[[activeTargets]] slot is empty, continue.
|
// 1. If observer.[[activeTargets]] slot is empty, continue.
|
||||||
if (observer->active_targets().is_empty()) {
|
if (observer->active_targets().is_empty()) {
|
||||||
|
@ -5878,9 +5881,9 @@ size_t Document::broadcast_active_resize_observations()
|
||||||
bool Document::has_active_resize_observations()
|
bool Document::has_active_resize_observations()
|
||||||
{
|
{
|
||||||
// 1. For each observer in [[resizeObservers]] run this step:
|
// 1. For each observer in [[resizeObservers]] run this step:
|
||||||
for (auto const& observer : m_resize_observers) {
|
for (auto& observer : m_resize_observers) {
|
||||||
// 1. If observer.[[activeTargets]] is not empty, return true.
|
// 1. If observer.[[activeTargets]] is not empty, return true.
|
||||||
if (!observer->active_targets().is_empty())
|
if (!observer.active_targets().is_empty())
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -5892,9 +5895,9 @@ bool Document::has_active_resize_observations()
|
||||||
bool Document::has_skipped_resize_observations()
|
bool Document::has_skipped_resize_observations()
|
||||||
{
|
{
|
||||||
// 1. For each observer in [[resizeObservers]] run this step:
|
// 1. For each observer in [[resizeObservers]] run this step:
|
||||||
for (auto const& observer : m_resize_observers) {
|
for (auto& observer : m_resize_observers) {
|
||||||
// 1. If observer.[[skippedTargets]] is not empty, return true.
|
// 1. If observer.[[skippedTargets]] is not empty, return true.
|
||||||
if (!observer->skipped_targets().is_empty())
|
if (!observer.skipped_targets().is_empty())
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -39,6 +39,7 @@
|
||||||
#include <LibWeb/HTML/Scripting/Environments.h>
|
#include <LibWeb/HTML/Scripting/Environments.h>
|
||||||
#include <LibWeb/HTML/VisibilityState.h>
|
#include <LibWeb/HTML/VisibilityState.h>
|
||||||
#include <LibWeb/InvalidateDisplayList.h>
|
#include <LibWeb/InvalidateDisplayList.h>
|
||||||
|
#include <LibWeb/ResizeObserver/ResizeObserver.h>
|
||||||
#include <LibWeb/TrustedTypes/InjectionSink.h>
|
#include <LibWeb/TrustedTypes/InjectionSink.h>
|
||||||
#include <LibWeb/WebIDL/ExceptionOr.h>
|
#include <LibWeb/WebIDL/ExceptionOr.h>
|
||||||
#include <LibWeb/WebIDL/ObservableArray.h>
|
#include <LibWeb/WebIDL/ObservableArray.h>
|
||||||
|
@ -1157,7 +1158,7 @@ private:
|
||||||
// Each Document has a lazy load intersection observer, initially set to null but can be set to an IntersectionObserver instance.
|
// Each Document has a lazy load intersection observer, initially set to null but can be set to an IntersectionObserver instance.
|
||||||
GC::Ptr<IntersectionObserver::IntersectionObserver> m_lazy_load_intersection_observer;
|
GC::Ptr<IntersectionObserver::IntersectionObserver> m_lazy_load_intersection_observer;
|
||||||
|
|
||||||
Vector<GC::Ref<ResizeObserver::ResizeObserver>> m_resize_observers;
|
ResizeObserver::ResizeObserver::ResizeObserversList m_resize_observers;
|
||||||
|
|
||||||
// https://html.spec.whatwg.org/multipage/semantics.html#will-declaratively-refresh
|
// https://html.spec.whatwg.org/multipage/semantics.html#will-declaratively-refresh
|
||||||
// A Document object has an associated will declaratively refresh (a boolean). It is initially false.
|
// A Document object has an associated will declaratively refresh (a boolean). It is initially false.
|
||||||
|
|
|
@ -1,6 +1,6 @@
|
||||||
/*
|
/*
|
||||||
* Copyright (c) 2021, Andreas Kling <andreas@ladybird.org>
|
* Copyright (c) 2021, Andreas Kling <andreas@ladybird.org>
|
||||||
* Copyright (c) 2024, Aliaksandr Kalenik <kalenik.aliaksandr@gmail.com>
|
* Copyright (c) 2024-2025, Aliaksandr Kalenik <kalenik.aliaksandr@gmail.com>
|
||||||
*
|
*
|
||||||
* SPDX-License-Identifier: BSD-2-Clause
|
* SPDX-License-Identifier: BSD-2-Clause
|
||||||
*/
|
*/
|
||||||
|
@ -29,7 +29,6 @@ ResizeObserver::ResizeObserver(JS::Realm& realm, WebIDL::CallbackType* callback)
|
||||||
{
|
{
|
||||||
auto navigable = as<HTML::Window>(HTML::relevant_global_object(*this)).navigable();
|
auto navigable = as<HTML::Window>(HTML::relevant_global_object(*this)).navigable();
|
||||||
m_document = navigable->active_document().ptr();
|
m_document = navigable->active_document().ptr();
|
||||||
m_document->register_resize_observer({}, *this);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
ResizeObserver::~ResizeObserver() = default;
|
ResizeObserver::~ResizeObserver() = default;
|
||||||
|
@ -51,7 +50,7 @@ void ResizeObserver::visit_edges(JS::Cell::Visitor& visitor)
|
||||||
|
|
||||||
void ResizeObserver::finalize()
|
void ResizeObserver::finalize()
|
||||||
{
|
{
|
||||||
if (m_document)
|
if (m_document && m_list_node.is_in_list())
|
||||||
m_document->unregister_resize_observer({}, *this);
|
m_document->unregister_resize_observer({}, *this);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -71,6 +70,10 @@ void ResizeObserver::observe(DOM::Element& target, ResizeObserverOptions options
|
||||||
|
|
||||||
// 4. Add the resizeObservation to the [[observationTargets]] slot.
|
// 4. Add the resizeObservation to the [[observationTargets]] slot.
|
||||||
m_observation_targets.append(resize_observation);
|
m_observation_targets.append(resize_observation);
|
||||||
|
|
||||||
|
if (!m_list_node.is_in_list()) {
|
||||||
|
m_document->register_resize_observer({}, *this);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// https://drafts.csswg.org/resize-observer-1/#dom-resizeobserver-unobserve
|
// https://drafts.csswg.org/resize-observer-1/#dom-resizeobserver-unobserve
|
||||||
|
@ -85,6 +88,8 @@ void ResizeObserver::unobserve(DOM::Element& target)
|
||||||
|
|
||||||
// 3. Remove observation from [[observationTargets]].
|
// 3. Remove observation from [[observationTargets]].
|
||||||
m_observation_targets.remove(observation.index());
|
m_observation_targets.remove(observation.index());
|
||||||
|
|
||||||
|
unregister_observer_if_needed();
|
||||||
}
|
}
|
||||||
|
|
||||||
// https://drafts.csswg.org/resize-observer-1/#dom-resizeobserver-disconnect
|
// https://drafts.csswg.org/resize-observer-1/#dom-resizeobserver-disconnect
|
||||||
|
@ -95,6 +100,8 @@ void ResizeObserver::disconnect()
|
||||||
|
|
||||||
// 2. Clear the [[activeTargets]] list.
|
// 2. Clear the [[activeTargets]] list.
|
||||||
m_active_targets.clear();
|
m_active_targets.clear();
|
||||||
|
|
||||||
|
unregister_observer_if_needed();
|
||||||
}
|
}
|
||||||
|
|
||||||
void ResizeObserver::invoke_callback(ReadonlySpan<GC::Ref<ResizeObserverEntry>> entries) const
|
void ResizeObserver::invoke_callback(ReadonlySpan<GC::Ref<ResizeObserverEntry>> entries) const
|
||||||
|
@ -112,4 +119,19 @@ void ResizeObserver::invoke_callback(ReadonlySpan<GC::Ref<ResizeObserverEntry>>
|
||||||
(void)WebIDL::invoke_callback(callback, JS::js_undefined(), WebIDL::ExceptionBehavior::Report, { { wrapped_records } });
|
(void)WebIDL::invoke_callback(callback, JS::js_undefined(), WebIDL::ExceptionBehavior::Report, { { wrapped_records } });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void ResizeObserver::unregister_observer_if_needed()
|
||||||
|
{
|
||||||
|
// https://drafts.csswg.org/resize-observer/#lifetime
|
||||||
|
// A ResizeObserver will remain alive until both of these conditions are met:
|
||||||
|
// - there are no scripting references to the observer.
|
||||||
|
// - the observer is not observing any targets.
|
||||||
|
|
||||||
|
// The first condition from the spec is handled by visiting ResizeObserver from
|
||||||
|
// JS environment that holds a reference to ResizeObserver.
|
||||||
|
// Here we handle the second condition.
|
||||||
|
if (m_observation_targets.is_empty() && m_list_node.is_in_list() && m_document) {
|
||||||
|
m_document->unregister_resize_observer({}, *this);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -1,6 +1,6 @@
|
||||||
/*
|
/*
|
||||||
* Copyright (c) 2021, Andreas Kling <andreas@ladybird.org>
|
* Copyright (c) 2021, Andreas Kling <andreas@ladybird.org>
|
||||||
* Copyright (c) 2024, Aliaksandr Kalenik <kalenik.aliaksandr@gmail.com>
|
* Copyright (c) 2024-2025, Aliaksandr Kalenik <kalenik.aliaksandr@gmail.com>
|
||||||
*
|
*
|
||||||
* SPDX-License-Identifier: BSD-2-Clause
|
* SPDX-License-Identifier: BSD-2-Clause
|
||||||
*/
|
*/
|
||||||
|
@ -44,6 +44,8 @@ private:
|
||||||
virtual void visit_edges(JS::Cell::Visitor&) override;
|
virtual void visit_edges(JS::Cell::Visitor&) override;
|
||||||
virtual void finalize() override;
|
virtual void finalize() override;
|
||||||
|
|
||||||
|
void unregister_observer_if_needed();
|
||||||
|
|
||||||
GC::Ptr<WebIDL::CallbackType> m_callback;
|
GC::Ptr<WebIDL::CallbackType> m_callback;
|
||||||
Vector<GC::Ref<ResizeObservation>> m_observation_targets;
|
Vector<GC::Ref<ResizeObservation>> m_observation_targets;
|
||||||
Vector<GC::Ref<ResizeObservation>> m_active_targets;
|
Vector<GC::Ref<ResizeObservation>> m_active_targets;
|
||||||
|
@ -51,6 +53,11 @@ private:
|
||||||
|
|
||||||
// AD-HOC: This is the document where we've registered the observer.
|
// AD-HOC: This is the document where we've registered the observer.
|
||||||
WeakPtr<DOM::Document> m_document;
|
WeakPtr<DOM::Document> m_document;
|
||||||
|
|
||||||
|
IntrusiveListNode<ResizeObserver> m_list_node;
|
||||||
|
|
||||||
|
public:
|
||||||
|
using ResizeObserversList = IntrusiveList<&ResizeObserver::m_list_node>;
|
||||||
};
|
};
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue