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 43 matching lines @@
   // |source_id| for BeginFrameArgs not created by a BeginFrameSource. Used to
   // avoid sequence number conflicts of BeginFrameArgs manually fed to an
   // observer with those fed to the observer by the its BeginFrameSource.
   static constexpr uint32_t kManualSourceId = UINT32_MAX;
 
   static constexpr uint64_t kInvalidFrameNumber = 0;
   static constexpr uint64_t kStartingFrameNumber = 1;
 
   // Creates an invalid set of values.
   BeginFrameArgs();
+  ~BeginFrameArgs();
 
 #ifdef NDEBUG
   typedef const void* CreationLocation;
 #else
   typedef const tracked_objects::Location& CreationLocation;
   tracked_objects::Location created_from;
 #endif
 
   // 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;

[vmpstr, 2017/04/05 19:11:33]

nit: source_frame_number to be consistent with the rest of the places

[brianderson, 2017/04/06 23:43:15]

A few comments:

* Make sure to initialize this in each of the constructors.

* Add a comment like:
"|source_frame_number| is set by the LayerTreeHost when the BeginFrame reaches
blink and corresponds to the commit count."

* "source" in this context is overloaded since it could correspond to the
BeginFrameSource. Maybe rename it to "commit_number" so it's more obvious it
relates to the commit. Although, I do still have a preference for a separate
BeginMainFrameArgs which would also avoid overloading the "source" concept.

[panicker, 2017/04/08 00:29:06]

Updated.
Happy to try out the BeginMainFrameArgs subsclassing as a follow up change, if
that's fine with you. Otherwise it's adding not necessary complexity to this
change.

+
+  // This is to report Activation time back to main.

[vmpstr, 2017/04/05 19:11:33]

Can you please elaborate on this comment a bit. Specifically, what the pair
represents and why it's a vector as opposed to a single value?

[panicker, 2017/04/08 00:29:06]

Done.

+  std::vector<std::pair<uint32_t, base::TimeTicks>> ready_to_activate_time;
 
   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 52 matching lines @@
   uint32_t source_id;  // |source_id| after above fields for packing.
 
   // |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_