cc: Allow sending BeginMainFrame before draw or activation

cc/scheduler/scheduler_state_machine_unittest.cc

 // Copyright 2011 The Chromium Authors. All rights reserved.
 // 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 "cc/scheduler/scheduler.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 #define EXPECT_ACTION_UPDATE_STATE(action)                                   \
   EXPECT_EQ(action, state.NextAction()) << *state.AsValue();                 \
   if (action == SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE ||   \
       action == SchedulerStateMachine::ACTION_DRAW_AND_SWAP_FORCED) {        \
-    if (SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW ==        \
-            state.CommitState() &&                                           \
-        SchedulerStateMachine::OUTPUT_SURFACE_ACTIVE !=                      \
-            state.output_surface_state())                                    \
-      return;                                                                \
     EXPECT_EQ(SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_INSIDE_DEADLINE, \
               state.begin_impl_frame_state())                                \
         << *state.AsValue();                                                 \
   }                                                                          \
   state.UpdateState(action);                                                 \
   if (action == SchedulerStateMachine::ACTION_NONE) {                        \
     if (state.begin_impl_frame_state() ==                                    \
         SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_BEGIN_FRAME_STARTING)  \
       state.OnBeginImplFrameDeadlinePending();                               \
     if (state.begin_impl_frame_state() ==                                    \
         SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_INSIDE_DEADLINE)       \
       state.OnBeginImplFrameIdle();                                          \
   }
 
 namespace cc {
 
 namespace {
 
 const SchedulerStateMachine::BeginImplFrameState all_begin_impl_frame_states[] =
     {SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_IDLE,
      SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_BEGIN_FRAME_STARTING,
      SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_INSIDE_BEGIN_FRAME,
      SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_INSIDE_DEADLINE, };
 
 const SchedulerStateMachine::CommitState all_commit_states[] = {
     SchedulerStateMachine::COMMIT_STATE_IDLE,
     SchedulerStateMachine::COMMIT_STATE_FRAME_IN_PROGRESS,
-    SchedulerStateMachine::COMMIT_STATE_READY_TO_COMMIT,
-    SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW, };
+    SchedulerStateMachine::COMMIT_STATE_READY_TO_COMMIT, };
 
 // Exposes the protected state fields of the SchedulerStateMachine for testing
 class StateMachine : public SchedulerStateMachine {
  public:
   explicit StateMachine(const SchedulerSettings& scheduler_settings)
       : SchedulerStateMachine(scheduler_settings) {}
 
   void CreateAndInitializeOutputSurfaceWithActivatedCommit() {
     DidCreateAndInitializeOutputSurface();
     output_surface_state_ = OUTPUT_SURFACE_ACTIVE;
@@ skipping 19 matching lines @@
   }
 
   SynchronousReadbackState readback_state() const { return readback_state_; }
 
   bool NeedsCommit() const { return needs_commit_; }
 
   void SetNeedsRedraw(bool b) { needs_redraw_ = b; }
 
   void SetNeedsForcedRedrawForTimeout(bool b) {
     forced_redraw_state_ = FORCED_REDRAW_STATE_WAITING_FOR_COMMIT;
-    commit_state_ = COMMIT_STATE_WAITING_FOR_FIRST_DRAW;
+    active_tree_needs_first_draw_ = true;
   }
   bool NeedsForcedRedrawForTimeout() const {
     return forced_redraw_state_ != FORCED_REDRAW_STATE_IDLE;
   }
 
   void SetNeedsForcedRedrawForReadback() {
     readback_state_ = READBACK_STATE_WAITING_FOR_DRAW_AND_READBACK;
-    commit_state_ = COMMIT_STATE_WAITING_FOR_FIRST_DRAW;
+    active_tree_needs_first_draw_ = true;
   }
 
   bool NeedsForcedRedrawForReadback() const {
     return readback_state_ != READBACK_STATE_IDLE;
   }
 
   void SetActiveTreeNeedsFirstDraw(bool needs_first_draw) {
     active_tree_needs_first_draw_ = needs_first_draw;
   }
 
@@ skipping 71 matching lines @@
     state.UpdateState(state.NextAction());
     state.CreateAndInitializeOutputSurfaceWithActivatedCommit();
     state.SetVisible(true);
     state.UpdateState(SchedulerStateMachine::ACTION_SEND_BEGIN_MAIN_FRAME);
     EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_FRAME_IN_PROGRESS,
               state.CommitState());
     EXPECT_FALSE(state.NeedsCommit());
   }
 }
 
