[webkit-reviews] review granted: [Bug 226734] Drop legacy MainThreadTaskQueue & EventLoopTaskQueue classes : [Attachment 430772] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 226734: Drop legacy MainThreadTaskQueue & EventLoopTaskQueue classes
https://bugs.webkit.org/show_bug.cgi?id=226734

Attachment 430772: Patch

https://bugs.webkit.org/attachment.cgi?id=430772&action=review




--- Comment #4 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 430772
  --> https://bugs.webkit.org/attachment.cgi?id=430772
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=430772&action=review

> Source/WTF/wtf/CancellableTask.h:36
> +class TaskCanceller : public CanMakeWeakPtr<TaskCanceller> {

I would have called this TaskCancelGroup or something.

> Source/WTF/wtf/CancellableTask.h:54
> +	   void taskIsNoLongerPending() { m_impl = nullptr; }

This should be a verb instead.
maybe clear?

> Source/WebCore/dom/ActiveDOMObject.h:118
> +	   CancellableTask cancellableTask(taskCanceller, WTFMove(task));
>	   object.queueTaskInEventLoop(source, [protectedObject =
makeRef(object), activity =
object.ActiveDOMObject::makePendingActivity(object), cancellableTask =
WTFMove(cancellableTask)]() mutable {

Hm... this would mean that the object is kept alive with a pending activity
even when the task is canceled?
That doesn't seem ideal.
We should probably follow up so that we can delete the task object itself from
the event loop queue when we cancel a task.