Add WebTaskRunner::postCancellableTask

third_party/WebKit/Source/platform/WebTaskRunner.cpp

Index: third_party/WebKit/Source/platform/WebTaskRunner.cpp
diff --git a/third_party/WebKit/Source/platform/WebTaskRunner.cpp b/third_party/WebKit/Source/platform/WebTaskRunner.cpp
index 548dbc9ac1d500a31170856fbe2966d1ce612059..23406328c37136b9047076525d3bfa45ad692819 100644
--- a/third_party/WebKit/Source/platform/WebTaskRunner.cpp
+++ b/third_party/WebKit/Source/platform/WebTaskRunner.cpp
@@ -6,6 +6,65 @@
 
 namespace blink {
 
+// This class holds a reference to a TaskHandle to keep it alive while a task is
+// pending in a task queue, and clears the reference on the task disposal, so
+// that it doesn't leave a circular reference like below:
+//   struct Foo : GarbageCollected<Foo> {
+//     void bar() {}
+//     RefPtr<TaskHandle> m_handle;
+//   };
+//
+//   foo->m_handle = taskRunner->postCancellableTask(
+//       BLINK_FROM_HERE, WTF::bind(&Foo::bar, wrapPersistent(foo)));
+//
+// There is a circular reference in the example above as:
+//   foo -> m_handle -> m_task -> Persistent<Foo> in WTF::bind.
+// CancelOnTaskDestruction is needed to break the circle by clearing |m_task|

[alex clarke (OOO till 29th), 2016/10/26 12:40:42]

I wonder if we could do something in a follow on patch to detect if this is
necessary for a given WTF::Closure and use a "fast path" when it isn't?

Maybe that is a micro optimization too far?

[tzik, 2016/10/27 06:38:53]

Do you mean a "fast path" for closures that don't have any strong reference (or
maybe just detect trivially destructible objects)?

[alex clarke (OOO till 29th), 2016/10/27 09:46:37]

I'm wondering about things like this:

foo->m_handle = taskRunner->postCancellableTask(
       BLINK_FROM_HERE, WTF::bind(&Foo::bar, foo->GetWeakPtr()));

It might require some plumbing (not in this patch!) but it should be possible to
detect the weak pointer.

+// when the wrapped WTF::Closure is deleted.
+class TaskHandle::CancelOnTaskDestruction {
+ public:
+  explicit CancelOnTaskDestruction(RefPtr<TaskHandle> handle)
+      : m_handle(std::move(handle)) {}
+
+  CancelOnTaskDestruction(CancelOnTaskDestruction&&) = default;
+
+  void cancel() const {
+    if (m_handle)
+      m_handle->cancel();
+  }
+
+  ~CancelOnTaskDestruction() { cancel(); }
+
+ private:
+  RefPtr<TaskHandle> m_handle;

[Sami, 2016/10/26 12:32:51]

DISALLOW_COPY_AND_ASSIGN?

[tzik, 2016/10/27 06:38:53]

Done.

+};
+
+bool TaskHandle::isActive() const {
+  return static_cast<bool>(m_task);
+}
+
+void TaskHandle::cancel() {
+  std::unique_ptr<WTF::Closure> task = std::move(m_task);
+  m_weakPtrFactory.revokeAll();
+}
+
+TaskHandle::~TaskHandle() {}
+
+TaskHandle::TaskHandle(std::unique_ptr<WTF::Closure> task)
+    : m_task(std::move(task)), m_weakPtrFactory(this) {
+  DCHECK(m_task);
+}
+
+void TaskHandle::run(const CancelOnTaskDestruction& scoper) {
+  std::unique_ptr<WTF::Closure> task = std::move(m_task);
+  cancel();

[dcheng, 2016/10/26 01:58:54]

This call looks a bit interesting, because cancel() also moves m_task into a
local temporary.

(I wonder if cancel() can just reset() m_task after invalidating the weak
pointers?)

[tzik, 2016/10/27 06:38:53]

I think cancel() can be potentially reenterred through a destructor of bound
objects, and causes a double free if we call reset() directly there.
# libstdc++ implementation seems to safe for this, but libc++ implementation
doesn't.

+  (*task)();
+}
+
+WTF::WeakPtr<TaskHandle> TaskHandle::asWeakPtr() {
+  return m_weakPtrFactory.createWeakPtr();
+}
+
 void WebTaskRunner::postTask(const WebTraceLocation& location,
                              std::unique_ptr<CrossThreadClosure> task) {
   toSingleThreadTaskRunner()->PostTask(location,
@@ -34,4 +93,27 @@ void WebTaskRunner::postDelayedTask(const WebTraceLocation& location,
       base::TimeDelta::FromMilliseconds(delayMs));
 }
 
+RefPtr<TaskHandle> WebTaskRunner::postCancellableTask(
+    const WebTraceLocation& location,
+    std::unique_ptr<WTF::Closure> task) {
+  DCHECK(runsTasksOnCurrentThread());
+  RefPtr<TaskHandle> handle = adoptRef(new TaskHandle(std::move(task)));
+  postTask(location, WTF::bind(&TaskHandle::run, handle->asWeakPtr(),
+                               TaskHandle::CancelOnTaskDestruction(handle)));
+  return handle;
+}
+
+RefPtr<TaskHandle> WebTaskRunner::postDelayedCancellableTask(
+    const WebTraceLocation& location,
+    std::unique_ptr<WTF::Closure> task,
+    long long delayMs) {
+  DCHECK(runsTasksOnCurrentThread());
+  RefPtr<TaskHandle> handle = adoptRef(new TaskHandle(std::move(task)));
+  postDelayedTask(location,
+                  WTF::bind(&TaskHandle::run, handle->asWeakPtr(),
+                            TaskHandle::CancelOnTaskDestruction(handle)),
+                  delayMs);
+  return handle;
+}
+
 }  // namespace blink