+// start_commit_before_draw_enabled = false
+// This is enabled by default.

[danakj, 2014/03/05 19:52:43]

Can you reword this, I'm not sure if it's saying
start_commit_before_draw_enabled is "enabled" by default or is equal to false by
default. This will get out of date anyways really when I think about it..

+TEST(SchedulerStateMachineTest, StartCommitBeforeDrawDisabled) {

[brianderson, 2014/03/05 00:30:48]

I went ahead and added this test to make sure we explicitly test *not* starting
the commit before the draw.

+  SchedulerSettings scheduler_settings;
+  scheduler_settings.impl_side_painting = true;
+  scheduler_settings.start_commit_before_draw_enabled = false;
+  StateMachine state(scheduler_settings);
+  state.SetCommitState(SchedulerStateMachine::COMMIT_STATE_IDLE);
+  state.SetCanStart();
+  state.UpdateState(state.NextAction());
+  state.CreateAndInitializeOutputSurfaceWithActivatedCommit();
+  state.SetNeedsRedraw(false);
+  state.SetVisible(true);
+  state.SetCanDraw(true);
+  state.SetNeedsCommit();
+
+  EXPECT_TRUE(state.BeginImplFrameNeeded());
+
+  // Commit to the pending tree.
+  state.OnBeginImplFrame(BeginFrameArgs::CreateForTesting());
+  EXPECT_ACTION_UPDATE_STATE(
+      SchedulerStateMachine::ACTION_SEND_BEGIN_MAIN_FRAME);
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
+  state.FinishCommit();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
+  EXPECT_EQ(state.CommitState(),
+            SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW);
+
+  state.OnBeginImplFrameDeadline();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
+  EXPECT_EQ(state.CommitState(),
+            SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW);
+
+  // Verify that the next commit doesn't start until the previous
+  // commit has been drawn.
+  state.SetNeedsCommit();
+  state.OnBeginImplFrame(BeginFrameArgs::CreateForTesting());
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
+
+  // Verify NotifyReadyToActivate unblocks activation, draw, and
+  // commit in that order.
+  state.NotifyReadyToActivate();
+  EXPECT_ACTION_UPDATE_STATE(
+      SchedulerStateMachine::ACTION_ACTIVATE_PENDING_TREE);
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
+  EXPECT_EQ(state.CommitState(),
+            SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW);
+
+  EXPECT_TRUE(state.ShouldTriggerBeginImplFrameDeadlineEarly());
+  state.OnBeginImplFrameDeadline();
+  EXPECT_ACTION_UPDATE_STATE(
+      SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE);
+  EXPECT_EQ(state.CommitState(), SchedulerStateMachine::COMMIT_STATE_IDLE);
+  EXPECT_ACTION_UPDATE_STATE(
+      SchedulerStateMachine::ACTION_SEND_BEGIN_MAIN_FRAME);
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
+  EXPECT_EQ(state.CommitState(),
+            SchedulerStateMachine::COMMIT_STATE_FRAME_IN_PROGRESS);
+
+  state.FinishCommit();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
+  EXPECT_EQ(state.CommitState(),
+            SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW);
+}
+
+// start_commit_before_activate_enabled = true
+// This is disabled by default.
+TEST(SchedulerStateMachineTest, StartCommitBeforeActivateEnabled) {
+  SchedulerSettings scheduler_settings;
+  scheduler_settings.impl_side_painting = true;
+  scheduler_settings.start_commit_before_activate_enabled = true;
+  StateMachine state(scheduler_settings);
+  state.SetCommitState(SchedulerStateMachine::COMMIT_STATE_IDLE);
+  state.SetCanStart();
+  state.UpdateState(state.NextAction());
+  state.CreateAndInitializeOutputSurfaceWithActivatedCommit();
+  state.SetNeedsRedraw(false);
+  state.SetVisible(true);
+  state.SetCanDraw(true);
+  state.SetNeedsCommit();
+
+  EXPECT_TRUE(state.BeginImplFrameNeeded());
+
+  // Commit to the pending tree.
+  state.OnBeginImplFrame(BeginFrameArgs::CreateForTesting());
+  EXPECT_ACTION_UPDATE_STATE(
+      SchedulerStateMachine::ACTION_SEND_BEGIN_MAIN_FRAME);
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
+  state.FinishCommit();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
+  EXPECT_EQ(state.CommitState(), SchedulerStateMachine::COMMIT_STATE_IDLE);
+
+  state.OnBeginImplFrameDeadline();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
+
+  // Verify that the next commit starts while there is still a pending tree.
+  state.SetNeedsCommit();
+  state.OnBeginImplFrame(BeginFrameArgs::CreateForTesting());
+  EXPECT_ACTION_UPDATE_STATE(
+      SchedulerStateMachine::ACTION_SEND_BEGIN_MAIN_FRAME);
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
+
+  // Verify the pending commit doesn't overwrite the pending
+  // tree until the pending tree has been activated.
+  state.FinishCommit();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
+
+  // Verify NotifyReadyToActivate unblocks activation, draw, and
+  // commit in that order.
+  state.NotifyReadyToActivate();
+  EXPECT_ACTION_UPDATE_STATE(
+      SchedulerStateMachine::ACTION_ACTIVATE_PENDING_TREE);
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
+
+  EXPECT_TRUE(state.ShouldTriggerBeginImplFrameDeadlineEarly());
+  state.OnBeginImplFrameDeadline();
+  EXPECT_ACTION_UPDATE_STATE(
+      SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE);
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
+  EXPECT_EQ(state.CommitState(), SchedulerStateMachine::COMMIT_STATE_IDLE);
+}
+
 TEST(SchedulerStateMachineTest,
      TestFailedDrawForAnimationCheckerboardSetsNeedsCommitAndDoesNotDrawAgain) {
   SchedulerSettings default_scheduler_settings;
   StateMachine state(default_scheduler_settings);
   state.SetCanStart();
   state.UpdateState(state.NextAction());
   state.CreateAndInitializeOutputSurfaceWithActivatedCommit();
   state.SetVisible(true);
   state.SetCanDraw(true);
   state.SetNeedsRedraw(true);
@@ skipping 83 matching lines @@
 
   // Failing the draw for animation checkerboards makes us require a commit.
   state.DidDrawIfPossibleCompleted(
       DrawSwapReadbackResult::DRAW_ABORTED_CHECKERBOARD_ANIMATIONS);
   state.OnBeginImplFrame(BeginFrameArgs::CreateForTesting());
   EXPECT_ACTION_UPDATE_STATE(
       SchedulerStateMachine::ACTION_SEND_BEGIN_MAIN_FRAME);
   EXPECT_TRUE(state.RedrawPending());
 }
 
