cc: Post the missed BeginFrame to a new stack

cc/surfaces/display_scheduler.cc

 // Copyright 2015 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/surfaces/display_scheduler.h"
 
 #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"
 
 namespace cc {
 
 DisplayScheduler::DisplayScheduler(BeginFrameSource* begin_frame_source,
                                    base::SingleThreadTaskRunner* task_runner,
                                    int max_pending_swaps)
     : begin_frame_source_(begin_frame_source),
@@ skipping 71 matching lines @@
   } else {
     child_surface_ids_damaged_.insert(surface_id);
 
     // TODO(mithro): Use hints from SetNeedsBeginFrames and SwapAborts.
     all_active_child_surfaces_ready_to_draw_ = base::STLIncludes(
         child_surface_ids_damaged_, child_surface_ids_to_expect_damage_from_);
   }
 
   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);
   }
 
   ScheduleBeginFrameDeadline();
 }
 
 void DisplayScheduler::OutputSurfaceLost() {
   TRACE_EVENT0("cc", "DisplayScheduler::OutputSurfaceLost");
   output_surface_lost_ = true;
   ScheduleBeginFrameDeadline();
@@ skipping 21 matching lines @@
 
   expect_damage_from_root_surface_ = root_surface_damaged_;
   root_surface_damaged_ = false;
 }
 
 bool DisplayScheduler::OnBeginFrameDerivedImpl(const BeginFrameArgs& args) {
   base::TimeTicks now = base::TimeTicks::Now();
   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_) {
     OnBeginFrameDeadline();
   }
 
   // Schedule the deadline.
   current_begin_frame_args_ = args;
   current_begin_frame_args_.deadline -=
       BeginFrameArgs::DefaultEstimatedParentDrawTime();
   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;
 }
 
 void DisplayScheduler::OnBeginFrameSourcePausedChanged(bool paused) {
   // BeginFrameSources used with DisplayScheduler do not make use of this
   // feature.
   if (paused)
     NOTIMPLEMENTED();
 }
 
@@ skipping 112 matching lines @@
     // We are going idle, so reset expectations.
     child_surface_ids_to_expect_damage_from_.clear();
     child_surface_ids_damaged_prev_.clear();
     child_surface_ids_damaged_.clear();
     all_active_child_surfaces_ready_to_draw_ = true;
     expect_damage_from_root_surface_ = false;
 
     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();
     }
   }
 }
 
 void DisplayScheduler::OnBeginFrameDeadline() {
   TRACE_EVENT0("cc", "DisplayScheduler::OnBeginFrameDeadline");
 
   AttemptDrawAndSwap();
   begin_frame_source_->DidFinishFrame(this, 0);
 }
 
 void DisplayScheduler::DidSwapBuffers() {
   pending_swaps_++;
   TRACE_EVENT_ASYNC_BEGIN1("cc", "DisplayScheduler:pending_swaps", this,
                            "pending_frames", pending_swaps_);
 }
 
 void DisplayScheduler::DidSwapBuffersComplete() {
   pending_swaps_--;
   TRACE_EVENT_ASYNC_END1("cc", "DisplayScheduler:pending_swaps", this,
                          "pending_frames", pending_swaps_);
   ScheduleBeginFrameDeadline();
 }
 
 }  // namespace cc