mirror of
https://github.com/RGBCube/serenity
synced 2025-07-25 23:07:35 +00:00
LibWeb: Use JS::HeapFunction for HTML::Timer callback
Before the completion_steps for timer were casted from JS::SafeFunction to Function in HTML::Timer constructor, which is incorrect because then callback's captured GC-allocated objects are not protected from being deallocated. Let's modify HTML::Timer to use JS::HeapFunction for the callback instead.
This commit is contained in:
parent
883a97984c
commit
4e8654e31b
3 changed files with 17 additions and 9 deletions
|
@ -13,16 +13,17 @@ namespace Web::HTML {
|
|||
|
||||
JS::NonnullGCPtr<Timer> Timer::create(JS::Object& window_or_worker_global_scope, i32 milliseconds, Function<void()> callback, i32 id)
|
||||
{
|
||||
return window_or_worker_global_scope.heap().allocate_without_realm<Timer>(window_or_worker_global_scope, milliseconds, move(callback), id);
|
||||
auto heap_function_callback = JS::create_heap_function(window_or_worker_global_scope.heap(), move(callback));
|
||||
return window_or_worker_global_scope.heap().allocate_without_realm<Timer>(window_or_worker_global_scope, milliseconds, heap_function_callback, id);
|
||||
}
|
||||
|
||||
Timer::Timer(JS::Object& window_or_worker_global_scope, i32 milliseconds, Function<void()> callback, i32 id)
|
||||
Timer::Timer(JS::Object& window_or_worker_global_scope, i32 milliseconds, JS::NonnullGCPtr<JS::HeapFunction<void()>> callback, i32 id)
|
||||
: m_window_or_worker_global_scope(window_or_worker_global_scope)
|
||||
, m_callback(move(callback))
|
||||
, m_id(id)
|
||||
{
|
||||
m_timer = Core::Timer::create_single_shot(milliseconds, [this] {
|
||||
m_callback();
|
||||
m_callback->function()();
|
||||
}).release_value_but_fixme_should_propagate_errors();
|
||||
}
|
||||
|
||||
|
@ -30,6 +31,7 @@ void Timer::visit_edges(Cell::Visitor& visitor)
|
|||
{
|
||||
Base::visit_edges(visitor);
|
||||
visitor.visit(m_window_or_worker_global_scope.ptr());
|
||||
visitor.visit(m_callback);
|
||||
}
|
||||
|
||||
Timer::~Timer()
|
||||
|
|
|
@ -12,6 +12,7 @@
|
|||
#include <LibCore/Forward.h>
|
||||
#include <LibJS/Heap/Cell.h>
|
||||
#include <LibJS/Heap/GCPtr.h>
|
||||
#include <LibJS/Heap/HeapFunction.h>
|
||||
#include <LibWeb/Forward.h>
|
||||
|
||||
namespace Web::HTML {
|
||||
|
@ -27,13 +28,13 @@ public:
|
|||
void stop();
|
||||
|
||||
private:
|
||||
Timer(JS::Object& window, i32 milliseconds, Function<void()> callback, i32 id);
|
||||
Timer(JS::Object& window, i32 milliseconds, JS::NonnullGCPtr<JS::HeapFunction<void()>> callback, i32 id);
|
||||
|
||||
virtual void visit_edges(Cell::Visitor&) override;
|
||||
|
||||
RefPtr<Core::Timer> m_timer;
|
||||
JS::NonnullGCPtr<JS::Object> m_window_or_worker_global_scope;
|
||||
Function<void()> m_callback;
|
||||
JS::NonnullGCPtr<JS::HeapFunction<void()>> m_callback;
|
||||
i32 m_id { 0 };
|
||||
};
|
||||
|
||||
|
|
|
@ -11,6 +11,7 @@
|
|||
#include <AK/String.h>
|
||||
#include <AK/Utf8View.h>
|
||||
#include <AK/Vector.h>
|
||||
#include <LibJS/Heap/HeapFunction.h>
|
||||
#include <LibTextCodec/Decoder.h>
|
||||
#include <LibWeb/Bindings/MainThreadVM.h>
|
||||
#include <LibWeb/Fetch/FetchMethod.h>
|
||||
|
@ -226,8 +227,10 @@ i32 WindowOrWorkerGlobalScopeMixin::run_timer_initialization_steps(TimerHandler
|
|||
// 7. Let initiating script be the active script.
|
||||
auto const* initiating_script = Web::Bindings::active_script();
|
||||
|
||||
auto& vm = this_impl().vm();
|
||||
|
||||
// 8. Let task be a task that runs the following substeps:
|
||||
JS::SafeFunction<void()> task = [this, handler = move(handler), timeout, arguments = move(arguments), repeat, id, initiating_script]() mutable {
|
||||
auto task = JS::create_heap_function(vm.heap(), Function<void()>([this, handler = move(handler), timeout, arguments = move(arguments), repeat, id, initiating_script]() {
|
||||
// 1. If id does not exist in global's map of active timers, then abort these steps.
|
||||
if (!m_timers.contains(id))
|
||||
return;
|
||||
|
@ -288,14 +291,16 @@ i32 WindowOrWorkerGlobalScopeMixin::run_timer_initialization_steps(TimerHandler
|
|||
m_timers.remove(id);
|
||||
break;
|
||||
}
|
||||
};
|
||||
}));
|
||||
|
||||
// FIXME: 9. Increment nesting level by one.
|
||||
// FIXME: 10. Set task's timer nesting level to nesting level.
|
||||
|
||||
// 11. Let completionStep be an algorithm step which queues a global task on the timer task source given global to run task.
|
||||
JS::SafeFunction<void()> completion_step = [this, task = move(task)]() mutable {
|
||||
queue_global_task(Task::Source::TimerTask, this_impl(), move(task));
|
||||
Function<void()> completion_step = [this, task = move(task)]() mutable {
|
||||
queue_global_task(Task::Source::TimerTask, this_impl(), [task] {
|
||||
task->function()();
|
||||
});
|
||||
};
|
||||
|
||||
// 12. Run steps after a timeout given global, "setTimeout/setInterval", timeout, completionStep, and id.
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue