[throughput metrics] Fix handling when no frames are produced.

If a CompositorFrame is not produced in response to a begin-frame, then
it can be either because the compositor is attempting to recover from
high-latency, or because there was no update to the frame (i.e. there
was no damage). The latter cases should not cause the throughput metrics
to go down. This is currently handled for some cases (e.g. when the
main-frame is aborted due to no damage), but there are other cases where
the trackers are not notified, and thus they report lower throughput
metrics. To correct this, use the newly introduced FrameSkippedReason to
notify the trackers when a frame is not produced because there were no
damage.

BUG=1021963

Change-Id: I415e919738928c30322dd56cc67c23e4ebd73788
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1956567Reviewed-by: Xida Chen <xidachen@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#723586}

cc/trees/layer_tree_host_impl.cc

@@ -2665,7 +2665,12 @@ bool LayerTreeHostImpl::WillBeginImplFrame(const viz::BeginFrameArgs& args) {
   frame_trackers_.NotifyBeginImplFrame(args);
 
   begin_main_frame_expected_during_impl_ = client_->IsBeginMainFrameExpected();
-  begin_main_frame_sent_during_impl_ = false;
+  if (begin_main_frame_expected_during_impl_) {
+    frame_trackers_.NotifyBeginMainFrame(args);
+    begin_main_frame_sent_during_impl_ = true;
+  } else {
+    begin_main_frame_sent_during_impl_ = false;
+  }
 
   if (is_likely_to_require_a_draw_) {
     // Optimistically schedule a draw. This will let us expect the tile manager
@@ -2728,7 +2733,26 @@ void LayerTreeHostImpl::DidNotProduceFrame(const viz::BeginFrameAck& ack,
                                            FrameSkippedReason reason) {
   if (layer_tree_frame_sink_)
     layer_tree_frame_sink_->DidNotProduceFrame(ack);
-  // TODO(sad): Notify |frame_trackers_| if |reason| is no-damage.
+
+  // If a frame was not submitted because there was no damage, then notify the
+  // trackers.
+  if (reason == FrameSkippedReason::kNoDamage &&
+      impl_thread_phase_ == ImplThreadPhase::INSIDE_IMPL_FRAME) {
+    // It is possible that |ack| is for a 'future frame', i.e. for the next
+    // frame from the one currently being handled by the compositor (represented
+    // by the BeginFrameArgs instance in |current_begin_frame_tracker_|). This
+    // can happen, for example, when a frame is skipped early for
+    // latency-recovery, while the previous frame is still being processed.
+    // Notify the trackers only when this is *not* the case (since the trackers
+    // are not notified about the start of the future frame either).
+    const auto& args = current_begin_frame_tracker_.Current();
+    if (args.source_id == ack.source_id &&
+        args.sequence_number == ack.sequence_number) {
+      frame_trackers_.NotifyImplFrameCausedNoDamage(ack);
+      if (begin_main_frame_sent_during_impl_)
+        frame_trackers_.NotifyMainFrameCausedNoDamage(args);
+    }
+  }
 }
 
 void LayerTreeHostImpl::SynchronouslyInitializeAllTiles() {