Skip to content

Commit

Permalink
Replace EventLoopTimerPtr with EventLoopTimerHandle
Browse files Browse the repository at this point in the history 
https://bugs.webkit.org/show_bug.cgi?id=259955

Reviewed by Wenson Hsieh.

This PR replaces EventLoopTimerPtr, which is a type cast of EventLoopTimer*,
with a proper EventLoopTimerHandle which knows how to clean up itself at the end.

EventLoopTimerHandle is simply a wrapper around RefPtr<EventLoopTimer> now that
EventLoopTimer is reference counted.

This PR also removes cancelScheduledTask and cancelRepeatingTask from EventLoop
and EventLoopTaskGroup as the timer is automatically deleted when the handle dies.

* Source/WebCore/dom/EventLoop.cpp:
(WebCore::EventLoopTimerHandle::EventLoopTimerHandle):
(WebCore::EventLoopTimerHandle::~EventLoopTimerHandle):
(WebCore::EventLoopTimerHandle::operator=):
(WebCore::EventLoop::scheduleTask):
(WebCore::EventLoop::cancelScheduledTask): Deleted.
(WebCore::EventLoop::removeScheduledTimer):
(WebCore::EventLoop::didExecuteScheduledTask): Deleted.
(WebCore::EventLoop::scheduleRepeatingTask):
(WebCore::EventLoop::cancelRepeatingTask): Deleted.
(WebCore::EventLoop::removeRepeatingTimer):
(WebCore::EventLoopTaskGroup::scheduleTask):.
(WebCore::EventLoopTaskGroup::cancelScheduledTask): Deleted.
(WebCore::EventLoopTaskGroup::didExecuteScheduledTask): Deleted.
(WebCore::EventLoopTaskGroup::removeScheduledTimer):
(WebCore::EventLoopTaskGroup::scheduleRepeatingTask):
(WebCore::EventLoopTaskGroup::cancelRepeatingTask): Deleted.
(WebCore::EventLoopTaskGroup::removeRepeatingTimer):
* Source/WebCore/dom/EventLoop.h:
(WebCore::EventLoopTimerHandle::operator UnspecifiedBoolType const):
(WebCore::EventLoopTimerHandle::unspecifiedBoolTypeInstance const):
* Source/WebCore/editing/AlternativeTextController.cpp:
(WebCore::AlternativeTextController::stopAlternativeTextUITimer):
* Source/WebCore/editing/AlternativeTextController.h:
* Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:
(WebCore::WebGLRenderingContextBase::maybeRestoreContextSoon):
* Source/WebCore/html/canvas/WebGLRenderingContextBase.h:
* Source/WebCore/page/EventSource.cpp:
(WebCore::EventSource::scheduleInitialConnect):
(WebCore::EventSource::scheduleReconnect):
(WebCore::EventSource::close):
* Source/WebCore/page/EventSource.h:
* Source/WebCore/xml/XMLHttpRequestProgressEventThrottle.cpp:
(WebCore::XMLHttpRequestProgressEventThrottle::flushProgressEvent):
(WebCore::XMLHttpRequestProgressEventThrottle::dispatchThrottledProgressEventTimerFired):
* Source/WebCore/xml/XMLHttpRequestProgressEventThrottle.h:

Canonical link: https://commits.webkit.org/266715@main
  • Loading branch information
