Update EventLatency Termination for 0 Delta Scrolls

During a scroll we were only saving EventMetrics when the scroll itself
caused damage. We can have scroll events which lead to a scroll delta of
zero being applied.

We may still produce a frame for other effects in this VSync. However we
are not connecting these scrolls, leading to incorrect jank analysis.

This change updates `LayerTreeHostImpl` to always save events if a
scroll is occurring. This will cause them to be associated with the next
producded frame. We are also terminating them if `DidNotProduceFrame`
is triggered. To denote when there is no damage at all.

Bug: 394235951
Change-Id: I06b965fcfe58bc4e7de6e0746857383b184d4763
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6226871
Reviewed-by: Victor Miura <vmiura@chromium.org>
Commit-Queue: Jonathan Ross <jonross@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1418955}

cc/base/features.cc

@@ -209,4 +209,8 @@ bool StopExportDFCMetrics() {
   return base::FeatureList::IsEnabled(features::kStopExportDFCMetrics);
 }
 
+BASE_FEATURE(kZeroScrollMetricsUpdate,
+             "ZeroScrollMetricsUpdate",
+             base::FEATURE_DISABLED_BY_DEFAULT);
+
 }  // namespace features

cc/base/features.h

@@ -206,6 +206,11 @@ CC_BASE_EXPORT BASE_DECLARE_FEATURE(kThrottleMainFrameTo60Hz);
 CC_BASE_EXPORT BASE_DECLARE_FEATURE(kStopExportDFCMetrics);
 CC_BASE_EXPORT extern bool StopExportDFCMetrics();
 
+// When enabled, we save the `EventMetrics` for a scroll, even when the result
+// is no damage. So that the termination can be per properly attributed to the
+// end of frame production for the given VSync.
+CC_BASE_EXPORT BASE_DECLARE_FEATURE(kZeroScrollMetricsUpdate);
+
 }  // namespace features
 
 #endif  // CC_BASE_FEATURES_H_

cc/trees/layer_tree_host_impl.cc

@@ -495,7 +495,9 @@ LayerTreeHostImpl::LayerTreeHostImpl(
                       compositor_frame_reporting_controller_.get()),
       lcd_text_metrics_reporter_(LCDTextMetricsReporter::CreateIfNeeded(this)),
       frame_rate_estimator_(GetTaskRunner()),
-      contains_srgb_cache_(kContainsSrgbCacheSize) {
+      contains_srgb_cache_(kContainsSrgbCacheSize),
+      zero_scroll_metrics_update_enabled_(
+          base::FeatureList::IsEnabled(features::kZeroScrollMetricsUpdate)) {
   resource_provider_ = std::make_unique<viz::ClientResourceProvider>(
       task_runner_provider_->MainThreadTaskRunner(),
       task_runner_provider_->HasImplThread()
@@ -3494,6 +3496,14 @@ void LayerTreeHostImpl::DidNotProduceFrame(const viz::BeginFrameAck& ack,
       layer_tree_frame_sink_->DidNotProduceFrame(ack, reason);
     }
   }
+  // While scrolling, we save all event metrics. It is possible that this
+  // results in a 0 delta scroll, which has no damage. We take the metrics here
+  // so that they are terminated now. This prevents them from being incorrectly
+  // associated with a future produced frame. So that jank measurements have
+  // accurate deltas.
+  if (zero_scroll_metrics_update_enabled_) {
+    events_metrics_manager_.TakeSavedEventsMetrics();
+  }
 }
 
 void LayerTreeHostImpl::OnBeginImplFrameDeadline() {
@@ -4581,6 +4591,14 @@ void LayerTreeHostImpl::WillScrollContent(ElementId element_id) {
             ScrollbarAnimationControllerForElementId(element_id))
       animation_controller->WillUpdateScroll();
   }
+  // Whenever we are scrolling, we want to save the metrics. It is possible for
+  // the event to result in a 0 delta change. However we want to associate it
+  // with the subsequent frame. Otherwise this event will be attributed to jank.
+  //
+  // If there are on going animations, we may still submit a frame.
+  if (zero_scroll_metrics_update_enabled_) {
+    events_metrics_manager_.SaveActiveEventMetrics();
+  }
 }
 
 void LayerTreeHostImpl::DidScrollContent(ElementId element_id, bool animated) {

cc/trees/layer_tree_host_impl.h

@@ -1289,6 +1289,11 @@ class CC_EXPORT LayerTreeHostImpl : public TileManagerClient,
   // was reused.
   viz::BeginFrameArgs last_draw_active_tree_begin_frame_args_;
 
+  // When true we will save all `EventMetrics` associated with a scroll, even
+  // if they cause no damage. This way their termination time can be properly
+  // attributed to the end of frame production for the given VSync.
+  const bool zero_scroll_metrics_update_enabled_;
+
   // Must be the last member to ensure this is destroyed first in the
   // destruction order and invalidates all weak pointers.
   base::WeakPtrFactory<LayerTreeHostImpl> weak_factory_{this};

testing/variations/fieldtrial_testing_config.json

@@ -26666,6 +26666,25 @@
             ]
         }
     ],
+    "ZeroScrollMetricsUpdate": [
+        {
+            "platforms": [
+                "android",
+                "chromeos",
+                "linux",
+                "mac",
+                "windows"
+            ],
+            "experiments": [
+                {
+                    "name": "Enabled",
+                    "enable_features": [
+                        "ZeroScrollMetricsUpdate"
+                    ]
+                }
+            ]
+        }
+    ],
     "iOSCPEPerformanceImprovements": [
         {
             "platforms": [