-TEST(SchedulerStateMachineTest,
-     TestFailedDrawsWillEventuallyForceADrawAfterTheNextCommit) {
+void TestFailedDrawsEventuallyForceDrawAfterNextCommit(
+    bool start_commit_before_draw_enabled) {
   SchedulerSettings scheduler_settings;
+  scheduler_settings.start_commit_before_draw_enabled =
+      start_commit_before_draw_enabled;
   scheduler_settings.maximum_number_of_failed_draws_before_draw_is_forced_ = 1;
   StateMachine state(scheduler_settings);
   state.SetCanStart();
   state.UpdateState(state.NextAction());
   state.CreateAndInitializeOutputSurfaceWithActivatedCommit();
   state.SetVisible(true);
   state.SetCanDraw(true);
 
   // Start a commit.
   state.SetNeedsCommit();
@@ skipping 21 matching lines @@
 
   // Finish the commit. Note, we should not yet be forcing a draw, but should
   // continue the commit as usual.
   state.FinishCommit();
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   EXPECT_TRUE(state.RedrawPending());
 
   // The redraw should be forced at the end of the next BeginImplFrame.
   state.OnBeginImplFrame(BeginFrameArgs::CreateForTesting());
+  if (start_commit_before_draw_enabled) {
+    EXPECT_ACTION_UPDATE_STATE(
+        SchedulerStateMachine::ACTION_SEND_BEGIN_MAIN_FRAME);
+  }
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   state.OnBeginImplFrameDeadline();
   EXPECT_ACTION_UPDATE_STATE(
       SchedulerStateMachine::ACTION_DRAW_AND_SWAP_FORCED);
 }
 
+TEST(SchedulerStateMachineTest,
+     TestFailedDrawsEventuallyForceDrawAfterNextCommit) {
+  bool start_commit_before_draw_enabled = false;
+  TestFailedDrawsEventuallyForceDrawAfterNextCommit(
+      start_commit_before_draw_enabled);
+}
+
+TEST(SchedulerStateMachineTest,
+     TestFailedDrawsEventuallyForceDrawAfterNextCommit_CommitBeforeDraw) {
+  bool start_commit_before_draw_enabled = true;
+  TestFailedDrawsEventuallyForceDrawAfterNextCommit(
+      start_commit_before_draw_enabled);
+}
+
 TEST(SchedulerStateMachineTest, TestFailedDrawsDoNotRestartForcedDraw) {
   SchedulerSettings scheduler_settings;
   int draw_limit = 1;
   scheduler_settings.maximum_number_of_failed_draws_before_draw_is_forced_ =
       draw_limit;
   scheduler_settings.impl_side_painting = true;
   StateMachine state(scheduler_settings);
   state.SetCanStart();
   state.UpdateState(state.NextAction());
   state.CreateAndInitializeOutputSurfaceWithActivatedCommit();
@@ skipping 160 matching lines @@
                 state.NextAction());
 
       // Case 2: needs_commit=true
       state.SetNeedsCommit();
       EXPECT_NE(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE,
                 state.NextAction())
           << *state.AsValue();
     }
   }
 
