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

cc/scheduler/begin_frame_source.cc

 // Copyright 2014 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/begin_frame_source.h"
 
 #include <stddef.h>
 
 #include "base/atomic_sequence_num.h"
 #include "base/auto_reset.h"
@@ skipping 180 matching lines @@
       current_begin_frame_args_.interval == time_source_->Interval()) {
     // Ensure that the args have the right type.
     current_begin_frame_args_.type = BeginFrameArgs::MISSED;
   } else {
     // The args are not up to date and we need to create new ones with the
     // missed tick's time and a new sequence number.
     current_begin_frame_args_ =
         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_);
+    }
   }
 }
 
 void DelayBasedBeginFrameSource::RemoveObserver(BeginFrameObserver* obs) {
   DCHECK(obs);
   DCHECK(observers_.find(obs) != observers_.end());
 
   observers_.erase(obs);
   if (observers_.empty())
     time_source_->SetActive(false);
@@ skipping 125 matching lines @@
   DCHECK(obs);
   DCHECK(observers_.find(obs) == observers_.end());
 
   bool observers_was_empty = observers_.empty();
   observers_.insert(obs);
   ack_tracker_.OnObserverAdded(obs);
   obs->OnBeginFrameSourcePausedChanged(paused_);
   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)) {
       DCHECK((missed_begin_frame_args_.source_id != last_args.source_id) ||
              (missed_begin_frame_args_.sequence_number >
               last_args.sequence_number))
           << "current " << missed_begin_frame_args_.AsValue()->ToString()
           << ", last " << last_args.AsValue()->ToString();
       obs->OnBeginFrame(missed_begin_frame_args_);
     }
@@ skipping 66 matching lines @@
 void ExternalBeginFrameSource::FinishFrame() {
   frame_active_ = false;
 
   BeginFrameAck ack(missed_begin_frame_args_.source_id,
                     missed_begin_frame_args_.sequence_number,
                     ack_tracker_.LatestConfirmedSequenceNumber(), 0,
                     ack_tracker_.AnyObserversHadDamage());
   client_->OnDidFinishFrame(ack);
 }
 
+base::TimeTicks ExternalBeginFrameSource::Now() const {
+  return base::TimeTicks::Now();
+}
+
 }  // namespace cc