mirror of
https://github.com/LadybirdBrowser/ladybird.git
synced 2025-04-27 23:09:08 +00:00
LibWeb: Use finalize for cleaning up all navigables
The use of this HashMap looks very spooky, but let's at least use finalize when cleaning them up on destruction to make things slightly less dangerous looking.
This commit is contained in:
parent
59162c8155
commit
76397c9ecd
Notes:
github-actions[bot]
2025-01-17 09:11:51 +00:00
Author: https://github.com/shannonbooth
Commit: 76397c9ecd
Pull-request: https://github.com/LadybirdBrowser/ladybird/pull/3284
7 changed files with 17 additions and 13 deletions
|
@ -3787,8 +3787,8 @@ void Document::destroy()
|
|||
|
||||
// Not in the spec:
|
||||
for (auto& navigable_container : HTML::NavigableContainer::all_instances()) {
|
||||
if (&navigable_container->document() == this)
|
||||
HTML::all_navigables().remove(navigable_container->content_navigable());
|
||||
if (&navigable_container->document() == this && navigable_container->content_navigable())
|
||||
HTML::all_navigables().remove(*navigable_container->content_navigable());
|
||||
}
|
||||
|
||||
// 9. Set document's node navigable's active session history entry's document state's document to null.
|
||||
|
|
|
@ -73,9 +73,9 @@ private:
|
|||
|
||||
GC_DEFINE_ALLOCATOR(ResponseHolder);
|
||||
|
||||
HashTable<Navigable*>& all_navigables()
|
||||
HashTable<GC::RawRef<Navigable>>& all_navigables()
|
||||
{
|
||||
static HashTable<Navigable*> set;
|
||||
static HashTable<GC::RawRef<Navigable>> set;
|
||||
return set;
|
||||
}
|
||||
|
||||
|
@ -111,12 +111,15 @@ Navigable::Navigable(GC::Ref<Page> page)
|
|||
: m_page(page)
|
||||
, m_event_handler({}, *this)
|
||||
{
|
||||
all_navigables().set(this);
|
||||
all_navigables().set(*this);
|
||||
}
|
||||
|
||||
Navigable::~Navigable()
|
||||
Navigable::~Navigable() = default;
|
||||
|
||||
void Navigable::finalize()
|
||||
{
|
||||
all_navigables().remove(this);
|
||||
all_navigables().remove(*this);
|
||||
Base::finalize();
|
||||
}
|
||||
|
||||
void Navigable::visit_edges(Cell::Visitor& visitor)
|
||||
|
@ -159,7 +162,7 @@ void Navigable::set_delaying_load_events(bool value)
|
|||
|
||||
GC::Ptr<Navigable> Navigable::navigable_with_active_document(GC::Ref<DOM::Document> document)
|
||||
{
|
||||
for (auto* navigable : all_navigables()) {
|
||||
for (auto navigable : all_navigables()) {
|
||||
if (navigable->active_document() == document)
|
||||
return navigable;
|
||||
}
|
||||
|
|
|
@ -192,6 +192,7 @@ protected:
|
|||
explicit Navigable(GC::Ref<Page>);
|
||||
|
||||
virtual void visit_edges(Cell::Visitor&) override;
|
||||
virtual void finalize() override;
|
||||
|
||||
// https://html.spec.whatwg.org/multipage/browsing-the-web.html#ongoing-navigation
|
||||
Variant<Empty, Traversal, String> m_ongoing_navigation;
|
||||
|
@ -236,7 +237,7 @@ private:
|
|||
Web::EventHandler m_event_handler;
|
||||
};
|
||||
|
||||
HashTable<Navigable*>& all_navigables();
|
||||
HashTable<GC::RawRef<Navigable>>& all_navigables();
|
||||
|
||||
bool navigation_must_be_a_replace(URL::URL const& url, DOM::Document const& document);
|
||||
void finalize_a_cross_document_navigation(GC::Ref<Navigable>, HistoryHandlingBehavior, UserNavigationInvolvement, GC::Ref<SessionHistoryEntry>);
|
||||
|
|
|
@ -283,7 +283,7 @@ void NavigableContainer::destroy_the_child_navigable()
|
|||
m_content_navigable = nullptr;
|
||||
|
||||
// Not in the spec:
|
||||
HTML::all_navigables().remove(navigable);
|
||||
HTML::all_navigables().remove(*navigable);
|
||||
|
||||
// 6. Let parentDocState be container's node navigable's active session history entry's document state.
|
||||
auto parent_doc_state = this->navigable()->active_session_history_entry()->document_state();
|
||||
|
|
|
@ -1268,7 +1268,7 @@ void TraversableNavigable::destroy_top_level_traversable()
|
|||
// FIXME: Figure out why we need to do this... we shouldn't be leaking Navigables for all time.
|
||||
// However, without this, we can keep stale destroyed traversables around.
|
||||
set_has_been_destroyed();
|
||||
all_navigables().remove(this);
|
||||
all_navigables().remove(*this);
|
||||
}
|
||||
|
||||
// https://html.spec.whatwg.org/multipage/browsing-the-web.html#finalize-a-same-document-navigation
|
||||
|
|
|
@ -46,7 +46,7 @@ JsonObject window_proxy_reference_object(HTML::WindowProxy const& window)
|
|||
|
||||
static GC::Ptr<HTML::Navigable> find_navigable_with_handle(StringView handle, bool should_be_top_level)
|
||||
{
|
||||
for (auto* navigable : Web::HTML::all_navigables()) {
|
||||
for (auto navigable : Web::HTML::all_navigables()) {
|
||||
if (navigable->is_top_level_traversable() != should_be_top_level)
|
||||
continue;
|
||||
|
||||
|
|
|
@ -581,7 +581,7 @@ Messages::WebDriverClient::SwitchToWindowResponse WebDriverConnection::switch_to
|
|||
// Otherwise, return error with error code no such window.
|
||||
bool found_matching_context = false;
|
||||
|
||||
for (auto* navigable : Web::HTML::all_navigables()) {
|
||||
for (auto navigable : Web::HTML::all_navigables()) {
|
||||
auto traversable = navigable->top_level_traversable();
|
||||
if (!traversable || !traversable->active_browsing_context())
|
||||
continue;
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue