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..5b311bdf6afaff74731defeceed79b2aa44bf5b4 100644
--- a/third_party/WebKit/Source/platform/WebTaskRunner.cpp
+++ b/third_party/WebKit/Source/platform/WebTaskRunner.cpp
@@ -6,6 +6,50 @@
 
 namespace blink {
 
+class TaskHandle::ClearOnScopeOut {

[haraken, 2016/10/19 15:20:39]

Sorry, would you help me understand why we need ClearOnScopeOut?

[tzik, 2016/10/19 20:46:47]

I'll add a comment for this. This is to avoid a circular reference among related
stuff:
If we don't clear it on the destruction of the wrapped callback, below will make
a reference cycle for foo -> m_handle -> m_task -> Bind()'s internal storage ->
foo.

struct Foo : GarbageCollected<Foo> {
  RefPtr<TaskHandle> m_handle;
  void bar();
};

Foo* foo = new Foo;
foo->m_handle = taskRunner->postCancellableTask(WTF::bind(&Foo::bar,
wrapPersistent(foo)));

[haraken, 2016/10/20 09:42:46]

That makes sense.

However, then why is it not enough to call TaskHandle::cancel() after running
the task? I don't fully understand why we need the comlexity of ClearOnScopeOut.

[tzik, 2016/10/20 16:15:12]

That still leaves a circular reference if a task is deleted before running.

+ public:
+  explicit ClearOnScopeOut(WTF::PassRefPtr<TaskHandle> handle)
+      : m_handle(std::move(handle)) {}
+
+  ClearOnScopeOut(ClearOnScopeOut&&) = default;
+
+  void cancel() const {
+    if (m_handle)
+      m_handle->cancel();
+  }
+
+  ~ClearOnScopeOut() { cancel(); }

[haraken, 2016/10/19 15:20:39]

For example, why we need to call cancel() when ClearOnScopeOut is destructed?

[tzik, 2016/10/20 16:15:12]

This is to clear the TaskHandle::m_task to cut the circular reference.

+
+ private:
+  RefPtr<TaskHandle> m_handle;
+};
+
+bool TaskHandle::isActive() const {
+  return static_cast<bool>(m_task);
+}
+
+void TaskHandle::cancel() {
+  std::unique_ptr<WTF::Closure> task = std::move(m_task);

[alex clarke (OOO till 29th), 2016/10/19 15:36:46]

Is there any reason to prefer this over: m_task = WTF::Closure()

[tzik, 2016/10/19 20:46:47]

Just in case. This is to avoid potential reentrancy. If the destructor of m_task
touches TaskHandle, m_task can be an undefined state.

[dcheng, 2016/10/19 22:21:47]

FWIW, probably worth adding a comment for this.

+  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 ClearOnScopeOut& scoper) {
+  std::unique_ptr<WTF::Closure> task = std::move(m_task);
+  scoper.cancel();

[haraken, 2016/10/19 15:20:39]

Can we simply call cancel() instead of scoper.cancel()?

[alex clarke (OOO till 29th), 2016/10/19 15:36:46]

+1 Haraken is essentially suggesting what TimerBase::runInternal does today. In
so far as cancellation goes.

[tzik, 2016/10/20 16:15:12]

Done.

+  (*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 +78,26 @@ void WebTaskRunner::postDelayedTask(const WebTraceLocation& location,
       base::TimeDelta::FromMilliseconds(delayMs));
 }
 
+PassRefPtr<TaskHandle> WebTaskRunner::postCancellableTask(

[dcheng, 2016/10/19 22:21:47]

My understanding is that PassRefPtr is deprecated, and we should just use RefPtr
directly.

[tzik, 2016/10/20 16:15:12]

Done.

+    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::ClearOnScopeOut(handle)));
+  return handle.release();
+}
+
+PassRefPtr<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::ClearOnScopeOut(handle)),
+                  delayMs);
+  return handle.release();
+}
+
 }  // namespace blink