[cc] Pass on BeginFrameAcks from CompositorEBFS through RWHVAura, DFH.

cc/ipc/cc_param_traits_macros.h

Index: cc/ipc/cc_param_traits_macros.h
diff --git a/cc/ipc/cc_param_traits_macros.h b/cc/ipc/cc_param_traits_macros.h
index 42e97e419e13449a165990f1b0f48beaee13e291..e2a6a8d971abf6c4a88ee93708cf74f1ee6b9230 100644
--- a/cc/ipc/cc_param_traits_macros.h
+++ b/cc/ipc/cc_param_traits_macros.h
@@ -175,6 +175,14 @@ IPC_STRUCT_TRAITS_BEGIN(cc::BeginFrameArgs)
   IPC_STRUCT_TRAITS_MEMBER(type)
 IPC_STRUCT_TRAITS_END()
 
+IPC_STRUCT_TRAITS_BEGIN(cc::BeginFrameAck)
+  IPC_STRUCT_TRAITS_MEMBER(sequence_number)
+  IPC_STRUCT_TRAITS_MEMBER(latest_confirmed_sequence_number)

[piman, 2017/03/14 20:06:44]

Are there assumptions about this? E.g. I assume it would be invalid for the
latest_confirmed_sequence_number to be more than the latest sent BeginFrame
sequence_number? Should there be sanity checks in RenderWidgetHostImpl?

[Eric Seckler, 2017/03/15 11:56:48]

Since BeginFrameAcks can generally be sent asynchronously to the compositor, we
check for consistency / relevancy when we inspect the acks in the compositor. We
do this in BeginFrameObserverAckTracker and throw away any acks that aren't for
the current BeginFrame's sequence_number:
https://cs.chromium.org/chromium/src/cc/scheduler/begin_frame_source.cc?type=...

Given that, I'm not sure if we need to add more checks for these constraints in
the IPC receivers (RWHViews in this case)? Added a comment to
BeginFrameSource::DidFinishFrame() that the source shouldn't assume that the ack
is for a valid/current/past BeginFrame (which all implementors already do).

+  IPC_STRUCT_TRAITS_MEMBER(source_id)
+  IPC_STRUCT_TRAITS_MEMBER(remaining_frames)

[piman, 2017/03/14 20:06:44]

Ditto here, any assumptions relative to
sequence_number/latest_confirmed_sequence_number/last sent BeginFrame
sequence_number?

[Eric Seckler, 2017/03/15 11:56:48]

remaining_frames is a remnant that doesn't have a purpose anymore - it's always
0 AFAIK. Removing from the struct traits / mojom. There's already a TODO in
BeginFrameAck that we should get rid of it completely (it's a little less
straight forward b/c cc::BackToBackBeginFrameSource needs to be changed for
this).

+  IPC_STRUCT_TRAITS_MEMBER(has_damage)

[piman, 2017/03/14 20:06:44]

Is it necessary to send |has_damage| over IPC? It seems redundant with the info
in the CompositorFrame, and I worry about privileged code assuming consistency
between both, whereas a compromised/malicious renderer could generate
inconsistent data.

[Eric Seckler, 2017/03/15 11:56:48]

Yes, it's not necessary - we can set it when we receive the SwapCompositorFrame
/ BeginFrameDidNotSwap IPCs. Removed from struct traits / mojom.

+IPC_STRUCT_TRAITS_END()
+
 IPC_STRUCT_TRAITS_BEGIN(cc::CompositorFrameMetadata)
   IPC_STRUCT_TRAITS_MEMBER(device_scale_factor)
   IPC_STRUCT_TRAITS_MEMBER(root_scroll_offset)
@@ -198,6 +206,7 @@ IPC_STRUCT_TRAITS_BEGIN(cc::CompositorFrameMetadata)
   IPC_STRUCT_TRAITS_MEMBER(latency_info)
   IPC_STRUCT_TRAITS_MEMBER(referenced_surfaces)
   IPC_STRUCT_TRAITS_MEMBER(content_source_id)
+  IPC_STRUCT_TRAITS_MEMBER(begin_frame_ack)
 IPC_STRUCT_TRAITS_END()
 
 #endif  // CC_IPC_CC_PARAM_TRAITS_MACROS_H_