-  // When in BeginImplFrame deadline we should always draw for SetNeedsRedraw or
-  // SetNeedsForcedRedrawForReadback have been called... except if we're
-  // ready to commit, in which case we expect a commit first.
+  // When in BeginImplFrame deadline we should always draw for SetNeedsRedraw
+  // except if we're ready to commit, in which case we expect a commit first.
+  // SetNeedsForcedRedrawForReadback should take precedence over all and
+  // issue a readback.
   for (size_t i = 0; i < num_commit_states; ++i) {
     for (size_t j = 0; j < 2; ++j) {
       bool request_readback = j;
 
-      // Skip invalid states
-      if (request_readback &&
-          (SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW !=
-           all_commit_states[i]))
-        continue;
-
       StateMachine state(default_scheduler_settings);
       state.SetCanStart();
       state.UpdateState(state.NextAction());
       state.CreateAndInitializeOutputSurfaceWithActivatedCommit();
       state.SetCanDraw(true);
       state.SetCommitState(all_commit_states[i]);
       state.SetBeginImplFrameState(
           SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_INSIDE_DEADLINE);
       if (request_readback) {
         state.SetNeedsForcedRedrawForReadback();
       } else {
         state.SetNeedsRedraw(true);
         state.SetVisible(true);
       }
 
       SchedulerStateMachine::Action expected_action;
-      if (all_commit_states[i] ==
-          SchedulerStateMachine::COMMIT_STATE_READY_TO_COMMIT) {
+      if (request_readback) {
+        expected_action = SchedulerStateMachine::ACTION_DRAW_AND_READBACK;
+      } else if (all_commit_states[i] ==
+                 SchedulerStateMachine::COMMIT_STATE_READY_TO_COMMIT) {
         expected_action = SchedulerStateMachine::ACTION_COMMIT;
-      } else if (request_readback) {
-        if (all_commit_states[i] ==
-            SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW)
-          expected_action = SchedulerStateMachine::ACTION_DRAW_AND_READBACK;
-        else
-          expected_action = SchedulerStateMachine::ACTION_NONE;
       } else {
         expected_action =
             SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE;
       }
 
       // Case 1: needs_commit=false.
       EXPECT_NE(state.BeginImplFrameNeeded(), request_readback)
           << *state.AsValue();
-      EXPECT_EQ(expected_action, state.NextAction()) << *state.AsValue();
+      EXPECT_EQ(state.NextAction(), expected_action) << *state.AsValue();
 
       // Case 2: needs_commit=true.
       state.SetNeedsCommit();
       EXPECT_NE(state.BeginImplFrameNeeded(), request_readback)
           << *state.AsValue();
-      EXPECT_EQ(expected_action, state.NextAction()) << *state.AsValue();
+      EXPECT_EQ(state.NextAction(), expected_action) << *state.AsValue();
     }
   }
 }
 
 TEST(SchedulerStateMachineTest, TestNoCommitStatesRedrawWhenInvisible) {
   SchedulerSettings default_scheduler_settings;
 
   size_t num_commit_states =
       sizeof(all_commit_states) / sizeof(SchedulerStateMachine::CommitState);
   for (size_t i = 0; i < num_commit_states; ++i) {
@@ skipping 50 matching lines @@
 }
 
 TEST(SchedulerStateMachineTest,
      TestCanRedrawWithWaitingForFirstDrawMakesProgress) {
   SchedulerSettings default_scheduler_settings;
   StateMachine state(default_scheduler_settings);
   state.SetCanStart();
   state.UpdateState(state.NextAction());
   state.CreateAndInitializeOutputSurfaceWithActivatedCommit();
 
-  state.SetCommitState(
-      SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW);
   state.SetActiveTreeNeedsFirstDraw(true);
   state.SetNeedsCommit();
   state.SetNeedsRedraw(true);
   state.SetVisible(true);
   state.SetCanDraw(false);
   state.OnBeginImplFrame(BeginFrameArgs::CreateForTesting());
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_ABORT);
   EXPECT_ACTION_UPDATE_STATE(
       SchedulerStateMachine::ACTION_SEND_BEGIN_MAIN_FRAME);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   state.FinishCommit();
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
   state.OnBeginImplFrameDeadline();
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_ABORT);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 }
 
