cc: Post the missed BeginFrame to a new stack

cc/surfaces/display_scheduler.cc

Index: cc/surfaces/display_scheduler.cc
diff --git a/cc/surfaces/display_scheduler.cc b/cc/surfaces/display_scheduler.cc
index d26d1b6306973debcf5abde447dfa0c928b981f7..bf8fc0be7ef4d4cc551e9af7c86196d6cc7fd7f5 100644
--- a/cc/surfaces/display_scheduler.cc
+++ b/cc/surfaces/display_scheduler.cc
@@ -6,6 +6,7 @@
 
 #include <vector>
 
+#include "base/auto_reset.h"
 #include "base/stl_util.h"
 #include "base/trace_event/trace_event.h"
 #include "cc/output/output_surface.h"
@@ -97,6 +98,10 @@ void DisplayScheduler::SurfaceDamaged(const SurfaceId& surface_id) {
 
   if (!output_surface_lost_ && !observing_begin_frame_source_) {
     observing_begin_frame_source_ = true;
+    // When we call AddObserver() this will cause a missed BeginFrame to
+    // occur. We need to detect this and not process it immediately in the
+    // same call stack.
+    base::AutoReset<bool>(&inside_external_action_, true);
     begin_frame_source_->AddObserver(this);
   }
 
@@ -138,6 +143,22 @@ bool DisplayScheduler::OnBeginFrameDerivedImpl(const BeginFrameArgs& args) {
   TRACE_EVENT2("cc", "DisplayScheduler::BeginFrame", "args", args.AsValue(),
                "now", now);
 
+  if (inside_external_action_) {
+    // Repost this so that we don't run a missed BeginFrame on the same
+    // callstack. Otherwise we end up running unexpected scheduler actions
+    // immediately while inside some other action (such as submitting a
+    // CompositorFrame for a SurfaceFactory).
+    DCHECK_EQ(args.type, BeginFrameArgs::MISSED);
+    DCHECK(missed_begin_frame_task_.IsCancelled());
+    missed_begin_frame_task_.Reset(base::Bind(
+        base::IgnoreResult(&DisplayScheduler::OnBeginFrameDerivedImpl),
+        // The CancelableCallback will not run after it is destroyed, which
+        // happens when |this| is destroyed.
+        base::Unretained(this), args));
+    task_runner_->PostTask(FROM_HERE, missed_begin_frame_task_.callback());
+    return true;
+  }
+
   // If we get another BeginFrame before the previous deadline,
   // synchronously trigger the previous deadline before progressing.
   if (inside_begin_frame_deadline_interval_) {
@@ -151,6 +172,11 @@ bool DisplayScheduler::OnBeginFrameDerivedImpl(const BeginFrameArgs& args) {
   inside_begin_frame_deadline_interval_ = true;
   ScheduleBeginFrameDeadline();
 
+  // If we get another BeginFrame before a posted missed frame, just drop the

[sunnyps, 2016/07/27 05:21:02]

nit: We also need to cancel the missed frame in case of a missed frame otherwise
our DCHECKs above will fail (IsCancelled won't return true unless we explicitly
Cancel or Reset).

[danakj, 2016/07/27 22:34:55]

I'm not sure what you mean here sorry, can you explain?

  bool IsCancelled() const {
    return callback_.is_null();
  }
 
IsCancelled() returns true for a new CancelableCallback if not created with a
callback (which is the case here).

[sunnyps, 2016/07/27 22:44:38]

Just a nit about the comment.

The comment seems to imply that we need to cancel the missed frame only if we
get another BeginFrame before the posted missed frame. But we also need to
cancel it when missed frame callback is called.

CancelableCallback doesn't cancel the callback on its own even after running the
callback.

+  // missed frame. (Do this last because this might be the missed frame and

[sunnyps, 2016/07/27 05:21:02]

nit: dangling parens

[danakj, 2016/07/27 22:34:55]

Done.

+  // we don't want to destroy |args| prematurely.
+  missed_begin_frame_task_.Cancel();
+
   return true;
 }
 
@@ -283,6 +309,9 @@ void DisplayScheduler::AttemptDrawAndSwap() {
     if (observing_begin_frame_source_) {
       observing_begin_frame_source_ = false;
       begin_frame_source_->RemoveObserver(this);
+      // A missed BeginFrame may be queued, so drop that too if we're going to
+      // stop listening.
+      missed_begin_frame_task_.Cancel();
     }
   }
 }