Reset a flag on first update event in a gesture

The flag not being reset to false means that if the last update event in
a previous gesture was marked as janky, the flag for "previous event
reported" would be set to true. This means that the first event in the
next gesture would never be able to be reported, which has been observed
in recorded traces while investigating discrepancy between UMA and
trace-based jank metrics.

This CL extracts all the per-scroll state into a struct, allowing to
reset all the variables in one line, avoiding this and potentially
other similar problems.

Bug: b/185991751
Change-Id: I178ef4caa2e224445d83521160c34ebeec069b1b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3077945
Commit-Queue: Andrew Shulaev <ddrone@google.com>
Reviewed-by: Stephen Nusko <nuskos@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#909767}

ui/latency/latency_tracker.cc

@@ -142,34 +142,31 @@ void LatencyTracker::ReportJankyFrame(base::TimeTicks original_timestamp,
   base::TimeDelta dur = gpu_swap_end_timestamp - original_timestamp;
 
   if (first_frame) {
-    if (total_update_events_ > 0) {
+    if (jank_state_.total_update_events_ > 0) {
       // If we have some data from previous scroll, report it to UMA.
       UMA_HISTOGRAM_MEDIUM_TIMES("Event.Latency.ScrollUpdate.TotalDuration",
-                                 total_update_duration_);
+                                 jank_state_.total_update_duration_);
       UMA_HISTOGRAM_MEDIUM_TIMES("Event.Latency.ScrollUpdate.JankyDuration",
-                                 janky_update_duration_);
+                                 jank_state_.janky_update_duration_);
 
       UMA_HISTOGRAM_COUNTS_10000("Event.Latency.ScrollUpdate.TotalEvents",
-                                 total_update_events_);
+                                 jank_state_.total_update_events_);
       UMA_HISTOGRAM_COUNTS_10000("Event.Latency.ScrollUpdate.JankyEvents",
-                                 janky_update_events_);
+                                 jank_state_.janky_update_events_);
 
-      if (!total_update_duration_.is_zero()) {
+      if (!jank_state_.total_update_duration_.is_zero()) {
         UMA_HISTOGRAM_PERCENTAGE(
             "Event.Latency.ScrollUpdate.JankyDurationPercentage",
-            static_cast<int>(
-                100 * (janky_update_duration_ / total_update_duration_)));
+            static_cast<int>(100 * (jank_state_.janky_update_duration_ /
+                                    jank_state_.total_update_duration_)));
       }
     }
 
-    total_update_events_ = 0;
-    janky_update_events_ = 0;
-    total_update_duration_ = base::TimeDelta{};
-    janky_update_duration_ = base::TimeDelta{};
+    jank_state_ = JankTrackerState{};
   }
 
-  total_update_events_++;
-  total_update_duration_ += dur;
+  jank_state_.total_update_events_++;
+  jank_state_.total_update_duration_ += dur;
 
   // When processing first frame in a scroll, we do not have any other frames to
   // compare it to, and thus no way to detect the jank.
@@ -180,21 +177,21 @@ void LatencyTracker::ReportJankyFrame(base::TimeTicks original_timestamp,
     // To see how many of those intervals fit into the real frame timing,
     // we divide it on 1/60 which is the same thing as multiplying by 60.
     double frames_taken = dur.InSecondsF() * 60;
-    double prev_frames_taken = prev_duration_.InSecondsF() * 60;
+    double prev_frames_taken = jank_state_.prev_duration_.InSecondsF() * 60;
 
     // For each GestureScroll update, we would like to report whether it was
     // janky. However, in order to do that, we need to compare it both to the
     // previous as well as to the next event. This condition means that no jank
     // was reported for the previous frame (as compared to the one before that),
     // so we need to compare it to the current one and report whether it's
     // janky:
-    if (!prev_scroll_update_reported_) {
+    if (!jank_state_.prev_scroll_update_reported_) {
       // The information about previous GestureScrollUpdate was not reported:
       // check whether it's janky by comparing to the current frame and report.
       if (prev_frames_taken > frames_taken + 0.5) {
         UMA_HISTOGRAM_BOOLEAN("Event.Latency.ScrollJank", true);
-        janky_update_events_++;
-        janky_update_duration_ += prev_duration_;
+        jank_state_.janky_update_events_++;
+        jank_state_.janky_update_duration_ += jank_state_.prev_duration_;
       } else {
         UMA_HISTOGRAM_BOOLEAN("Event.Latency.ScrollJank", false);
       }
@@ -203,22 +200,22 @@ void LatencyTracker::ReportJankyFrame(base::TimeTicks original_timestamp,
     // The current GestureScrollUpdate is janky compared to the previous one.
     if (frames_taken > prev_frames_taken + 0.5) {
       UMA_HISTOGRAM_BOOLEAN("Event.Latency.ScrollJank", true);
-      janky_update_events_++;
-      janky_update_duration_ += dur;
+      jank_state_.janky_update_events_++;
+      jank_state_.janky_update_duration_ += dur;
 
       // Since we have reported the current event as janky, there is no need to
       // report anything about it on the next iteration, as we would like to
       // report every GestureScrollUpdate only once.
-      prev_scroll_update_reported_ = true;
+      jank_state_.prev_scroll_update_reported_ = true;
     } else {
       // We do not have enough information to report whether the current event
       // is janky, and need to compare it to the next one before reporting
       // anything about it.
-      prev_scroll_update_reported_ = false;
+      jank_state_.prev_scroll_update_reported_ = false;
     }
   }
 
-  prev_duration_ = dur;
+  jank_state_.prev_duration_ = dur;
 }
 
 void LatencyTracker::ComputeEndToEndLatencyHistograms(

ui/latency/latency_tracker.h

@@ -34,12 +34,16 @@ class LatencyTracker {
     INPUT_METRIC_EVENT_MAX = SCROLL_UPDATE_WHEEL
   };
 
-  base::TimeDelta prev_duration_;
-  bool prev_scroll_update_reported_ = false;
-  int total_update_events_ = 0;
-  int janky_update_events_ = 0;
-  base::TimeDelta total_update_duration_;
-  base::TimeDelta janky_update_duration_;
+  // Data holder for all intermediate state for jank tracking.
+  struct JankTrackerState {
+    int total_update_events_ = 0;
+    int janky_update_events_ = 0;
+    bool prev_scroll_update_reported_ = false;
+    base::TimeDelta prev_duration_;
+    base::TimeDelta total_update_duration_;
+    base::TimeDelta janky_update_duration_;
+  };
+  JankTrackerState jank_state_;
 
   void ReportUkmScrollLatency(
       const InputMetricEvent& metric_event,