-TEST(SchedulerStateMachineTest, TestsetNeedsCommitIsNotLost) {
-  SchedulerSettings default_scheduler_settings;
-  StateMachine state(default_scheduler_settings);
+void TestsetNeedsCommitIsNotLost(bool start_commit_before_draw_enabled) {

[danakj, 2014/03/05 19:52:43]

s/TestsetNeeds/TestSetNeeds/g

Looks like that s got missed in our chromifying.

+  SchedulerSettings scheduler_settings;
+  scheduler_settings.start_commit_before_draw_enabled =
+      start_commit_before_draw_enabled;
+  StateMachine state(scheduler_settings);
   state.SetCanStart();
   state.UpdateState(state.NextAction());
   state.CreateAndInitializeOutputSurfaceWithActivatedCommit();
   state.SetNeedsCommit();
   state.SetVisible(true);
   state.SetCanDraw(true);
 
   EXPECT_TRUE(state.BeginImplFrameNeeded());
 
   // Begin the frame.
@@ skipping 30 matching lines @@
   state.OnBeginImplFrameIdle();
   EXPECT_EQ(SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_IDLE,
             state.begin_impl_frame_state());
   EXPECT_EQ(SchedulerStateMachine::ACTION_COMMIT, state.NextAction());
 
   state.OnBeginImplFrame(BeginFrameArgs::CreateForTesting());
   EXPECT_EQ(SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_BEGIN_FRAME_STARTING,
             state.begin_impl_frame_state());
   EXPECT_EQ(SchedulerStateMachine::ACTION_COMMIT, state.NextAction());
 
-  // Commit and make sure we draw on next BeginImplFrame
+  // Finish the commit, then make sure we start the next commit immediately
+  // and draw on the next BeginImplFrame.
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
+  if (start_commit_before_draw_enabled) {
+    EXPECT_ACTION_UPDATE_STATE(
+        SchedulerStateMachine::ACTION_SEND_BEGIN_MAIN_FRAME);
+  }
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
+
   state.OnBeginImplFrameDeadline();
-  EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW,
-            state.CommitState());
+
+  EXPECT_TRUE(state.active_tree_needs_first_draw());
   EXPECT_ACTION_UPDATE_STATE(
       SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE);
   state.DidDrawIfPossibleCompleted(DrawSwapReadbackResult::DRAW_SUCCESS);
+  if (!start_commit_before_draw_enabled) {
+    EXPECT_ACTION_UPDATE_STATE(
+        SchedulerStateMachine::ACTION_SEND_BEGIN_MAIN_FRAME);
+  }
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
+}
 
