Propagate scroll state for backfill PRs.

Set scroll state to fetch from active scrolling tracker
when making pipeline reporters for backfilled frames.

Presented frame pr:
https://screenshot.googleplex.com/AAds6BJNYfCHPtX
Backfill pr:
https://screenshot.googleplex.com/6iwvCBgqPhAuhNb

Bug: 40213368
Change-Id: Ic3f0ff90d564b6d640a7440ce6999856a135e9ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5952948
Reviewed-by: Kyle Charbonneau <kylechar@chromium.org>
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Commit-Queue: Stacy Gaikovaia <gaiko@google.com>
Auto-Submit: Stacy Gaikovaia <gaiko@google.com>
Cr-Commit-Position: refs/heads/main@{#1379708}

cc/metrics/compositor_frame_reporter.h

@@ -381,6 +381,8 @@ class CC_EXPORT CompositorFrameReporter {
     tick_clock_ = tick_clock;
   }
 
+  SmoothThread get_smooth_thread() { return smooth_thread_; }
+
   void set_checkerboarded_needs_raster(bool checkerboarded_needs_raster) {
     checkerboarded_needs_raster_ = checkerboarded_needs_raster;
   }

cc/metrics/compositor_frame_reporting_controller.cc

@@ -891,15 +891,16 @@ void CompositorFrameReportingController::CreateReportersForDroppedFrames(
         viz::BeginFrameArgs::NORMAL);
     devtools_instrumentation::DidBeginFrame(
         layer_tree_host_id_, args.frame_time, args.frame_id.sequence_number);
-    // ThreadType::kUnknown is used here for scrolling thread, because the
-    // frames reported here could have a scroll interaction active at their
-    // start time, but they were skipped and history of scrolling thread might
-    // change in the diff of start time and report time.
+
+    // Set the scrolling thread based on the global frame sequence trackers
+    // rather than the `scrolling_thread_` member, because the scrolling thread
+    // might have changed for a skipped or backfilled frame.
     auto reporter = std::make_unique<CompositorFrameReporter>(
         active_trackers_, args, should_report_histograms_,
         GetSmoothThreadAtTime(timestamp),
-        FrameInfo::SmoothEffectDrivingThread::kUnknown, layer_tree_host_id_,
-        global_trackers_);
+        global_trackers_.frame_sequence_trackers
+            ->GetSmoothEffectDrivingThread(),
+        layer_tree_host_id_, global_trackers_);
     reporter->set_tick_clock(tick_clock_);
     reporter->StartStage(StageType::kBeginImplFrameToSendBeginMainFrame,
                          timestamp);

cc/metrics/compositor_frame_reporting_controller_unittest.cc

@@ -22,6 +22,8 @@
 #include "base/time/time.h"
 #include "cc/metrics/dropped_frame_counter.h"
 #include "cc/metrics/event_metrics.h"
+#include "cc/metrics/frame_sequence_tracker.h"
+#include "cc/metrics/frame_sequence_tracker_collection.h"
 #include "cc/metrics/total_frame_counter.h"
 #include "cc/scheduler/commit_earlyout_reason.h"
 #include "components/viz/common/frame_timing_details.h"
@@ -125,9 +127,14 @@ class CompositorFrameReportingControllerTest : public testing::Test {
     reporting_controller_.set_tick_clock(&test_tick_clock_);
     args_ = SimulateBeginFrameArgs(current_id_);
     reporting_controller_.SetDroppedFrameCounter(&dropped_counter_);
+    reporting_controller_.SetFrameSequenceTrackerCollection(
+        &frame_seq_tracker_collection_);
     dropped_counter_.set_total_counter(&total_frame_counter_);
   }
 
