Plumb activation time to main

cc/output/begin_frame_args.h

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CC_OUTPUT_BEGIN_FRAME_ARGS_H_
 #define CC_OUTPUT_BEGIN_FRAME_ARGS_H_
 
 #include <stdint.h>
 #include <memory>
 
@@ skipping 61 matching lines @@
   // You should be able to find all instances where a BeginFrame has been
   // created by searching for "BeginFrameArgs::Create".
   // The location argument should **always** be BEGINFRAME_FROM_HERE macro.
   static BeginFrameArgs Create(CreationLocation location,
                                uint32_t source_id,
                                uint64_t sequence_number,
                                base::TimeTicks frame_time,
                                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.
   static base::TimeDelta DefaultEstimatedParentDrawTime();
 
   // This is the default interval to use to avoid sprinkling the code with
   // magic numbers.
   static base::TimeDelta DefaultInterval();
 
   bool IsValid() const { return interval >= base::TimeDelta(); }
 
   std::unique_ptr<base::trace_event::ConvertableToTraceFormat> AsValue() const;
   void AsValueInto(base::trace_event::TracedValue* dict) const;
 
   base::TimeTicks frame_time;
   base::TimeTicks deadline;
   base::TimeDelta interval;
 
   // |source_id| and |sequence_number| identify a BeginFrame within a single
   // process and are set by the original BeginFrameSource that created the
   // BeginFrameArgs. When |source_id| of consecutive BeginFrameArgs changes,
   // 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;
 
  private:
   BeginFrameArgs(uint32_t source_id,
                  uint64_t sequence_number,
                  base::TimeTicks frame_time,
                  base::TimeTicks deadline,
                  base::TimeDelta interval,
@@ skipping 57 matching lines @@
   uint32_t remaining_frames;
 
   // |true| if the observer has produced damage (e.g. sent a CompositorFrame or
   // damaged a surface) as part of responding to the BeginFrame.
   bool has_damage;
 };
 
 }  // namespace cc
 
 #endif  // CC_OUTPUT_BEGIN_FRAME_ARGS_H_