Add PostCommitSteps scheduler action

The commit step releases the main thread before calling
LTHI::CommitComplete(), but it will not have an opportunity to issue a
BeginMainFrame until LTHI::CommitComplete() finishes and the scheduler
loop runs again. This may be sub-optimal, because it denies the main
thread the opportunity to get the earliest possible start on the next
main frame.

This CL splits the commit step into commit and post-commit, to give
the scheduler an opportunity to send BeginMainFrame before running
LTHI::CommitComplete().

Change-Id: I3623ada45ccee509266b4549c689ff65655bc37d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3558985
Reviewed-by: Vladimir Levin <vmpstr@chromium.org>
Reviewed-by: Khushal Sagar <khushalsagar@chromium.org>
Commit-Queue: Stefan Zager <szager@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1001328}

cc/scheduler/scheduler.cc

@@ -217,11 +217,6 @@ void Scheduler::NotifyReadyToCommit(
   ProcessScheduledActions();
 }
 
-void Scheduler::DidCommit() {
-  compositor_timing_history_->DidCommit();
-  compositor_frame_reporting_controller_->DidCommit();
-}
-
 void Scheduler::BeginMainFrameAborted(CommitEarlyOutReason reason) {
   TRACE_EVENT1("cc", "Scheduler::BeginMainFrameAborted", "reason",
                CommitEarlyOutReasonToString(reason));
@@ -885,15 +880,20 @@ void Scheduler::ProcessScheduledActions() {
         state_machine_.WillNotifyBeginMainFrameNotExpectedSoon();
         BeginMainFrameNotExpectedSoon();
         break;
-      case SchedulerStateMachine::Action::COMMIT: {
-        bool commit_has_no_updates = false;
-        state_machine_.WillCommit(commit_has_no_updates);
+      case SchedulerStateMachine::Action::COMMIT:
+        state_machine_.WillCommit(/*commit_had_no_updates=*/false);
         compositor_timing_history_->WillCommit();
         compositor_frame_reporting_controller_->WillCommit();
         client_->ScheduledActionCommit();
+        compositor_timing_history_->DidCommit();
+        compositor_frame_reporting_controller_->DidCommit();
+        state_machine_.DidCommit();
         last_commit_origin_frame_args_ = last_dispatched_begin_main_frame_args_;
         break;
-      }
+      case SchedulerStateMachine::Action::POST_COMMIT:
+        client_->ScheduledActionPostCommit();
+        state_machine_.DidPostCommit();
+        break;
       case SchedulerStateMachine::Action::ACTIVATE_SYNC_TREE:
         compositor_timing_history_->WillActivate();
         compositor_frame_reporting_controller_->WillActivate();

cc/scheduler/scheduler.h

@@ -67,6 +67,7 @@ class SchedulerClient {
   // compositor thread, which allows Compositor thread to update its layer tree
   // to match the state of the layer tree on the main thread.
   virtual void ScheduledActionCommit() = 0;
+  virtual void ScheduledActionPostCommit() = 0;
   virtual void ScheduledActionActivateSyncTree() = 0;
   virtual void ScheduledActionBeginLayerTreeFrameSinkCreation() = 0;
   virtual void ScheduledActionPrepareTiles() = 0;
@@ -196,7 +197,6 @@ class CC_EXPORT Scheduler : public viz::BeginFrameObserverBase {
   // main thread updates are completed to signal it is ready for the commmit.
   void NotifyReadyToCommit(std::unique_ptr<BeginMainFrameMetrics> details);
   void BeginMainFrameAborted(CommitEarlyOutReason reason);
-  void DidCommit();
 
   // In the PrepareTiles step, compositor thread divides the layers into tiles
   // to reduce cost of raster large layers. Then, each tile is rastered by a

cc/scheduler/scheduler_state_machine.cc

@@ -167,6 +167,8 @@ SchedulerStateMachine::ActionToProtozeroEnum(Action action) {
     case Action::SEND_BEGIN_MAIN_FRAME:
       return pbzeroSchedulerAction::CC_SCHEDULER_ACTION_SEND_BEGIN_MAIN_FRAME;
     case Action::COMMIT:
+    case Action::POST_COMMIT:
+      // TODO(szager): Add CC_SCHEDULER_ACTION_POST_COMMIT to perfetto
       return pbzeroSchedulerAction::CC_SCHEDULER_ACTION_COMMIT;
     case Action::ACTIVATE_SYNC_TREE:
       return pbzeroSchedulerAction::CC_SCHEDULER_ACTION_ACTIVATE_SYNC_TREE;
@@ -621,6 +623,20 @@ bool SchedulerStateMachine::ShouldCommit() const {
   return true;
 }
 
+void SchedulerStateMachine::DidCommit() {
+  DCHECK(!needs_post_commit_);
+  needs_post_commit_ = true;
+}
+
+bool SchedulerStateMachine::ShouldRunPostCommit() const {
+  return needs_post_commit_;
+}
+
+void SchedulerStateMachine::DidPostCommit() {
+  DCHECK(needs_post_commit_);
+  needs_post_commit_ = false;
+}
+
 bool SchedulerStateMachine::ShouldPrepareTiles() const {
   // In full-pipeline mode, we need to prepare tiles ASAP to ensure that we
   // don't get stuck.
@@ -664,6 +680,8 @@ bool SchedulerStateMachine::ShouldInvalidateLayerTreeFrameSink() const {
 SchedulerStateMachine::Action SchedulerStateMachine::NextAction() const {
   if (ShouldSendBeginMainFrame())
     return Action::SEND_BEGIN_MAIN_FRAME;
+  if (ShouldRunPostCommit())
+    return Action::POST_COMMIT;
   if (ShouldActivateSyncTree())
     return Action::ACTIVATE_SYNC_TREE;
   if (ShouldCommit())
@@ -1436,8 +1454,7 @@ void SchedulerStateMachine::BeginMainFrameAborted(CommitEarlyOutReason reason) {
       SetNeedsBeginMainFrame();
       return;
     case CommitEarlyOutReason::FINISHED_NO_UPDATES:
-      bool commit_has_no_updates = true;
-      WillCommit(commit_has_no_updates);
+      WillCommit(/*commit_had_no_updates=*/true);
       return;
   }
 }

cc/scheduler/scheduler_state_machine.h

@@ -136,6 +136,7 @@ class CC_EXPORT SchedulerStateMachine {
     NONE,
     SEND_BEGIN_MAIN_FRAME,
     COMMIT,
+    POST_COMMIT,
     ACTIVATE_SYNC_TREE,
     PERFORM_IMPL_SIDE_INVALIDATION,
     DRAW_IF_POSSIBLE,
@@ -158,6 +159,8 @@ class CC_EXPORT SchedulerStateMachine {
   void WillNotifyBeginMainFrameNotExpectedUntil();
   void WillNotifyBeginMainFrameNotExpectedSoon();
   void WillCommit(bool commit_had_no_updates);
+  void DidCommit();
+  void DidPostCommit();
   void WillActivate();
   void WillDraw();
   void WillBeginLayerTreeFrameSinkCreation();
@@ -381,6 +384,7 @@ class CC_EXPORT SchedulerStateMachine {
   bool ShouldActivateSyncTree() const;
   bool ShouldSendBeginMainFrame() const;
   bool ShouldCommit() const;
+  bool ShouldRunPostCommit() const;
   bool ShouldPrepareTiles() const;
   bool ShouldInvalidateLayerTreeFrameSink() const;
   bool ShouldNotifyBeginMainFrameNotExpectedUntil() const;
@@ -441,6 +445,7 @@ class CC_EXPORT SchedulerStateMachine {
   bool needs_prepare_tiles_ = false;
   bool needs_begin_main_frame_ = false;
   bool needs_one_begin_impl_frame_ = false;
+  bool needs_post_commit_ = false;
   bool visible_ = false;
   bool begin_frame_source_paused_ = false;
   bool resourceless_draw_ = false;