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 6d05fe059f6f8a7ddb8b1370866f43cd5abda5ea..b2f462537c383bd43f809732ecaa11c2a59bfa7c 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,56 @@ void ScrollManager::customizedScroll(const Node& startNode,
   scrollState.distributeToScrollChainDescendant();
 }
 
+uint32_t ScrollManager::computeNonCompositedMainThreadScrollingReasons() {
+  if (!m_scrollGestureHandlingNode->layoutObject())
+    return 0;
+  uint32_t reasons = 0;
+  LayoutBox* curBox =
+      m_scrollGestureHandlingNode->layoutObject()->enclosingBox();
+  while (curBox) {
+    if (auto* scrollableArea = curBox->getScrollableArea()) {
+      if (scrollableArea->isScrollable()) {

[bokan, 2017/03/28 15:30:15]

On second look, I don't think this is enough. If we have a non-composited
scroller, the region that scroller occupies is added to the FrameView's non-fast
scroll regions. There's no notion of overlap there, meaning that if the
non-composited scroller is behind a composited scroller, it'll cause scrolls
over the latter to go to the main thread. Worse still, the non-composited
scroller may not be in the scroll chain of the composited, so we have no way to
get back to the original reason we were sent to main. e.g.:

<div id="a" style="width: 100px; height: 100px; overflow:scroll; opacity: 0.5;
will-change: transform">
  ...
</div>
<div id="b" style="width: 100px; height: 100px; overflow:scroll; position:
absolute; background-color: white; contains: paint">
  ...
</div>

In this case #b is composited and shows up overtop of #a. Scrolling it doesn't
cause scroll to chain up to #a but #a's presence means that the scrolls will be
sent to main. We'll miss the reason in this case.

I'm not sure on how to proceed here - another idea might be to attach the reason
to each shape in the NonFastScrollableRegion so that we record all this on CC
(again). This seemed like a good idea at first, but taking a look it seems
there'd be some significant changes in how we store the NonFastScrollRegions. I
believe we'll union together contained regions as an optimisation - we'd have to
stop doing that and store a complete list of rects/shapes in Region with a
reason attached to each. I think that'd work but could have a perf impact on
detecting when we're in a non-fast scroll region, I'm not sure how critical that
code is.

Perhaps adding an "Unknown" bucket and just accepting some inaccuracy in these
cases is preferable. I'd like to hear what others think.

[tdresser, 2017/03/28 17:10:22]

Can you describe with a bit more detail how we'd identify when we're in the
"Unknown" case? My intuition is that using an unknown bucket would be a good
first step.

[bokan, 2017/03/28 18:23:02]

We do this walk up the containingBlock chain until we get to the first
non-composited scroller and use its main thread scroll reason. If we hit the
root and all the scrollers in the chain are composited, it must be because of an
element not in the containing block chain, so we'd put these scrolls into a
separate bucket. I think this works since I can't find a way to position a child
scroller outside a parent scroller but keep them scroll chained, changing
position schemes changes the containingBlock hierarchy.

I'm not sure how common that "non-composited underlay" will be but at least it
might give us an idea if we need to improve the metric collection or not to
worry about it, before we do intrusive rearchitecting of NonFastScrollRegions. 

[tdresser, 2017/03/28 19:25:14]

Thanks. Yeah, considering that bucket separately SGTM.

+        reasons = scrollableArea->getNonCompositedMainThreadScrollingReasons();
+        break;
+      }
+    }
+    curBox = curBox->containingBlock();
+  }
+  return reasons;
+}
+
+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) {
+    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 +272,8 @@ WebInputEventResult ScrollManager::handleGestureScrollBegin(
   passScrollGestureEvent(gestureEvent,
                          m_scrollGestureHandlingNode->layoutObject());
 
+  recordNonCompositedMainThreadScrollingReasons(gestureEvent.sourceDevice);
+
   m_currentScrollChain.clear();
   std::unique_ptr<ScrollStateData> scrollStateData =
       WTF::makeUnique<ScrollStateData>();