RGBCube/serenity
1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-25 06:57:44 +00:00
Code Issues Activity Actions

LibWeb: Make ListOfAvailableImages actually visit images during GC

Browse source 
Instead of making the "Entry" inner struct GC-allocated and marking
*that*, we now mark the image instead.

This fixes an issue found by ASAN on https://mozilla.com/
This commit is contained in:
Andreas Kling 2023-12-23 18:17:04 +01:00
parent f4fa37afd2
commit f953a70965
3 changed files with 19 additions and 39 deletions
Download patch file Download diff file Expand all files Collapse all files

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

@ -373,7 +373,7 @@ ErrorOr<void> HTMLImageElement::update_the_image_data(bool restart_animations, b
key.origin = document().origin(); key.origin = document().origin();
// 3. If the list of available images contains an entry for key, then: // 3. If the list of available images contains an entry for key, then:
if (auto entry = document().list_of_available_images().get(key)) { if (auto* entry = document().list_of_available_images().get(key)) {
// 1. Set the ignore higher-layer caching flag for that entry. // 1. Set the ignore higher-layer caching flag for that entry.
entry->ignore_higher_layer_caching = true; entry->ignore_higher_layer_caching = true;
@ -598,7 +598,7 @@ void HTMLImageElement::add_callbacks_to_image_request(JS::NonnullGCPtr<ImageRequ
image_request->set_state(ImageRequest::State::CompletelyAvailable); image_request->set_state(ImageRequest::State::CompletelyAvailable);
// 3. Add the image to the list of available images using the key key, with the ignore higher-layer caching flag set. // 3. Add the image to the list of available images using the key key, with the ignore higher-layer caching flag set.
document().list_of_available_images().add(key, *image_data, true).release_value_but_fixme_should_propagate_errors(); document().list_of_available_images().add(key, *image_data, true);
// 4. If maybe omit events is not set or previousURL is not equal to urlString, then fire an event named load at the img element. // 4. If maybe omit events is not set or previousURL is not equal to urlString, then fire an event named load at the img element.
if (!maybe_omit_events || previous_url != url_string) if (!maybe_omit_events || previous_url != url_string)
@ -731,7 +731,7 @@ void HTMLImageElement::react_to_changes_in_the_environment()
image_request->set_state(ImageRequest::State::CompletelyAvailable); image_request->set_state(ImageRequest::State::CompletelyAvailable);
// 4. Add the image to the list of available images using the key key, with the ignore higher-layer caching flag set. // 4. Add the image to the list of available images using the key key, with the ignore higher-layer caching flag set.
document().list_of_available_images().add(key, image_data, true).release_value_but_fixme_should_propagate_errors(); document().list_of_available_images().add(key, image_data, true);
// 5. Upgrade the pending request to the current request. // 5. Upgrade the pending request to the current request.
upgrade_pending_request_to_current_request(); upgrade_pending_request_to_current_request();
@ -749,7 +749,7 @@ void HTMLImageElement::react_to_changes_in_the_environment()
// 14. If the list of available images contains an entry for key, then set image request's image data to that of the entry. // 14. If the list of available images contains an entry for key, then set image request's image data to that of the entry.
// Continue to the next step. // Continue to the next step.
if (auto entry = document().list_of_available_images().get(key)) { if (auto* entry = document().list_of_available_images().get(key)) {
image_request->set_image_data(entry->image_data); image_request->set_image_data(entry->image_data);
step_15(selected_source.value(), *image_request, key, entry->image_data); step_15(selected_source.value(), *image_request, key, entry->image_data);
} }

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

@ -10,7 +10,6 @@
namespace Web::HTML { namespace Web::HTML {
JS_DEFINE_ALLOCATOR(ListOfAvailableImages); JS_DEFINE_ALLOCATOR(ListOfAvailableImages);
JS_DEFINE_ALLOCATOR(ListOfAvailableImages::Entry);
ListOfAvailableImages::ListOfAvailableImages() = default; ListOfAvailableImages::ListOfAvailableImages() = default;
ListOfAvailableImages::~ListOfAvailableImages() = default; ListOfAvailableImages::~ListOfAvailableImages() = default;
@ -33,31 +32,16 @@ u32 ListOfAvailableImages::Key::hash() const
return cached_hash.value(); return cached_hash.value();
} }
JS::NonnullGCPtr<ListOfAvailableImages::Entry> ListOfAvailableImages::Entry::create(JS::VM& vm, JS::NonnullGCPtr<DecodedImageData> image_data, bool ignore_higher_layer_caching)
{
return vm.heap().allocate_without_realm<Entry>(image_data, ignore_higher_layer_caching);
}
void ListOfAvailableImages::visit_edges(JS::Cell::Visitor& visitor) void ListOfAvailableImages::visit_edges(JS::Cell::Visitor& visitor)
{ {
Base::visit_edges(visitor); Base::visit_edges(visitor);
for (auto& it : m_images) for (auto& it : m_images)
visitor.visit(it.value); visitor.visit(it.value->image_data);
} }
ListOfAvailableImages::Entry::Entry(JS::NonnullGCPtr<DecodedImageData> data, bool ignore_higher_layer_caching) void ListOfAvailableImages::add(Key const& key, JS::NonnullGCPtr<DecodedImageData> image_data, bool ignore_higher_layer_caching)
: ignore_higher_layer_caching(ignore_higher_layer_caching)
, image_data(data)
{ {
} m_images.set(key, make<Entry>(image_data, ignore_higher_layer_caching));
ListOfAvailableImages::Entry::~Entry() = default;
ErrorOr<void> ListOfAvailableImages::add(Key const& key, JS::NonnullGCPtr<DecodedImageData> image_data, bool ignore_higher_layer_caching)
{
auto entry = Entry::create(vm(), image_data, ignore_higher_layer_caching);
TRY(m_images.try_set(key, entry));
return {};
} }
void ListOfAvailableImages::remove(Key const& key) void ListOfAvailableImages::remove(Key const& key)
@ -65,12 +49,12 @@ void ListOfAvailableImages::remove(Key const& key)
m_images.remove(key); m_images.remove(key);
} }
JS::GCPtr<ListOfAvailableImages::Entry> ListOfAvailableImages::get(Key const& key) const ListOfAvailableImages::Entry* ListOfAvailableImages::get(Key const& key)
{ {
auto it = m_images.find(key); auto it = m_images.find(key);
if (it == m_images.end()) if (it == m_images.end())
return nullptr; return nullptr;
return it->value; return it->value.ptr();
} }
} }

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

