mirror of
https://github.com/RGBCube/serenity
synced 2025-07-26 19:37:36 +00:00
LibWeb: Fix memory leak in CSS::ImageStyleValue
Before this change, whenever ImageStyleValue had a non-null `m_image_request`, it was always leaked along with everything related to the document to which this value belongs. The issue arises due to the use of `JS::Handle` for the image request, as it introduces a cyclic dependency where `ImageRequest` prevents the `CSSStyleSheet`, that owns `ImageStyleValue`, from being deallocated: - ImageRequest - FetchController - FetchParams - Window - HTMLDocument - HTMLHtmlElement - HTMLBodyElement - Text - HTMLHeadElement - Text - HTMLMetaElement - Text - HTMLTitleElement - Text - HTMLStyleElement - CSSStyleSheet This change solves this by visiting `m_image_request` from `visit_edges` instead of introducing new heap root by using `JS::Handle`.
This commit is contained in:
parent
d81b0e3c86
commit
707ca984bd
3 changed files with 22 additions and 1 deletions
|
@ -9,6 +9,7 @@
|
||||||
#include <LibWeb/Bindings/Intrinsics.h>
|
#include <LibWeb/Bindings/Intrinsics.h>
|
||||||
#include <LibWeb/CSS/CSSStyleDeclaration.h>
|
#include <LibWeb/CSS/CSSStyleDeclaration.h>
|
||||||
#include <LibWeb/CSS/Parser/Parser.h>
|
#include <LibWeb/CSS/Parser/Parser.h>
|
||||||
|
#include <LibWeb/CSS/StyleValues/ImageStyleValue.h>
|
||||||
#include <LibWeb/DOM/Document.h>
|
#include <LibWeb/DOM/Document.h>
|
||||||
#include <LibWeb/DOM/Element.h>
|
#include <LibWeb/DOM/Element.h>
|
||||||
#include <LibWeb/Infra/Strings.h>
|
#include <LibWeb/Infra/Strings.h>
|
||||||
|
@ -38,6 +39,15 @@ PropertyOwningCSSStyleDeclaration::PropertyOwningCSSStyleDeclaration(JS::Realm&
|
||||||
{
|
{
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void PropertyOwningCSSStyleDeclaration::visit_edges(Cell::Visitor& visitor)
|
||||||
|
{
|
||||||
|
Base::visit_edges(visitor);
|
||||||
|
for (auto& property : m_properties) {
|
||||||
|
if (property.value->is_image())
|
||||||
|
property.value->as_image().visit_edges(visitor);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
String PropertyOwningCSSStyleDeclaration::item(size_t index) const
|
String PropertyOwningCSSStyleDeclaration::item(size_t index) const
|
||||||
{
|
{
|
||||||
if (index >= m_properties.size())
|
if (index >= m_properties.size())
|
||||||
|
|
|
@ -85,6 +85,8 @@ protected:
|
||||||
private:
|
private:
|
||||||
bool set_a_css_declaration(PropertyID, NonnullRefPtr<StyleValue const>, Important);
|
bool set_a_css_declaration(PropertyID, NonnullRefPtr<StyleValue const>, Important);
|
||||||
|
|
||||||
|
virtual void visit_edges(Cell::Visitor&) override;
|
||||||
|
|
||||||
Vector<StyleProperty> m_properties;
|
Vector<StyleProperty> m_properties;
|
||||||
HashMap<DeprecatedString, StyleProperty> m_custom_properties;
|
HashMap<DeprecatedString, StyleProperty> m_custom_properties;
|
||||||
};
|
};
|
||||||
|
|
|
@ -10,9 +10,11 @@
|
||||||
#pragma once
|
#pragma once
|
||||||
|
|
||||||
#include <AK/URL.h>
|
#include <AK/URL.h>
|
||||||
|
#include <LibJS/Heap/Cell.h>
|
||||||
#include <LibJS/Heap/Handle.h>
|
#include <LibJS/Heap/Handle.h>
|
||||||
#include <LibWeb/CSS/Enums.h>
|
#include <LibWeb/CSS/Enums.h>
|
||||||
#include <LibWeb/CSS/StyleValues/AbstractImageStyleValue.h>
|
#include <LibWeb/CSS/StyleValues/AbstractImageStyleValue.h>
|
||||||
|
#include <LibWeb/HTML/SharedImageRequest.h>
|
||||||
|
|
||||||
namespace Web::CSS {
|
namespace Web::CSS {
|
||||||
|
|
||||||
|
@ -26,6 +28,13 @@ public:
|
||||||
}
|
}
|
||||||
virtual ~ImageStyleValue() override = default;
|
virtual ~ImageStyleValue() override = default;
|
||||||
|
|
||||||
|
void visit_edges(JS::Cell::Visitor& visitor) const
|
||||||
|
{
|
||||||
|
// FIXME: visit_edges in non-GC allocated classes is confusing pattern.
|
||||||
|
// Consider making StyleValue to be GC allocated instead.
|
||||||
|
visitor.visit(m_image_request);
|
||||||
|
}
|
||||||
|
|
||||||
virtual String to_string() const override;
|
virtual String to_string() const override;
|
||||||
virtual bool equals(StyleValue const& other) const override;
|
virtual bool equals(StyleValue const& other) const override;
|
||||||
|
|
||||||
|
@ -44,7 +53,7 @@ public:
|
||||||
private:
|
private:
|
||||||
ImageStyleValue(AK::URL const&);
|
ImageStyleValue(AK::URL const&);
|
||||||
|
|
||||||
JS::Handle<HTML::SharedImageRequest> m_image_request;
|
JS::GCPtr<HTML::SharedImageRequest> m_image_request;
|
||||||
|
|
||||||
void animate();
|
void animate();
|
||||||
Gfx::Bitmap const* bitmap(size_t frame_index, Gfx::IntSize = {}) const;
|
Gfx::Bitmap const* bitmap(size_t frame_index, Gfx::IntSize = {}) const;
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue