cc: Always use SetNeedsBeginFrame to request the next BeginFrame

cc/scheduler/scheduler.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.h"
 
 #include "base/auto_reset.h"
 #include "base/debug/trace_event.h"
 #include "base/logging.h"
 #include "cc/debug/traced_value.h"
@@ skipping 156 matching lines @@
           last_begin_frame_args_.interval);
     }
   } else {
     poll_for_draw_triggers_closure_.Cancel();
   }
 }
 
 void Scheduler::BeginFrame(const BeginFrameArgs& args) {
   TRACE_EVENT0("cc", "Scheduler::BeginFrame");
   DCHECK(!has_pending_begin_frame_);
   has_pending_begin_frame_ = true;

[danakj, 2013/09/12 20:57:29]

Can you explain the purpose of this variable then? It seems like it stays true
until we SetupNextBeginFrameIfNeeded() which would unlock begin frame by setting
it to false? Am I reading right? How does this syncrhronize with the variable by
the same name in OutputSurface? (Why are there two?)

[brianderson, 2013/09/12 21:54:22]

I was able to remove the has_pending_begin_frame in the deadline patch. I think
I can remove it in this patch as well since it is now (almost) a synonym for
state_machine_.inside_begin_frame().
It was supposed to be synchronized with OutputSurface::begin_frame_pending_.
Part of the problem this bug was running into was that they became out of sync
and the Scheduler didn't think it needed to request another BeginFrame: The swap
was setting has_pending_begin_frame_ to false in the Scheduler, but not setting
begin_frame_pending_ to false in OutputSurface. 

SetupNextBeginFrameIfNeeded() would only request another BeginFrame if
begin_frame_pending_ was true (the swap could have set it to false.)

   last_begin_frame_args_ = args;
   state_machine_.DidEnterBeginFrame(args);
   ProcessScheduledActions();
   state_machine_.DidLeaveBeginFrame();
 }
 
 void Scheduler::PollForAnticipatedDrawTriggers() {
   TRACE_EVENT0("cc", "Scheduler::PollForAnticipatedDrawTriggers");
   state_machine_.PollForAnticipatedDrawTriggers();
   ProcessScheduledActions();
 }
 
 void Scheduler::DrawAndSwapIfPossible() {
   DrawSwapReadbackResult result =
       client_->ScheduledActionDrawAndSwapIfPossible();
   state_machine_.DidDrawIfPossibleCompleted(result.did_draw);
-  if (result.did_swap)
-    has_pending_begin_frame_ = false;
 }
 
 void Scheduler::DrawAndSwapForced() {
-  DrawSwapReadbackResult result = client_->ScheduledActionDrawAndSwapForced();
-  if (result.did_swap)
-    has_pending_begin_frame_ = false;
+  client_->ScheduledActionDrawAndSwapForced();
 }
 
 void Scheduler::DrawAndReadback() {
   DrawSwapReadbackResult result = client_->ScheduledActionDrawAndReadback();
   DCHECK(!result.did_swap);
 }
 
 void Scheduler::ProcessScheduledActions() {
   // We do not allow ProcessScheduledActions to be recursive.
   // The top-level call will iteratively execute the next action for us anyway.
@@ skipping 50 matching lines @@
 
   SetupNextBeginFrameIfNeeded();
   client_->DidAnticipatedDrawTimeChange(AnticipatedDrawTime());
 }
 
 bool Scheduler::WillDrawIfNeeded() const {
   return !state_machine_.PendingDrawsShouldBeAborted();
 }
 
 }  // namespace cc