cc: Add main frame timing info to frame timing tracker.

cc/trees/layer_tree_host_impl.cc

Index: cc/trees/layer_tree_host_impl.cc
diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc
index e66a3906e17db7d3124377331a65b3a661aa3ad5..1a663725119f53f08da40c7391e6c4f5bd868ac5 100644
--- a/cc/trees/layer_tree_host_impl.cc
+++ b/cc/trees/layer_tree_host_impl.cc
@@ -2103,6 +2103,25 @@ void LayerTreeHostImpl::CreateResourceAndTileTaskWorkerPool(
                                      settings_.renderer_settings.refresh_rate));
 }
 
+void LayerTreeHostImpl::RecordMainFrameTiming(
+    const BeginFrameArgs& start_of_main_frame_args,
+    const BeginFrameArgs& current_frame_args) {

[mithro-old, 2015/03/23 00:15:08]

This is "current_main_frame_args" or "current_impl_frame_args"?

[vmpstr, 2015/03/23 20:26:43]

That's where I'm a little bit hesitant. It's really impl frame args, but my
understanding is that it would become main frame args if there is a main frame?
So I don't really wanna call it impl frame, since then it's kind of weird that
I'm measuring main.start to impl.start, but I also don't want to call it a main
frame arg, since it isn't at this point.

Do you have any suggestions?

[mithro-old, 2015/03/24 07:37:42]

I think you need to be *explicitly* clear what is going on here.

I think you should be trying to measure the time between two successive
start_of_main_frame_args right?

I think this confusion here is probably pointing to it being in the wrong place?

[vmpstr, 2015/03/24 18:02:15]

In the situation where main frames don't happen one after the other, we still
want to capture the end of the frame as the time where the next main frame
_would_ begin. That's what these frame args are. That is, they are impl side
that are slated to become main frame args _if_ there is a main frame that is
going to happen. 

In situations where main frame does happen, this is the main frame args. In
situations where it doesn't, however, this isn't main frame args it's just args
that _would_ be main frame. 

I renamed this to be "expected_next_main_frame_args" and left a comment in the
header. I'm not sure how better to be explicit about it while at the same time
being correct in that it doesn't have to be main frame args here.

+  std::vector<int64_t> request_ids;
+  active_tree_->GatherFrameTimingRequestIds(&request_ids);
+  if (request_ids.empty())
+    return;
+
+  base::TimeTicks start_time = start_of_main_frame_args.frame_time;
+  // Since we finished the work in the current frame, we should be sending the
+  // next frame's frame_time. However, at this stage we don't know what the next
+  // frame_time will be. We can estimate this value by adding interval to the
+  // current frame's frame_time.

[mithro-old, 2015/03/23 00:15:08]

I really don't think we should be sending an estimate because the frame timing
specification is about providing developers with accurate information about what
is going on.

[vmpstr, 2015/03/23 20:26:43]

Done. With the latest patch, this is not an estimate.

+  base::TimeTicks end_time =
+      current_frame_args.frame_time + current_frame_args.interval;
+  frame_timing_tracker_->SaveMainFrameTimeStamps(
+      request_ids, start_time, end_time, active_tree_->source_frame_number());
+}
+
 void LayerTreeHostImpl::DestroyTileManager() {
   tile_manager_ = nullptr;
   resource_pool_ = nullptr;