Call DidNotProduceFrame if begin frames are not needed

Begin frames were being processed when there was no BeginFrameObserver,
occasionally causing |last_begin_frame_args_| in
ExternalBeginFrameSource to be overwritten. These lost frames create a
backlog in |pipeline_reporting_frame_times_|, which eventually causes a
DCHECK.

Remember whether or not begin frames are needed. If they aren't, call
DidNotProduceFrame() instead of OnBeginFrame(). This mirrors a check in
AsyncLayerTreeFrameSink::OnBeginFrame().

Bug: 916354
Change-Id: Id5361ec6c48044a21c4946585f0da5ef862dd6d5
Reviewed-on: https://chromium-review.googlesource.com/c/1393441Reviewed-by: kylechar <kylechar@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619883}

cc/mojo_embedder/async_layer_tree_frame_sink.cc

@@ -329,7 +329,8 @@ void AsyncLayerTreeFrameSink::OnBeginFrame(
                            TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT,
                            "step", "ReceiveBeginFrameDiscard");
     // We had a race with SetNeedsBeginFrame(false) and still need to let the
-    // sink know that we didn't use this BeginFrame.
+    // sink know that we didn't use this BeginFrame. OnBeginFrame() can also be
+    // called to deliver presentation feedback.
     DidNotProduceFrame(viz::BeginFrameAck(args, false));
     return;
   }

components/viz/service/frame_sinks/direct_layer_tree_frame_sink.cc

@@ -327,6 +327,20 @@ void DirectLayerTreeFrameSink::OnBeginFrame(
           base::TimeDelta::FromMilliseconds(100), 50);
     }
   }
+  if (!needs_begin_frames_) {
+    TRACE_EVENT_WITH_FLOW1("viz,benchmark", "Graphics.Pipeline",
+                           TRACE_ID_GLOBAL(args.trace_id),
+                           TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT,
+                           "step", "ReceiveBeginFrameDiscard");
+    // OnBeginFrame() can be called just to deliver presentation feedback, so
+    // report that we didn't use this BeginFrame.
+    DidNotProduceFrame(BeginFrameAck(args, false));
+    return;
+  }
+  TRACE_EVENT_WITH_FLOW1("viz,benchmark", "Graphics.Pipeline",
+                         TRACE_ID_GLOBAL(args.trace_id),
+                         TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT,
+                         "step", "ReceiveBeginFrame");
 
   begin_frame_source_->OnBeginFrame(args);
 }
@@ -340,8 +354,9 @@ void DirectLayerTreeFrameSink::OnBeginFramePausedChanged(bool paused) {
   begin_frame_source_->OnSetBeginFrameSourcePaused(paused);
 }
 
-void DirectLayerTreeFrameSink::OnNeedsBeginFrames(bool needs_begin_frame) {
-  support_->SetNeedsBeginFrame(needs_begin_frame);
+void DirectLayerTreeFrameSink::OnNeedsBeginFrames(bool needs_begin_frames) {
+  needs_begin_frames_ = needs_begin_frames;
+  support_->SetNeedsBeginFrame(needs_begin_frames);
 }
 
 void DirectLayerTreeFrameSink::OnContextLost() {

components/viz/service/frame_sinks/direct_layer_tree_frame_sink.h

@@ -99,7 +99,7 @@ class VIZ_SERVICE_EXPORT DirectLayerTreeFrameSink
   void OnBeginFramePausedChanged(bool paused) override;
 
   // ExternalBeginFrameSourceClient implementation:
-  void OnNeedsBeginFrames(bool needs_begin_frame) override;
+  void OnNeedsBeginFrames(bool needs_begin_frames) override;
 
   // ContextLostObserver implementation:
   void OnContextLost() override;
@@ -112,6 +112,7 @@ class VIZ_SERVICE_EXPORT DirectLayerTreeFrameSink
 
   std::unique_ptr<CompositorFrameSinkSupport> support_;
 
+  bool needs_begin_frames_ = false;
   const FrameSinkId frame_sink_id_;
   CompositorFrameSinkSupportManager* const support_manager_;
   FrameSinkManagerImpl* frame_sink_manager_;