[Code health] Cleanup FrameSequenceTrackerTest

There were two tests written in the way that its test body is inside
the FrameSequenceTrackerTest class. This was because there were a few
APIs that were not ready at the moment.

Now that we have all the instruments in the FrameSequenceTrackerTest
class to access APIs in FrameSequenceTracker, this CL cleans up the
test.

Bug: None
Change-Id: I88e09893500381669fd2d3d5bc04eccad839455c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2126249
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Yi Gu <yigu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#754640}

cc/metrics/frame_sequence_tracker.cc

@@ -559,10 +559,6 @@ void FrameSequenceTracker::ScheduleTerminate() {
   }
 }
 
-void FrameSequenceTracker::ReportMetricsForTesting() {
-  metrics_->ReportMetrics();
-}
-
 void FrameSequenceTracker::ReportBeginImplFrame(
     const viz::BeginFrameArgs& args) {
   if (termination_status_ != TerminationStatus::kActive)

cc/metrics/frame_sequence_tracker.h

@@ -357,8 +357,6 @@ class CC_EXPORT FrameSequenceTracker {
 
   bool ShouldIgnoreSequence(uint64_t sequence_number) const;
 
-  void ReportMetricsForTesting();
-
   const FrameSequenceTrackerType type_;
 
   TerminationStatus termination_status_ = TerminationStatus::kActive;

cc/metrics/frame_sequence_tracker_unittest.cc

@@ -103,83 +103,10 @@ class FrameSequenceTrackerTest : public testing::Test {
     return collection_.frame_trackers_.contains(type);
   }
 
-  void TestNotifyFramePresented() {
-    collection_.StartSequence(FrameSequenceTrackerType::kCompositorAnimation);
-    collection_.StartSequence(FrameSequenceTrackerType::kMainThreadAnimation);
-    // The kTouchScroll tracker is created in the test constructor, and the
-    // kUniversal tracker is created in the FrameSequenceTrackerCollection
-    // constructor.
-    EXPECT_EQ(collection_.frame_trackers_.size(), 3u);
-    collection_.StartSequence(FrameSequenceTrackerType::kUniversal);
-    EXPECT_EQ(collection_.frame_trackers_.size(), 4u);
-
-    collection_.StopSequence(FrameSequenceTrackerType::kCompositorAnimation);
-    EXPECT_EQ(collection_.frame_trackers_.size(), 3u);
-    EXPECT_TRUE(collection_.frame_trackers_.contains(
-        FrameSequenceTrackerType::kMainThreadAnimation));
-    EXPECT_TRUE(collection_.frame_trackers_.contains(
-        FrameSequenceTrackerType::kTouchScroll));
-    ASSERT_EQ(collection_.removal_trackers_.size(), 1u);
-    EXPECT_EQ(collection_.removal_trackers_[0]->type_,
-              FrameSequenceTrackerType::kCompositorAnimation);
-
-    gfx::PresentationFeedback feedback;
-    collection_.NotifyFramePresented(1u, feedback);
-    // NotifyFramePresented should call ReportFramePresented on all the
-    // |removal_trackers_|, which changes their termination_status_ to
-    // kReadyForTermination. So at this point, the |removal_trackers_| should be
-    // empty.
-    EXPECT_TRUE(collection_.removal_trackers_.empty());
-  }
-
-  void ReportMetricsTest() {
-    base::HistogramTester histogram_tester;
-
-    // Test that there is no main thread frames expected.
-    tracker_->impl_throughput().frames_expected = 100u;
-    tracker_->impl_throughput().frames_produced = 85u;
-    tracker_->ReportMetricsForTesting();
-    histogram_tester.ExpectTotalCount(
-        "Graphics.Smoothness.PercentDroppedFrames.CompositorThread.TouchScroll",
-        1u);
-    histogram_tester.ExpectTotalCount(
-        "Graphics.Smoothness.PercentDroppedFrames.MainThread.TouchScroll", 0u);
-
-    // Test that both are reported.
-    tracker_->impl_throughput().frames_expected = 100u;
-    tracker_->impl_throughput().frames_produced = 85u;
-    tracker_->main_throughput().frames_expected = 150u;
-    tracker_->main_throughput().frames_produced = 25u;
-    tracker_->ReportMetricsForTesting();
-    histogram_tester.ExpectTotalCount(
-        "Graphics.Smoothness.PercentDroppedFrames.CompositorThread.TouchScroll",
-        2u);
-    histogram_tester.ExpectTotalCount(
-        "Graphics.Smoothness.PercentDroppedFrames.MainThread.TouchScroll", 1u);
-
-    // Test that none is reported.
-    tracker_->main_throughput().frames_expected = 2u;
-    tracker_->main_throughput().frames_produced = 1u;
-    tracker_->impl_throughput().frames_expected = 2u;
-    tracker_->impl_throughput().frames_produced = 1u;
-    tracker_->ReportMetricsForTesting();
-    histogram_tester.ExpectTotalCount(
-        "Graphics.Smoothness.PercentDroppedFrames.CompositorThread.TouchScroll",
-        2u);
-    histogram_tester.ExpectTotalCount(
-        "Graphics.Smoothness.PercentDroppedFrames.MainThread.TouchScroll", 1u);
-
-    // Test the case where compositor and main thread have the same throughput.
-    tracker_->impl_throughput().frames_expected = 120u;
-    tracker_->impl_throughput().frames_produced = 118u;
-    tracker_->main_throughput().frames_expected = 120u;
-    tracker_->main_throughput().frames_produced = 118u;
-    tracker_->ReportMetricsForTesting();
-    histogram_tester.ExpectTotalCount(
-        "Graphics.Smoothness.PercentDroppedFrames.CompositorThread.TouchScroll",
-        3u);
-    histogram_tester.ExpectTotalCount(
-        "Graphics.Smoothness.PercentDroppedFrames.MainThread.TouchScroll", 2u);
+  bool RemovalTrackerExists(unsigned index,
+                            FrameSequenceTrackerType type) const {
+    DCHECK_GT(collection_.removal_trackers_.size(), index);
+    return collection_.removal_trackers_[index]->type_ == type;
   }
 
   void GenerateSequence(const char* str) {
@@ -297,7 +224,7 @@ class FrameSequenceTrackerTest : public testing::Test {
     }
   }
 
-  void ReportMetrics() { tracker_->ReportMetricsForTesting(); }
+  void ReportMetrics() { tracker_->metrics_->ReportMetrics(); }
 
   base::TimeDelta TimeDeltaToReort() const {
     return tracker_->time_delta_to_report_;
@@ -395,7 +322,30 @@ TEST_F(FrameSequenceTrackerTest, UniversalTrackerRestartableAfterClearAll) {
 }
 
 TEST_F(FrameSequenceTrackerTest, TestNotifyFramePresented) {
-  TestNotifyFramePresented();
+  collection_.StartSequence(FrameSequenceTrackerType::kCompositorAnimation);
+  collection_.StartSequence(FrameSequenceTrackerType::kMainThreadAnimation);
+  // The kTouchScroll tracker is created in the test constructor, and the
+  // kUniversal tracker is created in the FrameSequenceTrackerCollection
+  // constructor.
+  EXPECT_EQ(NumberOfTrackers(), 3u);
+  collection_.StartSequence(FrameSequenceTrackerType::kUniversal);
+  EXPECT_EQ(NumberOfTrackers(), 4u);
+
+  collection_.StopSequence(FrameSequenceTrackerType::kCompositorAnimation);
+  EXPECT_EQ(NumberOfTrackers(), 3u);
+  EXPECT_TRUE(TrackerExists(FrameSequenceTrackerType::kMainThreadAnimation));
+  EXPECT_TRUE(TrackerExists(FrameSequenceTrackerType::kTouchScroll));
+  ASSERT_EQ(NumberOfRemovalTrackers(), 1u);
+  EXPECT_TRUE(
+      RemovalTrackerExists(0, FrameSequenceTrackerType::kCompositorAnimation));
+
+  gfx::PresentationFeedback feedback;
+  collection_.NotifyFramePresented(1u, feedback);
+  // NotifyFramePresented should call ReportFramePresented on all the
+  // |removal_trackers_|, which changes their termination_status_ to
+  // kReadyForTermination. So at this point, the |removal_trackers_| should be
+  // empty.
+  EXPECT_EQ(NumberOfRemovalTrackers(), 0u);
 }
 
 // Base case for checkerboarding: present a single frame with checkerboarding,
@@ -499,7 +449,61 @@ TEST_F(FrameSequenceTrackerTest, MultipleCheckerboardingFrames) {
 }
 
 TEST_F(FrameSequenceTrackerTest, ReportMetrics) {
-  ReportMetricsTest();
+  base::HistogramTester histogram_tester;
+
+  // Test that there is no main thread frames expected.
+  ImplThroughput().frames_expected = 100u;
+  ImplThroughput().frames_produced = 85u;
+  ReportMetrics();
+  histogram_tester.ExpectTotalCount(
+      "Graphics.Smoothness.PercentDroppedFrames.CompositorThread.TouchScroll",
+      1u);
+  histogram_tester.ExpectTotalCount(
+      "Graphics.Smoothness.PercentDroppedFrames.MainThread.TouchScroll", 0u);
+  histogram_tester.ExpectTotalCount(
+      "Graphics.Smoothness.PercentDroppedFrames.SlowerThread.TouchScroll", 1u);
+
+  // Test that both are reported.
+  ImplThroughput().frames_expected = 100u;
+  ImplThroughput().frames_produced = 85u;
+  MainThroughput().frames_expected = 150u;
+  MainThroughput().frames_produced = 25u;
+  ReportMetrics();
+  histogram_tester.ExpectTotalCount(
+      "Graphics.Smoothness.PercentDroppedFrames.CompositorThread.TouchScroll",
+      2u);
+  histogram_tester.ExpectTotalCount(
+      "Graphics.Smoothness.PercentDroppedFrames.MainThread.TouchScroll", 1u);
+  histogram_tester.ExpectTotalCount(
+      "Graphics.Smoothness.PercentDroppedFrames.SlowerThread.TouchScroll", 2u);
+
+  // Test that none is reported.
+  MainThroughput().frames_expected = 2u;
+  MainThroughput().frames_produced = 1u;
+  ImplThroughput().frames_expected = 2u;
+  ImplThroughput().frames_produced = 1u;
+  ReportMetrics();
+  histogram_tester.ExpectTotalCount(
+      "Graphics.Smoothness.PercentDroppedFrames.CompositorThread.TouchScroll",
+      2u);
+  histogram_tester.ExpectTotalCount(
+      "Graphics.Smoothness.PercentDroppedFrames.MainThread.TouchScroll", 1u);
+  histogram_tester.ExpectTotalCount(
+      "Graphics.Smoothness.PercentDroppedFrames.SlowerThread.TouchScroll", 2u);
+
+  // Test the case where compositor and main thread have the same throughput.
+  ImplThroughput().frames_expected = 120u;
+  ImplThroughput().frames_produced = 118u;
+  MainThroughput().frames_expected = 120u;
+  MainThroughput().frames_produced = 118u;
+  ReportMetrics();
+  histogram_tester.ExpectTotalCount(
+      "Graphics.Smoothness.PercentDroppedFrames.CompositorThread.TouchScroll",
+      3u);
+  histogram_tester.ExpectTotalCount(
+      "Graphics.Smoothness.PercentDroppedFrames.MainThread.TouchScroll", 2u);
+  histogram_tester.ExpectTotalCount(
+      "Graphics.Smoothness.PercentDroppedFrames.SlowerThread.TouchScroll", 3u);
 }
 
 TEST_F(FrameSequenceTrackerTest, ReportMetricsAtFixedInterval) {