+  ~CompositorFrameReportingControllerTest() override {
+    reporting_controller_.SetFrameSequenceTrackerCollection(nullptr);
+  }
   // The following functions simulate the actions that would
   // occur for each phase of the reporting controller.
   void SimulateBeginImplFrame() {
@@ -339,6 +346,8 @@ class CompositorFrameReportingControllerTest : public testing::Test {
   DroppedFrameCounter dropped_counter_;
   TotalFrameCounter total_frame_counter_;
   TestCompositorFrameReportingController reporting_controller_;
+  FrameSequenceTrackerCollection frame_seq_tracker_collection_{
+      /*is_single_threaded=*/false, &reporting_controller_};
   ::base::test::TracingEnvironment tracing_environment_;
 };
 

cc/metrics/dropped_frame_counter.h

@@ -97,7 +97,8 @@ class CC_EXPORT DroppedFrameCounter {
   void ReportFramesOnEveryFrameForUI();
 
   void OnBeginFrame(const viz::BeginFrameArgs& args);
-  void OnEndFrame(const viz::BeginFrameArgs& args, const FrameInfo& frame_info);
+  virtual void OnEndFrame(const viz::BeginFrameArgs& args,
+                          const FrameInfo& frame_info);
   void SetUkmSmoothnessDestination(UkmSmoothnessDataShared* smoothness_data);
   void OnFcpReceived();
 

cc/metrics/frame_sequence_tracker_collection.cc

@@ -10,6 +10,7 @@
 #include "base/containers/contains.h"
 #include "base/memory/ptr_util.h"
 #include "cc/metrics/compositor_frame_reporting_controller.h"
+#include "cc/metrics/frame_info.h"
 #include "cc/metrics/frame_sequence_tracker.h"
 
 namespace cc {
@@ -336,4 +337,14 @@ void FrameSequenceTrackerCollection::AddSortedFrame(
   DestroyTrackers();
 }
 
+FrameInfo::SmoothEffectDrivingThread
+FrameSequenceTrackerCollection::GetSmoothEffectDrivingThread() {
+  for (auto const& tracker : frame_trackers_) {
+    if (IsScrollType(tracker.first.first)) {
+      return tracker.first.second;
+    }
+  }
+  return FrameInfo::SmoothEffectDrivingThread::kUnknown;
+}
+
 }  // namespace cc

cc/metrics/frame_sequence_tracker_collection.h

@@ -100,6 +100,8 @@ class CC_EXPORT FrameSequenceTrackerCollection {
   void AddSortedFrame(const viz::BeginFrameArgs& args,
                       const FrameInfo& frame_info);
 
+  FrameInfo::SmoothEffectDrivingThread GetSmoothEffectDrivingThread();
+
  private:
   friend class FrameSequenceTrackerTest;
 

cc/metrics/frame_sequence_tracker_unittest.cc

@@ -20,6 +20,7 @@
 #include "cc/metrics/compositor_frame_reporting_controller.h"
 #include "cc/metrics/frame_sequence_tracker_collection.h"
 #include "cc/metrics/frame_sorter.h"
+#include "cc/trees/layer_tree_host_impl.h"
 #include "components/viz/common/frame_sinks/begin_frame_args.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -851,4 +852,63 @@ TEST_F(FrameSequenceTrackerTest, CustomTrackerOutOfOrderFramesMissingV3Data) {
   EXPECT_EQ(2u, results[1].frames_expected_v3);
 }
 
+// Mock DroppedFrameCounter class in order to test the number of times that
+// frames get backfilled. This is necessary since `WillBeginImplFrame` creates
+// `CompositorFrameReporter`s for backfilled frames which submit to the DFC
+// without a good interim spot to analyze the frame info contents.
+class DroppedFrameCounterMock : public DroppedFrameCounter {
+ public:
+  MOCK_METHOD2(OnEndFrame, void(const viz::BeginFrameArgs&, const FrameInfo&));
+};
+
+TEST_F(FrameSequenceTrackerTest,
+       FrameTrackerSkippedFramesPreservesSmoothThread) {
+  const uint64_t source = 1;
+  uint64_t sequence = 0;
+  const uint64_t kNumFramesSkipped = 5;
+
+  DroppedFrameCounterMock dfc = DroppedFrameCounterMock();
+
+  // Expect that kNumFramesSkipped are backfilled with the appropriate smooth
+  // thread set.
+  EXPECT_CALL(dfc, OnEndFrame(testing::_, testing::_))
+      .Times(kNumFramesSkipped)
+      .WillRepeatedly([=](const viz::BeginFrameArgs& args,
+                          const FrameInfo& frame_info) {
+        EXPECT_EQ(frame_info.final_state, FrameInfo::FrameFinalState::kDropped);
+        EXPECT_EQ(frame_info.scroll_thread,
+                  FrameInfo::SmoothEffectDrivingThread::kCompositor);
+      });
+
+  compositor_frame_reporting_controller_->SetDroppedFrameCounter(&dfc);
+  compositor_frame_reporting_controller_->SetFrameSequenceTrackerCollection(
+      &collection_);
+  auto frame0_args = CreateBeginFrameArgs(source, ++sequence);
+  compositor_frame_reporting_controller_->WillBeginImplFrame(frame0_args);
+  compositor_frame_reporting_controller_->OnFinishImplFrame(
+      frame0_args.frame_id);
+
+  // Starting frame 5 will trigger the callback expectation.
+  auto frame5_args =
+      CreateBeginFrameArgs(source, sequence + kNumFramesSkipped,
+                           base::TimeTicks::Now() /*+ base::Seconds(5)*/);
+  compositor_frame_reporting_controller_->WillBeginImplFrame(frame5_args);
+  // Clear the expectation before simulating finishing the frame.
+  testing::Mock::VerifyAndClearExpectations(&dfc);
+  compositor_frame_reporting_controller_->WillBeginMainFrame(frame5_args);
+  compositor_frame_reporting_controller_->NotifyReadyToCommit(nullptr);
+  compositor_frame_reporting_controller_->WillCommit();
+  compositor_frame_reporting_controller_->DidCommit();
+  compositor_frame_reporting_controller_->WillActivate();
+  compositor_frame_reporting_controller_->DidActivate();
+  SubmitInfo submit_info;
+  compositor_frame_reporting_controller_->DidSubmitCompositorFrame(
+      submit_info, frame5_args.frame_id, frame5_args.frame_id);
+  compositor_frame_reporting_controller_->OnFinishImplFrame(
+      frame5_args.frame_id);
+  viz::FrameTimingDetails ftd;
+  compositor_frame_reporting_controller_->DidPresentCompositorFrame(
+      submit_info.frame_token, ftd);
+}
+
 }  // namespace cc