[Code health] Remove kUniversal tracker related code

We have seen that the kUniversal tracker penalizes the throughput
too much, so remove this code.

Bug: 1122176
Change-Id: I378eaa1327af7fa065c548abff7e99e822fa2d0c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2392768
Reviewed-by: Robert Flack <flackr@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805756}

cc/metrics/compositor_frame_reporter.cc

@@ -214,18 +214,11 @@ constexpr base::TimeDelta kEventLatencyHistogramMax =
     base::TimeDelta::FromSeconds(5);
 constexpr int kEventLatencyHistogramBucketCount = 100;
 
-bool ShouldReportLatencyMetricsForSequenceType(
-    FrameSequenceTrackerType sequence_type) {
-  return sequence_type != FrameSequenceTrackerType::kUniversal;
-}
-
 std::string GetCompositorLatencyHistogramName(
     const int report_type_index,
     FrameSequenceTrackerType frame_sequence_tracker_type,
     const int stage_type_index) {
   DCHECK_LE(frame_sequence_tracker_type, FrameSequenceTrackerType::kMaxType);
-  DCHECK(
-      ShouldReportLatencyMetricsForSequenceType(frame_sequence_tracker_type));
   const char* tracker_type_name =
       FrameSequenceTracker::GetFrameSequenceTrackerTypeName(
           frame_sequence_tracker_type);
@@ -538,7 +531,6 @@ void CompositorFrameReporter::ReportCompositorLatencyHistograms() const {
     for (size_t fst_type = 0; fst_type < active_trackers_.size(); ++fst_type) {
       const auto tracker_type = static_cast<FrameSequenceTrackerType>(fst_type);
       if (!active_trackers_.test(fst_type) ||
-          tracker_type == FrameSequenceTrackerType::kUniversal ||
           tracker_type == FrameSequenceTrackerType::kCustom ||
           tracker_type == FrameSequenceTrackerType::kMaxType) {
         continue;
@@ -576,7 +568,6 @@ void CompositorFrameReporter::ReportCompositorLatencyHistograms() const {
           UMA_HISTOGRAM_ENUMERATION("CompositorLatency.Type.ScrollbarScroll",
                                     report_type);
           break;
-        case FrameSequenceTrackerType::kUniversal:
         case FrameSequenceTrackerType::kCustom:
         case FrameSequenceTrackerType::kMaxType:
           NOTREACHED();
@@ -596,8 +587,6 @@ void CompositorFrameReporter::ReportCompositorLatencyHistograms() const {
 void CompositorFrameReporter::ReportStageHistogramWithBreakdown(
     const CompositorFrameReporter::StageData& stage,
     FrameSequenceTrackerType frame_sequence_tracker_type) const {
-  if (!ShouldReportLatencyMetricsForSequenceType(frame_sequence_tracker_type))
-    return;
   base::TimeDelta stage_delta = stage.end_time - stage.start_time;
   ReportCompositorLatencyHistogram(frame_sequence_tracker_type,
                                    static_cast<int>(stage.stage_type),

cc/metrics/frame_sequence_metrics.cc

@@ -105,7 +105,6 @@ FrameSequenceMetrics::FrameSequenceMetrics(FrameSequenceTrackerType type,
   // SetScrollingThread().
   if ((thread_type == ThreadType::kCompositor ||
        thread_type == ThreadType::kMain) &&
-      type != FrameSequenceTrackerType::kUniversal &&
       type != FrameSequenceTrackerType::kCustom)
     jank_reporter_ = std::make_unique<JankMetrics>(type, thread_type);
 }
@@ -113,17 +112,8 @@ FrameSequenceMetrics::FrameSequenceMetrics(FrameSequenceTrackerType type,
 FrameSequenceMetrics::~FrameSequenceMetrics() = default;
 
 void FrameSequenceMetrics::ReportLeftoverData() {
-  if (HasDataLeftForReporting()) {
-    // Do this before ReportMetrics() which clears the throughput data.
-    // TODO(xidachen): Find a way to make ThroughputUkmReporter to directly talk
-    // to LayerTreeHostClient, and submit throughput data. Instead of storing
-    // the values in ThroughputUkmReporter.
-    if (type_ == FrameSequenceTrackerType::kUniversal &&
-        HasEnoughDataForReporting()) {
-      throughput_ukm_reporter_->ComputeUniversalThroughput(this);
-    }
+  if (HasDataLeftForReporting())
     ReportMetrics();
-  }
 }
 
 void FrameSequenceMetrics::SetScrollingThread(ThreadType scrolling_thread) {
@@ -161,9 +151,6 @@ FrameSequenceMetrics::ThreadType FrameSequenceMetrics::GetEffectiveThread()
     case FrameSequenceTrackerType::kWheelScroll:
       return scrolling_thread_;
 
-    case FrameSequenceTrackerType::kUniversal:
-      return ThreadType::kSlower;
-
     case FrameSequenceTrackerType::kCustom:
       return ThreadType::kMain;
 
@@ -246,16 +233,14 @@ void FrameSequenceMetrics::ReportMetrics() {
       main_throughput_);
 
   // Report for the 'slower thread' for the metrics where it makes sense.
-  bool should_report_slower_thread =
-      IsInteractionType(type_) || type_ == FrameSequenceTrackerType::kUniversal;
+  bool should_report_slower_thread = IsInteractionType(type_);
   base::Optional<int> aggregated_throughput_percent;
   if (should_report_slower_thread) {
     aggregated_throughput_percent = ThroughputData::ReportHistogram(
         this, ThreadType::kSlower,
         GetIndexForMetric(FrameSequenceMetrics::ThreadType::kSlower, type_),
         aggregated_throughput_);
-    if (aggregated_throughput_percent.has_value() && throughput_ukm_reporter_ &&
-        type_ != FrameSequenceTrackerType::kUniversal) {
+    if (aggregated_throughput_percent.has_value() && throughput_ukm_reporter_) {
       throughput_ukm_reporter_->ReportThroughputUkm(
           aggregated_throughput_percent, impl_throughput_percent,
           main_throughput_percent, type_);
@@ -426,7 +411,6 @@ base::Optional<int> FrameSequenceMetrics::ThroughputData::ReportHistogram(
   }
 
   if (!is_animation && !IsInteractionType(sequence_type) &&
-      sequence_type != FrameSequenceTrackerType::kUniversal &&
       sequence_type != FrameSequenceTrackerType::kVideo) {
     return base::nullopt;
   }

cc/metrics/frame_sequence_metrics.h

@@ -24,7 +24,6 @@ enum class FrameSequenceTrackerType {
   kPinchZoom = 2,
   kRAF = 3,
   kTouchScroll = 4,
-  kUniversal = 5,
   kVideo = 6,
   kWheelScroll = 7,
   kScrollbarScroll = 8,

cc/metrics/frame_sequence_metrics_unittest.cc

@@ -27,28 +27,6 @@ TEST(FrameSequenceMetricsTest, AggregatedThroughputClearedAfterReport) {
   EXPECT_EQ(first.aggregated_throughput().frames_produced, 0u);
 }
 
-// Test that ThroughputUkmReporter::ReportThroughputUkm isn't called for the
-// kUniversal tracker.
-TEST(FrameSequenceMetricsTest, UniversalNotReportUkmAtRenderer) {
-  auto recorder = std::make_unique<ukm::TestUkmRecorder>();
-  auto ukm_manager = std::make_unique<UkmManager>(std::move(recorder));
-  ThroughputUkmReporter reporter(ukm_manager.get());
-  auto metric = std::make_unique<FrameSequenceMetrics>(
-      FrameSequenceTrackerType::kUniversal, &reporter);
-
-  metric->impl_throughput().frames_expected = 200u;
-  metric->impl_throughput().frames_produced = 190u;
-  metric->aggregated_throughput().frames_expected = 170u;
-  metric->aggregated_throughput().frames_produced = 150u;
-  metric->ReportMetrics();
-
-  // The corresponding |samples_to_next_event_| element is 0 if the
-  // ReportThroughputUkm isn't called.
-  EXPECT_EQ(reporter.GetSamplesToNextEventForTesting(
-                static_cast<int>(FrameSequenceTrackerType::kUniversal)),
-            1u);
-}
-
 TEST(FrameSequenceMetricsTest, MergeMetrics) {
   // Create a metric with only a small number of frames. It shouldn't report any
   // metrics.
@@ -139,20 +117,6 @@ TEST(FrameSequenceMetricsTest, AllMetricsReported) {
       "Graphics.Smoothness.PercentDroppedFrames.MainThread.TouchScroll", 1u);
   // All the metrics have now been reported. No data should be left over.
   EXPECT_FALSE(first.HasDataLeftForReporting());
-
-  FrameSequenceMetrics third(FrameSequenceTrackerType::kUniversal, nullptr);
-  third.impl_throughput().frames_expected = 120;
-  third.impl_throughput().frames_produced = 80;
-  third.main_throughput().frames_expected = 120;
-  third.main_throughput().frames_produced = 80;
-  EXPECT_TRUE(third.HasEnoughDataForReporting());
-  third.ReportMetrics();
-
-  histograms.ExpectTotalCount(
-      "Graphics.Smoothness.PercentDroppedFrames.CompositorThread.Universal",
-      1u);
-  histograms.ExpectTotalCount(
-      "Graphics.Smoothness.PercentDroppedFrames.MainThread.Universal", 1u);
 }
 
 TEST(FrameSequenceMetricsTest, IrrelevantMetricsNotReported) {

cc/metrics/frame_sequence_tracker.cc

@@ -52,8 +52,6 @@ const char* FrameSequenceTracker::GetFrameSequenceTrackerTypeName(
       return "RAF";
     case FrameSequenceTrackerType::kTouchScroll:
       return "TouchScroll";
-    case FrameSequenceTrackerType::kUniversal:
-      return "Universal";
     case FrameSequenceTrackerType::kVideo:
       return "Video";
     case FrameSequenceTrackerType::kWheelScroll:

cc/metrics/frame_sequence_tracker_collection.cc

@@ -67,7 +67,6 @@ FrameSequenceTracker* FrameSequenceTrackerCollection::StartSequenceInternal(
     metrics->SetScrollingThread(scrolling_thread);
   }
 
-  if (type != FrameSequenceTrackerType::kUniversal) {
     if (metrics->GetEffectiveThread() == ThreadType::kCompositor) {
       if (compositor_frame_reporting_controller_ &&
           compositor_thread_driving_smoothness_ == 0) {
@@ -84,7 +83,6 @@ FrameSequenceTracker* FrameSequenceTrackerCollection::StartSequenceInternal(
       }
       ++main_thread_driving_smoothness_;
     }
-  }
   return frame_trackers_[key].get();
 }
 
@@ -133,7 +131,6 @@ void FrameSequenceTrackerCollection::StopSequence(
         tracker->type());
   }
 
-  if (type != FrameSequenceTrackerType::kUniversal) {
     if (tracker->metrics()->GetEffectiveThread() == ThreadType::kCompositor) {
       DCHECK_GT(compositor_thread_driving_smoothness_, 0u);
       --compositor_thread_driving_smoothness_;
@@ -151,7 +148,6 @@ void FrameSequenceTrackerCollection::StopSequence(
             ThreadType::kMain, false);
       }
     }
-  }
 
   frame_trackers_.erase(key);
   tracker->ScheduleTerminate();
@@ -298,35 +294,6 @@ void FrameSequenceTrackerCollection::NotifyFramePresented(
   DestroyTrackers();
 }
 
-bool FrameSequenceTrackerCollection::HasThroughputData() const {
-  return throughput_ukm_reporter_ &&
-         throughput_ukm_reporter_->HasThroughputData();
-}
-
-int FrameSequenceTrackerCollection::TakeLastAggregatedPercent() {
-  DCHECK(throughput_ukm_reporter_);
-  return throughput_ukm_reporter_->TakeLastAggregatedPercent();
-}
-
-int FrameSequenceTrackerCollection::TakeLastImplPercent() {
-  DCHECK(throughput_ukm_reporter_);
-  return throughput_ukm_reporter_->TakeLastImplPercent();
-}
-
-base::Optional<int> FrameSequenceTrackerCollection::TakeLastMainPercent() {
-  DCHECK(throughput_ukm_reporter_);
-  return throughput_ukm_reporter_->TakeLastMainPercent();
-}
-
-void FrameSequenceTrackerCollection::ComputeUniversalThroughputForTesting() {
-  DCHECK(throughput_ukm_reporter_);
-  const auto type = FrameSequenceTrackerType::kUniversal;
-  auto key = std::make_pair(type, ThreadType::kUnknown);
-  DCHECK(frame_trackers_.contains(key));
-  throughput_ukm_reporter_->ComputeUniversalThroughput(
-      frame_trackers_[key]->metrics());
-}
-
 void FrameSequenceTrackerCollection::DestroyTrackers() {
   for (auto& tracker : removal_trackers_) {
     if (tracker->termination_status() ==
@@ -351,15 +318,10 @@ void FrameSequenceTrackerCollection::DestroyTrackers() {
         accumulated_metrics_.erase(key);
       }
 
-      if (metrics->HasEnoughDataForReporting()) {
-        // Do this before ReportMetrics() which clears the throughput data.
-        if (metrics->type() == FrameSequenceTrackerType::kUniversal)
-          throughput_ukm_reporter_->ComputeUniversalThroughput(metrics.get());
+      if (metrics->HasEnoughDataForReporting())
         metrics->ReportMetrics();
-      }
-      if (metrics->HasDataLeftForReporting()) {
+      if (metrics->HasDataLeftForReporting())
         accumulated_metrics_[key] = std::move(metrics);
-      }
     }
   }
 

cc/metrics/frame_sequence_tracker_collection.h

@@ -112,15 +112,6 @@ class CC_EXPORT FrameSequenceTrackerCollection {
 
   void SetUkmManager(UkmManager* manager);
 
-  // These methods directly calls corresponding APIs in ThroughputUkmReporter,
-  // please refer to the ThroughputUkmReporter for details.
-  bool HasThroughputData() const;
-  int TakeLastAggregatedPercent();
-  int TakeLastImplPercent();
-  base::Optional<int> TakeLastMainPercent();
-
-  void ComputeUniversalThroughputForTesting();
-
   using NotifyCustomerTrackerResutlsCallback =
       base::RepeatingCallback<void(CustomTrackerResults)>;
   void set_custom_tracker_results_added_callback(

cc/metrics/frame_sequence_tracker_unittest.cc

@@ -347,35 +347,13 @@ TEST_F(FrameSequenceTrackerTest, SourceIdChangeDuringSequence) {
                                 viz::BeginFrameAck(args_2, true), args_1);
 }
 
-TEST_F(FrameSequenceTrackerTest, UniversalTrackerCreation) {
-  // The universal tracker should be explicitly created by the object that
-  // manages the |collection_|
-  EXPECT_FALSE(TrackerExists(FrameSequenceTrackerType::kUniversal));
-}
-
-TEST_F(FrameSequenceTrackerTest, UniversalTrackerRestartableAfterClearAll) {
-  collection_.StartSequence(FrameSequenceTrackerType::kUniversal);
-  EXPECT_TRUE(TrackerExists(FrameSequenceTrackerType::kUniversal));
-
-  collection_.ClearAll();
-  EXPECT_FALSE(TrackerExists(FrameSequenceTrackerType::kUniversal));
-
-  collection_.StartSequence(FrameSequenceTrackerType::kUniversal);
-  EXPECT_TRUE(TrackerExists(FrameSequenceTrackerType::kUniversal));
-}
-
 TEST_F(FrameSequenceTrackerTest, 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_EQ(NumberOfTrackers(), 2u);
   EXPECT_TRUE(TrackerExists(FrameSequenceTrackerType::kMainThreadAnimation));
   EXPECT_TRUE(TrackerExists(FrameSequenceTrackerType::kTouchScroll));
   // StopSequence should have destroyed all trackers because there is no frame
@@ -1904,29 +1882,6 @@ TEST_F(FrameSequenceTrackerTest, TrackerTypeEncoding) {
   EXPECT_EQ(active_encoded, 16);  // 1 << 4
 }
 
-TEST_F(FrameSequenceTrackerTest, UniversalTrackerSubmitThroughput) {
-  auto recorder = std::make_unique<ukm::TestUkmRecorder>();
-  auto ukm_manager = std::make_unique<UkmManager>(std::move(recorder));
-
-  collection_.ClearAll();
-  collection_.SetUkmManager(ukm_manager.get());
-  auto* tracker =
-      collection_.StartSequence(FrameSequenceTrackerType::kUniversal);
-  ImplThroughput(tracker).frames_expected = 200u;
-  ImplThroughput(tracker).frames_produced = 190u;
-  MainThroughput(tracker).frames_expected = 100u;
-  MainThroughput(tracker).frames_produced = 50u;
-  AggregatedThroughput(tracker).frames_expected = 200u;
-  AggregatedThroughput(tracker).frames_produced = 150u;
-
-  collection_.ComputeUniversalThroughputForTesting();
-  DCHECK(collection_.HasThroughputData());
-  EXPECT_EQ(collection_.TakeLastAggregatedPercent(), 25);
-  EXPECT_EQ(collection_.TakeLastImplPercent(), 5);
-  EXPECT_EQ(collection_.TakeLastMainPercent().value(), 50);
-  EXPECT_FALSE(collection_.HasThroughputData());
-}
-
 // Test that when an impl frame caused no damage is due to waiting on main, the
 // computation of aggregated throughput is correct.
 TEST_F(FrameSequenceTrackerTest, ImplFrameNoDamageWaitingOnMain1) {

cc/metrics/jank_metrics.cc

@@ -99,8 +99,7 @@ void JankMetrics::AddPresentedFrame(
 }
 
 void JankMetrics::ReportJankMetrics(int frames_expected) {
-  if (tracker_type_ == FrameSequenceTrackerType::kUniversal ||
-      tracker_type_ == FrameSequenceTrackerType::kCustom)
+  if (tracker_type_ == FrameSequenceTrackerType::kCustom)
     return;
 
   int jank_percent = static_cast<int>(100 * jank_count_ / frames_expected);

cc/metrics/jank_metrics_unittest.cc

@@ -228,28 +228,6 @@ TEST_F(JankMetricsTest, RAFMergeJanks) {
   histogram_tester.ExpectTotalCount(invalid_metric, 0u);
 }
 
-// Test if jank reporting is correctly disabled for Universal trackers.
-TEST_F(JankMetricsTest, UniversalNotReported) {
-  base::HistogramTester histogram_tester;
-  FrameSequenceTrackerType tracker_type = FrameSequenceTrackerType::kUniversal;
-  FrameSequenceMetrics::ThreadType thread_type =
-      FrameSequenceMetrics::ThreadType::kCompositor;
-  JankMetrics jank_reporter{tracker_type, thread_type};
-
-  // There should be 4 janks, but the jank reporter does not track or report
-  // them.
-  auto feedbacks = CreateFeedbackSequence({16, +33, +48, 16, +33, +48}, 16.67);
-
-  AddPresentedFramesToJankReporter(&jank_reporter, feedbacks);
-  jank_reporter.ReportJankMetrics(100u);
-
-  // Expect no jank reports even though the sequence contains jank
-  histogram_tester.ExpectTotalCount("Graphics.Smoothness.Jank.Main.Universal",
-                                    0u);
-  histogram_tester.ExpectTotalCount(
-      "Graphics.Smoothness.Jank.Compositor.Universal", 0u);
-}
-
 // Test if jank reporting is correctly disabled for Custom trackers.
 TEST_F(JankMetricsTest, CustomNotReported) {
   base::HistogramTester histogram_tester;

cc/metrics/throughput_ukm_reporter.cc

@@ -16,11 +16,6 @@ constexpr unsigned kNumberOfSamplesToReport = 100u;
 ThroughputUkmReporter::ThroughputUkmReporter(UkmManager* ukm_manager)
     : ukm_manager_(ukm_manager) {
   DCHECK(ukm_manager_);
-  // TODO(crbug.com/1040634): Setting it to 1 such that the first sample is
-  // ignored. TThis is because the universal tracker is active during the page
-  // load and the first sample is heavily biased by loading as a result.
-  samples_to_next_event_[static_cast<int>(
-      FrameSequenceTrackerType::kUniversal)] = 1;
 }
 
 ThroughputUkmReporter::~ThroughputUkmReporter() = default;
@@ -36,10 +31,9 @@ void ThroughputUkmReporter::ReportThroughputUkm(
   if (!ukm_manager_)
     return;
   if (samples_to_next_event_[static_cast<int>(type)] == 0) {
-    // Sample every 100 events. Using the Universal tracker as an example
-    // which reports UMA every 5s, then the system collects UKM once per
-    // 100*5 = 500 seconds. This number may need to be tuned to not throttle
-    // the UKM system.
+    // Sample every 100 events. If a tracker reports UMA every 5s, then the
+    // system collects UKM once per 100*5 = 500 seconds. This number may need to
+    // be tuned to not throttle the UKM system.
     samples_to_next_event_[static_cast<int>(type)] = kNumberOfSamplesToReport;
     if (impl_throughput_percent) {
       ukm_manager_->RecordThroughputUKM(
@@ -70,39 +64,6 @@ void ThroughputUkmReporter::ReportAggregateThroughput(
   --samples_for_aggregated_report_;
 }
 
-void ThroughputUkmReporter::ComputeUniversalThroughput(
-    FrameSequenceMetrics* metrics) {
-  last_impl_percent_ = metrics->impl_throughput().DroppedFramePercent();
-  last_main_percent_ = metrics->main_throughput().DroppedFramePercent();
-  last_aggregated_percent_ =
-      metrics->aggregated_throughput().DroppedFramePercent();
-}
-
-bool ThroughputUkmReporter::HasThroughputData() const {
-  return last_aggregated_percent_.has_value();
-}
-
-int ThroughputUkmReporter::TakeLastAggregatedPercent() {
-  int ret_value = last_aggregated_percent_.value();
-  DCHECK(ret_value >= 0 && ret_value <= 100);
-  last_aggregated_percent_ = base::nullopt;
-  return ret_value;
-}
-
-int ThroughputUkmReporter::TakeLastImplPercent() {
-  int ret_value = last_impl_percent_.value();
-  DCHECK(ret_value >= 0 && ret_value <= 100);
-  last_impl_percent_ = base::nullopt;
-  return ret_value;
-}
-
-base::Optional<int> ThroughputUkmReporter::TakeLastMainPercent() {
-  base::Optional<int> ret_value = last_main_percent_;
-  DCHECK(!ret_value || (ret_value.value() >= 0 && ret_value.value() <= 100));
-  last_main_percent_ = base::nullopt;
-  return ret_value;
-}
-
 uint32_t ThroughputUkmReporter::GetSamplesToNextEventForTesting(int index) {
   DCHECK_LT(index, static_cast<int>(FrameSequenceTrackerType::kMaxType));
   return samples_to_next_event_[index];

cc/metrics/throughput_ukm_reporter.h

@@ -36,21 +36,6 @@ class CC_EXPORT ThroughputUkmReporter {
   void ReportAggregateThroughput(AggregationType aggregation_type,
                                  int throughput);
 
-  void ComputeUniversalThroughput(FrameSequenceMetrics* metrics);
-
-  // Once the kUniversal tracker reported its throughput to UMA, this returns
-  // true. In this case, the |last_aggregated_percent_| and |last_impl_percent_|
-  // must have value.
-  bool HasThroughputData() const;
-  // These functions are called only when HasThroughputData is true. They return
-  // the throughput value of the corresponding thread, and reset them to nullopt
-  // to indicate the value has been reported.
-  int TakeLastAggregatedPercent();
-  int TakeLastImplPercent();
-  // This could be nullopt even if the HasThroughputData() is true, because it
-  // could happen that all the frames are generated from the compositor thread.
-  base::Optional<int> TakeLastMainPercent();
-
   uint32_t GetSamplesToNextEventForTesting(int index);
 
  private:
@@ -65,18 +50,6 @@ class CC_EXPORT ThroughputUkmReporter {
   // initialized right after the LayerTreeHostImpl is created. So when this
   // pointer is initialized, there should be no trackers yet.
   UkmManager* const ukm_manager_;
-
-  // The last "PercentDroppedFrames" reported to UMA. LayerTreeHostImpl will
-  // read this number and send to the GPU process. When this page is done, we
-  // will report to UKM using these numbers. Currently only meaningful to the
-  // kUniversal tracker.
-  // Possible values:
-  //   1. A non-negative int value which is the percent of frames dropped.
-  //   2. base::nullopt: when they are fetched by LayerTreeHostImpl, so that it
-  //      knows that the last value has been reported.
-  base::Optional<int> last_aggregated_percent_;
-  base::Optional<int> last_main_percent_;
-  base::Optional<int> last_impl_percent_;
 };
 
 }  // namespace cc

cc/trees/layer_tree_host_impl.cc

@@ -2315,19 +2315,6 @@ bool LayerTreeHostImpl::DrawLayers(FrameData* frame) {
       std::move(compositor_frame),
       /*hit_test_data_changed=*/false, debug_state_.show_hit_test_borders);
 
-  // This is expected to be true roughly every 5 seconds.
-  if (frame_trackers_.HasThroughputData()) {
-    ukm::SourceId source_id = ukm_manager_->source_id();
-    // source_id can be invalid in tests.
-    if (source_id != ukm::kInvalidSourceId) {
-      int aggregated_percent = frame_trackers_.TakeLastAggregatedPercent();
-      int impl_percent = frame_trackers_.TakeLastImplPercent();
-      base::Optional<int> main_percent = frame_trackers_.TakeLastMainPercent();
-      client_->SubmitThroughputData(source_id, aggregated_percent, impl_percent,
-                                    main_percent);
-    }
-  }
-
 #if DCHECK_IS_ON()
   if (!doing_sync_draw_) {
     // The throughput computation (in |FrameSequenceTracker|) depends on the
@@ -3602,7 +3589,6 @@ bool LayerTreeHostImpl::InitializeFrameSink(
   has_valid_layer_tree_frame_sink_ = true;
 
   auto* context_provider = layer_tree_frame_sink_->context_provider();
-  frame_trackers_.StartSequence(FrameSequenceTrackerType::kUniversal);
 
   if (context_provider) {
     max_texture_size_ =

cc/trees/ukm_manager.cc

@@ -114,7 +114,6 @@ void UkmManager::RecordThroughputUKM(
         CASE_FOR_MAIN_THREAD_TRACKER(RAF);
         CASE_FOR_MAIN_THREAD_TRACKER(ScrollbarScroll);
         CASE_FOR_MAIN_THREAD_TRACKER(TouchScroll);
-        CASE_FOR_MAIN_THREAD_TRACKER(Universal);
         CASE_FOR_MAIN_THREAD_TRACKER(Video);
         CASE_FOR_MAIN_THREAD_TRACKER(WheelScroll);
 #undef CASE_FOR_MAIN_THREAD_TRACKER
@@ -138,7 +137,6 @@ void UkmManager::RecordThroughputUKM(
         CASE_FOR_COMPOSITOR_THREAD_TRACKER(RAF);
         CASE_FOR_COMPOSITOR_THREAD_TRACKER(ScrollbarScroll);
         CASE_FOR_COMPOSITOR_THREAD_TRACKER(TouchScroll);
-        CASE_FOR_COMPOSITOR_THREAD_TRACKER(Universal);
         CASE_FOR_COMPOSITOR_THREAD_TRACKER(Video);
         CASE_FOR_COMPOSITOR_THREAD_TRACKER(WheelScroll);
 #undef CASE_FOR_COMPOSITOR_THREAD_TRACKER
@@ -161,7 +159,6 @@ void UkmManager::RecordThroughputUKM(
         CASE_FOR_SLOWER_THREAD_TRACKER(RAF);
         CASE_FOR_SLOWER_THREAD_TRACKER(ScrollbarScroll);
         CASE_FOR_SLOWER_THREAD_TRACKER(TouchScroll);
-        CASE_FOR_SLOWER_THREAD_TRACKER(Universal);
         CASE_FOR_SLOWER_THREAD_TRACKER(Video);
         CASE_FOR_SLOWER_THREAD_TRACKER(WheelScroll);
 #undef CASE_FOR_SLOWER_THREAD_TRACKER
@@ -272,8 +269,6 @@ void UkmManager::RecordCompositorLatencyUKM(
       continue;
     const auto frame_sequence_tracker_type =
         static_cast<FrameSequenceTrackerType>(type);
-    if (frame_sequence_tracker_type == FrameSequenceTrackerType::kUniversal)
-      continue;
     switch (frame_sequence_tracker_type) {
 #define CASE_FOR_TRACKER(name)            \
   case FrameSequenceTrackerType::k##name: \

cc/trees/ukm_manager_unittest.cc

@@ -77,7 +77,6 @@ const char kPinchZoom[] = "PinchZoom";
 const char kRAF[] = "RAF";
 const char kScrollbarScroll[] = "ScrollbarScroll";
 const char kTouchScroll[] = "TouchScroll";
-const char kUniversal[] = "Universal";
 const char kVideo[] = "Video";
 const char kWheelScroll[] = "WheelScroll";
 
@@ -268,8 +267,6 @@ TEST_P(UkmManagerCompositorLatencyTest, CompositorLatency) {
       static_cast<size_t>(FrameSequenceTrackerType::kTouchScroll));
   active_trackers.set(
       static_cast<size_t>(FrameSequenceTrackerType::kCompositorAnimation));
-  active_trackers.set(
-      static_cast<size_t>(FrameSequenceTrackerType::kUniversal));
 
   manager_->RecordCompositorLatencyUKM(report_type(), stage_history,
                                        active_trackers, viz_breakdown);
@@ -344,7 +341,6 @@ TEST_P(UkmManagerCompositorLatencyTest, CompositorLatency) {
   EXPECT_FALSE(test_ukm_recorder_->EntryHasMetric(entry, kMainThreadAnimation));
   EXPECT_FALSE(test_ukm_recorder_->EntryHasMetric(entry, kPinchZoom));
   EXPECT_FALSE(test_ukm_recorder_->EntryHasMetric(entry, kRAF));
-  EXPECT_FALSE(test_ukm_recorder_->EntryHasMetric(entry, kUniversal));
   EXPECT_FALSE(test_ukm_recorder_->EntryHasMetric(entry, kVideo));
   EXPECT_FALSE(test_ukm_recorder_->EntryHasMetric(entry, kWheelScroll));
 }

chrome/browser/page_load_metrics/observers/ukm_page_load_metrics_observer.cc

@@ -99,21 +99,6 @@ std::unique_ptr<base::trace_event::TracedValue> CumulativeShiftScoreTraceData(
   return data;
 }
 
-int8_t ComputeMedianForThroughput(const base::flat_map<int8_t, int>& data) {
-  int total_samples = 0;
-  for (const auto& e : data)
-    total_samples += e.second;
-  int half_samples = total_samples / 2;
-  int current_samples = 0;
-  for (const auto& e : data) {
-    current_samples += e.second;
-    if (current_samples > half_samples)
-      return e.first;
-  }
-  NOTREACHED();
-  return 0;
-}
-
 bool ValidatePercent(int8_t percent) {
   if (percent >= 0 && percent <= 100)
     return true;
@@ -612,9 +597,6 @@ void UkmPageLoadMetricsObserver::RecordTimingMetrics(
     ReportMainResourceTimingMetrics(timing, &builder);
 
   builder.Record(ukm::UkmRecorder::Get());
-
-  if (throughput_source_id_ != ukm::kInvalidSourceId)
-    ReportThroughputUkm();
 }
 
 void UkmPageLoadMetricsObserver::RecordInternalTimingMetrics(
@@ -1038,29 +1020,6 @@ void UkmPageLoadMetricsObserver::OnThroughputUpdate(
     ++main_throughput_data_[main_throughput_percent->percent];
 }
 
-void UkmPageLoadMetricsObserver::ReportThroughputUkm() {
-  DCHECK_NE(throughput_source_id_, ukm::kInvalidSourceId);
-
-  ukm::builders::Graphics_Smoothness_PercentDroppedFrames builder(
-      throughput_source_id_);
-  if (aggregated_throughput_data_.size() > 0) {
-    builder.SetSlowerThread_Universal(
-        ComputeMedianForThroughput(aggregated_throughput_data_));
-    aggregated_throughput_data_.clear();
-  }
-  if (impl_throughput_data_.size() > 0) {
-    builder.SetCompositorThread_Universal(
-        ComputeMedianForThroughput(impl_throughput_data_));
-    impl_throughput_data_.clear();
-  }
-  if (main_throughput_data_.size() > 0) {
-    builder.SetMainThread_Universal(
-        ComputeMedianForThroughput(main_throughput_data_));
-    main_throughput_data_.clear();
-  }
-  builder.Record(ukm::UkmRecorder::Get());
-}
-
 void UkmPageLoadMetricsObserver::OnCpuTimingUpdate(
     content::RenderFrameHost* subframe_rfh,
     const page_load_metrics::mojom::CpuTiming& timing) {

chrome/browser/page_load_metrics/observers/ukm_page_load_metrics_observer.h

@@ -149,9 +149,6 @@ class UkmPageLoadMetricsObserver
 
   void RecordInputTimingMetrics();
 
-  // Report throughput to Ukm.
-  void ReportThroughputUkm();
-
   // Captures the site engagement score for the committed URL and
   // returns the score rounded to the nearest 10.
   base::Optional<int64_t> GetRoundedSiteEngagementScore() const;

tools/metrics/histograms/histograms.xml

@@ -217836,6 +217836,9 @@ regressions. -->
 </histogram_suffixes>
 
 <histogram_suffixes name="SmoothnessSequenceTypes.Universal" separator=".">
+  <obsolete>
+    Removed in 9/2020, due to over penalize smoothness.
+  </obsolete>
   <suffix name="Universal" label="All frame production"/>
   <affected-histogram name="Graphics.Smoothness.Checkerboarding"/>
   <affected-histogram name="Graphics.Smoothness.FrameSequenceLength"/>

tools/metrics/ukm/ukm.xml

@@ -4726,6 +4726,9 @@ be describing additional metrics about the same event.
     </summary>
   </metric>
   <metric name="CompositorThread.Universal">
+    <obsolete>
+      Removed in 9/2020, due to over penalize smoothness.
+    </obsolete>
     <summary>
       The throughput of the compositor thread for all frame production.
     </summary>
@@ -4773,6 +4776,9 @@ be describing additional metrics about the same event.
     </summary>
   </metric>
   <metric name="MainThread.Universal">
+    <obsolete>
+      Removed in 9/2020, due to over penalize smoothness.
+    </obsolete>
     <summary>
       The throughput of the main thread for all frame production.
     </summary>
@@ -4873,6 +4879,9 @@ be describing additional metrics about the same event.
     </aggregation>
   </metric>
   <metric name="SlowerThread.Universal">
+    <obsolete>
+      Removed in 9/2020, due to over penalize smoothness.
+    </obsolete>
     <summary>
       The worse throughput of the main and the compositor thread for all frame
       production.