cc: Always use SetNeedsBeginFrame to request the next BeginFrame

cc/scheduler/scheduler_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.h"
 
 #include <string>
 #include <vector>
 
 #include "base/logging.h"
@@ skipping 191 matching lines @@
   // FinishCommit should commit
   scheduler->FinishCommit();
   EXPECT_SINGLE_ACTION("ScheduledActionCommit", client);
   EXPECT_TRUE(client.needs_begin_frame());
   client.Reset();
 
   // BeginFrame should draw.
   scheduler->BeginFrame(BeginFrameArgs::CreateForTesting());
   EXPECT_ACTION("ScheduledActionDrawAndSwapIfPossible", client, 0, 2);
   EXPECT_ACTION("SetNeedsBeginFrameOnImplThread", client, 1, 2);
-  EXPECT_FALSE(client.needs_begin_frame());
+  EXPECT_TRUE(client.needs_begin_frame());
   client.Reset();
 }
 
 TEST(SchedulerTest, RequestCommitAfterBeginFrameSentToMainThread) {
   FakeSchedulerClient client;
   SchedulerSettings default_scheduler_settings;
   Scheduler* scheduler = client.CreateScheduler(default_scheduler_settings);
   scheduler->SetCanStart();
   scheduler->SetVisible(true);
   scheduler->SetCanDraw(true);
@@ skipping 10 matching lines @@
 
   // Now SetNeedsCommit again. Calling here means we need a second frame.
   scheduler->SetNeedsCommit();
 
   // Finish the commit for the first frame.
   scheduler->FinishCommit();
   EXPECT_SINGLE_ACTION("ScheduledActionCommit", client);
   client.Reset();
 
   // Tick should draw but then begin another frame for the second commit.
+  // Because we just swapped, the Scheduler should also request the next
+  // BeginFrame from the OutputSurface.
   scheduler->BeginFrame(BeginFrameArgs::CreateForTesting());
   EXPECT_TRUE(client.needs_begin_frame());
-  EXPECT_ACTION("ScheduledActionDrawAndSwapIfPossible", client, 0, 2);
-  EXPECT_ACTION("ScheduledActionSendBeginFrameToMainThread", client, 1, 2);
+  EXPECT_ACTION("ScheduledActionDrawAndSwapIfPossible", client, 0, 3);
+  EXPECT_ACTION("ScheduledActionSendBeginFrameToMainThread", client, 1, 3);
+  EXPECT_ACTION("SetNeedsBeginFrameOnImplThread", client, 2, 3);
   client.Reset();
 
-  // Finish the second commit to go back to quiescent state and verify we no
-  // longer request BeginFrames.
+  // Finish the second commit.
   scheduler->FinishCommit();
   scheduler->BeginFrame(BeginFrameArgs::CreateForTesting());
+  EXPECT_ACTION("ScheduledActionCommit", client, 0, 3);
+  EXPECT_ACTION("ScheduledActionDrawAndSwapIfPossible", client, 1, 3);
+  EXPECT_ACTION("SetNeedsBeginFrameOnImplThread", client, 2, 3);
+  EXPECT_TRUE(client.needs_begin_frame());
+  client.Reset();
+
+  // On the next BeginFrame, verify we go back to a quiescent state and
+  // no longer request BeginFrames.
+  scheduler->BeginFrame(BeginFrameArgs::CreateForTesting());
   EXPECT_FALSE(client.needs_begin_frame());
 }
 
 TEST(SchedulerTest, TextureAcquisitionCausesCommitInsteadOfDraw) {
   FakeSchedulerClient client;
   SchedulerSettings default_scheduler_settings;
   Scheduler* scheduler = client.CreateScheduler(default_scheduler_settings);
   scheduler->SetCanStart();
   scheduler->SetVisible(true);
   scheduler->SetCanDraw(true);
   EXPECT_SINGLE_ACTION("ScheduledActionBeginOutputSurfaceCreation", client);
 
   InitializeOutputSurfaceAndFirstCommit(scheduler);
   client.Reset();
   scheduler->SetNeedsRedraw();
   EXPECT_TRUE(scheduler->RedrawPending());
   EXPECT_SINGLE_ACTION("SetNeedsBeginFrameOnImplThread", client);
   EXPECT_TRUE(client.needs_begin_frame());
 
   client.Reset();
   scheduler->BeginFrame(BeginFrameArgs::CreateForTesting());
   EXPECT_ACTION("ScheduledActionDrawAndSwapIfPossible", client, 0, 2);
   EXPECT_ACTION("SetNeedsBeginFrameOnImplThread", client, 1, 2);
   EXPECT_FALSE(scheduler->RedrawPending());
+  EXPECT_TRUE(client.needs_begin_frame());
+
+  client.Reset();
+  scheduler->BeginFrame(BeginFrameArgs::CreateForTesting());
+  EXPECT_SINGLE_ACTION("SetNeedsBeginFrameOnImplThread", client);
+  EXPECT_FALSE(scheduler->RedrawPending());
   EXPECT_FALSE(client.needs_begin_frame());
 
   client.Reset();
   scheduler->SetMainThreadNeedsLayerTextures();
   EXPECT_SINGLE_ACTION("ScheduledActionAcquireLayerTexturesForMainThread",
                        client);
 
   // We should request a BeginFrame in anticipation of a draw.
   client.Reset();
   scheduler->SetNeedsRedraw();
@@ skipping 19 matching lines @@
   EXPECT_SINGLE_ACTION("ScheduledActionCommit", client);
   EXPECT_TRUE(scheduler->RedrawPending());
   EXPECT_TRUE(client.needs_begin_frame());
 
   // Now we can draw again after the commit happens.
   client.Reset();
   scheduler->BeginFrame(BeginFrameArgs::CreateForTesting());
   EXPECT_ACTION("ScheduledActionDrawAndSwapIfPossible", client, 0, 2);
   EXPECT_ACTION("SetNeedsBeginFrameOnImplThread", client, 1, 2);
   EXPECT_FALSE(scheduler->RedrawPending());
+  EXPECT_TRUE(client.needs_begin_frame());
+
+  // Make sure we stop requesting BeginFrames if we don't swap.
+  client.Reset();
+  scheduler->BeginFrame(BeginFrameArgs::CreateForTesting());
+  EXPECT_SINGLE_ACTION("SetNeedsBeginFrameOnImplThread", client);
   EXPECT_FALSE(client.needs_begin_frame());
-  client.Reset();
 }
 
 TEST(SchedulerTest, TextureAcquisitionCollision) {
   FakeSchedulerClient client;
   SchedulerSettings default_scheduler_settings;
   Scheduler* scheduler = client.CreateScheduler(default_scheduler_settings);
   scheduler->SetCanStart();
   scheduler->SetVisible(true);
   scheduler->SetCanDraw(true);
 
@@ skipping 26 matching lines @@
   client.Reset();
 
   // No implicit commit is expected.
   scheduler->BeginFrame(BeginFrameArgs::CreateForTesting());
   EXPECT_ACTION("ScheduledActionDrawAndSwapIfPossible", client, 0, 3);
   EXPECT_ACTION("ScheduledActionAcquireLayerTexturesForMainThread",
                 client,
                 1,
                 3);
   EXPECT_ACTION("SetNeedsBeginFrameOnImplThread", client, 2, 3);
+  EXPECT_TRUE(client.needs_begin_frame());
   client.Reset();
 
   // Compositor not scheduled to draw because textures are locked by main
   // thread.
+  scheduler->BeginFrame(BeginFrameArgs::CreateForTesting());
+  EXPECT_SINGLE_ACTION("SetNeedsBeginFrameOnImplThread", client);
   EXPECT_FALSE(client.needs_begin_frame());
+  client.Reset();
 
   // Needs an explicit commit from the main thread.
   scheduler->SetNeedsCommit();
   EXPECT_ACTION("ScheduledActionSendBeginFrameToMainThread", client, 0, 2);
   EXPECT_ACTION("SetNeedsBeginFrameOnImplThread", client, 1, 2);
   client.Reset();
 
   // Trigger the commit
   scheduler->FinishCommit();
+  EXPECT_SINGLE_ACTION("ScheduledActionCommit", client);
   EXPECT_TRUE(client.needs_begin_frame());
+  client.Reset();
+
+  // Verify we draw on the next BeginFrame.
+  scheduler->BeginFrame(BeginFrameArgs::CreateForTesting());
+  EXPECT_ACTION("ScheduledActionDrawAndSwapIfPossible", client, 0, 2);
+  EXPECT_ACTION("SetNeedsBeginFrameOnImplThread", client, 1, 2);
+  EXPECT_TRUE(client.needs_begin_frame());
+  client.Reset();
 }
 
 TEST(SchedulerTest, VisibilitySwitchWithTextureAcquisition) {
   FakeSchedulerClient client;
   SchedulerSettings default_scheduler_settings;
   Scheduler* scheduler = client.CreateScheduler(default_scheduler_settings);
   scheduler->SetCanStart();
   scheduler->SetVisible(true);
   scheduler->SetCanDraw(true);
 
@@ skipping 70 matching lines @@
   EXPECT_EQ(0, client.num_draws());
 
   scheduler->BeginFrame(BeginFrameArgs::CreateForTesting());
   EXPECT_EQ(1, client.num_draws());
   EXPECT_TRUE(scheduler->RedrawPending());
   EXPECT_TRUE(client.needs_begin_frame());
 
   scheduler->BeginFrame(BeginFrameArgs::CreateForTesting());
   EXPECT_EQ(2, client.num_draws());
   EXPECT_FALSE(scheduler->RedrawPending());
+  EXPECT_TRUE(client.needs_begin_frame());
+
+  // We stop requesting BeginFrames after a BeginFrame where we don't swap.
+  scheduler->BeginFrame(BeginFrameArgs::CreateForTesting());
+  EXPECT_EQ(2, client.num_draws());
+  EXPECT_FALSE(scheduler->RedrawPending());
   EXPECT_FALSE(client.needs_begin_frame());
 }
 
 // Test that requesting redraw inside a failed draw doesn't lose the request.
 TEST(SchedulerTest, RequestRedrawInsideFailedDraw) {
   SchedulerClientThatsetNeedsDrawInsideDraw client;
   SchedulerSettings default_scheduler_settings;
   Scheduler* scheduler = client.CreateScheduler(default_scheduler_settings);
   scheduler->SetCanStart();
   scheduler->SetVisible(true);
@@ skipping 89 matching lines @@
   scheduler->BeginFrame(BeginFrameArgs::CreateForTesting());
   EXPECT_EQ(1, client.num_draws());
   EXPECT_TRUE(scheduler->CommitPending());
   EXPECT_TRUE(client.needs_begin_frame());
   scheduler->FinishCommit();
 
   scheduler->BeginFrame(BeginFrameArgs::CreateForTesting());
   EXPECT_EQ(2, client.num_draws());;
   EXPECT_FALSE(scheduler->RedrawPending());
   EXPECT_FALSE(scheduler->CommitPending());
+  EXPECT_TRUE(client.needs_begin_frame());
+
+  // We stop requesting BeginFrames after a BeginFrame where we don't swap.
+  scheduler->BeginFrame(BeginFrameArgs::CreateForTesting());
+  EXPECT_EQ(2, client.num_draws());
+  EXPECT_FALSE(scheduler->RedrawPending());
+  EXPECT_FALSE(scheduler->CommitPending());
   EXPECT_FALSE(client.needs_begin_frame());
 }
 
 // Tests that when a draw fails then the pending commit should not be dropped.
 TEST(SchedulerTest, RequestCommitInsideFailedDraw) {
   SchedulerClientThatsetNeedsDrawInsideDraw client;
   SchedulerSettings default_scheduler_settings;
   Scheduler* scheduler = client.CreateScheduler(default_scheduler_settings);
   scheduler->SetCanStart();
   scheduler->SetVisible(true);
@@ skipping 161 matching lines @@
   // Afterwards, neither a draw nor ManageTiles are pending.
   scheduler->BeginFrame(BeginFrameArgs::CreateForTesting());
   EXPECT_EQ(1, client.num_draws());
   EXPECT_TRUE(client.HasAction("ScheduledActionDrawAndSwapIfPossible"));
   EXPECT_TRUE(client.HasAction("ScheduledActionManageTiles"));
   EXPECT_LT(client.ActionIndex("ScheduledActionDrawAndSwapIfPossible"),
             client.ActionIndex("ScheduledActionManageTiles"));
   EXPECT_FALSE(scheduler->RedrawPending());
   EXPECT_FALSE(scheduler->ManageTilesPending());
 
+  // We need a BeginFrame where we don't swap to go idle.

[brianderson, 2013/09/13 20:19:10]

Had to add this to @epenner's test to make sure we go idle.

+  client.Reset();
+  scheduler->BeginFrame(BeginFrameArgs::CreateForTesting());
+  EXPECT_SINGLE_ACTION("SetNeedsBeginFrameOnImplThread", client);
+  EXPECT_EQ(0, client.num_draws());
+
   // Now trigger a ManageTiles outside of a draw. We will then need
   // a begin-frame for the ManageTiles, but we don't need a draw.
   client.Reset();
   EXPECT_FALSE(client.needs_begin_frame());
   scheduler->SetNeedsManageTiles();
   EXPECT_TRUE(client.needs_begin_frame());
   EXPECT_TRUE(scheduler->ManageTilesPending());
   EXPECT_FALSE(scheduler->RedrawPending());
 
   // BeginFrame. There will be no draw, only ManageTiles.
   scheduler->BeginFrame(BeginFrameArgs::CreateForTesting());
   EXPECT_EQ(0, client.num_draws());
   EXPECT_FALSE(client.HasAction("ScheduledActionDrawAndSwapIfPossible"));
   EXPECT_TRUE(client.HasAction("ScheduledActionManageTiles"));
 }
 
 }  // namespace
 }  // namespace cc