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..b42b7c85b6be1b6cda26a48aa131191423ccdf32 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,81 @@ void ScrollManager::customizedScroll(const Node& startNode,
   scrollState.distributeToScrollChainDescendant();
 }
 
+uint32_t ScrollManager::computeNonCompositedMainThreadScrollingReasons() {
+  // When scrolling on the main thread, the scrollableArea may or may not be
+  // composited. 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

[bokan, 2017/04/06 14:25:24]

I think this line and the section below are out of date. We don't "find the
first" anymore, we look at all the scrollers in the chain.

In general, you should avoid comments that are basically pseudo code for what's
happening below, the reader can follow the code to find out what it's doing.
Instead: describe why you're doing it and/or provide missing context. You're
lines above are great, the fact that the other mainThreadScrollingReasons are
recorded on CC is missing context and needed to understand what we're doing
here.

What we're trying to do here is "record scrolls that occurred on main due to a
non-composited scroller". How that's done is described below by the code. 

+  // reasons accordingly. If no such region is found or it doesn't have any main
+  // thread 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.
+  if (!m_scrollGestureHandlingNode->layoutObject() || !m_frame->view())
+    return 0;
+
+  uint32_t reasons = 0;

[bokan, 2017/04/06 14:25:24]

rename to nonCompositedReasons or similar.

+  bool hasScrollsOverflowAreaInChain = false;
+
+  for (auto* curBox =
+           m_scrollGestureHandlingNode->layoutObject()->enclosingBox();
+       curBox; curBox = curBox->containingBlock()) {
+    PaintLayerScrollableArea* scrollableArea = curBox->getScrollableArea();
+
+    if (!scrollableArea || !scrollableArea->scrollsOverflow())
+      continue;
+
+    hasScrollsOverflowAreaInChain = true;
+    uint32_t nonCompositedMainThreadScrollingReasons =
+        scrollableArea->getNonCompositedMainThreadScrollingReasons();
+    if (scrollableArea->usesCompositedScrolling()) {
+      DCHECK(!nonCompositedMainThreadScrollingReasons);

[bokan, 2017/04/06 14:25:24]

You shouldn't branch just because you have a DCHECK. Instead,
DCHECK(!scrollableArea->usesCompositedScrolling() ||
!nonCompositedMainThreadScrollingReasons). And if
nonCompositedMainThreadScrollingReasons == 0 in the usesCompositedScrolling
case, you can just |= it into reasons unconditionally.

+    } else {
+      reasons |= nonCompositedMainThreadScrollingReasons;
+    }
+  }
+
+  if (!reasons && hasScrollsOverflowAreaInChain &&
+      !m_frame->view()->mainThreadScrollingReasons())
+    return MainThreadScrollingReason::kUnknownNonCompositedRegion;

[bokan, 2017/04/06 14:25:24]

Add a comment here describing this reason. i.e. There must be a non-composited
scroller in the region the scroll event occurred in but it's overlapped by
another scroller that has no reason to go to main. 

+
+  return reasons;
+}
+
+void ScrollManager::recordNonCompositedMainThreadScrollingReasons(
+    const WebGestureDevice device) {
+  if (device != WebGestureDeviceTouchpad &&
+      device != WebGestureDeviceTouchscreen) {
+    return;
+  }
+
+  uint32_t reasons = computeNonCompositedMainThreadScrollingReasons();
+  if (!reasons)
+    return;
+  DCHECK(MainThreadScrollingReason::NonCompositedScrollReasons(reasons));
+
+  uint32_t mainThreadScrollingReasonEnumMax =
+      MainThreadScrollingReason::kMainThreadScrollingReasonCount + 1;
+  for (uint32_t i = MainThreadScrollingReason::kNonCompositedReasonsFirst;
+       i <= MainThreadScrollingReason::kNonCompositedReasonsLast; ++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 +297,8 @@ WebInputEventResult ScrollManager::handleGestureScrollBegin(
   passScrollGestureEvent(gestureEvent,
                          m_scrollGestureHandlingNode->layoutObject());
 
+  recordNonCompositedMainThreadScrollingReasons(gestureEvent.sourceDevice);
+
   m_currentScrollChain.clear();
   std::unique_ptr<ScrollStateData> scrollStateData =
       WTF::makeUnique<ScrollStateData>();