-  // Verify that another commit will start immediately after draw.
-  EXPECT_ACTION_UPDATE_STATE(
-      SchedulerStateMachine::ACTION_SEND_BEGIN_MAIN_FRAME);
-  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
+TEST(SchedulerStateMachineTest, TestsetNeedsCommitIsNotLost) {
+  bool start_commit_before_draw_enabled = false;
+  TestsetNeedsCommitIsNotLost(start_commit_before_draw_enabled);
+}
+
+TEST(SchedulerStateMachineTest, TestsetNeedsCommitIsNotLost_CommitBeforeDraw) {
+  bool start_commit_before_draw_enabled = true;
+  TestsetNeedsCommitIsNotLost(start_commit_before_draw_enabled);
 }
 
 TEST(SchedulerStateMachineTest, TestFullCycle) {
   SchedulerSettings default_scheduler_settings;
   StateMachine state(default_scheduler_settings);
   state.SetCanStart();
   state.UpdateState(state.NextAction());
   state.CreateAndInitializeOutputSurfaceWithActivatedCommit();
   state.SetVisible(true);
   state.SetCanDraw(true);
@@ skipping 10 matching lines @@
   EXPECT_FALSE(state.NeedsCommit());
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 
   // Tell the scheduler the frame finished.
   state.FinishCommit();
   EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_READY_TO_COMMIT,
             state.CommitState());
 
   // Commit.
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
-  EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW,
-            state.CommitState());
+  EXPECT_TRUE(state.active_tree_needs_first_draw());
   EXPECT_TRUE(state.needs_redraw());
 
   // Expect to do nothing until BeginImplFrame deadline
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 
   // At BeginImplFrame deadline, draw.
   state.OnBeginImplFrameDeadline();
   EXPECT_ACTION_UPDATE_STATE(
       SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE);
   state.DidDrawIfPossibleCompleted(DrawSwapReadbackResult::DRAW_SUCCESS);
@@ skipping 29 matching lines @@
   state.SetNeedsCommit();
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 
   // Tell the scheduler the frame finished.
   state.FinishCommit();
   EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_READY_TO_COMMIT,
             state.CommitState());
 
   // First commit.
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
-  EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW,
-            state.CommitState());
+  EXPECT_TRUE(state.active_tree_needs_first_draw());
   EXPECT_TRUE(state.needs_redraw());
 
   // Expect to do nothing until BeginImplFrame deadline.
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 
   // At BeginImplFrame deadline, draw.
   state.OnBeginImplFrameDeadline();
   EXPECT_ACTION_UPDATE_STATE(
       SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE);
   state.DidDrawIfPossibleCompleted(DrawSwapReadbackResult::DRAW_SUCCESS);
@@ skipping 266 matching lines @@
   // Ask for another draw. Expect nothing happens.
   state.SetNeedsRedraw(true);
   EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
 
   // Finish the frame, and commit.
   state.FinishCommit();
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
 
   // We will abort the draw when the output surface is lost if we are
   // waiting for the first draw to unblock the main thread.
