Added BeginFrameId to BeginFrameArgs

source_id and sequence_number is used on BeginFrameArgs and BeginFrameAck for identifying the frame. This Change creates a single struct for these info. TBR=sky@chromium.org, eseckler@chromium.org, meacer@chromium.org Bug: none Change-Id: Iabafca83472af0facd39ada5f11c6ad2017bf84d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1966606 Commit-Queue: Behdad Bakhshinategh <behdadb@chromium.org> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org> Reviewed-by: Stephen Chenney <schenney@chromium.org> Cr-Commit-Position: refs/heads/master@{#726018}
Denger-Network · Dec 18, 2019 · 2adc423 · 2adc423
1 parent 26f7fd9
commit 2adc423
Show file tree

Hide file tree

Showing 41 changed files with 326 additions and 297 deletions.
diff --git a/ash/components/fast_ink/fast_ink_view.cc b/ash/components/fast_ink/fast_ink_view.cc
@@ -144,10 +144,9 @@ class FastInkView::LayerTreeFrameSinkHolder
     // Submit an empty frame to ensure that pending release callbacks will be
     // processed in a finite amount of time.
     viz::CompositorFrame frame;
-    frame.metadata.begin_frame_ack.source_id =
-        viz::BeginFrameArgs::kManualSourceId;
-    frame.metadata.begin_frame_ack.sequence_number =
-        viz::BeginFrameArgs::kStartingFrameNumber;
+    frame.metadata.begin_frame_ack.frame_id =
+        viz::BeginFrameId(viz::BeginFrameArgs::kManualSourceId,
+                          viz::BeginFrameArgs::kStartingFrameNumber);
     frame.metadata.begin_frame_ack.has_damage = true;
     frame.metadata.device_scale_factor =
         holder->last_frame_device_scale_factor_;

diff --git a/cc/metrics/frame_sequence_tracker.cc b/cc/metrics/frame_sequence_tracker.cc
@@ -426,20 +426,19 @@ void FrameSequenceTracker::ReportBeginImplFrame(
   if (termination_status_ != TerminationStatus::kActive)
     return;
 
-  if (ShouldIgnoreBeginFrameSource(args.source_id))
+  if (ShouldIgnoreBeginFrameSource(args.frame_id.source_id))
     return;
 
 #if DCHECK_IS_ON()
   DCHECK(!is_inside_frame_) << TRACKER_DCHECK_MSG;
   is_inside_frame_ = true;
 
   if (args.type == viz::BeginFrameArgs::NORMAL)
-    impl_frames_.insert(std::make_pair(args.source_id, args.sequence_number));
+    impl_frames_.insert(args.frame_id);
 #endif
-
-  TRACKER_TRACE_STREAM << "b(" << args.sequence_number << ")";
-  UpdateTrackedFrameData(&begin_impl_frame_data_, args.source_id,
-                         args.sequence_number);
+  TRACKER_TRACE_STREAM << "b(" << args.frame_id.sequence_number << ")";
+  UpdateTrackedFrameData(&begin_impl_frame_data_, args.frame_id.source_id,
+                         args.frame_id.sequence_number);
   impl_throughput().frames_expected +=
       begin_impl_frame_data_.previous_sequence_delta;
 
@@ -452,28 +451,26 @@ void FrameSequenceTracker::ReportBeginMainFrame(
   if (termination_status_ != TerminationStatus::kActive)
     return;
 
-  if (ShouldIgnoreBeginFrameSource(args.source_id))
+  if (ShouldIgnoreBeginFrameSource(args.frame_id.source_id))
     return;
 
-  if (ShouldIgnoreSequence(args.sequence_number))
+  if (ShouldIgnoreSequence(args.frame_id.sequence_number))
     return;
 
 #if DCHECK_IS_ON()
   if (args.type == viz::BeginFrameArgs::NORMAL) {
-    DCHECK(impl_frames_.contains(
-        std::make_pair(args.source_id, args.sequence_number)));
+    DCHECK(impl_frames_.contains(args.frame_id));
   }
 #endif
 
   TRACKER_TRACE_STREAM << 'B';
   TRACKER_TRACE_STREAM << "(" << begin_main_frame_data_.previous_sequence << ","
-                       << args.sequence_number << ")";
-  UpdateTrackedFrameData(&begin_main_frame_data_, args.source_id,
-                         args.sequence_number);
+                       << args.frame_id.sequence_number << ")";
+  UpdateTrackedFrameData(&begin_main_frame_data_, args.frame_id.source_id,
+                         args.frame_id.sequence_number);
   if (!first_received_main_sequence_ ||
-      first_received_main_sequence_ <= last_no_main_damage_sequence_) {
-    first_received_main_sequence_ = args.sequence_number;
-  }
+      first_received_main_sequence_ <= last_no_main_damage_sequence_)
+    first_received_main_sequence_ = args.frame_id.sequence_number;
   main_throughput().frames_expected +=
       begin_main_frame_data_.previous_sequence_delta;
 }
@@ -484,8 +481,8 @@ void FrameSequenceTracker::ReportSubmitFrame(
     const viz::BeginFrameAck& ack,
     const viz::BeginFrameArgs& origin_args) {
   if (termination_status_ != TerminationStatus::kActive ||
-      ShouldIgnoreBeginFrameSource(ack.source_id) ||
-      ShouldIgnoreSequence(ack.sequence_number)) {
+      ShouldIgnoreBeginFrameSource(ack.frame_id.source_id) ||
+      ShouldIgnoreSequence(ack.frame_id.sequence_number)) {
     ignored_frame_tokens_.insert(frame_token);
     return;
   }
@@ -500,20 +497,20 @@ void FrameSequenceTracker::ReportSubmitFrame(
   TRACKER_TRACE_STREAM << 's';
   const bool main_changes_after_sequence_started =
       first_received_main_sequence_ &&
-      origin_args.sequence_number >= first_received_main_sequence_;
+      origin_args.frame_id.sequence_number >= first_received_main_sequence_;
   const bool main_changes_include_new_changes =
       last_submitted_main_sequence_ == 0 ||
-      origin_args.sequence_number > last_submitted_main_sequence_;
+      origin_args.frame_id.sequence_number > last_submitted_main_sequence_;
   const bool main_change_had_no_damage =
       last_no_main_damage_sequence_ != 0 &&
-      origin_args.sequence_number == last_no_main_damage_sequence_;
+      origin_args.frame_id.sequence_number == last_no_main_damage_sequence_;
 
-  if (!ShouldIgnoreBeginFrameSource(origin_args.source_id) &&
+  if (!ShouldIgnoreBeginFrameSource(origin_args.frame_id.source_id) &&
       main_changes_after_sequence_started && main_changes_include_new_changes &&
       !main_change_had_no_damage) {
     TRACKER_TRACE_STREAM << 'S';
 
-    last_submitted_main_sequence_ = origin_args.sequence_number;
+    last_submitted_main_sequence_ = origin_args.frame_id.sequence_number;
     main_frames_.push_back(frame_token);
     DCHECK_GE(main_throughput().frames_expected, main_frames_.size())
         << TRACKER_DCHECK_MSG;
@@ -529,15 +526,15 @@ void FrameSequenceTracker::ReportFrameEnd(const viz::BeginFrameArgs& args) {
   if (termination_status_ != TerminationStatus::kActive)
     return;
 
-  if (ShouldIgnoreBeginFrameSource(args.source_id))
+  if (ShouldIgnoreBeginFrameSource(args.frame_id.source_id))
     return;
 
-  if (ShouldIgnoreSequence(args.sequence_number)) {
+  if (ShouldIgnoreSequence(args.frame_id.sequence_number)) {
     is_inside_frame_ = false;
     return;
   }
 
-  TRACKER_TRACE_STREAM << "e(" << args.sequence_number << ")";
+  TRACKER_TRACE_STREAM << "e(" << args.frame_id.sequence_number << ")";
   DCHECK(is_inside_frame_) << TRACKER_DCHECK_MSG;
   is_inside_frame_ = false;
 #endif
@@ -635,12 +632,12 @@ void FrameSequenceTracker::ReportImplFrameCausedNoDamage(
   if (termination_status_ != TerminationStatus::kActive)
     return;
 
-  if (ShouldIgnoreBeginFrameSource(ack.source_id))
+  if (ShouldIgnoreBeginFrameSource(ack.frame_id.source_id))
     return;
 
   // It is possible that this is called before a begin-impl-frame has been
   // dispatched for this frame-sequence. In such cases, ignore this call.
-  if (ShouldIgnoreSequence(ack.sequence_number))
+  if (ShouldIgnoreSequence(ack.frame_id.sequence_number))
     return;
 
   TRACKER_TRACE_STREAM << 'n';
@@ -650,7 +647,7 @@ void FrameSequenceTracker::ReportImplFrameCausedNoDamage(
       << TRACKER_DCHECK_MSG;
   --impl_throughput().frames_expected;
 
-  if (begin_impl_frame_data_.previous_sequence == ack.sequence_number)
+  if (begin_impl_frame_data_.previous_sequence == ack.frame_id.sequence_number)
     begin_impl_frame_data_.previous_sequence = 0;
 }
 
@@ -659,25 +656,25 @@ void FrameSequenceTracker::ReportMainFrameCausedNoDamage(
   if (termination_status_ != TerminationStatus::kActive)
     return;
 
-  if (ShouldIgnoreBeginFrameSource(args.source_id))
+  if (ShouldIgnoreBeginFrameSource(args.frame_id.source_id))
     return;
 
-  if (ShouldIgnoreSequence(args.sequence_number))
+  if (ShouldIgnoreSequence(args.frame_id.sequence_number))
     return;
 
   TRACKER_TRACE_STREAM << 'N';
   TRACKER_TRACE_STREAM << "(" << begin_main_frame_data_.previous_sequence << ","
-                       << args.sequence_number << ")";
+                       << args.frame_id.sequence_number << ")";
   DCHECK_GT(main_throughput().frames_expected, 0u) << TRACKER_DCHECK_MSG;
   DCHECK_GT(main_throughput().frames_expected,
             main_throughput().frames_produced)
       << TRACKER_DCHECK_MSG;
-  last_no_main_damage_sequence_ = args.sequence_number;
+  last_no_main_damage_sequence_ = args.frame_id.sequence_number;
   --main_throughput().frames_expected;
   DCHECK_GE(main_throughput().frames_expected, main_frames_.size())
       << TRACKER_DCHECK_MSG;
 
-  if (begin_main_frame_data_.previous_sequence == args.sequence_number)
+  if (begin_main_frame_data_.previous_sequence == args.frame_id.sequence_number)
     begin_main_frame_data_.previous_sequence = 0;
 }
 

diff --git a/cc/metrics/frame_sequence_tracker.h b/cc/metrics/frame_sequence_tracker.h
@@ -27,6 +27,7 @@ struct PresentationFeedback;
 namespace viz {
 struct BeginFrameAck;
 struct BeginFrameArgs;
+struct BeginFrameId;
 }  // namespace viz
 
 namespace cc {
@@ -395,7 +396,7 @@ class CC_EXPORT FrameSequenceTracker {
 
   // If ReportBeginImplFrame is never called on a arg, then ReportBeginMainFrame
   // should ignore that arg.
-  base::flat_set<std::pair<uint64_t, uint64_t>> impl_frames_;
+  base::flat_set<viz::BeginFrameId> impl_frames_;
 #endif
 };
 

diff --git a/cc/mojo_embedder/async_layer_tree_frame_sink.cc b/cc/mojo_embedder/async_layer_tree_frame_sink.cc
@@ -160,8 +160,7 @@ void AsyncLayerTreeFrameSink::SubmitCompositorFrame(
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   DCHECK(compositor_frame_sink_ptr_);
   DCHECK(frame.metadata.begin_frame_ack.has_damage);
-  DCHECK_LE(viz::BeginFrameArgs::kStartingFrameNumber,
-            frame.metadata.begin_frame_ack.sequence_number);
+  DCHECK(frame.metadata.begin_frame_ack.frame_id.IsSequenceValid());
 
   // It's possible to request an immediate composite from cc which will bypass
   // BeginFrame. In that case, we cannot collect full graphics pipeline data.
@@ -247,7 +246,7 @@ void AsyncLayerTreeFrameSink::DidNotProduceFrame(
     const viz::BeginFrameAck& ack) {
   DCHECK(compositor_frame_sink_ptr_);
   DCHECK(!ack.has_damage);
-  DCHECK_LE(viz::BeginFrameArgs::kStartingFrameNumber, ack.sequence_number);
+  DCHECK(ack.frame_id.IsSequenceValid());
 
   // TODO(yiyix): Remove duplicated calls of DidNotProduceFrame from the same
   // BeginFrames. https://crbug.com/881949

diff --git a/cc/scheduler/scheduler.cc b/cc/scheduler/scheduler.cc
@@ -595,8 +595,7 @@ void Scheduler::FinishImplFrame() {
 
 void Scheduler::SendDidNotProduceFrame(const viz::BeginFrameArgs& args,
                                        FrameSkippedReason reason) {
-  if (last_begin_frame_ack_.source_id == args.source_id &&
-      last_begin_frame_ack_.sequence_number == args.sequence_number)
+  if (last_begin_frame_ack_.frame_id == args.frame_id)
     return;
   last_begin_frame_ack_ = viz::BeginFrameAck(args, false /* has_damage */);
   client_->DidNotProduceFrame(last_begin_frame_ack_, reason);
@@ -617,8 +616,7 @@ void Scheduler::BeginImplFrame(const viz::BeginFrameArgs& args,
     base::AutoReset<bool> mark_inside(&inside_scheduled_action_, true);
 
     begin_impl_frame_tracker_.Start(args);
-    state_machine_.OnBeginImplFrame(args.source_id, args.sequence_number,
-                                    args.animate_only);
+    state_machine_.OnBeginImplFrame(args.frame_id, args.animate_only);
     devtools_instrumentation::DidBeginFrame(layer_tree_host_id_);
     compositor_timing_history_->WillBeginImplFrame(
         args, state_machine_.NewActiveTreeLikely(), now);

diff --git a/cc/scheduler/scheduler_state_machine.cc b/cc/scheduler/scheduler_state_machine.cc
@@ -1133,8 +1133,7 @@ bool SchedulerStateMachine::ProactiveBeginFrameWanted() const {
   return false;
 }
 
-void SchedulerStateMachine::OnBeginImplFrame(uint64_t source_id,
-                                             uint64_t sequence_number,
+void SchedulerStateMachine::OnBeginImplFrame(const viz::BeginFrameId& frame_id,
                                              bool animate_only) {
   begin_impl_frame_state_ = BeginImplFrameState::INSIDE_BEGIN_FRAME;
   current_frame_number_++;

diff --git a/cc/scheduler/scheduler_state_machine.h b/cc/scheduler/scheduler_state_machine.h
@@ -172,9 +172,7 @@ class CC_EXPORT SchedulerStateMachine {
   // Indicates that the system has entered and left a BeginImplFrame callback.
   // The scheduler will not draw more than once in a given BeginImplFrame
   // callback nor send more than one BeginMainFrame message.
-  void OnBeginImplFrame(uint64_t source_id,
-                        uint64_t sequence_number,
-                        bool animate_only);
+  void OnBeginImplFrame(const viz::BeginFrameId& frame_id, bool animate_only);
   // Indicates that the scheduler has entered the draw phase. The scheduler
   // will not draw more than once in a single draw phase.
   // TODO(sunnyps): Rename OnBeginImplFrameDeadline to OnDraw or similar.