Plumb activation time to main

cc/output/begin_frame_args.h

Index: cc/output/begin_frame_args.h
diff --git a/cc/output/begin_frame_args.h b/cc/output/begin_frame_args.h
index 41b156ad708d33692eed652b8c58a4466df3b4f7..fa402b9f9f8df60fd241d587e9be094b4d2e24a3 100644
--- a/cc/output/begin_frame_args.h
+++ b/cc/output/begin_frame_args.h
@@ -79,6 +79,7 @@ struct CC_EXPORT BeginFrameArgs {
                                base::TimeTicks deadline,
                                base::TimeDelta interval,
                                BeginFrameArgsType type);
+  BeginFrameArgs(const BeginFrameArgs&);
 
   // This is the default delta that will be used to adjust the deadline when
   // proper draw-time estimations are not yet available.
@@ -103,6 +104,12 @@ struct CC_EXPORT BeginFrameArgs {
   // observers should expect the continuity of |sequence_number| to break.
   uint64_t sequence_number;
   uint32_t source_id;  // |source_id| after |sequence_number| for packing.
+  uint32_t frame_source_number;
+
+  // TODO: move these into a separate struct?
+  // TODO: report a time delta - otherwise this doesn't make sense
+  base::TimeTicks ready_to_activate_time;
+  uint32_t ready_to_activate_source_frame_number;

[brianderson, 2017/03/30 18:32:03]

BeginFrameArgs are used everywhere, but these fields only apply to the main|impl
interface. It's probably better to store a vector of a new struct with these
fields on either 1) the ProxyImpl to be added to the
BeginMainFrameAndCommitState or 2) a new BeginMainFrameArgs class and store/pass
that around accordingly.

[panicker, 2017/03/30 21:31:25]

The reason I added it to BeginFrameArgs instead of BeginMainFrameAndCommitState
is that we need to send this down to the Blink Scheduler which is called from
here:
https://cs.chromium.org/chromium/src/content/renderer/gpu/render_widget_compo...
https://cs.chromium.org/chromium/src/cc/trees/layer_tree_host.cc?dr=C&l=241
render_widget_compositor is a client of layer_tree_host and we don't (and
shouldn't?) pass BeginMainFrameAndCommitState through all these layers.

Could you clarify your #2 suggestion - what do you mean by "new
BeginMainFrameArgs class"?

[panicker, 2017/03/30 23:09:19]

I tried moving to BeginMainFrameAndCommitState - however:
a. this is not available at all in the single_thread_proxy codepath, so we still
need to pass around BeginFrameArgs
b. BeginMainFrameAndCommitState is now getting exposed to browser eg.
content/renderer/gpu/render_widget_compositor.h, ui/compositor/compositor.h and
in the public interface here:
third_party/WebKit/public/platform/scheduler/renderer/renderer_scheduler.h
c. including "cc/trees/proxy_common.h" from
platform/scheduler/renderer/renderer_scheduler_impl.h seems wrong to me.

So using BeginFrameArgs still seems like the better approach, as it seems to be
intended for this type of thing (and BeginMainFrameAndCommitState doesn't seem
to be)

[brianderson, 2017/03/31 22:19:48]

Re My #2 suggestion: That would be adding a BeginMainFrameArgs to
begin_frame_args.h that is either a subclass of BeginFrameArgs or has a
BeginFrameArgs. Then BeginMainFrameAndCommitState could have a
BeginMainFrameArgs rather than a BeginFrameArgs.
SingleThreadProxy::ScheduledActionSendBeginMainFrame could do the same thing as
the threaded ProxyImpl.

Then, if you add a BeginMainFrameArgs to begin_frame_args.h, you wouldn't need
to include proxy_common.h from the higher level files. Depending on the level,
they could take either BeginMainFrameAndCommitState, BeginMainFrameArgs, or
BeginFrameArgs as needed.

 
   BeginFrameArgsType type;
   bool on_critical_path;