summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authormithro <mithro@mithis.com>2015-05-27 06:08:01 -0700
committerCommit bot <commit-bot@chromium.org>2015-05-27 13:08:44 +0000
commit0bdb49d47d685409d8f69dd0eb749a4f9e7bca6a (patch)
tree6fc1bc2145b84ee6bf1ace6a9ae3964ad7a7b73a
parent9f8274214490593da392bd9541fdc646f9d0766e (diff)
downloadchromium_src-0bdb49d47d685409d8f69dd0eb749a4f9e7bca6a.zip
chromium_src-0bdb49d47d685409d8f69dd0eb749a4f9e7bca6a.tar.gz
chromium_src-0bdb49d47d685409d8f69dd0eb749a4f9e7bca6a.tar.bz2
cc: Adding BeginFrameTracker object and removing Now() from LTHI.
**Sheriffs**: If this patch is making a test flaky / fail, then the test was already broken and this just makes the problem visible. This patch reduces the jitter in LTHI's animation time to be identical to the vsync jitter rather than being affected by the system load and OS scheduling delays. BeginFrameTracker uses DCHECKs and TRACE_EVENT to strictly track how the BeginFrameArgs are used throughout the LTHI. Using this information incorrect usage of Now() has been eliminated. Methods which violate correct usage of BeginFrameArgs are now clearly marked to prevent regressions. BUG=346230 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Review URL: https://codereview.chromium.org/787763006 Cr-Commit-Position: refs/heads/master@{#331563}
-rw-r--r--cc/BUILD.gn2
-rw-r--r--cc/cc.gyp2
-rw-r--r--cc/scheduler/begin_frame_tracker.cc121
-rw-r--r--cc/scheduler/begin_frame_tracker.h97
-rw-r--r--cc/scheduler/scheduler.cc84
-rw-r--r--cc/scheduler/scheduler.h5
-rw-r--r--cc/scheduler/scheduler_unittest.cc11
-rw-r--r--cc/test/fake_layer_tree_host_impl.cc9
-rw-r--r--cc/test/fake_layer_tree_host_impl.h1
-rw-r--r--cc/test/scheduler_test_common.h5
-rw-r--r--cc/trees/layer_tree_host_impl.cc40
-rw-r--r--cc/trees/layer_tree_host_impl.h14
-rw-r--r--cc/trees/layer_tree_host_impl_unittest.cc11
-rw-r--r--cc/trees/layer_tree_host_unittest_context.cc14
-rw-r--r--cc/trees/layer_tree_impl.cc4
-rw-r--r--cc/trees/layer_tree_impl.h2
16 files changed, 320 insertions, 102 deletions
diff --git a/cc/BUILD.gn b/cc/BUILD.gn
index 9091567..f38ff7d 100644
--- a/cc/BUILD.gn
+++ b/cc/BUILD.gn
@@ -446,6 +446,8 @@ component("cc") {
"resources/video_resource_updater.h",
"scheduler/begin_frame_source.cc",
"scheduler/begin_frame_source.h",
+ "scheduler/begin_frame_tracker.cc",
+ "scheduler/begin_frame_tracker.h",
"scheduler/commit_earlyout_reason.h",
"scheduler/delay_based_time_source.cc",
"scheduler/delay_based_time_source.h",
diff --git a/cc/cc.gyp b/cc/cc.gyp
index abdd958..90b8d4f 100644
--- a/cc/cc.gyp
+++ b/cc/cc.gyp
@@ -500,6 +500,8 @@
'resources/video_resource_updater.h',
'scheduler/begin_frame_source.cc',
'scheduler/begin_frame_source.h',
+ 'scheduler/begin_frame_tracker.cc',
+ 'scheduler/begin_frame_tracker.h',
'scheduler/commit_earlyout_reason.h',
'scheduler/delay_based_time_source.cc',
'scheduler/delay_based_time_source.h',
diff --git a/cc/scheduler/begin_frame_tracker.cc b/cc/scheduler/begin_frame_tracker.cc
new file mode 100644
index 0000000..fcc3313
--- /dev/null
+++ b/cc/scheduler/begin_frame_tracker.cc
@@ -0,0 +1,121 @@
+// Copyright 2014 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.
+
+#include "cc/scheduler/begin_frame_tracker.h"
+
+namespace cc {
+
+BeginFrameTracker::BeginFrameTracker(const tracked_objects::Location& location)
+ : location_(location),
+ location_string_(location.ToString()),
+ current_updated_at_(),
+ current_args_(),
+ current_finished_at_(base::TimeTicks::FromInternalValue(-1)) {
+}
+
+BeginFrameTracker::~BeginFrameTracker() {
+}
+
+void BeginFrameTracker::Start(BeginFrameArgs new_args) {
+ // Trace the frame time being passed between BeginFrameTrackers.
+ TRACE_EVENT_FLOW_STEP0(
+ TRACE_DISABLED_BY_DEFAULT("cc.debug.scheduler.frames"), "BeginFrameArgs",
+ new_args.frame_time.ToInternalValue(), location_string_);
+
+ // Trace this specific begin frame tracker Start/Finish times.
+ TRACE_EVENT_ASYNC_BEGIN2(
+ TRACE_DISABLED_BY_DEFAULT("cc.debug.scheduler.frames"),
+ location_string_.c_str(), new_args.frame_time.ToInternalValue(),
+ "new args", new_args.AsValue(), "current args", current_args_.AsValue());
+
+ // Check the new BeginFrameArgs are valid and monotonically increasing.
+ DCHECK(new_args.IsValid());
+ DCHECK_LE(current_args_.frame_time, new_args.frame_time);
+
+ DCHECK(HasFinished())
+ << "Tried to start a new frame before finishing an existing frame.";
+ current_updated_at_ = base::TimeTicks::NowFromSystemTraceTime();
+ current_args_ = new_args;
+ current_finished_at_ = base::TimeTicks();
+
+ // TODO(mithro): Add UMA tracking of delta between current_updated_at_ time
+ // and the new_args.frame_time argument. This will give us how long after a
+ // BeginFrameArgs message was created before we started processing it.
+}
+
+const BeginFrameArgs& BeginFrameTracker::Current() const {
+ DCHECK(!HasFinished())
+ << "Tried to use BeginFrameArgs after marking the frame finished.";
+ DCHECK(current_args_.IsValid())
+ << "Tried to use BeginFrameArgs before starting a frame!";
+ return current_args_;
+}
+
+void BeginFrameTracker::Finish() {
+ DCHECK(!HasFinished()) << "Tried to finish an already finished frame";
+ current_finished_at_ = base::TimeTicks::NowFromSystemTraceTime();
+ TRACE_EVENT_ASYNC_END0(TRACE_DISABLED_BY_DEFAULT("cc.debug.scheduler.frames"),
+ location_string_.c_str(),
+ current_args_.frame_time.ToInternalValue());
+}
+
+const BeginFrameArgs& BeginFrameTracker::Last() const {
+ DCHECK(current_args_.IsValid())
+ << "Tried to use last BeginFrameArgs before starting a frame!";
+ DCHECK(HasFinished())
+ << "Tried to use last BeginFrameArgs before the frame is finished.";
+ return current_args_;
+}
+
+base::TimeDelta BeginFrameTracker::Interval() const {
+ base::TimeDelta interval = current_args_.interval;
+ // Normal interval will be ~16ms, 200Hz (5ms) screens are the fastest
+ // easily available so anything less than that is likely an error.
+ if (interval < base::TimeDelta::FromMilliseconds(1)) {
+ interval = BeginFrameArgs::DefaultInterval();
+ }
+ return interval;
+}
+
+void BeginFrameTracker::AsValueInto(
+ base::TimeTicks now,
+ base::trace_event::TracedValue* state) const {
+ state->SetInteger("updated_at_us", current_updated_at_.ToInternalValue());
+ state->SetInteger("finished_at_us", current_finished_at_.ToInternalValue());
+ if (HasFinished()) {
+ state->SetString("state", "FINISHED");
+ state->BeginDictionary("current_args_");
+ } else {
+ state->SetString("state", "USING");
+ state->BeginDictionary("last_args_");
+ }
+ current_args_.AsValueInto(state);
+ state->EndDictionary();
+
+ base::TimeTicks frame_time = current_args_.frame_time;
+ base::TimeTicks deadline = current_args_.deadline;
+ base::TimeDelta interval = current_args_.interval;
+ state->BeginDictionary("major_timestamps_in_ms");
+ state->SetDouble("0_interval", interval.InMillisecondsF());
+ state->SetDouble("1_now_to_deadline", (deadline - now).InMillisecondsF());
+ state->SetDouble("2_frame_time_to_now", (now - frame_time).InMillisecondsF());
+ state->SetDouble("3_frame_time_to_deadline",
+ (deadline - frame_time).InMillisecondsF());
+ state->SetDouble("4_now", (now - base::TimeTicks()).InMillisecondsF());
+ state->SetDouble("5_frame_time",
+ (frame_time - base::TimeTicks()).InMillisecondsF());
+ state->SetDouble("6_deadline",
+ (deadline - base::TimeTicks()).InMillisecondsF());
+ state->EndDictionary();
+}
+
+const BeginFrameArgs& BeginFrameTracker::DangerousMethodCurrentOrLast() const {
+ if (!HasFinished()) {
+ return Current();
+ } else {
+ return Last();
+ }
+}
+
+} // namespace cc
diff --git a/cc/scheduler/begin_frame_tracker.h b/cc/scheduler/begin_frame_tracker.h
new file mode 100644
index 0000000..98b1da4
--- /dev/null
+++ b/cc/scheduler/begin_frame_tracker.h
@@ -0,0 +1,97 @@
+// Copyright 2014 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_SCHEDULER_BEGIN_FRAME_TRACKER_H_
+#define CC_SCHEDULER_BEGIN_FRAME_TRACKER_H_
+
+#include <set>
+#include <string>
+
+#include "base/location.h"
+#include "base/trace_event/trace_event.h"
+#include "base/trace_event/trace_event_argument.h"
+#include "cc/output/begin_frame_args.h"
+
+#define BEGINFRAMETRACKER_FROM_HERE FROM_HERE_WITH_EXPLICIT_FUNCTION("")
+
+namespace cc {
+
+// Microclass to trace and check properties for correct BeginFrameArgs (BFA)
+// usage and provide a few helper methods.
+//
+// With DCHECKs enable, this class checks the following "invariants";
+// * BFA are monotonically increasing.
+// * BFA is valid.
+// * The BFA is only used inside a given period.
+// * A new BFA isn't used before the last BFA is finished with.
+//
+// With the tracing category "cc.debug.scheduler.frames" enabled the tracker
+// will output the following trace information;
+// * Time period for which the BFA is in usage.
+// * The flow of BFA as they are passed between tracking objects.
+//
+// TODO(mithro): Record stats about the BeginFrameArgs
+class CC_EXPORT BeginFrameTracker {
+ public:
+ explicit BeginFrameTracker(const tracked_objects::Location& location);
+ ~BeginFrameTracker();
+
+ // The Start and Finish methods manage the period that a BFA should be
+ // accessed for. This allows tight control over the BFA and prevents
+ // accidental usage in the wrong period when code is split across multiple
+ // locations.
+
+ // Start using a new BFA value and check invariant properties.
+ // **Must** only be called after finishing with any previous BFA.
+ void Start(BeginFrameArgs new_args);
+ // Finish using the current BFA.
+ // **Must** only be called while still using a BFA.
+ void Finish();
+
+ // The two accessors methods allow access to the BFA stored inside the
+ // tracker. They are mutually exclusive, at any time it is only valid to call
+ // one or the other. This makes sure you understand exactly which BFA you are
+ // intending to use and verifies that is the case.
+
+ // Get the current BFA object.
+ // **Must** only be called between the start and finish methods calls.
+ const BeginFrameArgs& Current() const;
+ // Get the last used BFA.
+ // **Must** only be called when **not** between the start and finish method
+ // calls.
+ const BeginFrameArgs& Last() const;
+
+ // Helper method to try and return a valid interval property. Defaults to
+ // BFA::DefaultInterval() is no other interval can be found. Can be called at
+ // any time.
+ base::TimeDelta Interval() const;
+
+ void AsValueInto(base::TimeTicks now,
+ base::trace_event::TracedValue* dict) const;
+
+ // The following methods violate principles of how BeginFrameArgs should be
+ // used. These methods should only be used when there is no other choice.
+ bool DangerousMethodHasStarted() const {
+ return !current_updated_at_.is_null();
+ }
+ bool DangerousMethodHasFinished() const { return HasFinished(); }
+ const BeginFrameArgs& DangerousMethodCurrentOrLast() const;
+
+ private:
+ // Return if currently not between the start/end period. This method should
+ // be used extremely sparingly and normal indicates incorrect management of
+ // the BFA object. Can be called at any time.
+ bool HasFinished() const { return !current_finished_at_.is_null(); }
+
+ const tracked_objects::Location location_;
+ const std::string location_string_;
+
+ base::TimeTicks current_updated_at_;
+ BeginFrameArgs current_args_;
+ base::TimeTicks current_finished_at_;
+};
+
+} // namespace cc
+
+#endif // CC_SCHEDULER_BEGIN_FRAME_TRACKER_H_
diff --git a/cc/scheduler/scheduler.cc b/cc/scheduler/scheduler.cc
index 535d105..8d407de 100644
--- a/cc/scheduler/scheduler.cc
+++ b/cc/scheduler/scheduler.cc
@@ -78,6 +78,7 @@ Scheduler::Scheduler(
task_runner_(task_runner),
begin_impl_frame_deadline_mode_(
SchedulerStateMachine::BEGIN_IMPL_FRAME_DEADLINE_MODE_NONE),
+ begin_impl_frame_tracker_(BEGINFRAMETRACKER_FROM_HERE),
state_machine_(scheduler_settings),
inside_process_scheduled_actions_(false),
inside_action_(SchedulerStateMachine::ACTION_NONE),
@@ -274,18 +275,19 @@ void Scheduler::NotifyBeginMainFrameStarted() {
base::TimeTicks Scheduler::AnticipatedDrawTime() const {
if (!frame_source_->NeedsBeginFrames() ||
- begin_impl_frame_args_.interval <= base::TimeDelta())
+ begin_impl_frame_tracker_.DangerousMethodHasFinished())
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_tracker_.Current();
+ base::TimeTicks timebase = std::max(args.frame_time, args.deadline);
+ int64 intervals =
+ 1 + ((now - timebase) / begin_impl_frame_tracker_.Interval());
+ return timebase + (begin_impl_frame_tracker_.Interval() * intervals);
}
base::TimeTicks Scheduler::LastBeginImplFrameTime() {
- return begin_impl_frame_args_.frame_time;
+ return begin_impl_frame_tracker_.Current().frame_time;
}
void Scheduler::SetupNextBeginFrameIfNeeded() {
@@ -319,13 +321,13 @@ void Scheduler::SetupPollingMechanisms() {
if (IsBeginMainFrameSentOrStarted() &&
!settings_.using_synchronous_renderer_compositor) {
if (advance_commit_state_task_.IsCancelled() &&
- begin_impl_frame_args_.IsValid()) {
+ begin_impl_frame_tracker_.DangerousMethodCurrentOrLast().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_tracker_.Interval() * 2);
}
} else {
advance_commit_state_task_.Cancel();
@@ -339,6 +341,11 @@ void Scheduler::SetupPollingMechanisms() {
bool Scheduler::OnBeginFrameMixInDelegate(const BeginFrameArgs& args) {
TRACE_EVENT1("cc,benchmark", "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",
+ args.frame_time.ToInternalValue());
+
// TODO(brianderson): Adjust deadline in the DisplayScheduler.
BeginFrameArgs adjusted_args(args);
adjusted_args.deadline -= EstimatedParentDrawTime();
@@ -494,8 +501,8 @@ void Scheduler::BeginImplFrameWithDeadline(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 &&
@@ -503,7 +510,7 @@ void Scheduler::BeginImplFrameWithDeadline(const BeginFrameArgs& args) {
state_machine_.SetSkipNextBeginMainFrameToReduceLatency();
}
- BeginImplFrame();
+ BeginImplFrame(adjusted_args);
// The deadline will be scheduled in ProcessScheduledActions.
state_machine_.OnBeginImplFrameDeadlinePending();
@@ -513,8 +520,7 @@ void Scheduler::BeginImplFrameWithDeadline(const BeginFrameArgs& args) {
void Scheduler::BeginImplFrameSynchronous(const BeginFrameArgs& args) {
TRACE_EVENT1("cc,benchmark", "Scheduler::BeginImplFrame", "args",
args.AsValue());
- begin_impl_frame_args_ = args;
- BeginImplFrame();
+ BeginImplFrame(args);
FinishImplFrame();
}
@@ -524,21 +530,23 @@ void Scheduler::FinishImplFrame() {
client_->DidFinishImplFrame();
frame_source_->DidFinishFrame(begin_retro_frame_args_.size());
+ begin_impl_frame_tracker_.Finish();
}
// BeginImplFrame starts a compositor frame that will wait up until a deadline
// for a BeginMainFrame+activation to complete before it times out and draws
// any asynchronous animation and scroll/pinch updates.
-void Scheduler::BeginImplFrame() {
+void Scheduler::BeginImplFrame(const BeginFrameArgs& args) {
DCHECK_EQ(state_machine_.begin_impl_frame_state(),
SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_IDLE);
DCHECK(!BeginImplFrameDeadlinePending());
DCHECK(state_machine_.HasInitializedOutputSurface());
DCHECK(advance_commit_state_task_.IsCancelled());
+ begin_impl_frame_tracker_.Start(args);
state_machine_.OnBeginImplFrame();
devtools_instrumentation::DidBeginFrame(layer_tree_host_id_);
- client_->WillBeginImplFrame(begin_impl_frame_args_);
+ client_->WillBeginImplFrame(begin_impl_frame_tracker_.Current());
ProcessScheduledActions();
}
@@ -565,14 +573,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_tracker_.Current().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_tracker_.Current().frame_time +
+ begin_impl_frame_tracker_.Current().interval;
break;
case SchedulerStateMachine::
BEGIN_IMPL_FRAME_DEADLINE_MODE_BLOCKED_ON_READY_TO_DRAW:
@@ -768,26 +776,8 @@ void Scheduler::AsValueInto(base::trace_event::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_tracker_.AsValueInto(Now(), state);
state->EndDictionary();
-
- base::TimeTicks now = Now();
- base::TimeTicks frame_time = begin_impl_frame_args_.frame_time;
- base::TimeTicks deadline = begin_impl_frame_args_.deadline;
- base::TimeDelta interval = begin_impl_frame_args_.interval;
- state->BeginDictionary("major_timestamps_in_ms");
- state->SetDouble("0_interval", interval.InMillisecondsF());
- state->SetDouble("1_now_to_deadline", (deadline - now).InMillisecondsF());
- state->SetDouble("2_frame_time_to_now", (now - frame_time).InMillisecondsF());
- state->SetDouble("3_frame_time_to_deadline",
- (deadline - frame_time).InMillisecondsF());
- state->SetDouble("4_now", (now - base::TimeTicks()).InMillisecondsF());
- state->SetDouble("5_frame_time",
- (frame_time - base::TimeTicks()).InMillisecondsF());
- state->SetDouble("6_deadline",
- (deadline - base::TimeTicks()).InMillisecondsF());
- state->EndDictionary();
-
state->EndDictionary();
state->BeginDictionary("client_state");
@@ -803,22 +793,22 @@ void Scheduler::AsValueInto(base::trace_event::TracedValue* state) const {
}
bool Scheduler::CanCommitAndActivateBeforeDeadline() const {
+ BeginFrameArgs args =
+ begin_impl_frame_tracker_.DangerousMethodCurrentOrLast();
+
// 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 {
diff --git a/cc/scheduler/scheduler.h b/cc/scheduler/scheduler.h
index 13daf00..98e92c0 100644
--- a/cc/scheduler/scheduler.h
+++ b/cc/scheduler/scheduler.h
@@ -16,6 +16,7 @@
#include "cc/output/begin_frame_args.h"
#include "cc/output/vsync_parameter_observer.h"
#include "cc/scheduler/begin_frame_source.h"
+#include "cc/scheduler/begin_frame_tracker.h"
#include "cc/scheduler/delay_based_time_source.h"
#include "cc/scheduler/draw_result.h"
#include "cc/scheduler/scheduler_settings.h"
@@ -206,9 +207,9 @@ class CC_EXPORT Scheduler : public BeginFrameObserverMixIn {
base::TimeDelta estimated_parent_draw_time_;
std::deque<BeginFrameArgs> begin_retro_frame_args_;
- BeginFrameArgs begin_impl_frame_args_;
SchedulerStateMachine::BeginImplFrameDeadlineMode
begin_impl_frame_deadline_mode_;
+ BeginFrameTracker begin_impl_frame_tracker_;
base::Closure begin_retro_frame_closure_;
base::Closure begin_impl_frame_deadline_closure_;
@@ -235,7 +236,7 @@ class CC_EXPORT Scheduler : public BeginFrameObserverMixIn {
void BeginRetroFrame();
void BeginImplFrameWithDeadline(const BeginFrameArgs& args);
void BeginImplFrameSynchronous(const BeginFrameArgs& args);
- void BeginImplFrame();
+ void BeginImplFrame(const BeginFrameArgs& args);
void FinishImplFrame();
void OnBeginImplFrameDeadline();
void PollToAdvanceCommitState();
diff --git a/cc/scheduler/scheduler_unittest.cc b/cc/scheduler/scheduler_unittest.cc
index f8a9d3f..eaa0138 100644
--- a/cc/scheduler/scheduler_unittest.cc
+++ b/cc/scheduler/scheduler_unittest.cc
@@ -2916,16 +2916,14 @@ TEST_F(SchedulerTest, SynchronousCompositorDoubleCommitWithoutDraw) {
TEST_F(SchedulerTest, AuthoritativeVSyncInterval) {
SetUpScheduler(true);
-
- base::TimeDelta initial_interval =
- scheduler_->begin_impl_frame_args().interval;
+ base::TimeDelta initial_interval = scheduler_->BeginImplFrameInterval();
base::TimeDelta authoritative_interval =
base::TimeDelta::FromMilliseconds(33);
scheduler_->SetNeedsCommit();
EXPECT_SCOPED(AdvanceFrame());
- EXPECT_EQ(initial_interval, scheduler_->begin_impl_frame_args().interval);
+ EXPECT_EQ(initial_interval, scheduler_->BeginImplFrameInterval());
scheduler_->NotifyBeginMainFrameStarted();
scheduler_->NotifyReadyToCommit();
@@ -2937,9 +2935,8 @@ TEST_F(SchedulerTest, AuthoritativeVSyncInterval) {
// At the next BeginFrame, authoritative interval is used instead of previous
// interval.
- EXPECT_NE(initial_interval, scheduler_->begin_impl_frame_args().interval);
- EXPECT_EQ(authoritative_interval,
- scheduler_->begin_impl_frame_args().interval);
+ EXPECT_NE(initial_interval, scheduler_->BeginImplFrameInterval());
+ EXPECT_EQ(authoritative_interval, scheduler_->BeginImplFrameInterval());
}
} // namespace
diff --git a/cc/test/fake_layer_tree_host_impl.cc b/cc/test/fake_layer_tree_host_impl.cc
index d026120..81216a0 100644
--- a/cc/test/fake_layer_tree_host_impl.cc
+++ b/cc/test/fake_layer_tree_host_impl.cc
@@ -61,14 +61,15 @@ void FakeLayerTreeHostImpl::CreatePendingTree() {
}
BeginFrameArgs FakeLayerTreeHostImpl::CurrentBeginFrameArgs() const {
- if (!current_begin_frame_args_.IsValid())
- return LayerTreeHostImpl::CurrentBeginFrameArgs();
- return current_begin_frame_args_;
+ return current_begin_frame_tracker_.DangerousMethodCurrentOrLast();
}
void FakeLayerTreeHostImpl::SetCurrentBeginFrameArgs(
const BeginFrameArgs& args) {
- current_begin_frame_args_ = args;
+ if (!current_begin_frame_tracker_.DangerousMethodHasFinished()) {
+ current_begin_frame_tracker_.Finish();
+ }
+ current_begin_frame_tracker_.Start(args);
}
int FakeLayerTreeHostImpl::RecursiveUpdateNumChildren(LayerImpl* layer) {
diff --git a/cc/test/fake_layer_tree_host_impl.h b/cc/test/fake_layer_tree_host_impl.h
index 23f38ab..439c632 100644
--- a/cc/test/fake_layer_tree_host_impl.h
+++ b/cc/test/fake_layer_tree_host_impl.h
@@ -43,7 +43,6 @@ class FakeLayerTreeHostImpl : public LayerTreeHostImpl {
using LayerTreeHostImpl::RemoveRenderPasses;
private:
- BeginFrameArgs current_begin_frame_args_;
FakeLayerTreeHostImplClient client_;
FakeRenderingStatsInstrumentation stats_instrumentation_;
};
diff --git a/cc/test/scheduler_test_common.h b/cc/test/scheduler_test_common.h
index 633289b..a2982ee 100644
--- a/cc/test/scheduler_test_common.h
+++ b/cc/test/scheduler_test_common.h
@@ -184,7 +184,6 @@ class TestScheduler : public Scheduler {
BeginFrameSource& frame_source() { return *frame_source_; }
bool FrameProductionThrottled() { return throttle_frame_production_; }
- BeginFrameArgs begin_impl_frame_args() { return begin_impl_frame_args_; }
~TestScheduler() override;
@@ -195,6 +194,10 @@ class TestScheduler : public Scheduler {
}
}
+ base::TimeDelta BeginImplFrameInterval() {
+ return begin_impl_frame_tracker_.Interval();
+ }
+
protected:
// Overridden from Scheduler.
base::TimeTicks Now() const override;
diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc
index 8063831..3be7e62 100644
--- a/cc/trees/layer_tree_host_impl.cc
+++ b/cc/trees/layer_tree_host_impl.cc
@@ -189,6 +189,7 @@ LayerTreeHostImpl::LayerTreeHostImpl(
int id)
: client_(client),
proxy_(proxy),
+ current_begin_frame_tracker_(BEGINFRAMETRACKER_FROM_HERE),
content_is_suitable_for_gpu_rasterization_(true),
has_gpu_rasterization_trigger_(false),
use_gpu_rasterization_(false),
@@ -221,7 +222,6 @@ LayerTreeHostImpl::LayerTreeHostImpl(
max_memory_needed_bytes_(0),
device_scale_factor_(1.f),
resourceless_software_draw_(false),
- begin_impl_frame_interval_(BeginFrameArgs::DefaultInterval()),
animation_registrar_(AnimationRegistrar::Create()),
rendering_stats_instrumentation_(rendering_stats_instrumentation),
micro_benchmark_controller_(this),
@@ -399,6 +399,10 @@ bool LayerTreeHostImpl::CanDraw() const {
}
void LayerTreeHostImpl::Animate(base::TimeTicks monotonic_time) {
+ // mithro(TODO): Enable these checks.
+ // DCHECK(!current_begin_frame_tracker_.HasFinished());
+ // DCHECK(monotonic_time == current_begin_frame_tracker_.Current().frame_time)
+ // << "Called animate with unknown frame time!?";
if (input_handler_client_)
input_handler_client_->Animate(monotonic_time);
AnimatePageScale(monotonic_time);
@@ -1708,16 +1712,7 @@ bool LayerTreeHostImpl::SwapBuffers(const LayerTreeHostImpl::FrameData& frame) {
}
void LayerTreeHostImpl::WillBeginImplFrame(const BeginFrameArgs& args) {
- // Sample the frame time now. This time will be used for updating animations
- // when we draw.
- DCHECK(!current_begin_frame_args_.IsValid());
- current_begin_frame_args_ = args;
- // TODO(mithro): Stop overriding the frame time once the usage of frame
- // timing is unified.
- current_begin_frame_args_.frame_time = base::TimeTicks::Now();
-
- // Cache the begin impl frame interval
- begin_impl_frame_interval_ = args.interval;
+ current_begin_frame_tracker_.Start(args);
if (is_likely_to_require_a_draw_) {
// Optimistically schedule a draw. This will let us expect the tile manager
@@ -1731,8 +1726,7 @@ void LayerTreeHostImpl::WillBeginImplFrame(const BeginFrameArgs& args) {
}
void LayerTreeHostImpl::DidFinishImplFrame() {
- DCHECK(current_begin_frame_args_.IsValid());
- current_begin_frame_args_ = BeginFrameArgs();
+ current_begin_frame_tracker_.Finish();
}
void LayerTreeHostImpl::UpdateViewportContainerSizes() {
@@ -3167,8 +3161,9 @@ void LayerTreeHostImpl::AddVideoFrameController(
VideoFrameController* controller) {
bool was_empty = video_frame_controllers_.empty();
video_frame_controllers_.insert(controller);
- if (current_begin_frame_args_.IsValid())
- controller->OnBeginFrame(current_begin_frame_args_);
+ if (current_begin_frame_tracker_.DangerousMethodHasStarted() &&
+ !current_begin_frame_tracker_.DangerousMethodHasFinished())
+ controller->OnBeginFrame(current_begin_frame_tracker_.Current());
if (was_empty)
client_->SetVideoNeedsBeginFrames(true);
}
@@ -3195,14 +3190,13 @@ TreePriority LayerTreeHostImpl::GetTreePriority() const {
}
BeginFrameArgs LayerTreeHostImpl::CurrentBeginFrameArgs() const {
- // Try to use the current frame time to keep animations non-jittery. But if
- // we're not in a frame (because this is during an input event or a delayed
- // task), fall back to physical time. This should still be monotonic.
- if (current_begin_frame_args_.IsValid())
- return current_begin_frame_args_;
- return BeginFrameArgs::Create(
- BEGINFRAME_FROM_HERE, base::TimeTicks::Now(), base::TimeTicks(),
- BeginFrameArgs::DefaultInterval(), BeginFrameArgs::NORMAL);
+ // TODO(mithro): Replace call with current_begin_frame_tracker_.Current()
+ // once all calls which happens outside impl frames are fixed.
+ return current_begin_frame_tracker_.DangerousMethodCurrentOrLast();
+}
+
+base::TimeDelta LayerTreeHostImpl::CurrentBeginFrameInterval() const {
+ return current_begin_frame_tracker_.Interval();
}
scoped_refptr<base::trace_event::ConvertableToTraceFormat>
diff --git a/cc/trees/layer_tree_host_impl.h b/cc/trees/layer_tree_host_impl.h
index 5c0babe..8cb7c57 100644
--- a/cc/trees/layer_tree_host_impl.h
+++ b/cc/trees/layer_tree_host_impl.h
@@ -32,6 +32,7 @@
#include "cc/quads/render_pass.h"
#include "cc/resources/resource_provider.h"
#include "cc/resources/ui_resource_client.h"
+#include "cc/scheduler/begin_frame_tracker.h"
#include "cc/scheduler/commit_earlyout_reason.h"
#include "cc/scheduler/draw_result.h"
#include "cc/scheduler/video_frame_controller.h"
@@ -451,12 +452,12 @@ class CC_EXPORT LayerTreeHostImpl
void SetTreePriority(TreePriority priority);
TreePriority GetTreePriority() const;
+ // TODO(mithro): Remove this methods which exposes the internal
+ // BeginFrameArgs to external callers.
virtual BeginFrameArgs CurrentBeginFrameArgs() const;
// Expected time between two begin impl frame calls.
- base::TimeDelta begin_impl_frame_interval() const {
- return begin_impl_frame_interval_;
- }
+ base::TimeDelta CurrentBeginFrameInterval() const;
void AsValueWithFrameInto(FrameData* frame,
base::trace_event::TracedValue* value) const;
@@ -561,6 +562,8 @@ class CC_EXPORT LayerTreeHostImpl
LayerTreeHostImplClient* client_;
Proxy* proxy_;
+ BeginFrameTracker current_begin_frame_tracker_;
+
private:
gfx::Vector2dF ScrollLayerWithViewportSpaceDelta(
LayerImpl* layer_impl,
@@ -736,11 +739,6 @@ class CC_EXPORT LayerTreeHostImpl
gfx::Rect viewport_damage_rect_;
- BeginFrameArgs current_begin_frame_args_;
-
- // Expected time between two begin impl frame calls.
- base::TimeDelta begin_impl_frame_interval_;
-
scoped_ptr<AnimationRegistrar> animation_registrar_;
std::set<ScrollbarAnimationController*> scrollbar_animation_controllers_;
std::set<VideoFrameController*> video_frame_controllers_;
diff --git a/cc/trees/layer_tree_host_impl_unittest.cc b/cc/trees/layer_tree_host_impl_unittest.cc
index 5c7488c..6377818 100644
--- a/cc/trees/layer_tree_host_impl_unittest.cc
+++ b/cc/trees/layer_tree_host_impl_unittest.cc
@@ -180,6 +180,9 @@ class LayerTreeHostImplTest : public testing::Test,
task_graph_runner_.get(), 0);
bool init = host_impl_->InitializeRenderer(output_surface.Pass());
host_impl_->SetViewportSize(gfx::Size(10, 10));
+ // Set the BeginFrameArgs so that methods which use it are able to.
+ host_impl_->WillBeginImplFrame(
+ CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE));
return init;
}
@@ -5119,6 +5122,8 @@ TEST_F(LayerTreeHostImplTest, PartialSwapReceivesDamageRect) {
settings, this, &proxy_, &stats_instrumentation_,
shared_bitmap_manager_.get(), NULL, task_graph_runner_.get(), 0);
layer_tree_host_impl->InitializeRenderer(output_surface.Pass());
+ layer_tree_host_impl->WillBeginImplFrame(
+ CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE));
layer_tree_host_impl->SetViewportSize(gfx::Size(500, 500));
scoped_ptr<LayerImpl> root =
@@ -5407,6 +5412,8 @@ static scoped_ptr<LayerTreeHostImpl> SetupLayersForOpacity(
scoped_ptr<LayerTreeHostImpl> my_host_impl = LayerTreeHostImpl::Create(
settings, client, proxy, stats_instrumentation, manager, NULL, NULL, 0);
my_host_impl->InitializeRenderer(output_surface.Pass());
+ my_host_impl->WillBeginImplFrame(
+ CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE));
my_host_impl->SetViewportSize(gfx::Size(100, 100));
/*
@@ -7758,6 +7765,8 @@ class FakeVideoFrameController : public VideoFrameController {
};
TEST_F(LayerTreeHostImplTest, AddVideoFrameControllerInsideFrame) {
+ host_impl_->DidFinishImplFrame();
+
BeginFrameArgs begin_frame_args =
CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE);
FakeVideoFrameController controller;
@@ -7770,6 +7779,8 @@ TEST_F(LayerTreeHostImplTest, AddVideoFrameControllerInsideFrame) {
}
TEST_F(LayerTreeHostImplTest, AddVideoFrameControllerOutsideFrame) {
+ host_impl_->DidFinishImplFrame();
+
BeginFrameArgs begin_frame_args =
CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE);
FakeVideoFrameController controller;
diff --git a/cc/trees/layer_tree_host_unittest_context.cc b/cc/trees/layer_tree_host_unittest_context.cc
index dedf959..e440260 100644
--- a/cc/trees/layer_tree_host_unittest_context.cc
+++ b/cc/trees/layer_tree_host_unittest_context.cc
@@ -395,8 +395,8 @@ class MultipleCompositeDoesNotCreateOutputSurface
}
void BeginTest() override {
- layer_tree_host()->Composite(base::TimeTicks());
- layer_tree_host()->Composite(base::TimeTicks());
+ layer_tree_host()->Composite(base::TimeTicks::FromInternalValue(1));
+ layer_tree_host()->Composite(base::TimeTicks::FromInternalValue(2));
}
scoped_ptr<OutputSurface> CreateOutputSurface() override {
@@ -442,12 +442,12 @@ class FailedCreateDoesNotCreateExtraOutputSurface
void BeginTest() override {
// First composite tries to create a surface.
- layer_tree_host()->Composite(base::TimeTicks());
+ layer_tree_host()->Composite(base::TimeTicks::FromInternalValue(1));
EXPECT_EQ(num_requests_, 2);
EXPECT_TRUE(has_failed_);
// Second composite should not request or fail.
- layer_tree_host()->Composite(base::TimeTicks());
+ layer_tree_host()->Composite(base::TimeTicks::FromInternalValue(2));
EXPECT_EQ(num_requests_, 2);
EndTest();
}
@@ -493,7 +493,9 @@ class LayerTreeHostContextTestCommitAfterDelayedOutputSurface
LayerTreeHostContextTest::CreateOutputSurface());
}
- void BeginTest() override { layer_tree_host()->Composite(base::TimeTicks()); }
+ void BeginTest() override {
+ layer_tree_host()->Composite(base::TimeTicks::FromInternalValue(1));
+ }
void ScheduleComposite() override {
if (creating_output_)
@@ -527,7 +529,7 @@ class LayerTreeHostContextTestAvoidUnnecessaryComposite
void BeginTest() override {
in_composite_ = true;
- layer_tree_host()->Composite(base::TimeTicks());
+ layer_tree_host()->Composite(base::TimeTicks::FromInternalValue(1));
in_composite_ = false;
}
diff --git a/cc/trees/layer_tree_impl.cc b/cc/trees/layer_tree_impl.cc
index 0dba33f..a09b4e3 100644
--- a/cc/trees/layer_tree_impl.cc
+++ b/cc/trees/layer_tree_impl.cc
@@ -895,8 +895,8 @@ BeginFrameArgs LayerTreeImpl::CurrentBeginFrameArgs() const {
return layer_tree_host_impl_->CurrentBeginFrameArgs();
}
-base::TimeDelta LayerTreeImpl::begin_impl_frame_interval() const {
- return layer_tree_host_impl_->begin_impl_frame_interval();
+base::TimeDelta LayerTreeImpl::CurrentBeginFrameInterval() const {
+ return layer_tree_host_impl_->CurrentBeginFrameInterval();
}
void LayerTreeImpl::SetNeedsCommit() {
diff --git a/cc/trees/layer_tree_impl.h b/cc/trees/layer_tree_impl.h
index 113062d..e179940 100644
--- a/cc/trees/layer_tree_impl.h
+++ b/cc/trees/layer_tree_impl.h
@@ -96,7 +96,7 @@ class CC_EXPORT LayerTreeImpl {
LayerImpl* FindPendingTreeLayerById(int id);
bool PinchGestureActive() const;
BeginFrameArgs CurrentBeginFrameArgs() const;
- base::TimeDelta begin_impl_frame_interval() const;
+ base::TimeDelta CurrentBeginFrameInterval() const;
void SetNeedsCommit();
gfx::Rect DeviceViewport() const;
gfx::Size DrawViewportSize() const;