[cc/scheduler] Add a feature to throttle main frames to 60Hz

This CL adds a feature to allow decoupling BeginImplFrame() from
BeginMainFrame(). It works by not sending a BeginMainFrame() (but not
marking it as sent either) in the scheduler, on high refresh rate
clients.

Concretely, when the feature is enabled, no more than one BeginMainFrame
is sent per 1/60th of a second, which in practice means that impl-side
scrolling (even when blocked on main, i.e. with a non-passive observer)
and CSS animations work at 120fps, but main updates
(i.e. requestAnimationFrame()) happen at 60fps.

This patch works, but is not 100% complete, hence the feature is
disabled, though exercized with unit tests.

Change-Id: Iab0b0aad76ee4437b4ed7a39137fd88b1b5b8a7e
Bug: 379678455
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6054335
Commit-Queue: Benoit Lize <lizeb@chromium.org>
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1395984}

cc/base/features.cc

@@ -196,4 +196,8 @@ BASE_FEATURE(kDynamicSafeAreaInsetsSupportedByCC,
              "DynamicSafeAreaInsetsSupportedByCC",
              base::FEATURE_ENABLED_BY_DEFAULT);
 
+BASE_FEATURE(kThrottleMainFrameTo60Hz,
+             "ThrottleMainFrameTo60Hz",
+             base::FEATURE_DISABLED_BY_DEFAULT);
+
 }  // namespace features

cc/base/features.h

@@ -199,6 +199,10 @@ CC_BASE_EXPORT BASE_DECLARE_FEATURE(kInitImageDecodeLastUseTime);
 // affected nodes.
 CC_BASE_EXPORT BASE_DECLARE_FEATURE(kDynamicSafeAreaInsetsSupportedByCC);
 
+// On devices with a high refresh rate, whether to throttle main (not impl)
+// frame production to 60Hz.
+CC_BASE_EXPORT BASE_DECLARE_FEATURE(kThrottleMainFrameTo60Hz);
+
 }  // namespace features
 
 #endif  // CC_BASE_FEATURES_H_

cc/scheduler/scheduler.cc

@@ -10,11 +10,13 @@
 
 #include "base/auto_reset.h"
 #include "base/check_op.h"
+#include "base/feature_list.h"
 #include "base/functional/bind.h"
 #include "base/location.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/task/delay_policy.h"
 #include "base/task/single_thread_task_runner.h"
+#include "base/time/time.h"
 #include "base/trace_event/trace_event.h"
 #include "base/trace_event/traced_value.h"
 #include "cc/base/devtools_instrumentation.h"