@ -33,32 +33,28 @@ public:
mutable Optional<u32> cached_hash; mutable Optional<u32> cached_hash;
}; };
struct Entry final : public JS::Cell { struct Entry {
JS_CELL(Entry, Cell); Entry(JS::NonnullGCPtr<DecodedImageData> image_data, bool ignore_higher_layer_caching)
JS_DECLARE_ALLOCATOR(Entry); : image_data(move(image_data))
, ignore_higher_layer_caching(ignore_higher_layer_caching)
{
}
public:
static JS::NonnullGCPtr<Entry> create(JS::VM&, JS::NonnullGCPtr<DecodedImageData>, bool ignore_higher_layer_caching);
~Entry();
bool ignore_higher_layer_caching { false };
JS::NonnullGCPtr<DecodedImageData> image_data; JS::NonnullGCPtr<DecodedImageData> image_data;
bool ignore_higher_layer_caching { false };
private:
Entry(JS::NonnullGCPtr<DecodedImageData>, bool ignore_higher_layer_caching);
}; };
ListOfAvailableImages(); ListOfAvailableImages();
~ListOfAvailableImages(); ~ListOfAvailableImages();
ErrorOr<void> add(Key const&, JS::NonnullGCPtr<DecodedImageData>, bool ignore_higher_layer_caching); void add(Key const&, JS::NonnullGCPtr<DecodedImageData>, bool ignore_higher_layer_caching);
void remove(Key const&); void remove(Key const&);
[[nodiscard]] JS::GCPtr<Entry> get(Key const&) const; [[nodiscard]] Entry* get(Key const&);
void visit_edges(JS::Cell::Visitor& visitor) override; void visit_edges(JS::Cell::Visitor& visitor) override;
private: private:
HashMap<Key, JS::NonnullGCPtr<Entry>> m_images; HashMap<Key, NonnullOwnPtr<Entry>> m_images;
}; };
} }