cc: Adding a CommitEarlyOutReason enum to replace bool in BeginMainFrameAbort method.

The bool is confusing and has meant a bunch of tests have ended up not testing
what they thought they were testing. The new enum clearly delineates between
cases were we abort without handing the commit and when we abort because the
commit was an effective nop.

This also allows new early out reasons to be easily added.

Using a C++ "enum class" to make sure all BeginMainFrameAborted cases are
converted by preventing implicit conversion to bool. Allowed on;
https://chromium-cpp.appspot.com/

BUG=416749

Review URL: https://codereview.chromium.org/793243006

Cr-Commit-Position: refs/heads/master@{#308739}

6 files changed, 66 insertions, 26 deletions

diff --git a/cc/scheduler/commit_earlyout_reason.h b/cc/scheduler/commit_earlyout_reason.h
new file mode 100644
index 0000000..b45e96f
--- /dev/null
+++ b/cc/scheduler/commit_earlyout_reason.h
@@ -0,0 +1,37 @@
+// Copyright 2014 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.
+
+#ifndef CC_SCHEDULER_COMMIT_EARLYOUT_REASON_H_
+#define CC_SCHEDULER_COMMIT_EARLYOUT_REASON_H_
+
+#include "base/logging.h"
+
+namespace cc {
+
+enum class CommitEarlyOutReason {
+  ABORTED_OUTPUT_SURFACE_LOST,
+  ABORTED_NOT_VISIBLE,
+  FINISHED_NO_UPDATES,
+};
+
+inline const char* CommitEarlyOutReasonToString(CommitEarlyOutReason reason) {
+  switch (reason) {
+    case CommitEarlyOutReason::ABORTED_OUTPUT_SURFACE_LOST:
+      return "CommitEarlyOutReason::ABORTED_OUTPUT_SURFACE_LOST";
+    case CommitEarlyOutReason::ABORTED_NOT_VISIBLE:
+      return "CommitEarlyOutReason::ABORTED_NOT_VISIBLE";
+    case CommitEarlyOutReason::FINISHED_NO_UPDATES:
+      return "CommitEarlyOutReason::FINISHED_NO_UPDATES";
+  }
+  NOTREACHED();
+  return "???";
+}
+
+inline bool CommitEarlyOutHandledCommit(CommitEarlyOutReason reason) {
+  return reason == CommitEarlyOutReason::FINISHED_NO_UPDATES;
+}
+
+}  // namespace cc
+
+#endif  // CC_SCHEDULER_COMMIT_EARLYOUT_REASON_H_
diff --git a/cc/scheduler/scheduler.cc b/cc/scheduler/scheduler.cc
index 59a22de..884c618 100644
--- a/cc/scheduler/scheduler.cc
+++ b/cc/scheduler/scheduler.cc
@@ -260,9 +260,10 @@ void Scheduler::NotifyReadyToCommit() {
   ProcessScheduledActions();
 }
 
-void Scheduler::BeginMainFrameAborted(bool did_handle) {
-  TRACE_EVENT0("cc", "Scheduler::BeginMainFrameAborted");
-  state_machine_.BeginMainFrameAborted(did_handle);
+void Scheduler::BeginMainFrameAborted(CommitEarlyOutReason reason) {
+  TRACE_EVENT1("cc", "Scheduler::BeginMainFrameAborted", "reason",
+               CommitEarlyOutReasonToString(reason));
+  state_machine_.BeginMainFrameAborted(reason);
   ProcessScheduledActions();
 }
 
diff --git a/cc/scheduler/scheduler.h b/cc/scheduler/scheduler.h
index dea7c3e..133b676 100644
--- a/cc/scheduler/scheduler.h
+++ b/cc/scheduler/scheduler.h
@@ -128,7 +128,7 @@ class CC_EXPORT Scheduler : public BeginFrameObserverMixIn,
   void SetImplLatencyTakesPriority(bool impl_latency_takes_priority);
 
   void NotifyReadyToCommit();
-  void BeginMainFrameAborted(bool did_handle);
+  void BeginMainFrameAborted(CommitEarlyOutReason reason);
 
   void DidPrepareTiles();
   void DidLoseOutputSurface();
diff --git a/cc/scheduler/scheduler_state_machine.cc b/cc/scheduler/scheduler_state_machine.cc
index dc3fa36..323def6 100644
--- a/cc/scheduler/scheduler_state_machine.cc
+++ b/cc/scheduler/scheduler_state_machine.cc
@@ -550,8 +550,8 @@ void SchedulerStateMachine::UpdateState(Action action) {
       return;
 
     case ACTION_COMMIT: {
-      bool commit_was_aborted = false;
-      UpdateStateOnCommit(commit_was_aborted);
+      bool commit_has_no_updates = false;
+      UpdateStateOnCommit(commit_has_no_updates);
       return;
     }
 
@@ -586,13 +586,13 @@ void SchedulerStateMachine::UpdateState(Action action) {
   }
 }
 
-void SchedulerStateMachine::UpdateStateOnCommit(bool commit_was_aborted) {
+void SchedulerStateMachine::UpdateStateOnCommit(bool commit_has_no_updates) {
   commit_count_++;
 
-  if (!commit_was_aborted && HasAnimatedThisFrame())
+  if (!commit_has_no_updates && HasAnimatedThisFrame())
     did_commit_after_animating_ = true;
 
-  if (commit_was_aborted || settings_.main_frame_before_activation_enabled) {
+  if (commit_has_no_updates || settings_.main_frame_before_activation_enabled) {
     commit_state_ = COMMIT_STATE_IDLE;
   } else if (settings_.impl_side_painting) {
     commit_state_ = COMMIT_STATE_WAITING_FOR_ACTIVATION;
@@ -604,7 +604,7 @@ void SchedulerStateMachine::UpdateStateOnCommit(bool commit_was_aborted) {
 
   // If we are impl-side-painting but the commit was aborted, then we behave
   // mostly as if we are not impl-side-painting since there is no pending tree.
-  has_pending_tree_ = settings_.impl_side_painting && !commit_was_aborted;
+  has_pending_tree_ = settings_.impl_side_painting && !commit_has_no_updates;
 
   // Update state related to forced draws.
   if (forced_redraw_state_ == FORCED_REDRAW_STATE_WAITING_FOR_COMMIT) {
@@ -627,7 +627,7 @@ void SchedulerStateMachine::UpdateStateOnCommit(bool commit_was_aborted) {
   // Update state if we have a new active tree to draw, or if the active tree
   // was unchanged but we need to do a forced draw.
   if (!has_pending_tree_ &&
-      (!commit_was_aborted ||
+      (!commit_has_no_updates ||
        forced_redraw_state_ == FORCED_REDRAW_STATE_WAITING_FOR_DRAW)) {
     needs_redraw_ = true;
     active_tree_needs_first_draw_ = true;
@@ -1025,14 +1025,18 @@ void SchedulerStateMachine::NotifyReadyToCommit() {
     DCHECK(ShouldCommit());
 }
 
-void SchedulerStateMachine::BeginMainFrameAborted(bool did_handle) {
+void SchedulerStateMachine::BeginMainFrameAborted(CommitEarlyOutReason reason) {
   DCHECK_EQ(commit_state_, COMMIT_STATE_BEGIN_MAIN_FRAME_SENT);
-  if (did_handle) {
-    bool commit_was_aborted = true;
-    UpdateStateOnCommit(commit_was_aborted);
-  } else {
-    commit_state_ = COMMIT_STATE_IDLE;
-    SetNeedsCommit();
+  switch (reason) {
+    case CommitEarlyOutReason::ABORTED_OUTPUT_SURFACE_LOST:
+    case CommitEarlyOutReason::ABORTED_NOT_VISIBLE:
+      commit_state_ = COMMIT_STATE_IDLE;
+      SetNeedsCommit();
+      return;
+    case CommitEarlyOutReason::FINISHED_NO_UPDATES:
+      bool commit_has_no_updates = true;
+      UpdateStateOnCommit(commit_has_no_updates);
+      return;
   }
 }
 
diff --git a/cc/scheduler/scheduler_state_machine.h b/cc/scheduler/scheduler_state_machine.h
index 7ee039a..4d7bf6f 100644
--- a/cc/scheduler/scheduler_state_machine.h
+++ b/cc/scheduler/scheduler_state_machine.h
@@ -12,6 +12,7 @@
 #include "base/time/time.h"
 #include "cc/base/cc_export.h"
 #include "cc/output/begin_frame_args.h"
+#include "cc/scheduler/commit_earlyout_reason.h"
 #include "cc/scheduler/draw_result.h"
 #include "cc/scheduler/scheduler_settings.h"
 
@@ -206,8 +207,7 @@ class CC_EXPORT SchedulerStateMachine {
 
   // Call this only in response to receiving an ACTION_SEND_BEGIN_MAIN_FRAME
   // from NextAction if the client rejects the BeginMainFrame message.
-  // If did_handle is false, then another commit will be retried soon.
-  void BeginMainFrameAborted(bool did_handle);
+  void BeginMainFrameAborted(CommitEarlyOutReason reason);
 
   // Set that we can create the first OutputSurface and start the scheduler.
   void SetCanStart() { can_start_ = true; }
@@ -286,7 +286,7 @@ class CC_EXPORT SchedulerStateMachine {
   bool HasRequestedSwapThisFrame() const;
   bool HasSwappedThisFrame() const;
 
-  void UpdateStateOnCommit(bool commit_was_aborted);
+  void UpdateStateOnCommit(bool commit_had_no_updates);
   void UpdateStateOnActivation();
   void UpdateStateOnDraw(bool did_request_swap);
   void UpdateStateOnPrepareTiles();
diff --git a/cc/scheduler/scheduler_state_machine_unittest.cc b/cc/scheduler/scheduler_state_machine_unittest.cc
index 2d896a2..5a60276 100644
--- a/cc/scheduler/scheduler_state_machine_unittest.cc
+++ b/cc/scheduler/scheduler_state_machine_unittest.cc
@@ -1104,9 +1104,7 @@ TEST(SchedulerStateMachineTest, TestAbortBeginMainFrameBecauseInvisible) {
 
   // Become invisible and abort BeginMainFrame.
   state.SetVisible(false);
-  // False means that the BeginMainFrame was sent but we never forwarded it to
-  // the main thread because by the time it arrived we couldn't process it.
-  state.BeginMainFrameAborted(false);
+  state.BeginMainFrameAborted(CommitEarlyOutReason::ABORTED_NOT_VISIBLE);
 
   // NeedsCommit should now be true again because we never actually did a
   // commit.
@@ -1162,7 +1160,7 @@ TEST(SchedulerStateMachineTest, TestAbortBeginMainFrameBecauseCommitNotNeeded) {
 
   // Abort the commit, true means that the BeginMainFrame was sent but there
   // was no work to do on the main thread.
-  state.BeginMainFrameAborted(true);
+  state.BeginMainFrameAborted(CommitEarlyOutReason::FINISHED_NO_UPDATES);
 
   // NeedsCommit should now be false because the commit was actually handled.
   EXPECT_FALSE(state.NeedsCommit());
@@ -1622,7 +1620,7 @@ TEST(SchedulerStateMachineTest,
 
   // Since only the scroll offset changed, the main thread will abort the
   // commit.
-  state.BeginMainFrameAborted(true);
+  state.BeginMainFrameAborted(CommitEarlyOutReason::FINISHED_NO_UPDATES);
 
   // Since the commit was aborted, we should draw right away instead of waiting
   // for the deadline.