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

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

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/input/ScrollManager.h"
 
 #include <memory>
 #include "core/dom/DOMNodeIds.h"
 #include "core/events/GestureEvent.h"
 #include "core/frame/BrowserControls.h"
 #include "core/frame/FrameView.h"
 #include "core/html/HTMLFrameOwnerElement.h"
 #include "core/input/EventHandler.h"
 #include "core/input/EventHandlingUtil.h"
 #include "core/layout/LayoutBlock.h"
 #include "core/layout/LayoutPart.h"
 #include "core/layout/api/LayoutViewItem.h"
 #include "core/loader/DocumentLoader.h"
 #include "core/page/AutoscrollController.h"
 #include "core/page/Page.h"
 #include "core/page/scrolling/OverscrollController.h"
 #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"
 
 namespace blink {
 
 ScrollManager::ScrollManager(LocalFrame& frame) : m_frame(frame) {
   clear();
 }
 
 void ScrollManager::clear() {
@@ skipping 154 matching lines @@
   if (scrollState.deltaX() || scrollState.deltaY())
     m_frame->document()->updateStyleAndLayoutIgnorePendingStylesheets();
 
   if (m_currentScrollChain.empty())
     recomputeScrollChain(startNode, m_currentScrollChain);
   scrollState.setScrollChain(m_currentScrollChain);
 
   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();
 
   if (document->layoutViewItem().isNull())
     return WebInputEventResult::NotHandled;
 
   // If there's no layoutObject on the node, send the event to the nearest
   // ancestor with a layoutObject.  Needed for <option> and <optgroup> elements
   // so we can touch scroll <select>s
   while (m_scrollGestureHandlingNode &&
          !m_scrollGestureHandlingNode->layoutObject())
     m_scrollGestureHandlingNode =
         m_scrollGestureHandlingNode->parentOrShadowHostNode();
 
   if (!m_scrollGestureHandlingNode)
     m_scrollGestureHandlingNode = m_frame->document()->documentElement();
 
   if (!m_scrollGestureHandlingNode ||
       !m_scrollGestureHandlingNode->layoutObject())
     return WebInputEventResult::NotHandled;
 
   passScrollGestureEvent(gestureEvent,
                          m_scrollGestureHandlingNode->layoutObject());
 
+  recordNonCompositedMainThreadScrollingReasons(gestureEvent.sourceDevice);
+
   m_currentScrollChain.clear();
   std::unique_ptr<ScrollStateData> scrollStateData =
       WTF::makeUnique<ScrollStateData>();
   IntPoint position = flooredIntPoint(gestureEvent.positionInRootFrame());
   scrollStateData->position_x = position.x();
   scrollStateData->position_y = position.y();
   scrollStateData->is_beginning = true;
   scrollStateData->from_user_input = true;
   scrollStateData->is_direct_manipulation =
       gestureEvent.sourceDevice == WebGestureDeviceTouchscreen;
@@ skipping 326 matching lines @@
     if (scrollbar->gestureEvent(targetedEvent.event(), &shouldUpdateCapture)) {
       if (shouldUpdateCapture)
         m_scrollbarHandlingScrollGesture = scrollbar;
       return true;
     }
   }
   return false;
 }
 
 }  // namespace blink