cc: Trigger BeginFrames that were just missed retroactively

If a SwapAck, SetNeedsBeginFrame, or a Swap occurs *just* after
a BeginFrame is received and skipped by the compositor, this
patch tries the BeginFrame retroactively if it's possible
we might still be able to produce a frame in time.

Also fixes the include guards of begin_frame_args.h.

BUG=243384,243500

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

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

7 files changed, 165 insertions, 31 deletions

diff --git a/cc/output/begin_frame_args.cc b/cc/output/begin_frame_args.cc
index ea14954..cefb22b 100644
--- a/cc/output/begin_frame_args.cc
+++ b/cc/output/begin_frame_args.cc
@@ -9,8 +9,7 @@ namespace cc {
 BeginFrameArgs::BeginFrameArgs()
   : frame_time(base::TimeTicks()),
     deadline(base::TimeTicks()),
-    interval(base::TimeDelta::FromMicroseconds(-1))
-{
+    interval(base::TimeDelta::FromMicroseconds(-1)) {
 }
 
 BeginFrameArgs::BeginFrameArgs(base::TimeTicks frame_time,
@@ -42,6 +41,13 @@ BeginFrameArgs BeginFrameArgs::CreateForTesting() {
                         DefaultInterval());
 }
 
+BeginFrameArgs BeginFrameArgs::CreateExpiredForTesting() {
+  base::TimeTicks now = base::TimeTicks::Now();
+  return BeginFrameArgs(now,
+                        now - DefaultInterval(),
+                        DefaultInterval());
+}
+
 base::TimeDelta BeginFrameArgs::DefaultDeadlineAdjustment() {
   // Using a large deadline adjustment will effectively revert BeginFrame
   // scheduling to the hard vsync scheduling we used to have.
@@ -52,4 +58,9 @@ base::TimeDelta BeginFrameArgs::DefaultInterval() {
   return base::TimeDelta::FromMicroseconds(16666);
 }
 
+base::TimeDelta BeginFrameArgs::DefaultRetroactiveBeginFramePeriod() {
+  return base::TimeDelta::FromMicroseconds(4444);
+}
+
+
 }  // namespace cc
diff --git a/cc/output/begin_frame_args.h b/cc/output/begin_frame_args.h
index 616a86a..33304cb 100644
--- a/cc/output/begin_frame_args.h
+++ b/cc/output/begin_frame_args.h
@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CC_OUTPUT_BEGIN_FRAME_H_
-#define CC_OUTPUT_BEGIN_FRAME_H_
+#ifndef CC_OUTPUT_BEGIN_FRAME_ARGS_H_
+#define CC_OUTPUT_BEGIN_FRAME_ARGS_H_
 
 #include "base/time.h"
 #include "cc/base/cc_export.h"
@@ -21,6 +21,7 @@ struct CC_EXPORT BeginFrameArgs {
                                base::TimeDelta interval);
   static BeginFrameArgs CreateForSynchronousCompositor();
   static BeginFrameArgs CreateForTesting();
+  static BeginFrameArgs CreateExpiredForTesting();
 
   // This is the default delta that will be used to adjust the deadline when
   // proper draw-time estimations are not yet available.
@@ -30,8 +31,14 @@ struct CC_EXPORT BeginFrameArgs {
   // magic numbers.
   static base::TimeDelta DefaultInterval();
 
-  bool IsInvalid() const {
-    return interval < base::TimeDelta();
+  // This is the default amount of time after the frame_time to retroactively
+  // send a BeginFrame that had been skipped. This only has an effect if the
+  // deadline has passed, since the deadline is also used to trigger BeginFrame
+  // retroactively.
+  static base::TimeDelta DefaultRetroactiveBeginFramePeriod();
+
+  bool IsValid() const {
+    return interval >= base::TimeDelta();
   }
 
   base::TimeTicks frame_time;
@@ -46,4 +53,4 @@ struct CC_EXPORT BeginFrameArgs {
 
 }  // namespace cc
 
-#endif  // CC_OUTPUT_BEGIN_FRAME_H_
+#endif  // CC_OUTPUT_BEGIN_FRAME_ARGS_H_
diff --git a/cc/output/output_surface.cc b/cc/output/output_surface.cc
index 60789b0..15112f6 100644
--- a/cc/output/output_surface.cc
+++ b/cc/output/output_surface.cc
@@ -58,6 +58,7 @@ OutputSurface::OutputSurface(
       weak_ptr_factory_(this),
       max_frames_pending_(0),
       pending_swap_buffers_(0),
+      needs_begin_frame_(false),
       begin_frame_pending_(false),
       client_(NULL) {
 }
@@ -71,6 +72,7 @@ OutputSurface::OutputSurface(
       weak_ptr_factory_(this),
       max_frames_pending_(0),
       pending_swap_buffers_(0),
+      needs_begin_frame_(false),
       begin_frame_pending_(false),
       client_(NULL) {
 }
@@ -86,6 +88,7 @@ OutputSurface::OutputSurface(
       weak_ptr_factory_(this),
       max_frames_pending_(0),
       pending_swap_buffers_(0),
+      needs_begin_frame_(false),
       begin_frame_pending_(false),
       client_(NULL) {
 }
@@ -94,7 +97,7 @@ void OutputSurface::InitializeBeginFrameEmulation(
     base::SingleThreadTaskRunner* task_runner,
     bool throttle_frame_production,
     base::TimeDelta interval) {
-  if (throttle_frame_production){
+  if (throttle_frame_production) {
     frame_rate_controller_.reset(
         new FrameRateController(
             DelayBasedTimeSource::Create(interval, task_runner)));
@@ -132,7 +135,9 @@ void OutputSurface::OnVSyncParametersChanged(base::TimeTicks timebase,
 void OutputSurface::FrameRateControllerTick(bool throttled,
                                             const BeginFrameArgs& args) {
   DCHECK(frame_rate_controller_);
-  if (!throttled)
+  if (throttled)
+    skipped_begin_frame_args_ = args;
+  else
     BeginFrame(args);
 }
 
@@ -144,20 +149,58 @@ 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;
-  if (frame_rate_controller_)
-    frame_rate_controller_->SetActive(enable);
+  if (frame_rate_controller_) {
+    BeginFrameArgs skipped = frame_rate_controller_->SetActive(enable);
+    if (skipped.IsValid())
+      skipped_begin_frame_args_ = skipped;
+  }
+  if (needs_begin_frame_)
+    PostCheckForRetroactiveBeginFrame();
 }
 
 void OutputSurface::BeginFrame(const BeginFrameArgs& args) {
   TRACE_EVENT2("cc", "OutputSurface::BeginFrame",
                "begin_frame_pending_", begin_frame_pending_,
                "pending_swap_buffers_", pending_swap_buffers_);
-  if (begin_frame_pending_ ||
-      (pending_swap_buffers_ >= max_frames_pending_ && max_frames_pending_ > 0))
+  if (!needs_begin_frame_ || begin_frame_pending_ ||
+      (pending_swap_buffers_ >= max_frames_pending_ &&
+       max_frames_pending_ > 0)) {
+    skipped_begin_frame_args_ = args;
+  } else {
+    begin_frame_pending_ = true;
+    client_->BeginFrame(args);
+    // args might be an alias for skipped_begin_frame_args_.
+    // Do not reset it before calling BeginFrame!
+    skipped_begin_frame_args_ = BeginFrameArgs();
+  }
+}
+
+base::TimeDelta OutputSurface::RetroactiveBeginFramePeriod() {
+  return BeginFrameArgs::DefaultRetroactiveBeginFramePeriod();
+}
+
+void OutputSurface::PostCheckForRetroactiveBeginFrame() {
+  if (!skipped_begin_frame_args_.IsValid())
     return;
-  begin_frame_pending_ = true;
-  client_->BeginFrame(args);
+
+  base::MessageLoop::current()->PostTask(
+     FROM_HERE,
+     base::Bind(&OutputSurface::CheckForRetroactiveBeginFrame,
+                weak_ptr_factory_.GetWeakPtr()));
+}
+
+void OutputSurface::CheckForRetroactiveBeginFrame() {
+  TRACE_EVENT0("cc", "OutputSurface::CheckForRetroactiveBeginFrame");
+  base::TimeTicks now = base::TimeTicks::Now();
+  base::TimeTicks alternative_deadline =
+      skipped_begin_frame_args_.frame_time +
+      RetroactiveBeginFramePeriod();
+  if (now < skipped_begin_frame_args_.deadline ||
+      now < alternative_deadline) {
+    BeginFrame(skipped_begin_frame_args_);
+  }
 }
 
 void OutputSurface::DidSwapBuffers() {
@@ -167,6 +210,7 @@ void OutputSurface::DidSwapBuffers() {
                "pending_swap_buffers_", pending_swap_buffers_);
   if (frame_rate_controller_)
     frame_rate_controller_->DidSwapBuffers();
+  PostCheckForRetroactiveBeginFrame();
 }
 
 void OutputSurface::OnSwapBuffersComplete(const CompositorFrameAck* ack) {
@@ -176,6 +220,7 @@ void OutputSurface::OnSwapBuffersComplete(const CompositorFrameAck* ack) {
   client_->OnSwapBuffersComplete(ack);
   if (frame_rate_controller_)
     frame_rate_controller_->DidSwapBuffersComplete();
+  PostCheckForRetroactiveBeginFrame();
 }
 
 void OutputSurface::DidLoseOutputSurface() {
diff --git a/cc/output/output_surface.h b/cc/output/output_surface.h
index 3172f41..738e650 100644
--- a/cc/output/output_surface.h
+++ b/cc/output/output_surface.h
@@ -154,6 +154,7 @@ class CC_EXPORT OutputSurface : public FrameRateControllerClient {
   scoped_ptr<FrameRateController> frame_rate_controller_;
   int max_frames_pending_;
   int pending_swap_buffers_;
+  bool needs_begin_frame_;
   bool begin_frame_pending_;
 
   // Forwarded to OutputSurfaceClient but threaded through OutputSurface
@@ -167,12 +168,19 @@ class CC_EXPORT OutputSurface : public FrameRateControllerClient {
   void SetExternalDrawConstraints(const gfx::Transform& transform,
                                   gfx::Rect viewport);
 
+  // virtual for testing.
+  virtual base::TimeDelta RetroactiveBeginFramePeriod();
+  virtual void PostCheckForRetroactiveBeginFrame();
+  void CheckForRetroactiveBeginFrame();
+
  private:
   OutputSurfaceClient* client_;
   friend class OutputSurfaceCallbacks;
 
   void SetContext3D(scoped_ptr<WebKit::WebGraphicsContext3D> context3d);
 
+  BeginFrameArgs skipped_begin_frame_args_;
+
   DISALLOW_COPY_AND_ASSIGN(OutputSurface);
 };
 
diff --git a/cc/output/output_surface_unittest.cc b/cc/output/output_surface_unittest.cc
index 68053e8..a73b779 100644
--- a/cc/output/output_surface_unittest.cc
+++ b/cc/output/output_surface_unittest.cc
@@ -43,7 +43,7 @@ class TestOutputSurface : public OutputSurface {
   }
 
   void BeginFrameForTesting() {
-    BeginFrame(BeginFrameArgs::CreateForTesting());
+    OutputSurface::BeginFrame(BeginFrameArgs::CreateExpiredForTesting());
   }
 
   void DidSwapBuffersForTesting() {
@@ -57,6 +57,22 @@ class TestOutputSurface : public OutputSurface {
   void OnSwapBuffersCompleteForTesting() {
     OnSwapBuffersComplete(NULL);
   }
+
+  void SetRetroactiveBeginFramePeriod(base::TimeDelta period) {
+    retroactive_begin_frame_period_ = period;
+  }
+
+ protected:
+  virtual void PostCheckForRetroactiveBeginFrame() OVERRIDE {
+    // For testing purposes, we check immediately rather than posting a task.
+    CheckForRetroactiveBeginFrame();
+  }
+
+  virtual base::TimeDelta RetroactiveBeginFramePeriod() OVERRIDE {
+    return retroactive_begin_frame_period_;
+  }
+
+  base::TimeDelta retroactive_begin_frame_period_;
 };
 
 class FakeOutputSurfaceClient : public OutputSurfaceClient {
@@ -221,6 +237,8 @@ TEST(OutputSurfaceTest, BeginFrameEmulation) {
       display_refresh_interval);
 
   output_surface.SetMaxFramesPending(2);
+  output_surface.SetRetroactiveBeginFramePeriod(
+      base::TimeDelta::FromSeconds(-1));
 
   // We should start off with 0 BeginFrames
   EXPECT_EQ(client.begin_frame_count(), 0);
@@ -278,27 +296,60 @@ TEST(OutputSurfaceTest, BeginFrameEmulation) {
   EXPECT_FALSE(task_runner->HasPendingTask());
   EXPECT_EQ(client.begin_frame_count(), 4);
   EXPECT_EQ(output_surface.pending_swap_buffers(), 1);
+}
+
+TEST(OutputSurfaceTest, OptimisticAndRetroactiveBeginFrames) {
+  scoped_ptr<TestWebGraphicsContext3D> context3d =
+      TestWebGraphicsContext3D::Create();
+
+  TestOutputSurface output_surface(
+      context3d.PassAs<WebKit::WebGraphicsContext3D>());
+  EXPECT_FALSE(output_surface.HasClientForTesting());
+
+  FakeOutputSurfaceClient client;
+  EXPECT_TRUE(output_surface.BindToClient(&client));
+  EXPECT_TRUE(output_surface.HasClientForTesting());
+  EXPECT_FALSE(client.deferred_initialize_called());
 
-  // Optimistically injected BeginFrames without a SetNeedsBeginFrame should be
-  // allowed.
+  output_surface.SetMaxFramesPending(2);
+
+  // Enable retroactive BeginFrames.
+  output_surface.SetRetroactiveBeginFramePeriod(
+    base::TimeDelta::FromSeconds(100000));
+
+  // Optimistically injected BeginFrames should be throttled if
+  // SetNeedsBeginFrame is false...
+  output_surface.SetNeedsBeginFrame(false);
   output_surface.BeginFrameForTesting();
-  EXPECT_EQ(client.begin_frame_count(), 5);
-  EXPECT_EQ(output_surface.pending_swap_buffers(), 1);
+  EXPECT_EQ(client.begin_frame_count(), 0);
+  // ...and retroactively triggered by a SetNeedsBeginFrame.
+  output_surface.SetNeedsBeginFrame(true);
+  EXPECT_EQ(client.begin_frame_count(), 1);
 
-  // Optimistically injected BeginFrames without a SetNeedsBeginFrame should
-  // still be throttled by pending begin frames however.
+  // Optimistically injected BeginFrames should be throttled by pending
+  // BeginFrames...
+  output_surface.BeginFrameForTesting();
+  EXPECT_EQ(client.begin_frame_count(), 1);
+  // ...and retroactively triggered by a SetNeedsBeginFrame.
+  output_surface.SetNeedsBeginFrame(true);
+  EXPECT_EQ(client.begin_frame_count(), 2);
+  // ...or retroactively triggered by a Swap.
   output_surface.BeginFrameForTesting();
-  EXPECT_EQ(client.begin_frame_count(), 5);
+  EXPECT_EQ(client.begin_frame_count(), 2);
+  output_surface.DidSwapBuffersForTesting();
+  EXPECT_EQ(client.begin_frame_count(), 3);
   EXPECT_EQ(output_surface.pending_swap_buffers(), 1);
 
-  // Optimistically injected BeginFrames without a SetNeedsBeginFrame should
-  // also be throttled by pending swap buffers.
+  // Optimistically injected BeginFrames should be by throttled by pending
+  // swap buffers...
   output_surface.DidSwapBuffersForTesting();
-  EXPECT_EQ(client.begin_frame_count(), 5);
+  EXPECT_EQ(client.begin_frame_count(), 3);
   EXPECT_EQ(output_surface.pending_swap_buffers(), 2);
   output_surface.BeginFrameForTesting();
-  EXPECT_EQ(client.begin_frame_count(), 5);
-  EXPECT_EQ(output_surface.pending_swap_buffers(), 2);
+  EXPECT_EQ(client.begin_frame_count(), 3);
+  // ...and retroactively triggered by OnSwapBuffersComplete
+  output_surface.OnSwapBuffersCompleteForTesting();
+  EXPECT_EQ(client.begin_frame_count(), 4);
 }
 
 }  // namespace
diff --git a/cc/scheduler/frame_rate_controller.cc b/cc/scheduler/frame_rate_controller.cc
index 811892d..c2294ea 100644
--- a/cc/scheduler/frame_rate_controller.cc
+++ b/cc/scheduler/frame_rate_controller.cc
@@ -65,10 +65,11 @@ FrameRateController::~FrameRateController() {
     time_source_->SetActive(false);
 }
 
-void FrameRateController::SetActive(bool active) {
+BeginFrameArgs FrameRateController::SetActive(bool active) {
   if (active_ == active)
-    return;
+    return BeginFrameArgs();
   TRACE_EVENT1("cc", "FrameRateController::SetActive", "active", active);
+  bool just_activated = active && !active_;
   active_ = active;
 
   if (is_time_source_throttling_) {
@@ -79,6 +80,14 @@ void FrameRateController::SetActive(bool active) {
     else
       weak_factory_.InvalidateWeakPtrs();
   }
+
+  if (just_activated) {
+    // TODO(brianderson): Use an adaptive parent compositor deadline.
+    base::TimeTicks frame_time = NextTickTime() - interval_;
+    base::TimeTicks deadline = NextTickTime();
+    return BeginFrameArgs::Create(frame_time, deadline, interval_);
+  }
+  return BeginFrameArgs();
 }
 
 void FrameRateController::SetMaxSwapsPending(int max_swaps_pending) {
diff --git a/cc/scheduler/frame_rate_controller.h b/cc/scheduler/frame_rate_controller.h
index 32e757b..0cfca0f 100644
--- a/cc/scheduler/frame_rate_controller.h
+++ b/cc/scheduler/frame_rate_controller.h
@@ -42,7 +42,10 @@ class CC_EXPORT FrameRateController {
 
   void SetClient(FrameRateControllerClient* client) { client_ = client; }
 
-  void SetActive(bool active);
+  // Returns a valid BeginFrame on activation to potentially be used
+  // retroactively.
+  BeginFrameArgs SetActive(bool active);
+
   bool IsActive() { return active_; }
 
   // Use the following methods to adjust target frame rate.