cc: Adding BeginFrameTracker object and removing Now() from LTHI.

cc/scheduler/scheduler.cc

Index: cc/scheduler/scheduler.cc
diff --git a/cc/scheduler/scheduler.cc b/cc/scheduler/scheduler.cc
index 884c6187728a245a80e958517c8468cd0eb41367..7204327f06f3f4c1933febf93b5958262a7ab504 100644
--- a/cc/scheduler/scheduler.cc
+++ b/cc/scheduler/scheduler.cc
@@ -91,6 +91,7 @@ Scheduler::Scheduler(
       task_runner_(task_runner),
       power_monitor_(power_monitor),
       begin_retro_frame_posted_(false),
+      begin_impl_frame_args_(BEGINFRAMETRACKER_FROM_HERE),
       state_machine_(scheduler_settings),
       inside_process_scheduled_actions_(false),
       inside_action_(SchedulerStateMachine::ACTION_NONE),
@@ -294,19 +295,18 @@ void Scheduler::NotifyBeginMainFrameStarted() {
 }
 
 base::TimeTicks Scheduler::AnticipatedDrawTime() const {
-  if (!frame_source_->NeedsBeginFrames() ||
-      begin_impl_frame_args_.interval <= base::TimeDelta())
+  if (!frame_source_->NeedsBeginFrames())
     return base::TimeTicks();
 
   base::TimeTicks now = Now();
-  base::TimeTicks timebase = std::max(begin_impl_frame_args_.frame_time,
-                                      begin_impl_frame_args_.deadline);
-  int64 intervals = 1 + ((now - timebase) / begin_impl_frame_args_.interval);
-  return timebase + (begin_impl_frame_args_.interval * intervals);
+  BeginFrameArgs args = begin_impl_frame_args_.Get();
+  base::TimeTicks timebase = std::max(args.frame_time, args.deadline);
+  int64 intervals = 1 + ((now - timebase) / begin_impl_frame_args_.Interval());
+  return timebase + (begin_impl_frame_args_.Interval() * intervals);
 }
 
 base::TimeTicks Scheduler::LastBeginImplFrameTime() {
-  return begin_impl_frame_args_.frame_time;
+  return begin_impl_frame_args_.Get().frame_time;
 }
 
 void Scheduler::SetupNextBeginFrameIfNeeded() {
@@ -350,9 +350,7 @@ void Scheduler::SetupPollingMechanisms(bool needs_begin_frame) {
     DCHECK(!needs_begin_frame);
     if (poll_for_draw_triggers_task_.IsCancelled()) {
       poll_for_draw_triggers_task_.Reset(poll_for_draw_triggers_closure_);
-      base::TimeDelta delay = begin_impl_frame_args_.IsValid()
-                                  ? begin_impl_frame_args_.interval
-                                  : BeginFrameArgs::DefaultInterval();
+      base::TimeDelta delay = begin_impl_frame_args_.Interval();
       task_runner_->PostDelayedTask(
           FROM_HERE, poll_for_draw_triggers_task_.callback(), delay);
     }
@@ -374,13 +372,13 @@ void Scheduler::SetupPollingMechanisms(bool needs_begin_frame) {
 
   if (needs_advance_commit_state_timer) {
     if (advance_commit_state_task_.IsCancelled() &&
-        begin_impl_frame_args_.IsValid()) {
+        begin_impl_frame_args_.Get().IsValid()) {
       // Since we'd rather get a BeginImplFrame by the normal mechanism, we
       // set the interval to twice the interval from the previous frame.
       advance_commit_state_task_.Reset(advance_commit_state_closure_);
       task_runner_->PostDelayedTask(FROM_HERE,
                                     advance_commit_state_task_.callback(),
-                                    begin_impl_frame_args_.interval * 2);
+                                    begin_impl_frame_args_.Interval() * 2);
     }
   } else {
     advance_commit_state_task_.Cancel();
@@ -394,6 +392,11 @@ void Scheduler::SetupPollingMechanisms(bool needs_begin_frame) {
 bool Scheduler::OnBeginFrameMixInDelegate(const BeginFrameArgs& args) {
   TRACE_EVENT1("cc", "Scheduler::BeginFrame", "args", args.AsValue());
 
+  // Trace this begin frame time through the Chrome stack
+  TRACE_EVENT_FLOW_BEGIN0(
+      TRACE_DISABLED_BY_DEFAULT("cc.debug.scheduler.frames"), "BeginFrameArgs",

[brianderson, 2014/12/18 21:57:08]

Is this a new category? If so, are we cluttering the tracing dialog box? Should
we keep it with cc.debug.scheduler?

[mithro-old, 2014/12/18 23:15:05]

This category was added a while back after discussion with Sami showed the
traces in BeginFrameSource were adding about 10-15% overhead.

+      args.frame_time.ToInternalValue());
+
   // Deliver BeginFrames to children.
   if (settings_.forward_begin_frames_to_children &&
       state_machine_.children_need_begin_frames()) {
@@ -544,8 +547,8 @@ void Scheduler::BeginImplFrame(const BeginFrameArgs& args) {
 
   advance_commit_state_task_.Cancel();
 
-  begin_impl_frame_args_ = args;
-  begin_impl_frame_args_.deadline -= client_->DrawDurationEstimate();
+  BeginFrameArgs adjusted_args = args;
+  adjusted_args.deadline -= client_->DrawDurationEstimate();
 
   if (!state_machine_.impl_latency_takes_priority() &&
       main_thread_is_in_high_latency_mode &&
@@ -553,8 +556,9 @@ void Scheduler::BeginImplFrame(const BeginFrameArgs& args) {
     state_machine_.SetSkipNextBeginMainFrameToReduceLatency();
   }
 
-  client_->WillBeginImplFrame(begin_impl_frame_args_);
-  state_machine_.OnBeginImplFrame(begin_impl_frame_args_);
+  begin_impl_frame_args_.Start(adjusted_args);
+  client_->WillBeginImplFrame(begin_impl_frame_args_.Get());
+  state_machine_.OnBeginImplFrame(begin_impl_frame_args_.Get());
   devtools_instrumentation::DidBeginFrame(layer_tree_host_id_);
 
   ProcessScheduledActions();
@@ -592,14 +596,14 @@ void Scheduler::ScheduleBeginImplFrameDeadline() {
       break;
     case SchedulerStateMachine::BEGIN_IMPL_FRAME_DEADLINE_MODE_REGULAR:
       // We are animating on the impl thread but we can wait for some time.
-      deadline = begin_impl_frame_args_.deadline;
+      deadline = begin_impl_frame_args_.Get().deadline;
       break;
     case SchedulerStateMachine::BEGIN_IMPL_FRAME_DEADLINE_MODE_LATE:
       // We are blocked for one reason or another and we should wait.
       // TODO(brianderson): Handle long deadlines (that are past the next
       // frame's frame time) properly instead of using this hack.
-      deadline =
-          begin_impl_frame_args_.frame_time + begin_impl_frame_args_.interval;
+      deadline = begin_impl_frame_args_.Get().frame_time +
+                 begin_impl_frame_args_.Get().interval;
       break;
   }
 
@@ -641,6 +645,7 @@ void Scheduler::OnBeginImplFrameDeadline() {
   state_machine_.OnBeginImplFrameIdle();
   ProcessScheduledActions();
 
+  begin_impl_frame_args_.Finish();

[brianderson, 2014/12/18 21:57:08]

I think there are various places outside of the deadline where it should be
valid to look at the begin_impl_frame_args_.

AnticipatedDrawTime might get updated in the polling mechanisms multiple times
after the deadline.  Also, AsValueInto might get called after the deadline.

[mithro-old, 2014/12/18 23:15:05]

As discuss, maybe we should add something like DangerousGetCurrentOrLast()
method for this case.
Yeah, I'm not sure what the correct solution there is. I have a work around
which seems to make the tests pass but the coverage here seems pretty poor.
I've already fixed AsValueInto in the latest patch.

   client_->DidBeginImplFrameDeadline();
 }
 
@@ -764,7 +769,7 @@ void Scheduler::AsValueInto(base::debug::TracedValue* state) const {
   state->SetBoolean("advance_commit_state_task_",
                     !advance_commit_state_task_.IsCancelled());
   state->BeginDictionary("begin_impl_frame_args");
-  begin_impl_frame_args_.AsValueInto(state);
+  begin_impl_frame_args_.Get().AsValueInto(state);
   state->EndDictionary();
 
   state->EndDictionary();
@@ -782,22 +787,21 @@ void Scheduler::AsValueInto(base::debug::TracedValue* state) const {
 }
 
 bool Scheduler::CanCommitAndActivateBeforeDeadline() const {
+  BeginFrameArgs args = begin_impl_frame_args_.Get();

[brianderson, 2014/12/18 21:57:08]

Can this be a reference?

[mithro-old, 2014/12/18 23:15:05]

Turns out this is location which needs the DangerousGetCurrentOrLast type
method.

[brianderson, 2014/12/19 01:18:25]

Yes, an "unsafe" or "dangerous" accessor would be nice.

[mithro-old, 2014/12/19 03:40:55]

Done.

+
   // Check if the main thread computation and commit can be finished before the
   // impl thread's deadline.
   base::TimeTicks estimated_draw_time =
-      begin_impl_frame_args_.frame_time +
-      client_->BeginMainFrameToCommitDurationEstimate() +
+      args.frame_time + client_->BeginMainFrameToCommitDurationEstimate() +
       client_->CommitToActivateDurationEstimate();
 
-  TRACE_EVENT2(
-      TRACE_DISABLED_BY_DEFAULT("cc.debug.scheduler"),
-      "CanCommitAndActivateBeforeDeadline",
-      "time_left_after_drawing_ms",
-      (begin_impl_frame_args_.deadline - estimated_draw_time).InMillisecondsF(),
-      "state",
-      AsValue());
+  TRACE_EVENT2(TRACE_DISABLED_BY_DEFAULT("cc.debug.scheduler"),
+               "CanCommitAndActivateBeforeDeadline",
+               "time_left_after_drawing_ms",
+               (args.deadline - estimated_draw_time).InMillisecondsF(), "state",
+               AsValue());
 
-  return estimated_draw_time < begin_impl_frame_args_.deadline;
+  return estimated_draw_time < args.deadline;
 }
 
 bool Scheduler::IsBeginMainFrameSentOrStarted() const {