[cc] Don't send missed BeginFrames with past deadline.

cc/scheduler/begin_frame_source.cc

Index: cc/scheduler/begin_frame_source.cc
diff --git a/cc/scheduler/begin_frame_source.cc b/cc/scheduler/begin_frame_source.cc
index 714f505037b230186cd85245f8f87f9b1b372ad9..262a91fa985fa9e9696cc760b886c08a4562592a 100644
--- a/cc/scheduler/begin_frame_source.cc
+++ b/cc/scheduler/begin_frame_source.cc
@@ -198,17 +198,20 @@ void DelayBasedBeginFrameSource::AddObserver(BeginFrameObserver* obs) {
         CreateBeginFrameArgs(last_or_missed_tick_time, BeginFrameArgs::MISSED);
   }
 
-  BeginFrameArgs last_args = obs->LastUsedBeginFrameArgs();
-  if (!last_args.IsValid() ||
-      (current_begin_frame_args_.frame_time >
-       last_args.frame_time +
-           current_begin_frame_args_.interval / kDoubleTickDivisor)) {
-    DCHECK(current_begin_frame_args_.sequence_number >
-               last_args.sequence_number ||
-           current_begin_frame_args_.source_id != last_args.source_id)
-        << "current " << current_begin_frame_args_.AsValue()->ToString()
-        << ", last " << last_args.AsValue()->ToString();
-    obs->OnBeginFrame(current_begin_frame_args_);
+  // Avoid issuing missed BeginFrames with a past deadline.
+  if (current_begin_frame_args_.deadline > time_source_->Now()) {

[brianderson, 2017/03/24 19:57:28]

Can you swap the nested if's, which would avoid making a syscall to get the
timestamp in some cases?

[Eric Seckler, 2017/03/27 09:53:25]

Actually, I think that DelayBasedBFS can't ever issue past deadlines anyway.
(The time source query above should avoid that.) So removed this if-statement
again.

I did swap the if-branches in ExternalBFS though. And I'm also special-casing
deadline == 0 there now, since that's what android's sync-compositor uses.

+    BeginFrameArgs last_args = obs->LastUsedBeginFrameArgs();
+    if (!last_args.IsValid() ||
+        (current_begin_frame_args_.frame_time >
+         last_args.frame_time +
+             current_begin_frame_args_.interval / kDoubleTickDivisor)) {
+      DCHECK(current_begin_frame_args_.sequence_number >
+                 last_args.sequence_number ||
+             current_begin_frame_args_.source_id != last_args.source_id)
+          << "current " << current_begin_frame_args_.AsValue()->ToString()
+          << ", last " << last_args.AsValue()->ToString();
+      obs->OnBeginFrame(current_begin_frame_args_);
+    }
   }
 }
 
@@ -354,8 +357,10 @@ void ExternalBeginFrameSource::AddObserver(BeginFrameObserver* obs) {
   if (observers_was_empty)
     client_->OnNeedsBeginFrames(true);
 
-  // Send a MISSED begin frame if necessary.
-  if (missed_begin_frame_args_.IsValid()) {
+  // Send a MISSED begin frame if necessary, while avoiding issuing missed
+  // BeginFrames with a past deadline.
+  if (missed_begin_frame_args_.IsValid() &&
+      missed_begin_frame_args_.deadline > Now()) {

[brianderson, 2017/03/24 19:57:28]

The changes in this file duplicate logic in
Scheduler::BeginImplFrameWithDeadline. Can we remove the dupe in Scheduler?

[Eric Seckler, 2017/03/27 09:53:25]

I think it's fine to leave the logic in the cc::Scheduler, as the time may
change between when the browser issues a missed BF and when the renderer's
Scheduler receives it. (And since we need the call to Now() in Scheduler for
other reasons it doesn't add much overhead there.)

     const BeginFrameArgs& last_args = obs->LastUsedBeginFrameArgs();
     if (!last_args.IsValid() ||
         (missed_begin_frame_args_.frame_time > last_args.frame_time)) {
@@ -442,4 +447,8 @@ void ExternalBeginFrameSource::FinishFrame() {
   client_->OnDidFinishFrame(ack);
 }
 
+base::TimeTicks ExternalBeginFrameSource::Now() const {
+  return base::TimeTicks::Now();
+}
+
 }  // namespace cc