cc: Always use SetNeedsBeginFrame to request the next BeginFrame

This avoids relying on SwapBuffers to implicitly trigger the next
BeginFrame. Only a single code path is used to request the next
BeginFrame now: SetNeedsBeginFrame(true).

This avoids issues where OutputSurface subclasses might not call
OutputSurface::DidSwap() when they need to or when we early out
a swap request and don't actually swap anything.

BUG=289755

Review URL: https://chromiumcodereview.appspot.com/23498035

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@223218 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 15 insertions, 11 deletions

diff --git a/cc/output/output_surface.cc b/cc/output/output_surface.cc
index 2ffcf95..812f46b 100644
--- a/cc/output/output_surface.cc
+++ b/cc/output/output_surface.cc
@@ -39,7 +39,7 @@ OutputSurface::OutputSurface(scoped_refptr<ContextProvider> context_provider)
       max_frames_pending_(0),
       pending_swap_buffers_(0),
       needs_begin_frame_(false),
-      begin_frame_pending_(false),
+      client_ready_for_begin_frame_(true),
       client_(NULL),
       check_for_retroactive_begin_frame_pending_(false),
       external_stencil_test_enabled_(false) {}
@@ -54,7 +54,7 @@ OutputSurface::OutputSurface(
       max_frames_pending_(0),
       pending_swap_buffers_(0),
       needs_begin_frame_(false),
-      begin_frame_pending_(false),
+      client_ready_for_begin_frame_(true),
       client_(NULL),
       check_for_retroactive_begin_frame_pending_(false),
       external_stencil_test_enabled_(false) {}
@@ -71,7 +71,7 @@ OutputSurface::OutputSurface(
       max_frames_pending_(0),
       pending_swap_buffers_(0),
       needs_begin_frame_(false),
-      begin_frame_pending_(false),
+      client_ready_for_begin_frame_(true),
       client_(NULL),
       check_for_retroactive_begin_frame_pending_(false),
       external_stencil_test_enabled_(false) {}
@@ -133,7 +133,7 @@ void OutputSurface::SetNeedsRedrawRect(gfx::Rect damage_rect) {
 void OutputSurface::SetNeedsBeginFrame(bool enable) {
   TRACE_EVENT1("cc", "OutputSurface::SetNeedsBeginFrame", "enable", enable);
   needs_begin_frame_ = enable;
-  begin_frame_pending_ = false;
+  client_ready_for_begin_frame_ = true;
   if (frame_rate_controller_) {
     BeginFrameArgs skipped = frame_rate_controller_->SetActive(enable);
     if (skipped.IsValid())
@@ -145,14 +145,14 @@ void OutputSurface::SetNeedsBeginFrame(bool enable) {
 
 void OutputSurface::BeginFrame(const BeginFrameArgs& args) {
   TRACE_EVENT2("cc", "OutputSurface::BeginFrame",
-               "begin_frame_pending_", begin_frame_pending_,
+               "client_ready_for_begin_frame_", client_ready_for_begin_frame_,
                "pending_swap_buffers_", pending_swap_buffers_);
-  if (!needs_begin_frame_ || begin_frame_pending_ ||
+  if (!needs_begin_frame_ || !client_ready_for_begin_frame_ ||
       (pending_swap_buffers_ >= max_frames_pending_ &&
        max_frames_pending_ > 0)) {
     skipped_begin_frame_args_ = args;
   } else {
-    begin_frame_pending_ = true;
+    client_ready_for_begin_frame_ = false;
     client_->BeginFrame(args);
     // args might be an alias for skipped_begin_frame_args_.
     // Do not reset it before calling BeginFrame!
@@ -192,7 +192,6 @@ void OutputSurface::CheckForRetroactiveBeginFrame() {
 }
 
 void OutputSurface::DidSwapBuffers() {
-  begin_frame_pending_ = false;
   pending_swap_buffers_++;
   TRACE_EVENT1("cc", "OutputSurface::DidSwapBuffers",
                "pending_swap_buffers_", pending_swap_buffers_);
@@ -213,7 +212,7 @@ void OutputSurface::OnSwapBuffersComplete(const CompositorFrameAck* ack) {
 
 void OutputSurface::DidLoseOutputSurface() {
   TRACE_EVENT0("cc", "OutputSurface::DidLoseOutputSurface");
-  begin_frame_pending_ = false;
+  client_ready_for_begin_frame_ = true;
   pending_swap_buffers_ = 0;
   client_->DidLoseOutputSurface();
 }
diff --git a/cc/output/output_surface.h b/cc/output/output_surface.h
index f30d3f3..c75092d 100644
--- a/cc/output/output_surface.h
+++ b/cc/output/output_surface.h
@@ -163,7 +163,7 @@ class CC_EXPORT OutputSurface : public FrameRateControllerClient {
   int max_frames_pending_;
   int pending_swap_buffers_;
   bool needs_begin_frame_;
-  bool begin_frame_pending_;
+  bool client_ready_for_begin_frame_;
 
   // Forwarded to OutputSurfaceClient but threaded through OutputSurface
   // first so OutputSurface has a chance to update the FrameRateController
diff --git a/cc/output/output_surface_unittest.cc b/cc/output/output_surface_unittest.cc
index 54fd3a1..cad5e4e 100644
--- a/cc/output/output_surface_unittest.cc
+++ b/cc/output/output_surface_unittest.cc
@@ -210,8 +210,10 @@ TEST(OutputSurfaceTest, BeginFrameEmulation) {
   EXPECT_EQ(client.begin_frame_count(), 1);
   EXPECT_EQ(output_surface.pending_swap_buffers(), 0);
 
-  // DidSwapBuffers should clear the pending BeginFrame.
+  // SetNeedsBeginFrame should clear the pending BeginFrame after
+  // a SwapBuffers.
   output_surface.DidSwapBuffersForTesting();
+  output_surface.SetNeedsBeginFrame(true);
   EXPECT_EQ(client.begin_frame_count(), 1);
   EXPECT_EQ(output_surface.pending_swap_buffers(), 1);
   task_runner->RunPendingTasks();
@@ -220,6 +222,7 @@ TEST(OutputSurfaceTest, BeginFrameEmulation) {
 
   // BeginFrame should be throttled by pending swap buffers.
   output_surface.DidSwapBuffersForTesting();
+  output_surface.SetNeedsBeginFrame(true);
   EXPECT_EQ(client.begin_frame_count(), 2);
   EXPECT_EQ(output_surface.pending_swap_buffers(), 2);
   task_runner->RunPendingTasks();
@@ -284,12 +287,14 @@ TEST(OutputSurfaceTest, OptimisticAndRetroactiveBeginFrames) {
   output_surface.BeginFrameForTesting();
   EXPECT_EQ(client.begin_frame_count(), 2);
   output_surface.DidSwapBuffersForTesting();
+  output_surface.SetNeedsBeginFrame(true);
   EXPECT_EQ(client.begin_frame_count(), 3);
   EXPECT_EQ(output_surface.pending_swap_buffers(), 1);
 
   // Optimistically injected BeginFrames should be by throttled by pending
   // swap buffers...
   output_surface.DidSwapBuffersForTesting();
+  output_surface.SetNeedsBeginFrame(true);
   EXPECT_EQ(client.begin_frame_count(), 3);
   EXPECT_EQ(output_surface.pending_swap_buffers(), 2);
   output_surface.BeginFrameForTesting();