Scheduler: Deprecate CancellableTaskFactory in favor of WebTaskRunner::postCancellableTask (3)

third_party/WebKit/Source/platform/mac/ScrollAnimatorMac.mm

Index: third_party/WebKit/Source/platform/mac/ScrollAnimatorMac.mm
diff --git a/third_party/WebKit/Source/platform/mac/ScrollAnimatorMac.mm b/third_party/WebKit/Source/platform/mac/ScrollAnimatorMac.mm
index edb3195e8739f2ff5065312f780f5f744acf98ca..e6b9b6253427ab372b903d1b9195e3f5552eb12e 100644
--- a/third_party/WebKit/Source/platform/mac/ScrollAnimatorMac.mm
+++ b/third_party/WebKit/Source/platform/mac/ScrollAnimatorMac.mm
@@ -692,12 +692,6 @@ ScrollAnimatorBase* ScrollAnimatorBase::create(ScrollableArea* scrollableArea) {
 
 ScrollAnimatorMac::ScrollAnimatorMac(ScrollableArea* scrollableArea)
     : ScrollAnimatorBase(scrollableArea),
-      m_initialScrollbarPaintTaskFactory(CancellableTaskFactory::create(
-          this,
-          &ScrollAnimatorMac::initialScrollbarPaintTask)),
-      m_sendContentAreaScrolledTaskFactory(CancellableTaskFactory::create(
-          this,
-          &ScrollAnimatorMac::sendContentAreaScrolledTask)),
       m_taskRunner(Platform::current()
                        ->currentThread()
                        ->scheduler()
@@ -736,8 +730,8 @@ void ScrollAnimatorMac::dispose() {
   [m_scrollAnimationHelperDelegate.get() invalidate];
   END_BLOCK_OBJC_EXCEPTIONS;
 
-  m_initialScrollbarPaintTaskFactory->cancel();
-  m_sendContentAreaScrolledTaskFactory->cancel();
+  m_initialScrollbarPaintTaskHandle.cancel();
+  m_sendContentAreaScrolledTaskHandle.cancel();
 }
 
 ScrollResult ScrollAnimatorMac::userScroll(ScrollGranularity granularity,
@@ -1068,17 +1062,18 @@ void ScrollAnimatorMac::updateScrollerStyle() {
 }
 
 void ScrollAnimatorMac::startScrollbarPaintTimer() {
-  m_taskRunner->postDelayedTask(
-      BLINK_FROM_HERE, m_initialScrollbarPaintTaskFactory->cancelAndCreate(),
+  m_initialScrollbarPaintTaskHandle = m_taskRunner->postDelayedCancellableTask(
+      BLINK_FROM_HERE, WTF::bind(&ScrollAnimatorMac::initialScrollbarPaintTask,
+                                 wrapWeakPersistent(this)),

[nhiroki, 2016/11/21 05:45:54]

Can we make this wrapPersistent(this)? I'm assuming that
ScrollAnimatorMac::dispose() is always called when ScrollAnimatorMac is no
longer necessary and it cuts a strong reference from a posted task to |this|.

(I tried to confirm this but I couldn't trace who calls
ScrollAnimatorMac::dispose() on the codesearch)

[haraken, 2016/11/21 06:42:24]

It's called by a pre-finalizer:

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/mac/S...

In other words, dispose() is called after all strong references to the scroll
animator object is gone and Oilpan has decided to destruct the object. So we
should use wrapWeakPersistent.

That said, using wrapPersistent won't cause a big issue because the timer will
be invoked and the persistent handle is dropped in a finite time period.

[nhiroki, 2016/11/21 08:15:16]

Thank you for the clarification :)

       0.1);

[nhiroki, 2016/11/21 05:45:54]

This fails compile on Mac bots as follows:

[29712/38626] OBJCXX
obj/third_party/WebKit/Source/platform/platform/ScrollAnimatorMac.o
FAILED: obj/third_party/WebKit/Source/platform/platform/ScrollAnimatorMac.o 
../../third_party/WebKit/Source/platform/mac/ScrollAnimatorMac.mm:1068:7: error:
implicit conversion from 'double' to 'long long' changes value from 0.1 to 0
[-Werror,-Wliteral-conversion]
      0.1);
      ^~~
1 error generated.

I'm not sure why the original code was compilable. postDelayedTask() also takes
"long long" as the 3rd argument. Anyway, should we replace this with
postCancellableTask()?

[haraken, 2016/11/21 06:42:24]

Yeah, 0.1 looks like a random number. +1 to changing it to postCancellableTask.

[nhiroki, 2016/11/21 08:15:16]

Done.

 }
 
 bool ScrollAnimatorMac::scrollbarPaintTimerIsActive() const {
-  return m_initialScrollbarPaintTaskFactory->isPending();
+  return m_initialScrollbarPaintTaskHandle.isActive();
 }
 
 void ScrollAnimatorMac::stopScrollbarPaintTimer() {
-  m_initialScrollbarPaintTaskFactory->cancel();
+  m_initialScrollbarPaintTaskHandle.cancel();
 }
 
 void ScrollAnimatorMac::initialScrollbarPaintTask() {
@@ -1092,10 +1087,12 @@ void ScrollAnimatorMac::initialScrollbarPaintTask() {
 void ScrollAnimatorMac::sendContentAreaScrolledSoon(const ScrollOffset& delta) {
   m_contentAreaScrolledTimerScrollDelta = delta;
 
-  if (!m_sendContentAreaScrolledTaskFactory->isPending())
-    m_taskRunner->postTask(
-        BLINK_FROM_HERE,
-        m_sendContentAreaScrolledTaskFactory->cancelAndCreate());
+  if (m_sendContentAreaScrolledTaskHandle.isActive())
+    return;
+  m_sendContentAreaScrolledTaskHandle = m_taskRunner->postCancellableTask(
+      BLINK_FROM_HERE,
+      WTF::bind(&ScrollAnimatorMac::sendContentAreaScrolledTask,
+                wrapWeakPersistent(this)));

[nhiroki, 2016/11/21 05:45:54]

ditto.

 }
 
 void ScrollAnimatorMac::sendContentAreaScrolledTask() {