RGBCube/serenity
1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-05-31 19:58:11 +00:00
Code Issues Activity Actions

LibWeb: Make HTML::DecodedImageData to be GC-allocated

Browse source 
This change fixes GC-leak caused by following mutual dependency:
- SVGDecodedImageData owns JS::Handle for Page.
- SVGDecodedImageData is owned by visited objects.
by making everything inherited from HTML::DecodedImageData and
ListOfAvailableImages to be GC-allocated.

Generally, if visited object has a handle, very likely we leak
everything visited from object in a handle.
This commit is contained in:
Aliaksandr Kalenik 2023-12-12 20:07:19 +01:00 • committed by Andreas Kling
parent 57a04c536c
commit 41a3c19cfe
18 changed files with 100 additions and 54 deletions
Download patch file Download diff file Expand all files Collapse all files

2
Userland/Libraries/LibWeb/CSS/StyleValues/ImageStyleValue.cpp
View file

@ -132,7 +132,7 @@ void ImageStyleValue::paint(PaintContext& context, DevicePixelRect const& dest_r
}
}
RefPtr<HTML::DecodedImageData const> ImageStyleValue::image_data() const
JS::GCPtr<HTML::DecodedImageData> ImageStyleValue::image_data() const
{
if (!m_image_request)
return nullptr;

2
Userland/Libraries/LibWeb/CSS/StyleValues/ImageStyleValue.h
View file

@ -48,7 +48,7 @@ public:
Function<void()> on_animate;
RefPtr<HTML::DecodedImageData const> image_data() const;
JS::GCPtr<HTML::DecodedImageData> image_data() const;
private:
ImageStyleValue(AK::URL const&);

4
Userland/Libraries/LibWeb/DOM/Document.cpp
View file

@ -347,7 +347,7 @@ void Document::initialize(JS::Realm& realm)
m_selection = heap().allocate<Selection::Selection>(realm, realm, *this);
m_list_of_available_images = make<HTML::ListOfAvailableImages>();
m_list_of_available_images = heap().allocate<HTML::ListOfAvailableImages>(realm);
}
void Document::visit_edges(Cell::Visitor& visitor)
@ -413,6 +413,8 @@ void Document::visit_edges(Cell::Visitor& visitor)
for (auto& timeline : m_associated_animation_timelines)
visitor.visit(timeline);
visitor.visit(m_list_of_available_images);
}
// https://w3c.github.io/selection-api/#dom-document-getselection

2
Userland/Libraries/LibWeb/DOM/Document.h
View file

@ -728,7 +728,7 @@ private:
JS::GCPtr<HTML::HTMLBaseElement const> m_first_base_element_with_href_in_tree_order;
// https://html.spec.whatwg.org/multipage/images.html#list-of-available-images
OwnPtr<HTML::ListOfAvailableImages> m_list_of_available_images;
JS::GCPtr<HTML::ListOfAvailableImages> m_list_of_available_images;
JS::GCPtr<CSS::VisualViewport> m_visual_viewport;

8
Userland/Libraries/LibWeb/HTML/AnimatedBitmapDecodedImageData.cpp
View file

