[cc] CompositorFrameSinkSupport: Defer BeginFrameAck of pending frames.

cc/surfaces/compositor_frame_sink_support.h

Index: cc/surfaces/compositor_frame_sink_support.h
diff --git a/cc/surfaces/compositor_frame_sink_support.h b/cc/surfaces/compositor_frame_sink_support.h
index bdafb4933e6c0f752edc1fde38a9d63fd003d6b5..087415d0d034efd21c8f61f9ace011486f7b5d3b 100644
--- a/cc/surfaces/compositor_frame_sink_support.h
+++ b/cc/surfaces/compositor_frame_sink_support.h
@@ -68,6 +68,7 @@ class CC_SURFACES_EXPORT CompositorFrameSinkSupport
   void RemoveTopLevelRootReference(const SurfaceId& surface_id);
 
   void DidReceiveCompositorFrameAck();
+  void PendingFrameDiscarded();
 
   // SurfaceFactoryClient implementation.
   void ReferencedSurfacesChanged(
@@ -78,6 +79,8 @@ class CC_SURFACES_EXPORT CompositorFrameSinkSupport
   void SetBeginFrameSource(BeginFrameSource* begin_frame_source) override;
   void WillDrawSurface(const LocalSurfaceId& local_surface_id,
                        const gfx::Rect& damage_rect) override;
+  void PendingFrameActivated(const LocalSurfaceId& local_surface_id) override;
+  void SurfaceDiscarded(const LocalSurfaceId& local_surface_id) override;
 
   // BeginFrameObserver implementation.
   void OnBeginFrame(const BeginFrameArgs& args) override;
@@ -85,6 +88,8 @@ class CC_SURFACES_EXPORT CompositorFrameSinkSupport
   void OnBeginFrameSourcePausedChanged(bool paused) override;
 
   void UpdateNeedsBeginFramesInternal();
+  void AcknowledgeBeginFrame(const BeginFrameAck& ack);

[Fady Samuel, 2017/03/31 00:09:02]

nit: comment.

[Eric Seckler, 2017/03/31 15:24:03]

Done.

+  void AcknowledgeLastBeginFrameWithoutUpdates();

[Fady Samuel, 2017/03/31 00:09:02]

nit: comment.

[Eric Seckler, 2017/03/31 15:24:03]

Done.

 
   CompositorFrameSinkSupportClient* const client_;
 
@@ -98,14 +103,29 @@ class CC_SURFACES_EXPORT CompositorFrameSinkSupport
   int ack_pending_count_ = 0;
   ReturnedResourceArray surface_returned_resources_;
 
+  // Whether a CompositorFrame is currently pending to be activated.
+  bool has_pending_frame_ = false;

[Fady Samuel, 2017/03/31 00:09:02]

Surface already knows this.

[Eric Seckler, 2017/03/31 15:24:03]

not quite: CFSSupport only acknowledges when a CompositorFrame that it itself
submitted is activated, not if some other CompositorFrame activates (e.g. one
queued by ClearSurface). Therefore, has_pending_frame_ was only tracking
CFSSupport-submitted pending frames.

but no need for this field with base::Optional below.

+  LocalSurfaceId surface_id_for_pending_frame_;

[Fady Samuel, 2017/03/31 00:09:02]

Do we need this?

[Eric Seckler, 2017/03/31 15:24:03]

yeah, see comment in SurfaceDiscarded: If we submit a CompositorFrame to a new
local_surface_id, the old surface will be destroyed after the frame becomes
pending.

+
   // The begin frame source being observered. Null if none.
   BeginFrameSource* begin_frame_source_ = nullptr;
 
   // The last begin frame args generated by the begin frame source.
   BeginFrameArgs last_begin_frame_args_;
 
+  // The BeginFrameAck that will be issued when the last submitted
+  // CompositorFrame is activated.
+  BeginFrameAck begin_frame_ack_for_pending_frame_;

[Fady Samuel, 2017/03/31 00:09:02]

Maybe either make this base::Optional<BeginFrameAck> or simplify modify in
place?

[Eric Seckler, 2017/03/31 15:24:03]

base::Optional it is! modify-in-place might be an option to explore, too. Not
sure if it would be simpler though.

+
+  // Whether the last BeginFrame was forwarded to the client.
+  bool last_begin_frame_sent_to_client_ = true;
+
+  uint32_t latest_confirmed_begin_frame_source_id_ = 0;
+  uint64_t latest_confirmed_begin_frame_sequence_number_ =
+      BeginFrameArgs::kInvalidFrameNumber;
+
   // Whether a request for begin frames has been issued.
-  bool needs_begin_frame_ = false;
+  bool client_needs_begin_frame_ = false;
 
   // Whether or not a frame observer has been added.
   bool added_frame_observer_ = false;