Move logic of recording main thread scrolling reasons from cc to blink::ScrollManager

third_party/WebKit/Source/core/input/ScrollManager.cpp

Index: third_party/WebKit/Source/core/input/ScrollManager.cpp
diff --git a/third_party/WebKit/Source/core/input/ScrollManager.cpp b/third_party/WebKit/Source/core/input/ScrollManager.cpp
index 727c3e74e57525196f5f4abb1ec12c18a21ecc4c..48d4b8b6d662f30ae2e0ee8c3487807dcce92d58 100644
--- a/third_party/WebKit/Source/core/input/ScrollManager.cpp
+++ b/third_party/WebKit/Source/core/input/ScrollManager.cpp
@@ -22,6 +22,7 @@
 #include "core/page/scrolling/RootScrollerController.h"
 #include "core/page/scrolling/ScrollState.h"
 #include "core/paint/PaintLayer.h"
+#include "platform/Histogram.h"
 #include "platform/RuntimeEnabledFeatures.h"
 #include "wtf/PtrUtil.h"
 
@@ -196,6 +197,77 @@ void ScrollManager::customizedScroll(const Node& startNode,
   scrollState.distributeToScrollChainDescendant();
 }
 
+// When scrolling on the main thread, the scrollableArea may or may not be
+// composited.

[bokan, 2017/03/31 16:55:53]

super-nit: No need for an early line break here, wrap the below line on this
line. Also, I think we generally put a comment like this inside the function.

+// If it's composited and has the main thread scrolling reasons
+// stored in its layer, the reasons have been recorded on cc side. If it dosen't
+// have existing reasons but scrolls on main, we walk up its containing scroll
+// chain to find the first non composited region and record reaons accordingly.

[bokan, 2017/03/31 16:55:53]

nit: reaons->reasons

+// If no such region is found or it doesn't have any main thread scorlling

[bokan, 2017/03/31 16:55:53]

nit: scorlling->scrolling

+// reason, we record the reason kUnknownNonCompositedRegion.
+// If it's not composited, we have recorded "NonFastScrollableRegion" on
+// the cc side. Here we try to record additional reasons that prevent promotion.
+uint32_t ScrollManager::computeNonCompositedMainThreadScrollingReasons() {
+  if (!m_scrollGestureHandlingNode->layoutObject() || !m_frame->view())
+    return 0;
+  uint32_t reasons = 0;
+  LayoutBox* curBox =
+      m_scrollGestureHandlingNode->layoutObject()->enclosingBox();
+  bool compositedMainThreadScrollForUnknownReasons = false;
+
+  while (curBox) {
+    if (auto* scrollableArea = curBox->getScrollableArea()) {

[bokan, 2017/03/31 16:55:52]

The repeated indents here make the code harder to follow. You can make it
simpler by using a for loop instead and avoiding the outermost block using:

auto* scrollableArea = curBox->getScrollableArea();
if (!scrollableArea)
  continue;

+      if (scrollableArea->usesCompositedScrolling()) {
+        if (m_frame->view()->mainThreadScrollingReasons()) {
+          return 0;

[bokan, 2017/03/31 16:55:53]

I don't think you need this early return since it might prevent us from
recording a NonComposited reason that's caused by a parent scroller. In that
case, I think we'd want to record both the mainTreadScrollingReason (which was
done in CC) and the NonCompositedReason from the parent since if we optimised
Chrome to fix the mainThreadScrollingReason so we could scroll on the
compositor, the scroller still wouldn't be scrolling on main because of the
NonFastScrollingRegion. So I think we want to record both reasons in that case.

+        }
+        compositedMainThreadScrollForUnknownReasons = true;

[bokan, 2017/03/31 16:55:53]

We want to record "Unknown" if and only if we didn't hit any non-composited
scrollers on the walk *and* none of the composited scrollers in the chain had a
mainThreadScrollingReason (since in that case, we were sent to main for that
reason).

Given that, I'd call this bool "haveMainThreadScrollingReason" and change the
code to match the name. This name better describes the "why" of recording the
Unknown reason and the code will read more naturally because of it.

+      } else {
+        reasons = scrollableArea->getNonCompositedMainThreadScrollingReasons();

[bokan, 2017/03/31 16:55:53]

Add a DCHECK here that we have a reason.

+        break;
+      }
+    }
+    curBox = curBox->containingBlock();
+  }
+
+  if (!reasons && compositedMainThreadScrollForUnknownReasons)
+    return MainThreadScrollingReason::kUnknownNonCompositedRegion;
+
+  return reasons;

[bokan, 2017/03/31 16:55:53]

Add a DCHECK here that reasons only contains NonCompositedScrollReasons

+}
+
+void ScrollManager::recordNonCompositedMainThreadScrollingReasons(
+    const WebGestureDevice device) {
+  if (device != WebGestureDeviceTouchpad &&
+      device != WebGestureDeviceTouchscreen) {
+    return;
+  }
+
+  uint32_t reasons = computeNonCompositedMainThreadScrollingReasons();
+  if (!reasons)
+    return;
+
+  uint32_t mainThreadScrollingReasonEnumMax =
+      MainThreadScrollingReason::kMainThreadScrollingReasonCount + 1;
+  for (uint32_t i = 0;
+       i < MainThreadScrollingReason::kMainThreadScrollingReasonCount; ++i) {

[bokan, 2017/03/31 16:55:52]

You only need to loop over the NonCompositedScrollReasons here. Add a
NonCompositedReasonsFirst, and NonCompositedReasonsLast in the enum and use
that.

+    unsigned val = 1 << i;
+    if (reasons & val) {
+      if (device == WebGestureDeviceTouchscreen) {
+        DEFINE_STATIC_LOCAL(EnumerationHistogram, touchHistogram,
+                            ("Renderer4.MainThreadGestureScrollReason",
+                             mainThreadScrollingReasonEnumMax));
+        touchHistogram.count(i + 1);
+      } else {
+        DEFINE_STATIC_LOCAL(EnumerationHistogram, wheelHistogram,
+                            ("Renderer4.MainThreadWheelScrollReason",
+                             mainThreadScrollingReasonEnumMax));
+        wheelHistogram.count(i + 1);
+      }
+    }
+  }
+}
+
 WebInputEventResult ScrollManager::handleGestureScrollBegin(
     const WebGestureEvent& gestureEvent) {
   Document* document = m_frame->document();
@@ -221,6 +293,8 @@ WebInputEventResult ScrollManager::handleGestureScrollBegin(
   passScrollGestureEvent(gestureEvent,
                          m_scrollGestureHandlingNode->layoutObject());
 
+  recordNonCompositedMainThreadScrollingReasons(gestureEvent.sourceDevice);
+
   m_currentScrollChain.clear();
   std::unique_ptr<ScrollStateData> scrollStateData =
       WTF::makeUnique<ScrollStateData>();