@ -5,13 +5,17 @@
*/
#include <LibGfx/Bitmap.h>
#include <LibJS/Heap/Heap.h>
#include <LibJS/Runtime/Realm.h>
#include <LibWeb/HTML/AnimatedBitmapDecodedImageData.h>
namespace Web::HTML {
ErrorOr<NonnullRefPtr<AnimatedBitmapDecodedImageData>> AnimatedBitmapDecodedImageData::create(Vector<Frame>&& frames, size_t loop_count, bool animated)
JS_DEFINE_ALLOCATOR(AnimatedBitmapDecodedImageData);
ErrorOr<JS::NonnullGCPtr<AnimatedBitmapDecodedImageData>> AnimatedBitmapDecodedImageData::create(JS::Realm& realm, Vector<Frame>&& frames, size_t loop_count, bool animated)
{
return adopt_nonnull_ref_or_enomem(new (nothrow) AnimatedBitmapDecodedImageData(move(frames), loop_count, animated));
return realm.heap().allocate<AnimatedBitmapDecodedImageData>(realm, move(frames), loop_count, animated);
}
AnimatedBitmapDecodedImageData::AnimatedBitmapDecodedImageData(Vector<Frame>&& frames, size_t loop_count, bool animated)

5
Userland/Libraries/LibWeb/HTML/AnimatedBitmapDecodedImageData.h
View file

@ -12,13 +12,16 @@
namespace Web::HTML {
class AnimatedBitmapDecodedImageData final : public DecodedImageData {
JS_CELL(AnimatedBitmapDecodedImageData, Cell);
JS_DECLARE_ALLOCATOR(AnimatedBitmapDecodedImageData);
public:
struct Frame {
RefPtr<Gfx::ImmutableBitmap> bitmap;
int duration { 0 };
};
static ErrorOr<NonnullRefPtr<AnimatedBitmapDecodedImageData>> create(Vector<Frame>&&, size_t loop_count, bool animated);
static ErrorOr<JS::NonnullGCPtr<AnimatedBitmapDecodedImageData>> create(JS::Realm&, Vector<Frame>&&, size_t loop_count, bool animated);
virtual ~AnimatedBitmapDecodedImageData() override;
virtual RefPtr<Gfx::ImmutableBitmap> bitmap(size_t frame_index, Gfx::IntSize = {}) const override;

2
Userland/Libraries/LibWeb/HTML/DecodedImageData.h
View file

@ -14,7 +14,7 @@
namespace Web::HTML {
// https://html.spec.whatwg.org/multipage/images.html#img-req-data
class DecodedImageData : public RefCounted<DecodedImageData> {
class DecodedImageData : public JS::Cell {
public:
virtual ~DecodedImageData();

4
Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp
View file

@ -715,7 +715,7 @@ void HTMLImageElement::react_to_changes_in_the_environment()
// FIXME: 13. End the synchronous section, continuing the remaining steps in parallel.
auto step_15 = [this](String const& selected_source, JS::NonnullGCPtr<ImageRequest> image_request, ListOfAvailableImages::Key const& key, NonnullRefPtr<DecodedImageData> const& image_data) {
auto step_15 = [this](String const& selected_source, JS::NonnullGCPtr<ImageRequest> image_request, ListOfAvailableImages::Key const& key, JS::NonnullGCPtr<DecodedImageData> image_data) {
// 15. Queue an element task on the DOM manipulation task source given the img element and the following steps:
queue_an_element_task(HTML::Task::Source::DOMManipulation, [this, selected_source, image_request, key, image_data] {
// 1. FIXME: If the img element has experienced relevant mutations since this algorithm started, then let pending request be null and abort these steps.
@ -789,7 +789,7 @@ void HTMLImageElement::react_to_changes_in_the_environment()
VERIFY(image_request->shared_image_request());
auto image_data = image_request->shared_image_request()->image_data();
image_request->set_image_data(image_data);
step_15(selected_source, image_request, key, NonnullRefPtr(*image_data));
step_15(selected_source, image_request, key, *image_data);
});
},
[this]() {

2
Userland/Libraries/LibWeb/HTML/HTMLObjectElement.cpp
View file

@ -354,7 +354,7 @@ i32 HTMLObjectElement::default_tab_index_value() const
return 0;
}
RefPtr<DecodedImageData const> HTMLObjectElement::image_data() const
JS::GCPtr<DecodedImageData> HTMLObjectElement::image_data() const
{
if (!m_image_request)
return nullptr;

2
Userland/Libraries/LibWeb/HTML/HTMLObjectElement.h
View file

@ -81,7 +81,7 @@ private:
Representation m_representation { Representation::Unknown };
RefPtr<DecodedImageData const> image_data() const;
JS::GCPtr<DecodedImageData> image_data() const;
JS::GCPtr<SharedImageRequest> m_image_request;
};

7
Userland/Libraries/LibWeb/HTML/ImageRequest.cpp
View file

@ -41,6 +41,7 @@ void ImageRequest::visit_edges(JS::Cell::Visitor& visitor)
Base::visit_edges(visitor);
visitor.visit(m_shared_image_request);
visitor.visit(m_page);
visitor.visit(m_image_data);
}
// https://html.spec.whatwg.org/multipage/images.html#img-available
@ -92,14 +93,14 @@ void abort_the_image_request(JS::Realm&, ImageRequest* image_request)
// AD-HOC: We simply don't do this, since our SharedImageRequest may be used by someone else.
}
RefPtr<DecodedImageData const> ImageRequest::image_data() const
JS::GCPtr<DecodedImageData> ImageRequest::image_data() const
{
return m_image_data;
}
void ImageRequest::set_image_data(RefPtr<DecodedImageData const> data)
void ImageRequest::set_image_data(JS::GCPtr<DecodedImageData> data)
{
m_image_data = move(data);
m_image_data = data;
}
// https://html.spec.whatwg.org/multipage/images.html#prepare-an-image-for-presentation

6
Userland/Libraries/LibWeb/HTML/ImageRequest.h
View file

@ -43,8 +43,8 @@ public:
AK::URL const& current_url() const;
void set_current_url(JS::Realm&, AK::URL);
[[nodiscard]] RefPtr<DecodedImageData const> image_data() const;
void set_image_data(RefPtr<DecodedImageData const>);
[[nodiscard]] JS::GCPtr<DecodedImageData> image_data() const;
void set_image_data(JS::GCPtr<DecodedImageData>);
[[nodiscard]] float current_pixel_density() const { return m_current_pixel_density; }
void set_current_pixel_density(float density) { m_current_pixel_density = density; }
@ -76,7 +76,7 @@ private:
AK::URL m_current_url;
// https://html.spec.whatwg.org/multipage/images.html#img-req-data
RefPtr<DecodedImageData const> m_image_data;
JS::GCPtr<DecodedImageData> m_image_data;
// https://html.spec.whatwg.org/multipage/images.html#current-pixel-density
// Each image request has a current pixel density, which must initially be 1.

26
Userland/Libraries/LibWeb/HTML/ListOfAvailableImages.cpp
View file

@ -9,6 +9,9 @@
namespace Web::HTML {
JS_DEFINE_ALLOCATOR(ListOfAvailableImages);
JS_DEFINE_ALLOCATOR(ListOfAvailableImages::Entry);
ListOfAvailableImages::ListOfAvailableImages() = default;
ListOfAvailableImages::~ListOfAvailableImages() = default;
@ -30,23 +33,30 @@ u32 ListOfAvailableImages::Key::hash() const
return cached_hash.value();
}
ErrorOr<NonnullRefPtr<ListOfAvailableImages::Entry>> ListOfAvailableImages::Entry::create(NonnullRefPtr<DecodedImageData> image_data, bool ignore_higher_layer_caching)
JS::NonnullGCPtr<ListOfAvailableImages::Entry> ListOfAvailableImages::Entry::create(JS::VM& vm, JS::NonnullGCPtr<DecodedImageData> image_data, bool ignore_higher_layer_caching)
{
return adopt_nonnull_ref_or_enomem(new (nothrow) Entry(move(image_data), ignore_higher_layer_caching));
return vm.heap().allocate_without_realm<Entry>(image_data, ignore_higher_layer_caching);
}
ListOfAvailableImages::Entry::Entry(NonnullRefPtr<DecodedImageData> data, bool ignore_higher_layer_caching)
void ListOfAvailableImages::visit_edges(JS::Cell::Visitor& visitor)
{
Base::visit_edges(visitor);
for (auto& it : m_images)
visitor.visit(it.value);
}
ListOfAvailableImages::Entry::Entry(JS::NonnullGCPtr<DecodedImageData> data, bool ignore_higher_layer_caching)
: ignore_higher_layer_caching(ignore_higher_layer_caching)
, image_data(move(data))
, image_data(data)
{
}
ListOfAvailableImages::Entry::~Entry() = default;
ErrorOr<void> ListOfAvailableImages::add(Key const& key, NonnullRefPtr<DecodedImageData> image_data, bool ignore_higher_layer_caching)
ErrorOr<void> ListOfAvailableImages::add(Key const& key, JS::NonnullGCPtr<DecodedImageData> image_data, bool ignore_higher_layer_caching)
{
auto entry = TRY(Entry::create(move(image_data), ignore_higher_layer_caching));
TRY(m_images.try_set(key, move(entry)));
auto entry = Entry::create(vm(), image_data, ignore_higher_layer_caching);
TRY(m_images.try_set(key, entry));
return {};
}
@ -55,7 +65,7 @@ void ListOfAvailableImages::remove(Key const& key)
m_images.remove(key);
}
RefPtr<ListOfAvailableImages::Entry> ListOfAvailableImages::get(Key const& key) const
JS::GCPtr<ListOfAvailableImages::Entry> ListOfAvailableImages::get(Key const& key) const
{
auto it = m_images.find(key);
if (it == m_images.end())

25
Userland/Libraries/LibWeb/HTML/ListOfAvailableImages.h
View file

@ -16,7 +16,10 @@
namespace Web::HTML {
// https://html.spec.whatwg.org/multipage/images.html#list-of-available-images
class ListOfAvailableImages {
class ListOfAvailableImages : public JS::Cell {
JS_CELL(ListOfAvailableImages, Cell);
JS_DECLARE_ALLOCATOR(ListOfAvailableImages);
public:
struct Key {
AK::URL url;
@ -30,26 +33,32 @@ public:
mutable Optional<u32> cached_hash;
};
struct Entry final : public RefCounted<Entry> {
static ErrorOr<NonnullRefPtr<Entry>> create(NonnullRefPtr<DecodedImageData>, bool ignore_higher_layer_caching);
struct Entry final : public JS::Cell {
JS_CELL(Entry, Cell);
JS_DECLARE_ALLOCATOR(Entry);
public:
static JS::NonnullGCPtr<Entry> create(JS::VM&, JS::NonnullGCPtr<DecodedImageData>, bool ignore_higher_layer_caching);
~Entry();
bool ignore_higher_layer_caching { false };
NonnullRefPtr<DecodedImageData> image_data;
JS::NonnullGCPtr<DecodedImageData> image_data;
private:
Entry(NonnullRefPtr<DecodedImageData>, bool ignore_higher_layer_caching);
Entry(JS::NonnullGCPtr<DecodedImageData>, bool ignore_higher_layer_caching);
};
ListOfAvailableImages();
~ListOfAvailableImages();
ErrorOr<void> add(Key const&, NonnullRefPtr<DecodedImageData>, bool ignore_higher_layer_caching);
ErrorOr<void> add(Key const&, JS::NonnullGCPtr<DecodedImageData>, bool ignore_higher_layer_caching);
void remove(Key const&);
[[nodiscard]] RefPtr<Entry> get(Key const&) const;
[[nodiscard]] JS::GCPtr<Entry> get(Key const&) const;
void visit_edges(JS::Cell::Visitor& visitor) override;
private:
HashMap<Key, NonnullRefPtr<Entry>> m_images;
HashMap<Key, JS::NonnullGCPtr<Entry>> m_images;
};
}

11
Userland/Libraries/LibWeb/HTML/SharedImageRequest.cpp
View file

@ -56,9 +56,10 @@ void SharedImageRequest::visit_edges(JS::Cell::Visitor& visitor)
visitor.visit(callback.on_finish);
visitor.visit(callback.on_fail);
}
visitor.visit(m_image_data);
}
RefPtr<DecodedImageData const> SharedImageRequest::image_data() const
JS::GCPtr<DecodedImageData> SharedImageRequest::image_data() const
{
return m_image_data;
}
@ -137,7 +138,7 @@ void SharedImageRequest::handle_successful_fetch(AK::URL const& url_string, Stri
bool const is_svg_image = mime_type == "image/svg+xml"sv || url_string.basename().ends_with(".svg"sv);
RefPtr<DecodedImageData> image_data;
JS::GCPtr<DecodedImageData> image_data;
auto handle_failed_decode = [&] {
m_state = State::Failed;
@ -148,7 +149,7 @@ void SharedImageRequest::handle_successful_fetch(AK::URL const& url_string, Stri
};
if (is_svg_image) {
auto result = SVG::SVGDecodedImageData::create(m_page, url_string, data);
auto result = SVG::SVGDecodedImageData::create(m_document->realm(), m_page, url_string, data);
if (result.is_error())
return handle_failed_decode();
@ -165,10 +166,10 @@ void SharedImageRequest::handle_successful_fetch(AK::URL const& url_string, Stri
.duration = static_cast<int>(frame.duration),
});
}
image_data = AnimatedBitmapDecodedImageData::create(move(frames), result.value().loop_count, result.value().is_animated).release_value_but_fixme_should_propagate_errors();
image_data = AnimatedBitmapDecodedImageData::create(m_document->realm(), move(frames), result.value().loop_count, result.value().is_animated).release_value_but_fixme_should_propagate_errors();
}
m_image_data = move(image_data);
m_image_data = image_data;
m_state = State::Finished;

4
Userland/Libraries/LibWeb/HTML/SharedImageRequest.h
View file

@ -28,7 +28,7 @@ public:
AK::URL const& url() const { return m_url; }
[[nodiscard]] RefPtr<DecodedImageData const> image_data() const;
[[nodiscard]] JS::GCPtr<DecodedImageData> image_data() const;
[[nodiscard]] JS::GCPtr<Fetch::Infrastructure::FetchController> fetch_controller();
void set_fetch_controller(JS::GCPtr<Fetch::Infrastructure::FetchController>);
@ -66,7 +66,7 @@ private:
Vector<Callbacks> m_callbacks;
AK::URL m_url;
RefPtr<DecodedImageData const> m_image_data;
JS::GCPtr<DecodedImageData> m_image_data;
JS::GCPtr<Fetch::Infrastructure::FetchController> m_fetch_controller;
JS::GCPtr<DOM::Document> m_document;

25
Userland/Libraries/LibWeb/SVG/SVGDecodedImageData.cpp
View file

@ -23,6 +23,8 @@
namespace Web::SVG {
JS_DEFINE_ALLOCATOR(SVGDecodedImageData);
class SVGDecodedImageData::SVGPageClient final : public PageClient {
JS_CELL(SVGDecodedImageData::SVGPageClient, PageClient);
@ -54,7 +56,7 @@ private:
}
};
ErrorOr<NonnullRefPtr<SVGDecodedImageData>> SVGDecodedImageData::create(Page& host_page, AK::URL const& url, ByteBuffer data)
ErrorOr<JS::NonnullGCPtr<SVGDecodedImageData>> SVGDecodedImageData::create(JS::Realm& realm, JS::NonnullGCPtr<Page> host_page, AK::URL const& url, ByteBuffer data)
{
auto page_client = SVGPageClient::create(Bindings::main_thread_vm(), host_page);
auto page = Page::create(Bindings::main_thread_vm(), *page_client);
@ -98,19 +100,28 @@ ErrorOr<NonnullRefPtr<SVGDecodedImageData>> SVGDecodedImageData::create(Page& ho
MUST(document->append_child(*svg_root));
return adopt_nonnull_ref_or_enomem(new (nothrow) SVGDecodedImageData(page, move(page_client), move(document), move(svg_root)));
return realm.heap().allocate<SVGDecodedImageData>(realm, page, page_client, document, *svg_root);
}
SVGDecodedImageData::SVGDecodedImageData(JS::NonnullGCPtr<Page> page, JS::Handle<SVGPageClient> page_client, JS::Handle<DOM::Document> document, JS::Handle<SVG::SVGSVGElement> root_element)
: m_page(move(page))
, m_page_client(move(page_client))
, m_document(move(document))
, m_root_element(move(root_element))
SVGDecodedImageData::SVGDecodedImageData(JS::NonnullGCPtr<Page> page, JS::NonnullGCPtr<SVGPageClient> page_client, JS::NonnullGCPtr<DOM::Document> document, JS::NonnullGCPtr<SVG::SVGSVGElement> root_element)
: m_page(page)
, m_page_client(page_client)
, m_document(document)
, m_root_element(root_element)
{
}
SVGDecodedImageData::~SVGDecodedImageData() = default;
void SVGDecodedImageData::visit_edges(Cell::Visitor& visitor)
{
Base::visit_edges(visitor);
visitor.visit(m_page);
visitor.visit(m_document);
visitor.visit(m_page_client);
visitor.visit(m_root_element);
}
RefPtr<Gfx::Bitmap> SVGDecodedImageData::render(Gfx::IntSize size) const
{
auto bitmap = Gfx::Bitmap::create(Gfx::BitmapFormat::BGRA8888, size).release_value_but_fixme_should_propagate_errors();

17
Userland/Libraries/LibWeb/SVG/SVGDecodedImageData.h
View file

@ -12,8 +12,11 @@
namespace Web::SVG {
class SVGDecodedImageData final : public HTML::DecodedImageData {
JS_CELL(SVGDecodedImageData, Cell);
JS_DECLARE_ALLOCATOR(SVGDecodedImageData);
public:
static ErrorOr<NonnullRefPtr<SVGDecodedImageData>> create(Page&, AK::URL const&, ByteBuffer encoded_svg);
static ErrorOr<JS::NonnullGCPtr<SVGDecodedImageData>> create(JS::Realm&, JS::NonnullGCPtr<Page>, AK::URL const&, ByteBuffer encoded_svg);
virtual ~SVGDecodedImageData() override;
virtual RefPtr<Gfx::ImmutableBitmap> bitmap(size_t frame_index, Gfx::IntSize) const override;
@ -30,18 +33,20 @@ public:
DOM::Document const& svg_document() const { return *m_document; }
virtual void visit_edges(Cell::Visitor& visitor) override;
private:
class SVGPageClient;
SVGDecodedImageData(JS::NonnullGCPtr<Page>, JS::Handle<SVGPageClient>, JS::Handle<DOM::Document>, JS::Handle<SVG::SVGSVGElement>);
SVGDecodedImageData(JS::NonnullGCPtr<Page>, JS::NonnullGCPtr<SVGPageClient>, JS::NonnullGCPtr<DOM::Document>, JS::NonnullGCPtr<SVG::SVGSVGElement>);
RefPtr<Gfx::Bitmap> render(Gfx::IntSize) const;
mutable RefPtr<Gfx::ImmutableBitmap> m_immutable_bitmap;
JS::Handle<Page> m_page;
JS::Handle<SVGPageClient> m_page_client;
JS::NonnullGCPtr<Page> m_page;
JS::NonnullGCPtr<SVGPageClient> m_page_client;
JS::Handle<DOM::Document> m_document;
JS::Handle<SVG::SVGSVGElement> m_root_element;
JS::NonnullGCPtr<DOM::Document> m_document;
JS::NonnullGCPtr<SVG::SVGSVGElement> m_root_element;
};
}