@rniwa
rniwa committed Aug 9, 2023
1 parent ff720c4 commit 6b2ef31
Show file tree
Hide file tree
Showing 10 changed files with 114 additions and 105 deletions.
124 changes: 68 additions & 56 deletions Source/WebCore/dom/EventLoop.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,10 @@ namespace WebCore {
class EventLoopTimer final : public RefCounted<EventLoopTimer>, public TimerBase, public CanMakeWeakPtr<EventLoopTimer> {
WTF_MAKE_FAST_ALLOCATED;
public:
static Ref<EventLoopTimer> create(std::unique_ptr<EventLoopTask>&& task) { return adoptRef(*new EventLoopTimer(WTFMove(task))); }
enum class Type : bool { OneShot, Repeating };
static Ref<EventLoopTimer> create(Type type, std::unique_ptr<EventLoopTask>&& task) { return adoptRef(*new EventLoopTimer(type, WTFMove(task))); }

Type type() const { return m_type; }
EventLoopTaskGroup* group() const { return m_task ? m_task->group() : nullptr; }

void stop()
Expand All @@ -45,6 +47,7 @@ class EventLoopTimer final : public RefCounted<EventLoopTimer>, public TimerBase
TimerBase::stop();
else
m_suspended = false;
m_task = nullptr;
}

void suspend()
Expand All @@ -69,6 +72,7 @@ class EventLoopTimer final : public RefCounted<EventLoopTimer>, public TimerBase

void startRepeating(Seconds nextFireInterval, Seconds repeatInterval)
{
ASSERT(m_type == Type::Repeating);
if (!m_suspended)
TimerBase::start(nextFireInterval, repeatInterval);
else {
Expand All @@ -80,6 +84,7 @@ class EventLoopTimer final : public RefCounted<EventLoopTimer>, public TimerBase

void startOneShot(Seconds interval)
{
ASSERT(m_type == Type::OneShot);
if (!m_suspended)
TimerBase::startOneShot(interval);
else {
Expand All @@ -90,28 +95,59 @@ class EventLoopTimer final : public RefCounted<EventLoopTimer>, public TimerBase
}

private:
EventLoopTimer(std::unique_ptr<EventLoopTask>&& task)
EventLoopTimer(Type type, std::unique_ptr<EventLoopTask>&& task)
: m_task(WTFMove(task))
, m_type(type)
{
}

void fired() final
{
Ref protectedThis { *this };
if (!m_task)
return;
m_task->execute();
auto* group = m_task->group();
ASSERT(group);
group->didExecuteScheduledTask(*this);
if (m_type == Type::OneShot)
m_task->group()->removeScheduledTimer(*this);
}

std::unique_ptr<EventLoopTask> m_task;

Seconds m_savedNextFireInterval;
Seconds m_savedRepeatInterval;
Type m_type;
bool m_suspended { false };
bool m_savedIsActive { false };
};

EventLoopTimerHandle::EventLoopTimerHandle() = default;

EventLoopTimerHandle::EventLoopTimerHandle(EventLoopTimer& timer)
: m_timer(&timer)
{ }

EventLoopTimerHandle::EventLoopTimerHandle(const EventLoopTimerHandle&) = default;
EventLoopTimerHandle::EventLoopTimerHandle(EventLoopTimerHandle&&) = default;

EventLoopTimerHandle::~EventLoopTimerHandle()
{
if (!m_timer)
return;
if (auto* group = m_timer->group(); group && m_timer->refCount() == 1) {
if (m_timer->type() == EventLoopTimer::Type::OneShot)
group->removeScheduledTimer(*m_timer);
else
group->removeRepeatingTimer(*m_timer);
}
}

EventLoopTimerHandle& EventLoopTimerHandle::operator=(const EventLoopTimerHandle&) = default;
EventLoopTimerHandle& EventLoopTimerHandle::operator=(std::nullptr_t)
{
m_timer = nullptr;
return *this;
}

EventLoop::EventLoop() = default;
EventLoop::~EventLoop() = default;

Expand All @@ -124,60 +160,42 @@ void EventLoop::queueTask(std::unique_ptr<EventLoopTask>&& task)
m_tasks.append(WTFMove(task));
}

EventLoopTimerPtr EventLoop::scheduleTask(Seconds timeout, std::unique_ptr<EventLoopTask>&& action)
EventLoopTimerHandle EventLoop::scheduleTask(Seconds timeout, std::unique_ptr<EventLoopTask>&& action)
{
auto timer = EventLoopTimer::create(WTFMove(action));
auto timer = EventLoopTimer::create(EventLoopTimer::Type::OneShot, WTFMove(action));
timer->startOneShot(timeout);

ASSERT(timer->group());
timer->group()->didAddTimer(timer);

auto handle = reinterpret_cast<EventLoopTimerPtr>(timer.ptr());
m_scheduledTasks.add(WTFMove(timer));
EventLoopTimerHandle handle { timer };
m_scheduledTasks.add(timer);
return handle;
}

void EventLoop::cancelScheduledTask(EventLoopTimerPtr handle)
void EventLoop::removeScheduledTimer(EventLoopTimer& timer)
{
auto it = m_scheduledTasks.find(reinterpret_cast<EventLoopTimer*>(handle));
if (it == m_scheduledTasks.end())
return;
Ref timer = *it;
timer->stop();
ASSERT(timer->group());
timer->group()->didRemoveTimer(timer);
m_scheduledTasks.remove(it);
ASSERT(timer.type() == EventLoopTimer::Type::OneShot);
m_scheduledTasks.remove(timer);
}

void EventLoop::didExecuteScheduledTask(EventLoopTimer& timer)
EventLoopTimerHandle EventLoop::scheduleRepeatingTask(Seconds nextTimeout, Seconds interval, std::unique_ptr<EventLoopTask>&& action)
{
m_scheduledTasks.remove(&timer);
}

// FIXME: Remove the dependency on ScriptExecutionContext.
EventLoopTimerPtr EventLoop::scheduleRepeatingTask(Seconds nextTimeout, Seconds interval, std::unique_ptr<EventLoopTask>&& action)
{
auto timer = EventLoopTimer::create(WTFMove(action));
auto timer = EventLoopTimer::create(EventLoopTimer::Type::Repeating, WTFMove(action));
timer->startRepeating(nextTimeout, interval);

ASSERT(timer->group());
timer->group()->didAddTimer(timer);

auto handle = reinterpret_cast<EventLoopTimerPtr>(timer.ptr());
m_repeatingTasks.add(WTFMove(timer));
EventLoopTimerHandle handle { timer };
m_repeatingTasks.add(timer);
return handle;
}

void EventLoop::cancelRepeatingTask(EventLoopTimerPtr handle)
void EventLoop::removeRepeatingTimer(EventLoopTimer& timer)
{
auto it = m_repeatingTasks.find(reinterpret_cast<EventLoopTimer*>(handle));
if (it == m_repeatingTasks.end())
return;
Ref timer = *it;
timer->stop();
ASSERT(timer->group());
timer->group()->didRemoveTimer(timer);
m_repeatingTasks.remove(it);
ASSERT(timer.type() == EventLoopTimer::Type::Repeating);
m_repeatingTasks.remove(timer);
}

void EventLoop::queueMicrotask(std::unique_ptr<EventLoopTask>&& microtask)
Expand Down Expand Up @@ -372,40 +390,34 @@ void EventLoopTaskGroup::runAtEndOfMicrotaskCheckpoint(EventLoop::TaskFunction&&
microtaskQueue().addCheckpointTask(makeUnique<EventLoopFunctionDispatchTask>(TaskSource::IndexedDB, *this, WTFMove(function)));
}

EventLoopTimerPtr EventLoopTaskGroup::scheduleTask(Seconds timeout, TaskSource source, EventLoop::TaskFunction&& function)
EventLoopTimerHandle EventLoopTaskGroup::scheduleTask(Seconds timeout, TaskSource source, EventLoop::TaskFunction&& function)
{
if (m_state == State::Stopped || !m_eventLoop)
return 0;
return { };
return m_eventLoop->scheduleTask(timeout, makeUnique<EventLoopFunctionDispatchTask>(source, *this, WTFMove(function)));
}

void EventLoopTaskGroup::cancelScheduledTask(EventLoopTimerPtr timer)
{
if (m_state == State::Stopped || !m_eventLoop)
return;
m_eventLoop->cancelScheduledTask(timer);
}

void EventLoopTaskGroup::didExecuteScheduledTask(EventLoopTimer& timer)
void EventLoopTaskGroup::removeScheduledTimer(EventLoopTimer& timer)
{
if (!m_eventLoop)
return;
m_eventLoop->didExecuteScheduledTask(timer);
ASSERT(timer.type() == EventLoopTimer::Type::OneShot);
if (RefPtr eventLoop = m_eventLoop.get())
eventLoop->removeScheduledTimer(timer);
m_timers.remove(timer);
}

EventLoopTimerPtr EventLoopTaskGroup::scheduleRepeatingTask(Seconds nextTimeout, Seconds interval, TaskSource source, EventLoop::TaskFunction&& function)
EventLoopTimerHandle EventLoopTaskGroup::scheduleRepeatingTask(Seconds nextTimeout, Seconds interval, TaskSource source, EventLoop::TaskFunction&& function)
{
if (m_state == State::Stopped || !m_eventLoop)
return 0;
return { };
return m_eventLoop->scheduleRepeatingTask(nextTimeout, interval, makeUnique<EventLoopFunctionDispatchTask>(source, *this, WTFMove(function)));
}

void EventLoopTaskGroup::cancelRepeatingTask(EventLoopTimerPtr timer)
void EventLoopTaskGroup::removeRepeatingTimer(EventLoopTimer& timer)
{
if (m_state == State::Stopped || !m_eventLoop)
return;
m_eventLoop->cancelRepeatingTask(timer);
ASSERT(timer.type() == EventLoopTimer::Type::Repeating);
if (RefPtr eventLoop = m_eventLoop.get())
eventLoop->removeRepeatingTimer(timer);
m_timers.remove(timer);
}

void EventLoopTaskGroup::didAddTimer(EventLoopTimer& timer)
Expand Down
45 changes: 33 additions & 12 deletions Source/WebCore/dom/EventLoop.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,29 @@ class EventLoopTask {
WeakPtr<EventLoopTaskGroup> m_group;
};

using EventLoopTimerPtr = uintptr_t;
class EventLoopTimerHandle {
WTF_MAKE_FAST_ALLOCATED;
public:
EventLoopTimerHandle();
EventLoopTimerHandle(EventLoopTimer&);
EventLoopTimerHandle(const EventLoopTimerHandle&);
EventLoopTimerHandle(EventLoopTimerHandle&&);
~EventLoopTimerHandle();

EventLoopTimerHandle& operator=(const EventLoopTimerHandle&);
EventLoopTimerHandle& operator=(std::nullptr_t);

// This conversion operator allows implicit conversion to bool but not to other integer types.
using UnspecifiedBoolType = void (EventLoopTimerHandle::*)() const;
operator UnspecifiedBoolType() const { return m_timer ? &EventLoopTimerHandle::unspecifiedBoolTypeInstance : nullptr; }

private:
friend class EventLoop;

void unspecifiedBoolTypeInstance() const { }

RefPtr<EventLoopTimer> m_timer;
};

// https://html.spec.whatwg.org/multipage/webappapis.html#event-loop
class EventLoop : public RefCounted<EventLoop>, public CanMakeWeakPtr<EventLoop> {
Expand All @@ -71,12 +93,11 @@ class EventLoop : public RefCounted<EventLoop>, public CanMakeWeakPtr<EventLoop>
typedef Function<void ()> TaskFunction;
void queueTask(std::unique_ptr<EventLoopTask>&&);

EventLoopTimerPtr scheduleTask(Seconds timeout, std::unique_ptr<EventLoopTask>&&);
void cancelScheduledTask(EventLoopTimerPtr);
void didExecuteScheduledTask(EventLoopTimer&);
EventLoopTimerHandle scheduleTask(Seconds timeout, std::unique_ptr<EventLoopTask>&&);
void removeScheduledTimer(EventLoopTimer&);

EventLoopTimerPtr scheduleRepeatingTask(Seconds nextTimeout, Seconds interval, std::unique_ptr<EventLoopTask>&&);
void cancelRepeatingTask(EventLoopTimerPtr);
EventLoopTimerHandle scheduleRepeatingTask(Seconds nextTimeout, Seconds interval, std::unique_ptr<EventLoopTask>&&);
void removeRepeatingTimer(EventLoopTimer&);

// https://html.spec.whatwg.org/multipage/webappapis.html#queue-a-microtask
void queueMicrotask(std::unique_ptr<EventLoopTask>&&);
Expand Down Expand Up @@ -111,8 +132,8 @@ class EventLoop : public RefCounted<EventLoop>, public CanMakeWeakPtr<EventLoop>

// Use a global queue instead of multiple task queues since HTML5 spec allows UA to pick arbitrary queue.
Vector<std::unique_ptr<EventLoopTask>> m_tasks;
HashSet<Ref<EventLoopTimer>> m_scheduledTasks;
HashSet<Ref<EventLoopTimer>> m_repeatingTasks;
WeakHashSet<EventLoopTimer> m_scheduledTasks;
WeakHashSet<EventLoopTimer> m_repeatingTasks;
WeakHashSet<EventLoopTaskGroup> m_associatedGroups;
WeakHashSet<EventLoopTaskGroup> m_groupsWithSuspendedTasks;
WeakHashSet<ScriptExecutionContext> m_associatedContexts;
Expand Down Expand Up @@ -180,12 +201,12 @@ class EventLoopTaskGroup : public CanMakeWeakPtr<EventLoopTaskGroup> {

void runAtEndOfMicrotaskCheckpoint(EventLoop::TaskFunction&&);

EventLoopTimerPtr scheduleTask(Seconds timeout, TaskSource, EventLoop::TaskFunction&&);
void cancelScheduledTask(EventLoopTimerPtr);
EventLoopTimerHandle scheduleTask(Seconds timeout, TaskSource, EventLoop::TaskFunction&&);
void didExecuteScheduledTask(EventLoopTimer&);
void removeScheduledTimer(EventLoopTimer&);

EventLoopTimerPtr scheduleRepeatingTask(Seconds nextTimeout, Seconds interval, TaskSource, EventLoop::TaskFunction&&);
void cancelRepeatingTask(EventLoopTimerPtr);
EventLoopTimerHandle scheduleRepeatingTask(Seconds nextTimeout, Seconds interval, TaskSource, EventLoop::TaskFunction&&);
void removeRepeatingTimer(EventLoopTimer&);

void didAddTimer(EventLoopTimer&);
void didRemoveTimer(EventLoopTimer&);
Expand Down
6 changes: 1 addition & 5 deletions Source/WebCore/editing/AlternativeTextController.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,6 @@ void AlternativeTextController::startAlternativeTextUITimer(AlternativeTextType
if (type == AlternativeTextType::Correction)
m_rangeWithAlternative = std::nullopt;
m_type = type;
if (m_timer)
m_document.eventLoop().cancelScheduledTask(m_timer);
m_timer = m_document.eventLoop().scheduleTask(correctionPanelTimerInterval, TaskSource::UserInteraction, [weakThis = WeakPtr { *this }] {
if (!weakThis)
return;
Expand All @@ -124,9 +122,7 @@ void AlternativeTextController::startAlternativeTextUITimer(AlternativeTextType

void AlternativeTextController::stopAlternativeTextUITimer()
{
if (m_timer)
m_document.eventLoop().cancelScheduledTask(m_timer);
m_timer = 0;
m_timer = nullptr;
m_rangeWithAlternative = std::nullopt;
}

Expand Down
5 changes: 2 additions & 3 deletions Source/WebCore/editing/AlternativeTextController.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

#include "AlternativeTextClient.h"
#include "DocumentMarker.h"
#include "EventLoop.h"
#include "Position.h"
#include <variant>
#include <wtf/Noncopyable.h>
Expand All @@ -46,8 +47,6 @@ struct DictationAlternative;
struct SimpleRange;
struct TextCheckingResult;

using EventLoopTimerPtr = uintptr_t;

#if USE(AUTOCORRECTION_PANEL)
// These backslashes are for making style checker happy.
#define UNLESS_ENABLED(functionBody) \
Expand Down Expand Up @@ -120,7 +119,7 @@ class AlternativeTextController : public CanMakeWeakPtr<AlternativeTextControlle
FloatRect rootViewRectForRange(const SimpleRange&) const;
void markPrecedingWhitespaceForDeletedAutocorrectionAfterCommand(EditCommand*);

EventLoopTimerPtr m_timer { 0 };
EventLoopTimerHandle m_timer;
std::optional<SimpleRange> m_rangeWithAlternative;
bool m_isActive { };
bool m_isDismissedByEditing { };
Expand Down
7 changes: 1 addition & 6 deletions Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5748,14 +5748,9 @@ void WebGLRenderingContextBase::maybeRestoreContextSoon(Seconds timeout)
if (!scriptExecutionContext)
return;

if (m_restoreTimer) {
scriptExecutionContext->eventLoop().cancelScheduledTask(m_restoreTimer);
m_restoreTimer = 0;
}

m_restoreTimer = scriptExecutionContext->eventLoop().scheduleTask(timeout, TaskSource::WebGL, [weakThis = WeakPtr { *this }] {
if (CheckedPtr checkedThis = weakThis.get()) {
checkedThis->m_restoreTimer = 0;
checkedThis->m_restoreTimer = nullptr;
checkedThis->maybeRestoreContext();
}
});
Expand Down
5 changes: 2 additions & 3 deletions Source/WebCore/html/canvas/WebGLRenderingContextBase.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#if ENABLE(WEBGL)

#include "ActivityStateChangeObserver.h"
#include "EventLoop.h"
#include "ExceptionOr.h"
#include "GPUBasedCanvasRenderingContext.h"
#include "GraphicsContextGL.h"
Expand Down Expand Up @@ -156,8 +157,6 @@ using WebGLCanvas = std::variant<RefPtr<HTMLCanvasElement>>;
class VideoFrame;
#endif

using EventLoopTimerPtr = uintptr_t;

class InspectorScopedShaderProgramHighlight {
public:
InspectorScopedShaderProgramHighlight(WebGLRenderingContextBase&, WebGLProgram*);
Expand Down Expand Up @@ -619,7 +618,7 @@ class WebGLRenderingContextBase : public GraphicsContextGL::Client, public GPUBa
RefPtr<WebGLContextGroup> m_contextGroup;
Lock m_objectGraphLock;

EventLoopTimerPtr m_restoreTimer { 0 };
EventLoopTimerHandle m_restoreTimer;
GCGLErrorCodeSet m_errors;
bool m_needsUpdate;
bool m_markedCanvasDirty;
Expand Down
Loading

0 comments on commit 6b2ef31

Please sign in to comment.