Clean up cc::MainThreadScrollingReason

- Group the reasons by repaint reasons, hit-test reasons, and other
  reasons, instead of the previous transient and non-transient reasons.
- Add checks for repaint and hit-test reasons.
- Remove ScrollStatus::main_thread_scrolling_reasons.

Bug: 1369739
Change-Id: I718fd5e9140e563a953672a21f17241b9a2c0ba8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4919598
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1209575}

cc/input/input_handler.cc

@@ -74,8 +74,6 @@ InputHandler::ScrollStatus InputHandler::ScrollBegin(ScrollState* scroll_state,
   DCHECK(scroll_state->delta_x() == 0 && scroll_state->delta_y() == 0);
 
   InputHandler::ScrollStatus scroll_status;
-  scroll_status.main_thread_scrolling_reasons =
-      MainThreadScrollingReason::kNotScrollingOnMain;
   TRACE_EVENT0("cc", "InputHandler::ScrollBegin");
 
   // If this ScrollBegin is non-animated then ensure we cancel any ongoing
@@ -174,6 +172,8 @@ InputHandler::ScrollStatus InputHandler::ScrollBegin(ScrollState* scroll_state,
         DCHECK(scroll_hit_test.main_thread_hit_test_reasons);
         scroll_status.main_thread_hit_test_reasons =
             scroll_hit_test.main_thread_hit_test_reasons;
+        CHECK(MainThreadScrollingReason::AreHitTestReasons(
+            scroll_status.main_thread_hit_test_reasons));
         return scroll_status;
       }
 
@@ -208,15 +208,15 @@ InputHandler::ScrollStatus InputHandler::ScrollBegin(ScrollState* scroll_state,
     return scroll_status;
   }
 
-  DCHECK_EQ(scroll_status.main_thread_scrolling_reasons,
-            MainThreadScrollingReason::kNotScrollingOnMain);
   DCHECK_EQ(scroll_status.thread,
             InputHandler::ScrollThread::kScrollOnImplThread);
   DCHECK(scrolling_node);
 
   ActiveTree().SetCurrentlyScrollingNode(scrolling_node);
   scroll_status.main_thread_repaint_reasons =
       scroll_tree.GetMainThreadRepaintReasons(*scrolling_node);
+  CHECK(MainThreadScrollingReason::AreRepaintReasons(
+      scroll_status.main_thread_repaint_reasons));
 
   DidLatchToScroller(*scroll_state, type);
 

cc/input/input_handler.h

@@ -192,13 +192,15 @@ class CC_EXPORT InputHandler : public InputDelegateForCompositor {
   // Note these are used in a histogram. Do not reorder or delete existing
   // entries.
   enum class ScrollThread {
-    kScrollOnMainThread = 0,
+    // kScrollOnMainThread is not used anymore. However we'll keep this entry
+    // as per the comment above.
+    kScrollOnMainThread_NotUsed = 0,
     kScrollOnImplThread,
     kScrollIgnored,
     // kScrollUnknown is not used anymore. However we'll keep this entry as per
     // the comment above.
-    kScrollUnknown,
-    kLastScrollStatus = kScrollUnknown
+    kScrollUnknown_NotUsed,
+    kLastScrollStatus = kScrollUnknown_NotUsed,
   };
 
   explicit InputHandler(CompositorDelegateForInput& compositor_delegate);
@@ -209,24 +211,20 @@ class CC_EXPORT InputHandler : public InputDelegateForCompositor {
 
   struct ScrollStatus {
     ScrollThread thread = ScrollThread::kScrollOnImplThread;
-    // This should be set to nonzero iff `thread` is SCROLL_ON_MAIN_THREAD.
-    uint32_t main_thread_scrolling_reasons =
-        MainThreadScrollingReason::kNotScrollingOnMain;
 
-    // Used only in scroll unification. If nonzero, it tells the caller that
-    // the input handler detected a case where it cannot reliably target a
-    // scroll node and needs the main thread to perform a hit test.
+    // If nonzero, it tells the caller that the input handler detected a case
+    // where it cannot reliably target a scroll node and needs the main thread
+    // to perform a hit test. If nonzero, this will be one or more values from
+    // MainThreadScrollingReason::kHitTestReasons.
     uint32_t main_thread_hit_test_reasons =
         MainThreadScrollingReason::kNotScrollingOnMain;
 
-    // Used only in scroll unification. A nonzero value means we have performed
-    // the scroll (i.e. updated the offset in the scroll tree) on the compositor
-    // thread, but we will need a main thread lifecycle update + commit before
-    // the user will see the new pixels (for example, because the scroller does
-    // not have a composited layer). If nonzero, this will be one or more values
-    // from the MainThreadScrollingReason enum. (Unification avoids setting
-    // main_thread_scrolling_reasons, to keep that field consistent with
-    // semantics of ScrollThread::SCROLL_ON_IMPL_THREAD.)
+    // A nonzero value means we have performed the scroll (i.e. updated the
+    // offset in the scroll tree) on the compositor thread, but we will need a
+    // main thread lifecycle update + commit before the user will see the new
+    // pixels (for example, because the scroller does not have a composited
+    // layer). If nonzero, this will be one or more values from the
+    // MainThreadScrollingReason::kRepaintReasons.
     uint32_t main_thread_repaint_reasons =
         MainThreadScrollingReason::kNotScrollingOnMain;
 

cc/input/main_thread_scrolling_reason.cc

@@ -30,25 +30,32 @@ void MainThreadScrollingReason::AddToTracedValue(
     base::trace_event::TracedValue& traced_value) {
   traced_value.BeginArray("main_thread_scrolling_reasons");
 
-  if (reasons & kHasBackgroundAttachmentFixedObjects)
-    traced_value.AppendString("Has background-attachment:fixed");
-  if (reasons & kScrollbarScrolling)
-    traced_value.AppendString("Scrollbar scrolling");
-  if (reasons & kNotOpaqueForTextAndLCDText)
-    traced_value.AppendString("Not opaque for text and LCD text");
+#define ADD_REASON(reason, string)       \
+  do                                     \
+    if (reasons & reason) {              \
+      traced_value.AppendString(string); \
+      reasons &= ~reason;                \
+    }                                    \
+  while (false)
 
-  // Transient scrolling reasons.
-  if (reasons & kNonFastScrollableRegion)
-    traced_value.AppendString("Non fast scrollable region");
-  if (reasons & kFailedHitTest)
-    traced_value.AppendString("Failed hit test");
-  if (reasons & kNoScrollingLayer)
-    traced_value.AppendString("No scrolling layer");
-  if (reasons & kWheelEventHandlerRegion)
-    traced_value.AppendString("Wheel event handler region");
-  if (reasons & kTouchEventHandlerRegion)
-    traced_value.AppendString("Touch event handler region");
+  ADD_REASON(kHasBackgroundAttachmentFixedObjects,
+             "Has background-attachment:fixed");
+  ADD_REASON(kNotOpaqueForTextAndLCDText, "Not opaque for text and LCD text");
+  ADD_REASON(kNoScrollingLayer, "No scrolling layer");
+  ADD_REASON(kPreferNonCompositedScrolling, "Prefer non-composited scrolling");
+  ADD_REASON(kBackgroundNeedsRepaintOnScroll,
+             "Background needs repaint on scroll");
+  ADD_REASON(kScrollbarScrolling, "Scrollbar scrolling");
+  ADD_REASON(kNonFastScrollableRegion, "Non fast scrollable region");
+  ADD_REASON(kFailedHitTest, "Failed hit test");
+  ADD_REASON(kPopupNoThreadedInput,
+             "Popup scrolling (no threaded input handler)");
+  ADD_REASON(kWheelEventHandlerRegion, "Wheel event handler region");
+  ADD_REASON(kTouchEventHandlerRegion, "Touch event handler region");
 
+#undef ADD_REASON
+
+  DCHECK_EQ(reasons, kNotScrollingOnMain);
   traced_value.EndArray();
 }
 

cc/input/main_thread_scrolling_reason.h

@@ -20,7 +20,7 @@ namespace cc {
 // Ensure this stays in sync with the "MainThreadScrollingReason" enum in:
 //   tools/metrics/histograms/enums.xml
 // When adding a new MainThreadScrollingReason, make sure the corresponding
-// [MainThread/Compositor]CanSetScrollReasons function is also updated.
+// kRepaintReasons and kHitTestReasons are also updated.
 struct CC_EXPORT MainThreadScrollingReason {
   enum : uint32_t {
     kNotScrollingOnMain = 0,
@@ -36,37 +36,64 @@ struct CC_EXPORT MainThreadScrollingReason {
     // respectively, so the 0th bit and the 1st bit should never be used.
     // See also blink::RecordScrollReasonsMetric().
 
-    // Non-transient scrolling reasons. These are set on the ScrollNode.
+    // Main-thread repaint reasons.
+    // See InputHandler::ScrollStatus::main_thread_repaint_reasons.
+    // They are set in ScrollNode::main_thread_scrolling_reasons, or
+    // kNoScrollingLayer is used for a ScrollNode that doesn't have
+    // main_thread_scrolling_reasons but is_composited is false.
     kHasBackgroundAttachmentFixedObjects = 1 << 2,
-    kPopupNoThreadedInput = 1 << 4,
+    // 1 << 4 is used by kPopupNoThreadedInput below.
     // Subpixel (LCD) text rendering requires blending glyphs with an opaque
     // background.
     kNotOpaqueForTextAndLCDText = 1 << 5,
+    kNoScrollingLayer = 1 << 10,
     kPreferNonCompositedScrolling = 1 << 15,
     kBackgroundNeedsRepaintOnScroll = 1 << 16,
 
-    // Transient scrolling reasons. These are computed for each scroll gesture.
-    // When computed inside ScrollBegin, these prevent the InputHandler from
-    // reporting a status with SCROLL_ON_IMPL_THREAD. In other cases, the
-    // InputHandler is scrolling "on impl", but we report a transient main
-    // thread scrolling reason to UMA when we determine that some other aspect
-    // of handling the scroll has been (or will be) blocked on the main thread.
+    // Main-thread hit-test reasons.
+    // See InputHandler::ScrollStatus::main_thread_hit_test_reasons.
     kScrollbarScrolling = 1 << 7,
     kNonFastScrollableRegion = 1 << 8,
     kFailedHitTest = 1 << 9,
-    kNoScrollingLayer = 1 << 10,
+    // 1 << 10 is used by kNoScrollingLayer above.
+
+    // The following reasons are neither repaint reasons nor hit-test reasons.
+    // They don't go through InputHandler::ScrollBegin() or set in
+    // InputHandler::ScrollStatus.
+
+    // We need main thread Scrolling in a popup because it doesn't have a
+    // threaded input handler. This flag is used in blink only, to prevent
+    // composited scroll animation in a popup.
+    // See blink::ScrollAnimator::SendAnimationToCompositor().
+    kPopupNoThreadedInput = 1 << 4,
+
+    // Scrolling can be handled on the compositor thread but it might be
+    // blocked on the main thread waiting for non-passive event handlers to
+    // process the wheel/touch events (i.e. were they preventDefaulted?).
     kWheelEventHandlerRegion = 1 << 13,
     kTouchEventHandlerRegion = 1 << 14,
 
-    // The following reasons are listed above so they are grouped with other
-    // non-transient scrolling reasons:
-    // kPreferNonCompositedScrolling = 1 << 15,
-    // kBackgroundNeedsRepaintOnScroll = 1 << 16,
+    // 1 << 15 is used by kPreferNonCompositedScrolling above.
+    // 1 << 16 is used by kBackgroundNeedsRepaintOnScroll above.
 
     // For blink::RecordScrollReasonsMetric() to know the number of used bits.
     kMainThreadScrollingReasonLast = 16,
   };
 
+  static constexpr uint32_t kRepaintReasons =
+      kHasBackgroundAttachmentFixedObjects | kNotOpaqueForTextAndLCDText |
+      kNoScrollingLayer | kPreferNonCompositedScrolling |
+      kBackgroundNeedsRepaintOnScroll;
+  static constexpr uint32_t kHitTestReasons =
+      kScrollbarScrolling | kNonFastScrollableRegion | kFailedHitTest;
+
+  static bool AreRepaintReasons(uint32_t reasons) {
+    return (reasons & ~kRepaintReasons) == 0;
+  }
+  static bool AreHitTestReasons(uint32_t reasons) {
+    return (reasons & ~kHitTestReasons) == 0;
+  }
+
   static int BucketIndexForTesting(uint32_t reason);
 
   static std::string AsText(uint32_t reasons);

cc/input/main_thread_scrolling_reason_unittest.cc

@@ -14,14 +14,26 @@ TEST_F(MainThreadScrollingReasonTest, AsText) {
   EXPECT_EQ("", MainThreadScrollingReason::AsText(0));
   EXPECT_EQ(
       "Has background-attachment:fixed, "
-      "Scrollbar scrolling, "
       "Not opaque for text and LCD text, "
-      "Non fast scrollable region, "
-      "Failed hit test, "
       "No scrolling layer, "
+      "Prefer non-composited scrolling, "
+      "Background needs repaint on scroll",
+      MainThreadScrollingReason::AsText(
+          MainThreadScrollingReason::kRepaintReasons));
+  EXPECT_EQ(
+      "Scrollbar scrolling, "
+      "Non fast scrollable region, "
+      "Failed hit test",
+      MainThreadScrollingReason::AsText(
+          MainThreadScrollingReason::kHitTestReasons));
+  EXPECT_EQ(
+      "Popup scrolling (no threaded input handler), "
       "Wheel event handler region, "
       "Touch event handler region",
-      MainThreadScrollingReason::AsText(0xffffffffu));
+      MainThreadScrollingReason::AsText(
+          MainThreadScrollingReason::kPopupNoThreadedInput |
+          MainThreadScrollingReason::kWheelEventHandlerRegion |
+          MainThreadScrollingReason::kTouchEventHandlerRegion));
 }
 
 }  // namespace cc