viz: Refactor BeginFrameArgs creation in BeginFrameSource.

The sequence numbers in BeginFrameArgs are expected to be incremented
based on vsync intervals. Refactor the code for this from
DelayBasedBeginFrameSource and ExternalBeginFrameSourceAndroid into a
BeginFrameArgsGenerator.

This is a simple refactor of the code, and does not change behaviour.

BUG=none

Change-Id: Idb51e1306dc6e6c64acbe2027cc28df0ba440055
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2058032Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Auto-Submit: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742256}

components/viz/common/frame_sinks/begin_frame_source.cc

@@ -24,11 +24,6 @@ namespace {
 // often to an observer.
 constexpr double kDoubleTickDivisor = 2.0;
 
-// kErrorMarginIntervalPct used to determine what percentage of the time tick
-// interval should be used as a margin of error when comparing times to
-// deadlines.
-constexpr double kErrorMarginIntervalPct = 0.05;
-
 base::AtomicSequenceNumber g_next_source_id;
 
 // Generates a source_id with upper 32 bits from |restart_id| and lower 32 bits
@@ -102,6 +97,40 @@ void BeginFrameObserverBase::AsProtozeroInto(
   last_begin_frame_args_.AsProtozeroInto(state->set_last_begin_frame_args());
 }
 
+BeginFrameArgs
+BeginFrameSource::BeginFrameArgsGenerator::GenerateBeginFrameArgs(
+    uint64_t source_id,
+    base::TimeTicks frame_time,
+    base::TimeTicks next_frame_time,
+    base::TimeDelta vsync_interval) {
+  uint64_t sequence_number =
+      next_sequence_number_ +
+      EstimateTickCountsBetween(frame_time, next_expected_frame_time_,
+                                vsync_interval);
+  next_expected_frame_time_ = next_frame_time;
+  next_sequence_number_ = sequence_number + 1;
+  return BeginFrameArgs::Create(BEGINFRAME_FROM_HERE, source_id,
+                                sequence_number, frame_time, next_frame_time,
+                                vsync_interval, BeginFrameArgs::NORMAL);
+}
+
+uint64_t BeginFrameSource::BeginFrameArgsGenerator::EstimateTickCountsBetween(
+    base::TimeTicks frame_time,
+    base::TimeTicks next_expected_frame_time,
+    base::TimeDelta vsync_interval) {
+  if (next_expected_frame_time.is_null())
+    return 0;
+
+  // kErrorMarginIntervalPct used to determine what percentage of the time tick
+  // interval should be used as a margin of error when comparing times to
+  // deadlines.
+  constexpr double kErrorMarginIntervalPct = 0.05;
+  base::TimeDelta error_margin = vsync_interval * kErrorMarginIntervalPct;
+  int ticks_since_estimated_frame_time =
+      (frame_time + error_margin - next_expected_frame_time) / vsync_interval;
+  return std::max(0, ticks_since_estimated_frame_time);
+}
+
 // BeginFrameSource -------------------------------------------------------
 
 // static
@@ -242,8 +271,7 @@ DelayBasedBeginFrameSource::DelayBasedBeginFrameSource(
     std::unique_ptr<DelayBasedTimeSource> time_source,
     uint32_t restart_id)
     : SyntheticBeginFrameSource(restart_id),
-      time_source_(std::move(time_source)),
-      next_sequence_number_(BeginFrameArgs::kStartingFrameNumber) {
+      time_source_(std::move(time_source)) {
   time_source_->SetClient(this);
 }
 
@@ -264,29 +292,8 @@ void DelayBasedBeginFrameSource::OnUpdateVSyncParameters(
 BeginFrameArgs DelayBasedBeginFrameSource::CreateBeginFrameArgs(
     base::TimeTicks frame_time) {
   base::TimeDelta interval = time_source_->Interval();
-  uint64_t sequence_number = next_sequence_number_;
-
-  base::TimeDelta error_margin = interval * kErrorMarginIntervalPct;
-
-  // We expect |sequence_number| to be the number for the frame at
-  // |expected_frame_time|. We adjust this sequence number according to the
-  // actual frame time in case it is later than expected.
-  if (next_expected_frame_time_ != base::TimeTicks()) {
-    // Add |error_margin| to round |frame_time| up to the next tick if it is
-    // close to the end of an interval. This happens when a timebase is a bit
-    // off because of an imperfect presentation timestamp that may be a bit
-    // later than the beginning of the next interval.
-    int ticks_since_estimated_frame_time =
-        (frame_time + error_margin - next_expected_frame_time_) / interval;
-    sequence_number += std::max(0, ticks_since_estimated_frame_time);
-  }
-
-  next_expected_frame_time_ = time_source_->NextTickTime();
-  next_sequence_number_ = sequence_number + 1;
-
-  return BeginFrameArgs::Create(
-      BEGINFRAME_FROM_HERE, source_id(), sequence_number, frame_time,
-      time_source_->NextTickTime(), interval, BeginFrameArgs::NORMAL);
+  return begin_frame_args_generator_.GenerateBeginFrameArgs(
+      source_id(), frame_time, time_source_->NextTickTime(), interval);
 }
 
 void DelayBasedBeginFrameSource::AddObserver(BeginFrameObserver* obs) {

components/viz/common/frame_sinks/begin_frame_source.h

@@ -130,6 +130,34 @@ class VIZ_COMMON_EXPORT BeginFrameObserverBase : public BeginFrameObserver {
 // all BeginFrameSources *must* provide.
 class VIZ_COMMON_EXPORT BeginFrameSource {
  public:
+  class VIZ_COMMON_EXPORT BeginFrameArgsGenerator {
+   public:
+    BeginFrameArgsGenerator() = default;
+    ~BeginFrameArgsGenerator() = default;
+
+    BeginFrameArgs GenerateBeginFrameArgs(uint64_t source_id,
+                                          base::TimeTicks frame_time,
+                                          base::TimeTicks next_frame_time,
+                                          base::TimeDelta vsync_interval);
+
+   private:
+    static uint64_t EstimateTickCountsBetween(
+        base::TimeTicks frame_time,
+        base::TimeTicks next_expected_frame_time,
+        base::TimeDelta vsync_interval);
+
+    // Used for determining what the sequence number should be on
+    // CreateBeginFrameArgs.
+    base::TimeTicks next_expected_frame_time_;
+
+    // This is what the sequence number should be for any args created between
+    // |next_expected_frame_time_| to |next_expected_frame_time_| + vsync
+    // interval. Args created outside of this range will have their sequence
+    // number assigned relative to this, based on how many intervals the frame
+    // time is off.
+    uint64_t next_sequence_number_ = BeginFrameArgs::kStartingFrameNumber;
+  };
+
   // This restart_id should be used for BeginFrameSources that don't have to
   // worry about process restart. For example, if a BeginFrameSource won't
   // generate and forward BeginFrameArgs to another process or the process can't
@@ -302,16 +330,7 @@ class VIZ_COMMON_EXPORT DelayBasedBeginFrameSource
   base::TimeTicks last_timebase_;
   BeginFrameArgs last_begin_frame_args_;
 
-  // Used for determining what the sequence number should be on
-  // CreateBeginFrameArgs.
-  base::TimeTicks next_expected_frame_time_;
-
-  // This is what the sequence number should be for any args created between
-  // |next_expected_frame_time_| to |next_expected_frame_time_| + vsync
-  // interval. Args created outside of this range will have their sequence
-  // number assigned relative to this, based on how many intervals the frame
-  // time is off.
-  uint64_t next_sequence_number_;
+  BeginFrameArgsGenerator begin_frame_args_generator_;
 
   DISALLOW_COPY_AND_ASSIGN(DelayBasedBeginFrameSource);
 };

components/viz/service/frame_sinks/external_begin_frame_source_android.cc

@@ -40,31 +40,11 @@ void ExternalBeginFrameSourceAndroid::OnVSync(
       base::TimeTicks() + base::TimeDelta::FromMicroseconds(time_micros);
   base::TimeDelta vsync_period(
       base::TimeDelta::FromMicroseconds(period_micros));
-
-  uint64_t sequence_number = next_sequence_number_;
-  // We expect |sequence_number| to be the number for the frame at
-  // |expected_frame_time|. We adjust this sequence number according to the
-  // actual frame time in case it is later than expected.
-  if (next_expected_frame_time_ != base::TimeTicks()) {
-    // Add |error_margin| to round |frame_time| up to the next tick if it is
-    // close to the end of an interval. This happens when a timebase is a bit
-    // off because of an imperfect presentation timestamp that may be a bit
-    // later than the beginning of the next interval.
-    constexpr double kErrorMarginIntervalPct = 0.05;
-    base::TimeDelta error_margin = vsync_period * kErrorMarginIntervalPct;
-    int ticks_since_estimated_frame_time =
-        (frame_time + error_margin - next_expected_frame_time_) / vsync_period;
-    sequence_number += std::max(0, ticks_since_estimated_frame_time);
-  }
-
   // Calculate the next frame deadline:
   base::TimeTicks deadline = frame_time + vsync_period;
-  auto begin_frame_args = BeginFrameArgs::Create(
-      BEGINFRAME_FROM_HERE, source_id(), sequence_number, frame_time, deadline,
-      vsync_period, BeginFrameArgs::NORMAL);
-  next_sequence_number_ = sequence_number + 1;
-  next_expected_frame_time_ = deadline;
 
+  auto begin_frame_args = begin_frame_args_generator_.GenerateBeginFrameArgs(
+      source_id(), frame_time, deadline, vsync_period);
   OnBeginFrame(begin_frame_args);
 }
 

components/viz/service/frame_sinks/external_begin_frame_source_android.h

@@ -35,11 +35,8 @@ class VIZ_SERVICE_EXPORT ExternalBeginFrameSourceAndroid
 
   void SetEnabled(bool enabled);
 
-  uint64_t next_sequence_number_ = BeginFrameArgs::kStartingFrameNumber;
   base::android::ScopedJavaGlobalRef<jobject> j_object_;
-  // Used for determining what the sequence number should be on
-  // CreateBeginFrameArgs.
-  base::TimeTicks next_expected_frame_time_;
+  BeginFrameArgsGenerator begin_frame_args_generator_;
 
   DISALLOW_COPY_AND_ASSIGN(ExternalBeginFrameSourceAndroid);
 };