-  EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW,
-            state.CommitState());
+  EXPECT_TRUE(state.active_tree_needs_first_draw());
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_ABORT);
 
   // Expect to begin context recreation only in BEGIN_IMPL_FRAME_STATE_IDLE
   EXPECT_EQ(SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_IDLE,
             state.begin_impl_frame_state());
   EXPECT_EQ(SchedulerStateMachine::ACTION_BEGIN_OUTPUT_SURFACE_CREATION,
             state.NextAction());
 
   state.OnBeginImplFrame(BeginFrameArgs::CreateForTesting());
   EXPECT_EQ(SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_BEGIN_FRAME_STARTING,
@@ skipping 40 matching lines @@
   state.DidLoseOutputSurface();
 
   // Ask for another draw and also set needs commit. Expect nothing happens.
   state.SetNeedsRedraw(true);
   state.SetNeedsCommit();
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 
   // Finish the frame, and commit.
   state.FinishCommit();
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
-  EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW,
-            state.CommitState());
+  EXPECT_TRUE(state.active_tree_needs_first_draw());
 
   // Because the output surface is missing, we expect the draw to abort.
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_ABORT);
 
   // Expect to begin context recreation only in BEGIN_IMPL_FRAME_STATE_IDLE
   EXPECT_EQ(SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_IDLE,
             state.begin_impl_frame_state());
   EXPECT_EQ(SchedulerStateMachine::ACTION_BEGIN_OUTPUT_SURFACE_CREATION,
             state.NextAction());
 
@@ skipping 37 matching lines @@
   state.SetCanStart();
   state.UpdateState(state.NextAction());
   state.CreateAndInitializeOutputSurfaceWithActivatedCommit();
   state.SetVisible(true);
   state.SetCanDraw(true);
 
   // Cause a lost context lost.
   state.DidLoseOutputSurface();
 
   // Ask a forced redraw for readback and verify it ocurrs.
-  state.SetCommitState(
-      SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW);
   state.SetNeedsForcedRedrawForReadback();
   state.OnBeginImplFrame(BeginFrameArgs::CreateForTesting());
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DRAW_AND_READBACK);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 
   // Forced redraws for readbacks need to be followed by a new commit
   // to replace the readback commit.
   EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_FRAME_IN_PROGRESS,
             state.CommitState());
   state.FinishCommit();
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
 
   // We don't yet have an output surface, so we the draw and swap should abort.
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_ABORT);
 
   // Expect to begin context recreation only in BEGIN_IMPL_FRAME_STATE_IDLE
   EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
 
   state.OnBeginImplFrameDeadline();
   EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
 
   state.OnBeginImplFrameIdle();
   EXPECT_ACTION_UPDATE_STATE(
       SchedulerStateMachine::ACTION_BEGIN_OUTPUT_SURFACE_CREATION);
 
   // Ask a readback and verify it occurs.
-  state.SetCommitState(
-      SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW);
   state.SetNeedsForcedRedrawForReadback();
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DRAW_AND_READBACK);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 }
 
 TEST(SchedulerStateMachineTest, DontDrawBeforeCommitAfterLostOutputSurface) {
   SchedulerSettings default_scheduler_settings;
   StateMachine state(default_scheduler_settings);
   state.SetCanStart();
   state.UpdateState(state.NextAction());
@@ skipping 88 matching lines @@
   state.UpdateState(state.NextAction());
   state.CreateAndInitializeOutputSurfaceWithActivatedCommit();
   state.SetVisible(false);
   state.SetCommitState(SchedulerStateMachine::COMMIT_STATE_FRAME_IN_PROGRESS);
   state.SetNeedsCommit();
 
   state.FinishCommit();
   EXPECT_EQ(SchedulerStateMachine::ACTION_COMMIT, state.NextAction());
   state.UpdateState(state.NextAction());
 
-  EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW,
-            state.CommitState());
+  EXPECT_TRUE(state.active_tree_needs_first_draw());
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_ABORT);
 }
 
 TEST(SchedulerStateMachineTest, TestFinishCommitWhenForcedCommitInProgress) {
   SchedulerSettings default_scheduler_settings;
   StateMachine state(default_scheduler_settings);
   state.SetCanStart();
   state.UpdateState(state.NextAction());
   state.CreateAndInitializeOutputSurfaceWithActivatedCommit();
   state.SetVisible(false);
   state.SetCommitState(SchedulerStateMachine::COMMIT_STATE_FRAME_IN_PROGRESS);
   state.SetNeedsForcedCommitForReadback();
 
   // The commit for readback interupts the normal commit.
   state.FinishCommit();
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
 
-  EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW,
-            state.CommitState());
+  EXPECT_TRUE(state.active_tree_needs_first_draw());
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DRAW_AND_READBACK);
 
   // When the readback interrupts the normal commit, we should not get
   // another BeginMainFrame when the readback completes.
   EXPECT_NE(SchedulerStateMachine::ACTION_SEND_BEGIN_MAIN_FRAME,
             state.NextAction());
 
   // The normal commit can then proceed.
   state.FinishCommit();
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
@@ skipping 47 matching lines @@
   // Schedule a readback, commit it, draw it.
   state.SetNeedsForcedCommitForReadback();
   EXPECT_ACTION_UPDATE_STATE(
       SchedulerStateMachine::ACTION_SEND_BEGIN_MAIN_FRAME);
   state.FinishCommit();
 
   EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_READY_TO_COMMIT,
             state.CommitState());
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
 