@@ -396,6 +398,27 @@ bool Scheduler::OnBeginFrameDerivedImpl(const viz::BeginFrameArgs& args) {
   if (args.interval != last_frame_interval_ && args.interval.is_positive()) {
     last_frame_interval_ = args.interval;
     client_->FrameIntervalUpdated(last_frame_interval_);
+
+    // Only query the feature (and thus enter the experiment group) if we see a
+    // short interval. This ignores 90Hz displays, on purpose, and adds some
+    // leeway.
+    //
+    // Apply some slack, so that if for some reason the interval is a bit larger
+    // than 8.33333333333333ms, then we catch it still.
+    constexpr float kSlackFactor = .9;
+    if (args.interval < base::Hertz(120) * (1 / kSlackFactor) &&
+        base::FeatureList::IsEnabled(features::kThrottleMainFrameTo60Hz)) {
+      TRACE_EVENT0("cc", "ThrottleMainFrameTo60Hz");
+      // Note that we don't change args.interval, so the next main frame will
+      // see e.g. 8ms, even though the next one will come in 16ms. This is not
+      // necessarily bad, as it is mostly used for idle period timing.
+      //
+      // Here as well, use a slack factor, to make sure that small timing
+      // variations don't result in uneven pacing.
+      state_machine_.SetThrottleMainFrames(base::Hertz(60.) * kSlackFactor);
+    } else {
+      state_machine_.SetThrottleMainFrames(base::TimeDelta());
+    }
   }
 
   // Drop the BeginFrame if we don't need one.
@@ -629,6 +652,7 @@ void Scheduler::BeginImplFrameSynchronous(const viz::BeginFrameArgs& args) {
 }
 
 void Scheduler::FinishImplFrame() {
+  TRACE_EVENT0("cc", __PRETTY_FUNCTION__);
   DCHECK(!needs_finish_frame_for_synchronous_compositor_);
   state_machine_.OnBeginImplFrameIdle();
 
@@ -641,6 +665,8 @@ void Scheduler::FinishImplFrame() {
                               SchedulerStateMachine::BeginMainFrameState::IDLE;
     bool is_draw_throttled =
         state_machine_.needs_redraw() && state_machine_.IsDrawThrottled();
+    TRACE_EVENT2("cc", "DidNotSubmitInLastFrame", "has_pending_tree",
+                 has_pending_tree, "is_waiting_on_main", is_waiting_on_main);
 
     FrameSkippedReason reason = FrameSkippedReason::kNoDamage;
 
@@ -678,6 +704,7 @@ void Scheduler::FinishImplFrame() {
 
 void Scheduler::SendDidNotProduceFrame(const viz::BeginFrameArgs& args,
                                        FrameSkippedReason reason) {
+  TRACE_EVENT1("cc", __PRETTY_FUNCTION__, "reason", reason);
   if (last_begin_frame_ack_.frame_id == args.frame_id)
     return;
   last_begin_frame_ack_ = viz::BeginFrameAck(args, false /* has_damage */);

cc/scheduler/scheduler_state_machine.cc

@@ -7,6 +7,7 @@
 #include "base/check_op.h"
 #include "base/format_macros.h"
 #include "base/notreached.h"
+#include "base/time/time.h"
 #include "base/trace_event/trace_event.h"
 #include "base/trace_event/traced_value.h"
 #include "base/values.h"
@@ -670,6 +671,14 @@ bool SchedulerStateMachine::ShouldSendBeginMainFrame() const {
     return false;
   }
 
+  // This comes last, because we only want to throttle main frame that would
+  // otherwise actually be sent, and we do not want to throttle forced redraws.
+  if (main_frame_throttled_interval_.is_positive() &&
+      Now() - last_sent_begin_main_frame_time_ <
+          main_frame_throttled_interval_) {
+    TRACE_EVENT0("cc", "ThrottleMainFrame");
+    return false;
+  }
   return true;
 }
 
@@ -930,6 +939,7 @@ void SchedulerStateMachine::WillSendBeginMainFrame() {
   did_send_begin_main_frame_for_current_frame_ = true;
   // TODO(szager): Make sure this doesn't break perfetto
   last_frame_number_begin_main_frame_sent_ = current_frame_number_;
+  last_sent_begin_main_frame_time_ = Now();
 }
 
 void SchedulerStateMachine::WillNotifyBeginMainFrameNotExpectedUntil() {
@@ -1517,6 +1527,10 @@ bool SchedulerStateMachine::ShouldBlockDeadlineIndefinitely() const {
   return false;
 }
 
+void SchedulerStateMachine::SetThrottleMainFrames(base::TimeDelta interval) {
+  main_frame_throttled_interval_ = interval;
+}
+
 bool SchedulerStateMachine::IsDrawThrottled() const {
   return pending_submit_frames_ >= kMaxPendingSubmitFrames &&
          !settings_.disable_frame_rate_limit;
@@ -1794,4 +1808,8 @@ bool SchedulerStateMachine::HasInitializedLayerTreeFrameSink() const {
   NOTREACHED();
 }
 
+base::TimeTicks SchedulerStateMachine::Now() const {
+  return base::TimeTicks::Now();
+}
+
 }  // namespace cc

cc/scheduler/scheduler_state_machine.h

@@ -7,6 +7,7 @@
 
 #include <stdint.h>
 
+#include "base/time/time.h"
 #include "base/tracing/protos/chrome_track_event.pbzero.h"
 #include "cc/cc_export.h"
 #include "cc/scheduler/commit_earlyout_reason.h"
@@ -208,6 +209,13 @@ class CC_EXPORT SchedulerStateMachine {
 
   bool IsDrawThrottled() const;
 
+  // Throttles main frame production to a given interval, but not compositor
+  // frames.
+  void SetThrottleMainFrames(base::TimeDelta interval);
+  base::TimeDelta main_frame_throttled_interval() const {
+    return main_frame_throttled_interval_;
+  }
+
   // Indicates whether the LayerTreeHostImpl is visible.
   void SetVisible(bool visible);
   bool visible() const { return visible_; }
@@ -421,6 +429,9 @@ class CC_EXPORT SchedulerStateMachine {
   void WillPerformImplSideInvalidationInternal();
   void DidDrawInternal(DrawResult draw_result);
 
+  // Virtual for testing.
+  virtual base::TimeTicks Now() const;
+
   const SchedulerSettings settings_;
 
   LayerTreeFrameSinkState layer_tree_frame_sink_state_ =
@@ -444,6 +455,9 @@ class CC_EXPORT SchedulerStateMachine {
   int last_frame_number_begin_main_frame_sent_ = -1;
   int last_frame_number_invalidate_layer_tree_frame_sink_performed_ = -1;
 
+  base::TimeTicks last_sent_begin_main_frame_time_;
+  base::TimeDelta main_frame_throttled_interval_;
+
   // Inputs from the last impl frame that are required for decisions made in
   // this impl frame. The values from the last frame are cached before being
   // reset in OnBeginImplFrame.

cc/scheduler/scheduler_state_machine_unittest.cc

@@ -1,13 +1,18 @@
 // Copyright 2011 The Chromium Authors
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
-
 #include "cc/scheduler/scheduler_state_machine.h"
 
 #include <stddef.h>
 
 #include <array>
 
+#include "base/time/time.h"
+#include "cc/metrics/begin_main_frame_metrics.h"
+#ifdef UNSAFE_BUFFERS_BUILD
+// TODO(crbug.com/351564777): Remove this and convert code to safer constructs.
+#pragma allow_unsafe_buffers
+#endif
 #include "base/test/gtest_util.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/trace_event/trace_event.h"
@@ -237,6 +242,8 @@ class StateMachine : public SchedulerStateMachine {
     return needs_impl_side_invalidation_;
   }
 
+  void AdvanceTimeBy(base::TimeDelta delta) { now_ticks_ += delta; }
+
   using SchedulerStateMachine::ProactiveBeginFrameWanted;
   using SchedulerStateMachine::ShouldDraw;
   using SchedulerStateMachine::ShouldPrepareTiles;
@@ -245,9 +252,13 @@ class StateMachine : public SchedulerStateMachine {
   using SchedulerStateMachine::ShouldWaitForScrollEvent;
   using SchedulerStateMachine::WillCommit;
 
+ private:
+  base::TimeTicks Now() const override { return now_ticks_; }
+
  protected:
   DrawResult draw_result_for_test_;
   uint64_t next_begin_frame_number_ = viz::BeginFrameArgs::kStartingFrameNumber;
+  base::TimeTicks now_ticks_;
 };
 
 void PerformAction(StateMachine* sm, SchedulerStateMachine::Action action) {
@@ -1148,6 +1159,61 @@ TEST(SchedulerStateMachineTest, TestFullCycle) {
   EXPECT_FALSE(state.needs_redraw());
 }
 
+TEST(SchedulerStateMachineTest, TestMainFrameThrottling) {
+  SchedulerSettings default_scheduler_settings;
+  StateMachine state(default_scheduler_settings);
+  SET_UP_STATE(state);
+
+  state.SetThrottleMainFrames(base::Hertz(60));
+  state.AdvanceTimeBy(base::Seconds(1280));  // Start at an arbitrary point.
+
+  int begin_main_frame_count = 0;
+  for (int i = 0; i < 10; i++) {
+    state.SetNeedsBeginMainFrame();
+    state.IssueNextBeginImplFrame();
+
+    // If we send a BeginMainFrame(), simulate the fast path, where main is fast
+    // enough to catch the next deadline.
+    if (state.ShouldSendBeginMainFrame()) {
+      begin_main_frame_count += 1;
+      EXPECT_ACTION_UPDATE_STATE(
+          SchedulerStateMachine::Action::SEND_BEGIN_MAIN_FRAME);
+      EXPECT_MAIN_FRAME_STATE(SchedulerStateMachine::BeginMainFrameState::SENT);
+      EXPECT_FALSE(state.NeedsCommit());
+      EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::Action::NONE);
+      state.NotifyReadyToCommit();
+      EXPECT_MAIN_FRAME_STATE(
+          SchedulerStateMachine::BeginMainFrameState::READY_TO_COMMIT);
+      EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::Action::COMMIT);
+      EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::Action::POST_COMMIT);
+      state.NotifyReadyToActivate();
+      EXPECT_ACTION_UPDATE_STATE(
+          SchedulerStateMachine::Action::ACTIVATE_SYNC_TREE);
+      EXPECT_TRUE(state.active_tree_needs_first_draw());
+      EXPECT_TRUE(state.needs_redraw());
+    } else {
+      // Still need to require a draw, otherwise nothing will happen below.
+      state.SetNeedsRedraw(true);
+    }
+
+    // Expect to do nothing until BeginImplFrame deadline
+    EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::Action::NONE);
+    state.OnBeginImplFrameDeadline();
+    EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::Action::DRAW_IF_POSSIBLE);
+    state.DidSubmitCompositorFrame();
+    state.DidReceiveCompositorFrameAck();
+
+    // Should be synchronized, no draw needed, no action needed.
+    EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::Action::NONE);
+    EXPECT_MAIN_FRAME_STATE(SchedulerStateMachine::BeginMainFrameState::IDLE);
+    EXPECT_FALSE(state.needs_redraw());
+
+    state.AdvanceTimeBy(base::Hertz(120));
+  }
+
+  EXPECT_EQ(begin_main_frame_count, 5);
+}
+
 TEST(SchedulerStateMachineTest, CommitWithoutDrawWithPendingTree) {
   SchedulerSettings default_scheduler_settings;
   StateMachine state(default_scheduler_settings);

cc/scheduler/scheduler_unittest.cc

@@ -1562,6 +1562,76 @@ TEST_F(SchedulerTest, FrameIntervalUpdated) {
   EXPECT_EQ(client_->frame_interval(), interval);
 }
 
+TEST_F(SchedulerTest, BeginMainFrameThrottling) {
+  // Verify that the SchedulerClient gets updates when the begin frame interval
+  // changes.
+  SetUpScheduler(EXTERNAL_BFS);
+  constexpr uint64_t kSourceId = viz::BeginFrameArgs::kStartingSourceId;
+  uint64_t sequence_number = viz::BeginFrameArgs::kStartingFrameNumber;
+
+  // No throttling at 60Hz.
+  base::TimeDelta interval = base::Hertz(60);
+  scheduler_->SetNeedsRedraw();
+  task_runner_->AdvanceMockTickClock(interval);
+  viz::BeginFrameArgs args = viz::BeginFrameArgs::Create(
+      BEGINFRAME_FROM_HERE, kSourceId, sequence_number++,
+      task_runner_->NowTicks(), task_runner_->NowTicks() + interval, interval,
+      viz::BeginFrameArgs::NORMAL);
+  fake_external_begin_frame_source_->TestOnBeginFrame(args);
+  EXPECT_EQ(client_->frame_interval(), interval);
+  EXPECT_TRUE(
+      scheduler_->state_machine().main_frame_throttled_interval().is_zero());
+
+  {
+    base::test::ScopedFeatureList feature_list;
+    feature_list.InitAndDisableFeature(features::kThrottleMainFrameTo60Hz);
+
+    // No throttling when the feature is disabled.
+    interval = base::Hertz(240);
+    scheduler_->SetNeedsRedraw();
+    task_runner_->AdvanceMockTickClock(interval);
+    args = viz::BeginFrameArgs::Create(
+        BEGINFRAME_FROM_HERE, kSourceId, sequence_number++,
+        task_runner_->NowTicks(), task_runner_->NowTicks() + interval, interval,
+        viz::BeginFrameArgs::NORMAL);
+    fake_external_begin_frame_source_->TestOnBeginFrame(args);
+    EXPECT_EQ(client_->frame_interval(), interval);
+    EXPECT_TRUE(
+        scheduler_->state_machine().main_frame_throttled_interval().is_zero());
+  }
+
+  // Enable the feature for the rest of the test.
+  base::test::ScopedFeatureList feature_list{
+      features::kThrottleMainFrameTo60Hz};
+
+  // Throttling at 120fps.
+  interval = base::Hertz(120);
+  scheduler_->SetNeedsRedraw();
+  task_runner_->AdvanceMockTickClock(interval);
+  args = viz::BeginFrameArgs::Create(
+      BEGINFRAME_FROM_HERE, kSourceId, sequence_number++,
+      task_runner_->NowTicks(), task_runner_->NowTicks() + interval, interval,
+      viz::BeginFrameArgs::NORMAL);
+  fake_external_begin_frame_source_->TestOnBeginFrame(args);
+  EXPECT_EQ(client_->frame_interval(), interval);
+  constexpr float kSlackFactor = .9;
+  EXPECT_EQ(scheduler_->state_machine().main_frame_throttled_interval(),
+            base::Hertz(60) * kSlackFactor);
+
+  // Not at 90Hz.
+  interval = base::Hertz(90);
+  scheduler_->SetNeedsRedraw();
+  task_runner_->AdvanceMockTickClock(interval);
+  args = viz::BeginFrameArgs::Create(
+      BEGINFRAME_FROM_HERE, kSourceId, sequence_number++,
+      task_runner_->NowTicks(), task_runner_->NowTicks() + interval, interval,
+      viz::BeginFrameArgs::NORMAL);
+  fake_external_begin_frame_source_->TestOnBeginFrame(args);
+  EXPECT_EQ(client_->frame_interval(), interval);
+  EXPECT_TRUE(
+      scheduler_->state_machine().main_frame_throttled_interval().is_zero());
+}
+
 TEST_F(SchedulerTest, MainFrameNotSkippedAfterLateCommit) {
   SetUpScheduler(EXTERNAL_BFS);
   fake_compositor_timing_history_->SetAllEstimatesTo(kFastDuration);