Fix flaky bugs introduced by AverageLagTrackingManager

After commit crrev.com/917ce177
introduced the AverageLagTrackingManager some cc_unittests started
failing.
These test tested scenarios where frames were Drawn but didn't receive
a PresentationFeedback on LayerTreeHostImpl. They were failing due to a
DCHECK at  AverageLagTrackingManager that checked the amount of pending
frames without feedback before destruction.

These frames never received a PresentationFeedback because
LayerTreeFrameSink was purposefully losing its context
(LayerTreeFrameSink::OnContextLost).
This prevents DisplayScheduler to post the tasks that will ultimately
call LayerTreeHostImpl::DidPresentCompositorFrame.

In this CL the list in AverageLagTrackingManager is cleaned whenever
LayerTreeFrameSink loses its context, as it is a sign that the current
pending frames will never receive a presentation feedback.
Also, frames that don't have relevant latency_info aren't added to the
list anymore as this was one of the reasons why the tests where flaky
too.

Bug: 1103289, 1104841, 1106020, 1104202
Change-Id: I9b74ac22adb6db6af7c8e727304dc2e0b02e8899
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2303363Reviewed-by: Xida Chen <xidachen@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: João Victor Almeida de Aguiar <joalmei@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#791413}

cc/metrics/average_lag_tracking_manager.cc

@@ -55,7 +55,8 @@ void AverageLagTrackingManager::CollectScrollEventsFromFrame(
     event_infos.push_back(event_info);
   }
 
-  frame_token_to_info_.push_back(std::make_pair(frame_token, event_infos));
+  if (event_infos.size() > 0)
+    frame_token_to_info_.push_back(std::make_pair(frame_token, event_infos));
 }
 
 void AverageLagTrackingManager::DidPresentCompositorFrame(
@@ -95,4 +96,8 @@ void AverageLagTrackingManager::DidPresentCompositorFrame(
     }
   }
 }
+
+void AverageLagTrackingManager::Clear() {
+  frame_token_to_info_.clear();
+}
 }  // namespace cc

cc/metrics/average_lag_tracking_manager.h

@@ -43,6 +43,10 @@ class CC_EXPORT AverageLagTrackingManager {
   void DidPresentCompositorFrame(uint32_t frame_token,
                                  const viz::FrameTimingDetails& frame_details);
 
+  // Clears the list of events |frame_token_to_info_| if the current frames wont
+  // receive a presentation feedback (e.g. LayerTreeFrameSink loses context)
+  void Clear();
+
  private:
   // TODO(https://crbug.com/1101005): Remove GpuSwap implementation after M86.
   // Tracker for the AverageLag metrics that uses the gpu swap begin timing as

cc/trees/layer_tree_host_impl.cc

@@ -3047,6 +3047,7 @@ void LayerTreeHostImpl::DidLoseLayerTreeFrameSink() {
     return;
   has_valid_layer_tree_frame_sink_ = false;
   client_->DidLoseLayerTreeFrameSinkOnImplThread();
+  lag_tracking_manager_.Clear();
 }
 
 bool LayerTreeHostImpl::ShouldPinTopControlsToContentTop() const {

cc/trees/layer_tree_host_impl_unittest.cc

@@ -13749,6 +13749,55 @@ TEST_P(ScrollUnifiedLayerTreeHostImplTest,
   host_impl_ = nullptr;
 }
 
+// Test if the AverageLagTrackingManager's pending frames list is cleared when
+// the LayerTreeFrameSink loses context. It is necessary since the frames won't
+// receive a presentation feedback if the context is lost, and the pending
+// frames will never be removed from the list otherwise.
+TEST_F(LayerTreeHostImplTest,
+       ClearTrackingManagerOnLayerTreeFrameSinkLoseContext) {
+  const gfx::Size content_size(1000, 10000);
+  const gfx::Size viewport_size(500, 500);
+  SetupViewportLayersOuterScrolls(viewport_size, content_size);
+  DrawFrame();
+
+  host_impl_->ScrollBegin(BeginState(gfx::Point(250, 250), gfx::Vector2dF(),
+                                     ui::ScrollInputType::kTouchscreen)
+                              .get(),
+                          ui::ScrollInputType::kTouchscreen);
+
+  // Draw 30 frames, each with 1 LatencyInfo object that will be added to the
+  // AverageLagTrackingManager.
+  for (int i = 0; i < 30; i++) {
+    // Makes a scroll update so the next frame is set to be processed
+    // (to force frame->has_no_damage = false).
+    host_impl_->ScrollUpdate(UpdateState(gfx::Point(), gfx::Vector2d(0, 10),
+                                         ui::ScrollInputType::kTouchscreen)
+                                 .get());
+
+    // Add a LatencyInfo object that will be accepted by
+    // AverageLagTrackingManager::CollectScrollEventsFromFrame.
+    ui::LatencyInfo latency_info;
+    latency_info.set_source_event_type(ui::SourceEventType::TOUCH);
+    latency_info.AddLatencyNumber(
+        ui::INPUT_EVENT_LATENCY_SCROLL_UPDATE_ORIGINAL_COMPONENT);
+    latency_info.AddLatencyNumberWithTimestamp(
+        ui::INPUT_EVENT_LATENCY_SCROLL_UPDATE_LAST_EVENT_COMPONENT,
+        base::TimeTicks::Now());
+    std::unique_ptr<SwapPromise> swap_promise(
+        new LatencyInfoSwapPromise(latency_info));
+    host_impl_->active_tree()->QueuePinnedSwapPromise(std::move(swap_promise));
+
+    DrawFrame();
+  }
+
+  // Make LayerTreeFrameSink lose context. It should clear
+  // |lag_tracking_manager_|.
+  host_impl_->DidLoseLayerTreeFrameSink();
+
+  // Finish the test. |lag_tracking_manager_| will check in its destructor if
+  // there is less than 20 frames in its pending frames list.
+}
+
 // Tests that the scheduled autoscroll task aborts if a 2nd mousedown occurs in
 // the same frame.
 TEST_F(LayerTreeHostImplTest, AutoscrollTaskAbort) {