-  EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW,
-            state.CommitState());
+  EXPECT_TRUE(state.active_tree_needs_first_draw());
 
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DRAW_AND_READBACK);
   state.DidDrawIfPossibleCompleted(DrawSwapReadbackResult::DRAW_SUCCESS);
 
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 
   // Should be waiting for the normal BeginMainFrame.
   EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_FRAME_IN_PROGRESS,
             state.CommitState());
 }
@@ skipping 14 matching lines @@
   // Schedule a readback, commit it, draw it.
   state.SetNeedsForcedCommitForReadback();
   EXPECT_ACTION_UPDATE_STATE(
       SchedulerStateMachine::ACTION_SEND_BEGIN_MAIN_FRAME);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   state.FinishCommit();
   EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_READY_TO_COMMIT,
             state.CommitState());
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
 
-  EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW,
-            state.CommitState());
+  EXPECT_TRUE(state.active_tree_needs_first_draw());
 
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DRAW_AND_READBACK);
   state.DidDrawIfPossibleCompleted(DrawSwapReadbackResult::DRAW_SUCCESS);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 
   // Should be waiting for the normal BeginMainFrame.
   EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_FRAME_IN_PROGRESS,
             state.CommitState())
       << *state.AsValue();
 }
@@ skipping 12 matching lines @@
 
   state.SetNeedsForcedCommitForReadback();
   EXPECT_ACTION_UPDATE_STATE(
       SchedulerStateMachine::ACTION_SEND_BEGIN_MAIN_FRAME);
   state.FinishCommit();
 
   EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_READY_TO_COMMIT,
             state.CommitState());
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
 
-  EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW,
-            state.CommitState());
+  EXPECT_TRUE(state.active_tree_needs_first_draw());
 
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DRAW_AND_READBACK);
   state.DidDrawIfPossibleCompleted(DrawSwapReadbackResult::DRAW_SUCCESS);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 
   // Should be waiting for BeginMainFrame.
   EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_FRAME_IN_PROGRESS,
             state.CommitState())
       << *state.AsValue();
 
@@ skipping 19 matching lines @@
   state.UpdateState(state.NextAction());
 
   state.SetNeedsForcedCommitForReadback();
   state.UpdateState(state.NextAction());
   state.FinishCommit();
 
   EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_READY_TO_COMMIT,
             state.CommitState());
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
 
-  EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW,
-            state.CommitState());
+  EXPECT_TRUE(state.active_tree_needs_first_draw());
 
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DRAW_AND_READBACK);
   state.DidDrawIfPossibleCompleted(DrawSwapReadbackResult::DRAW_SUCCESS);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 }
 
 TEST(SchedulerStateMachineTest, ReportIfNotDrawing) {
   SchedulerSettings default_scheduler_settings;
   StateMachine state(default_scheduler_settings);
   state.SetCanStart();
@@ skipping 140 matching lines @@
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 
   // The deadline is not triggered early until we enter prefer smoothness mode.
   EXPECT_FALSE(state.ShouldTriggerBeginImplFrameDeadlineEarly());
   state.SetSmoothnessTakesPriority(true);
   EXPECT_TRUE(state.ShouldTriggerBeginImplFrameDeadlineEarly());
 }
 
 }  // namespace
 }  // namespace cc