cc: Use BeginFrameArgs

cc/scheduler/scheduler.cc

Index: cc/scheduler/scheduler.cc
diff --git a/cc/scheduler/scheduler.cc b/cc/scheduler/scheduler.cc
index 2f43e8fd5ad22ca334de4f30a7d84e55b670cdfe..aebe761e4b394ac4982aa847d8fab2b40c1b04b2 100644
--- a/cc/scheduler/scheduler.cc
+++ b/cc/scheduler/scheduler.cc
@@ -126,35 +126,60 @@ base::TimeTicks Scheduler::LastBeginFrameOnImplThreadTime() {
 
 void Scheduler::SetupNextBeginFrameIfNeeded() {
   // Determine if we need BeginFrame notifications.
-  // If we do, always request the BeginFrame immediately.
-  // If not, only disable on the next BeginFrame to avoid unnecessary toggles.
-  // The synchronous renderer compositor requires immediate disables though.
   bool needs_begin_frame = state_machine_.BeginFrameNeededByImplThread();
-  if ((needs_begin_frame ||
-       state_machine_.inside_begin_frame() ||
-       settings_.using_synchronous_renderer_compositor) &&
-      (needs_begin_frame != last_set_needs_begin_frame_)) {
-    client_->SetNeedsBeginFrameOnImplThread(needs_begin_frame);
-    last_set_needs_begin_frame_ = needs_begin_frame;
-  }
-
-  // Request another BeginFrame if we haven't drawn for now until we have
-  // deadlines implemented.
-  if (state_machine_.inside_begin_frame() && has_pending_begin_frame_) {
-    has_pending_begin_frame_ = false;
-    client_->SetNeedsBeginFrameOnImplThread(true);
+  if (settings_.using_synchronous_renderer_compositor) {
+    // The synchronous renderer compositor needs immediate enables/disables.
+    if (needs_begin_frame != last_set_needs_begin_frame_) {
+      client_->SetNeedsBeginFrameOnImplThread(needs_begin_frame);
+      last_set_needs_begin_frame_ = needs_begin_frame;
+    }
+  } else {
+    // The non-synchronous renderer compositor is a bit more complicated:
+    if (needs_begin_frame && !last_set_needs_begin_frame_) {
+      // Always request the BeginFrame immediately if it wasn't needed before.
+      client_->SetNeedsBeginFrameOnImplThread(needs_begin_frame);

[Sami, 2013/06/14 15:59:20]

nit: This function might be a little easier to read if you moved the repeated
calls to SetNeedsBeginFrameOnImplThread and last_set_needs_begin_frame_ to the
end.

[brianderson, 2013/06/14 18:42:36]

Ok. This could definitely be simplified.

+      last_set_needs_begin_frame_ = needs_begin_frame;
+    } else if (!needs_begin_frame && last_set_needs_begin_frame_ &&
+               has_pending_begin_frame_ &&
+               state_machine_.begin_frame_state() ==
+                   SchedulerStateMachine::BEGIN_FRAME_IDLE) {
+      // Only disable the BeginFrame after a BeginFrame where we didn't swap.
+      client_->SetNeedsBeginFrameOnImplThread(needs_begin_frame);
+      last_set_needs_begin_frame_ = needs_begin_frame;
+      has_pending_begin_frame_ = false;
+    } else if (needs_begin_frame && has_pending_begin_frame_ &&
+               state_machine_.begin_frame_state() ==
+                   SchedulerStateMachine::BEGIN_FRAME_IDLE) {
+      // We did not draw and swap this BeginFrame,
+      // so we need to explicitly request another BeginFrame.
+      client_->SetNeedsBeginFrameOnImplThread(needs_begin_frame);
+      last_set_needs_begin_frame_ = needs_begin_frame;
+      has_pending_begin_frame_ = false;
+    }
   }
 }
 
 void Scheduler::BeginFrame(BeginFrameArgs args) {
   TRACE_EVENT0("cc", "Scheduler::BeginFrame");
-  last_begin_frame_args_ = args;
   DCHECK(!has_pending_begin_frame_);
   has_pending_begin_frame_ = true;
-  last_begin_frame_args_ = args;
-  state_machine_.DidEnterBeginFrame(args);
+  last_begin_frame_args_ = args.CreateBeginFrameWithAdjustedDeadline(
+      BeginFrameArgs::DefaultDeadlineAdjustment());
+  state_machine_.StartBeginFrame(args);
+  ProcessScheduledActions();
+  if (state_machine_.begin_frame_state() ==
+          SchedulerStateMachine::BEGIN_FRAME_WAITING) {
+    client_->PostBeginFrameDeadlineTask(last_begin_frame_args_.deadline());
+  } else {
+    DCHECK_EQ(state_machine_.begin_frame_state(),
+              SchedulerStateMachine::BEGIN_FRAME_IDLE);
+  }
+}
+
+void Scheduler::OnBeginFrameDeadline() {
+  TRACE_EVENT0("cc", "Scheduler::OnBeginFrameDeadline");
+  state_machine_.OnBeginFrameDeadline();
   ProcessScheduledActions();
-  state_machine_.DidLeaveBeginFrame();
 }
 
 void Scheduler::DrawAndSwapIfPossible() {
@@ -180,8 +205,11 @@ void Scheduler::ProcessScheduledActions() {
 
   base::AutoReset<bool> mark_inside(&inside_process_scheduled_actions_, true);
 
-  SchedulerStateMachine::Action action = state_machine_.NextAction();
-  while (action != SchedulerStateMachine::ACTION_NONE) {
+  SchedulerStateMachine::Action action;
+  do {

[brianderson, 2013/06/14 04:16:29]

Changing to a do-while loop allows UpdateState to consider ACTION_NONE.

[Sami, 2013/06/14 15:59:20]

Advancing the begin frame state on ACTION_NONE seems a little fishy, but I guess
there's no straightforward way to use explicit actions instead is there?

[brianderson, 2013/06/14 18:42:36]

It is a bit fishy, but I couldn't think of a simpler solution. It would be
pretty round about to create an explicit action just to advance the BeginFrame
state. If we could anticipate that the *next* action would be ACTION_NONE, we
could advance the begin_frame_state ahead of time, but I think that would be
hard to do since the current action might trigger further actions depending on
the current action's outcome.

+    TRACE_EVENT1("cc", "SchedulerStateMachine",
+                 "state", state_machine_.ToString());
+    action = state_machine_.NextAction();
     state_machine_.UpdateState(action);
     switch (action) {
       case SchedulerStateMachine::ACTION_NONE:
@@ -211,8 +239,7 @@ void Scheduler::ProcessScheduledActions() {
         client_->ScheduledActionAcquireLayerTexturesForMainThread();
         break;
     }
-    action = state_machine_.NextAction();
-  }
+  } while (action != SchedulerStateMachine::ACTION_NONE);
 
   SetupNextBeginFrameIfNeeded();
   client_->DidAnticipatedDrawTimeChange(AnticipatedDrawTime());