[Blink] Migrate scroll reason metrics to //base/metrics helpers.

Though the original code used blink::EnumerationHistogram to record
these metrics, they have been migrated to use UMA_HISTOGRAM_EXACT_LINEAR
since they do not conform to the standard expectations of enumeration
histograms.

Bug: 1047547q
Change-Id: I1f69cd1985874246061a7d13f93816a26c7a2487
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2524587Reviewed-by: Robert Flack <flackr@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Auto-Submit: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#830862}

third_party/blink/renderer/core/input/scroll_manager.cc

@@ -33,8 +33,8 @@
 #include "third_party/blink/renderer/core/scroll/scroll_animator_base.h"
 #include "third_party/blink/renderer/core/scroll/scroll_customization.h"
 #include "third_party/blink/renderer/platform/heap/heap.h"
-#include "third_party/blink/renderer/platform/instrumentation/histogram.h"
 #include "third_party/blink/renderer/platform/instrumentation/tracing/trace_event.h"
+#include "third_party/blink/renderer/platform/widget/input/input_metrics.h"
 
 namespace blink {
 namespace {
@@ -422,23 +422,23 @@ void ScrollManager::RecordScrollRelatedMetrics(const WebGestureDevice device) {
   if (non_composited_main_thread_scrolling_reasons) {
     DCHECK(cc::MainThreadScrollingReason::HasNonCompositedScrollReasons(
         non_composited_main_thread_scrolling_reasons));
-    uint32_t main_thread_scrolling_reason_enum_max =
-        cc::MainThreadScrollingReason::kMainThreadScrollingReasonCount + 1;
+    // The enum in cc::MainThreadScrollingReason simultaneously defines actual
+    // bitmask values and indices into the bitmask, making this loop a bit
+    // confusing.
+    //
+    // This stems from the fact that kNotScrollingOnMain is recorded in the
+    // histograms as value 0. However, the 0th bit is not actually reserved and
+    // has a separate, well-defined meaning. kNotScrollingOnMain is only
+    // recorded when *no* bits are set.
+    //
+    // As such, when recording any reason that's not kNotScrollingOnMain (i.e.
+    // recording the index of a set bit), the index must be incremented by 1 to
+    // be recorded properly.
     for (uint32_t i = cc::MainThreadScrollingReason::kNonCompositedReasonsFirst;
          i <= cc::MainThreadScrollingReason::kNonCompositedReasonsLast; ++i) {
       unsigned val = 1 << i;
       if (non_composited_main_thread_scrolling_reasons & val) {
-        if (device == WebGestureDevice::kTouchscreen) {
-          DEFINE_STATIC_LOCAL(EnumerationHistogram, touch_histogram,
-                              ("Renderer4.MainThreadGestureScrollReason",
-                               main_thread_scrolling_reason_enum_max));
-          touch_histogram.Count(i + 1);
-        } else {
-          DEFINE_STATIC_LOCAL(EnumerationHistogram, wheel_histogram,
-                              ("Renderer4.MainThreadWheelScrollReason",
-                               main_thread_scrolling_reason_enum_max));
-          wheel_histogram.Count(i + 1);
-        }
+        RecordScrollReasonMetric(device, i + 1);
       }
     }
   }

third_party/blink/renderer/platform/BUILD.gn

@@ -1489,6 +1489,8 @@ component("platform") {
     "widget/input/input_event_prediction.cc",
     "widget/input/input_event_prediction.h",
     "widget/input/input_handler_proxy.cc",
+    "widget/input/input_metrics.cc",
+    "widget/input/input_metrics.h",
     "widget/input/main_thread_event_queue.cc",
     "widget/input/main_thread_event_queue.h",
     "widget/input/main_thread_event_queue_task.h",

third_party/blink/renderer/platform/widget/input/input_handler_proxy.cc

@@ -39,6 +39,7 @@
 #include "third_party/blink/renderer/platform/widget/input/cursor_control_handler.h"
 #include "third_party/blink/renderer/platform/widget/input/elastic_overscroll_controller.h"
 #include "third_party/blink/renderer/platform/widget/input/event_with_callback.h"
+#include "third_party/blink/renderer/platform/widget/input/input_metrics.h"
 #include "third_party/blink/renderer/platform/widget/input/momentum_scroll_jank_tracker.h"
 #include "third_party/blink/renderer/platform/widget/input/scroll_predictor.h"
 #include "ui/events/types/scroll_input_type.h"
@@ -791,11 +792,6 @@ WebInputEventAttribution InputHandlerProxy::PerformEventAttribution(
 void InputHandlerProxy::RecordMainThreadScrollingReasons(
     WebGestureDevice device,
     uint32_t reasons) {
-  static const char* kGestureHistogramName =
-      "Renderer4.MainThreadGestureScrollReason";
-  static const char* kWheelHistogramName =
-      "Renderer4.MainThreadWheelScrollReason";
-
   if (device != WebGestureDevice::kTouchpad &&
       device != WebGestureDevice::kScrollbar &&
       device != WebGestureDevice::kTouchscreen) {
@@ -842,27 +838,23 @@ void InputHandlerProxy::RecordMainThreadScrollingReasons(
   const bool is_unblocked_compositor_scroll =
       reasons == cc::MainThreadScrollingReason::kNotScrollingOnMain;
 
-  // UMA_HISTOGRAM_ENUMERATION requires that the enum_max must be strictly
-  // greater than the sample value. kMainThreadScrollingReasonCount doesn't
-  // include the NotScrollingOnMain enum but the histograms do so adding
-  // the +1 is necessary.
-  // TODO(dcheng): Fix https://crbug.com/705169 so this isn't needed.
-  constexpr uint32_t kMainThreadScrollingReasonEnumMax =
-      cc::MainThreadScrollingReason::kMainThreadScrollingReasonCount + 1;
   if (is_unblocked_compositor_scroll) {
-    if (device == WebGestureDevice::kTouchscreen) {
-      UMA_HISTOGRAM_ENUMERATION(
-          kGestureHistogramName,
-          cc::MainThreadScrollingReason::kNotScrollingOnMain,
-          kMainThreadScrollingReasonEnumMax);
-    } else {
-      UMA_HISTOGRAM_ENUMERATION(
-          kWheelHistogramName,
-          cc::MainThreadScrollingReason::kNotScrollingOnMain,
-          kMainThreadScrollingReasonEnumMax);
-    }
+    RecordScrollReasonMetric(
+        device, cc::MainThreadScrollingReason::kNotScrollingOnMain);
   }
 
+  // The enum in cc::MainThreadScrollingReason simultaneously defines actual
+  // bitmask values and indices into the bitmask, making this loop a bit
+  // confusing.
+  //
+  // This stems from the fact that kNotScrollingOnMain is recorded in the
+  // histograms as value 0. However, the 0th bit is not actually reserved and
+  // has a separate, well-defined meaning. kNotScrollingOnMain is only recorded
+  // when *no* bits are set.
+  //
+  // As such, when recording any reason that's not kNotScrollingOnMain (i.e.
+  // recording the index of a set bit), the index must be incremented by 1 to be
+  // recorded properly.
   for (uint32_t i = 0;
        i < cc::MainThreadScrollingReason::kMainThreadScrollingReasonCount;
        ++i) {
@@ -876,13 +868,7 @@ void InputHandlerProxy::RecordMainThreadScrollingReasons(
         if (reasons & ~val)
           continue;
       }
-      if (device == WebGestureDevice::kTouchscreen) {
-        UMA_HISTOGRAM_ENUMERATION(kGestureHistogramName, i + 1,
-                                  kMainThreadScrollingReasonEnumMax);
-      } else {
-        UMA_HISTOGRAM_ENUMERATION(kWheelHistogramName, i + 1,
-                                  kMainThreadScrollingReasonEnumMax);
-      }
+      RecordScrollReasonMetric(device, i + 1);
     }
   }
 }

third_party/blink/renderer/platform/widget/input/input_metrics.cc

+// Copyright 2020 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 "third_party/blink/renderer/platform/widget/input/input_metrics.h"
+
+#include "base/metrics/histogram_macros.h"
+#include "cc/input/main_thread_scrolling_reason.h"
+#include "third_party/blink/public/common/input/web_gesture_device.h"
+
+namespace blink {
+
+void RecordScrollReasonMetric(WebGestureDevice device, uint32_t reason) {
+  constexpr uint32_t kMainThreadScrollingReasonEnumMax =
+      cc::MainThreadScrollingReason::kMainThreadScrollingReasonCount + 1;
+  // Note the use of `UMA_HISTOGRAM_EXACT_LINEAR` here. This is because the enum
+  // defined in cc::MainThreadScrollingReason defines both bitmasks and bitmask
+  // positions and doesn't correspond well to how the UMA helpers for
+  // enumerations are typically used.
+  if (device == WebGestureDevice::kTouchscreen) {
+    UMA_HISTOGRAM_EXACT_LINEAR("Renderer4.MainThreadGestureScrollReason",
+                               reason, kMainThreadScrollingReasonEnumMax);
+  } else {
+    UMA_HISTOGRAM_EXACT_LINEAR("Renderer4.MainThreadWheelScrollReason", reason,
+                               kMainThreadScrollingReasonEnumMax);
+  }
+}
+
+}  // namespace blink

third_party/blink/renderer/platform/widget/input/input_metrics.h

+// Copyright 2020 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.
+
+#ifndef THIRD_PARTY_BLINK_RENDERER_PLATFORM_WIDGET_INPUT_INPUT_METRICS_H_
+#define THIRD_PARTY_BLINK_RENDERER_PLATFORM_WIDGET_INPUT_INPUT_METRICS_H_
+
+#include "cc/input/main_thread_scrolling_reason.h"
+#include "third_party/blink/public/common/input/web_gesture_device.h"
+#include "third_party/blink/renderer/platform/platform_export.h"
+
+namespace blink {
+
+// `reason` is derived from `cc::MainThreadScrollingReason`. If recording
+// `kNotScrollingOnMain`, simply pass it as-is. Hoewver, if recording the
+// position of a set bit, the index of the set bit must be incremented by one.
+//
+// This stems from the fact that kNotScrollingOnMain is recorded in the
+// histograms as value 0. However, the 0th bit is not actually reserved and
+// has a separate, well-defined meaning. kNotScrollingOnMain is only
+// recorded when *no* bits are set.
+PLATFORM_EXPORT void RecordScrollReasonMetric(WebGestureDevice device,
+                                              uint32_t reason);
+
+}  // namespace blink
+
+#endif  // THIRD_PARTY_BLINK_RENDERER_PLATFORM_WIDGET_INPUT_INPUT_METRICS_H_