Commit b6e2cb9b authored by Nicolas Pena's avatar Nicolas Pena Committed by Commit Bot

[EventTiming] Always buffer firstInput

Before, we'd buffer the firstInput entry only if it happened before
onload. This CL forces firstInput to always be buffered. This means
EventTiming code is called more in some tests, so the event hardware
timestamp is no longer guaranteed to be nonzero, and a couple of xr
tests require double RAF to avoid competing with the swap promises from
Event Timing.

Bug: 841224, 843184

Change-Id: I942b934c387798c100da4ecfff52affb66ab94e8
Reviewed-on: https://chromium-review.googlesource.com/c/1355880Reviewed-by: default avatarTimothy Dresser <tdresser@chromium.org>
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612703}
parent e5dcd97d
......@@ -37,9 +37,8 @@ void EventTiming::WillDispatchEvent(const Event& event) {
// dispatched.
if ((performance_->ShouldBufferEventTiming() &&
!performance_->IsEventTimingBufferFull()) ||
performance_->HasObserverFor(PerformanceEntry::kEvent)
|| (performance_->HasObserverFor(PerformanceEntry::kFirstInput)
&& !performance_->FirstInputDetected())) {
performance_->HasObserverFor(PerformanceEntry::kEvent) ||
!performance_->FirstInputDetected()) {
processing_start_ = CurrentTimeTicks();
finished_will_dispatch_event_ = true;
}
......
......@@ -340,7 +340,7 @@ void WindowPerformance::RegisterEventTiming(const AtomicString& event_type,
bool cancelable) {
DCHECK(origin_trials::EventTimingEnabled(GetExecutionContext()));
DCHECK(!start_time.is_null());
// |start_time| could be null in some tests that inject input.
DCHECK(!processing_start.is_null());
DCHECK(!processing_end.is_null());
DCHECK_GE(processing_end, processing_start);
......@@ -374,7 +374,7 @@ void WindowPerformance::ReportEventTimings(WebLayerTreeView::SwapResult result,
for (const auto& entry : event_timings_) {
int duration_in_ms = std::ceil((end_time - entry->startTime()) / 8) * 8;
entry->SetDuration(duration_in_ms);
if (!first_input_detected_) {
if (!first_input_timing_) {
if (entry->name() == "pointerdown") {
first_pointer_down_event_timing_ =
PerformanceEventTiming::CreateFirstInputTiming(entry);
......@@ -413,7 +413,6 @@ void WindowPerformance::AddElementTiming(const AtomicString& name,
void WindowPerformance::DispatchFirstInputTiming(
PerformanceEventTiming* entry) {
DCHECK(origin_trials::EventTimingEnabled(GetExecutionContext()));
first_input_detected_ = true;
if (!entry)
return;
......@@ -424,8 +423,7 @@ void WindowPerformance::DispatchFirstInputTiming(
}
DCHECK(!first_input_timing_);
if (ShouldBufferEventTiming())
first_input_timing_ = entry;
first_input_timing_ = entry;
}
void WindowPerformance::AddLayoutJankFraction(double jank_fraction) {
......
......@@ -71,7 +71,7 @@ class CORE_EXPORT WindowPerformance final : public Performance,
bool ShouldBufferEventTiming();
bool FirstInputDetected() const { return first_input_detected_; }
bool FirstInputDetected() const { return !!first_input_timing_; }
// This method creates a PerformanceEventTiming and if needed creates a swap
// promise to calculate the |duration| attribute when such promise is
......@@ -119,10 +119,6 @@ class CORE_EXPORT WindowPerformance final : public Performance,
// dispatch has been completed but the swap promise used to determine
// |duration| has not been resolved.
HeapVector<Member<PerformanceEventTiming>> event_timings_;
// We use a bool separate from |first_input_timing_| because if the first
// input does not happen before onload then |first_input_timing_| will never
// be populated since it should not be accessible from the performance buffer.
bool first_input_detected_ = false;
Member<PerformanceEventTiming> first_pointer_down_event_timing_;
mutable Member<PerformanceNavigation> navigation_;
mutable Member<PerformanceTiming> timing_;
......
......@@ -31,8 +31,8 @@
if (numEventsObserved >= 2) {
assert_equals(performance.getEntriesByType('event').length, 0,
"There should be no buffered event entries.");
assert_equals(performance.getEntriesByType('firstInput').length, 0,
"There should be no buffered firstInput entries.");
assert_equals(performance.getEntriesByType('firstInput').length, 1,
"There should be a buffered firstInput entry.");
// There should be 2 event entries and one firstInput entry.
assert_equals(numEventsObserved, 2,
"There should be 2 observed event entries.");
......@@ -42,8 +42,8 @@
}
})).observe({ entryTypes: ['event', 'firstInput'] });
on_event(window, 'load', () => {
clickAndBlockMain('button').then(wait).then(() => {
clickAndBlockMain('button').then(wait);
clickAndBlockMain('button').then(() => {
clickAndBlockMain('button');
});
});
},
......
......@@ -62,9 +62,9 @@ registration are lost
*/
async_test(function(t) {
on_event(window, 'load', () => {
clickAndBlockMain('button').then(() => {
clickAndBlockMain('button').then(wait).then(() => {
startObserver(t);
clickAndBlockMain('button').then(wait);
clickAndBlockMain('button');
processingStartMin = performance.now();
});
});
......
<!DOCTYPE html>
<html>
<meta charset=utf-8 />
<title>Event Timing: firstInput entry should be buffered even without observer</title>
<button id='button'>Generate a 'click' event</button>
<script src=/resources/testharness.js></script>
<script src=/resources/testharnessreport.js></script>
<script src=/resources/testdriver.js></script>
<script src=/resources/testdriver-vendor.js></script>
<script src=resources/event-timing-support.js></script>
<script>
async_test(function(t) {
clickAndBlockMain('button').then(wait).then(t.step_func_done(() => {
assert_equals(performance.getEntriesByType('firstInput').length, 1,
"There should be a firstInput entry in the performance timeline");
const entry = performance.getEntriesByType('firstInput')[0];
assert_equals(entry.name, 'click');
assert_equals(entry.entryType, 'firstInput');
assert_greater_than(entry.duration, 50,
"The first input was a long one.");
}));
},
"Event Timing: check firstInput after onload, observer, click, click."
);
</script>
</html>
......@@ -52,7 +52,9 @@ let testFunction = function(session, t, fakeDeviceController) {
session.addEventListener("resetpose", onPoseReset, false);
// Trigger the reset pose.
session.requestAnimationFrame((time, xrFrame) => {});
session.requestAnimationFrame(() => {
session.requestAnimationFrame(() => {});
});
return eventPromise;
};
......
......@@ -38,12 +38,11 @@ let testFunction = function(session, t, fakeDeviceController) {
return session.requestFrameOfReference("stage")
.then((frameOfRef) => new Promise((resolve, reject) => {
function onFirstFrame(time, xrFrame) {
// On the first frame, the stage should be using an emulated frame of
// reference because it has no stageParameters yet. So the pose should
// be ~1.5 meters off the floor.
// On the first frame where the pose has been initialized, the stage
// should be using an emulated frame of reference because it has no
// stageParameters yet. So the pose should be ~1.5 meters off the floor.
t.step( () => {
let pose = xrFrame.getDevicePose(frameOfRef);
assert_not_equals(pose, null);
let poseMatrix = pose.poseModelMatrix;
assert_approx_equals(poseMatrix[12], 0.0, FLOAT_EPSILON);
......@@ -51,9 +50,10 @@ let testFunction = function(session, t, fakeDeviceController) {
assert_approx_equals(poseMatrix[14], 0.0, FLOAT_EPSILON);
fakeDeviceController.setStageTransform(VALID_STAGE_TRANSFORM);
session.requestAnimationFrame(() => {
session.requestAnimationFrame(onFrame);
});
});
session.requestAnimationFrame(onFrame);
}
function onFrame(time, xrFrame) {
......@@ -70,7 +70,9 @@ let testFunction = function(session, t, fakeDeviceController) {
resolve();
}
session.requestAnimationFrame(onFirstFrame);
session.requestAnimationFrame(() => {
session.requestAnimationFrame(onFirstFrame);
});
}));
};
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment