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

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

 /*
  * Copyright (C) 2010, 2011 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 674 matching lines @@
 @end
 
 namespace blink {
 
 ScrollAnimatorBase* ScrollAnimatorBase::create(ScrollableArea* scrollableArea) {
   return new ScrollAnimatorMac(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()
                        ->timerTaskRunner()
                        ->clone()),
       m_haveScrolledSincePageLoad(false),
       m_needsScrollerStyleUpdate(false) {
   ThreadState::current()->registerPreFinalizer(this);
 
   m_scrollAnimationHelperDelegate.adoptNS(
@@ skipping 18 matching lines @@
 
 void ScrollAnimatorMac::dispose() {
   BEGIN_BLOCK_OBJC_EXCEPTIONS;
   [m_scrollbarPainterControllerDelegate.get() invalidate];
   [m_scrollbarPainterController.get() setDelegate:nil];
   [m_horizontalScrollbarPainterDelegate.get() invalidate];
   [m_verticalScrollbarPainterDelegate.get() invalidate];
   [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,
                                            const ScrollOffset& delta) {
   m_haveScrolledSincePageLoad = true;
 
   if (!m_scrollableArea->scrollAnimatorEnabled())
     return ScrollAnimatorBase::userScroll(granularity, delta);
 
   if (granularity == ScrollByPixel || granularity == ScrollByPrecisePixel)
@@ skipping 310 matching lines @@
   // page cache, and
   // a relayout will happen on its own. Otherwise, we must initiate a re-layout
   // ourselves.
   if (!m_needsScrollerStyleUpdate)
     getScrollableArea()->scrollbarStyleChanged();
 
   m_needsScrollerStyleUpdate = false;
 }
 
 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() {
   // To force the scrollbars to flash, we have to call hide first. Otherwise,
   // the ScrollbarPainterController
   // might think that the scrollbars are already showing and bail early.
   [m_scrollbarPainterController.get() hideOverlayScrollers];
   [m_scrollbarPainterController.get() flashScrollers];
 }
 
 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() {
   if (supportsContentAreaScrolledInDirection()) {
     [m_scrollbarPainterController.get()
         contentAreaScrolledInDirection:NSMakePoint(
                                            m_contentAreaScrolledTimerScrollDelta
                                                .width(),
                                            m_contentAreaScrolledTimerScrollDelta
                                                .height())];
     m_contentAreaScrolledTimerScrollDelta = ScrollOffset();
   } else
     [m_scrollbarPainterController.get() contentAreaScrolled];
 }
 
 void ScrollAnimatorMac::setVisibleScrollerThumbRect(
     const IntRect& scrollerThumb) {
   IntRect rectInViewCoordinates = scrollerThumb;
   if (Scrollbar* verticalScrollbar = m_scrollableArea->verticalScrollbar())
     rectInViewCoordinates =
         verticalScrollbar->convertToContainingWidget(scrollerThumb);
 
   if (rectInViewCoordinates == m_visibleScrollerThumbRect)
     return;
 
   m_visibleScrollerThumbRect = rectInViewCoordinates;
 }
 
 }  // namespace blink