Commit ba4d92c9 authored by Sebastien Seguin-Gagnon's avatar Sebastien Seguin-Gagnon Committed by Commit Bot

Revert "[EventTiming] Always buffer firstInput"

This reverts commit b6e2cb9b.

Reason for revert: Causing WebKitLayout tests failure on linux:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20Tests%20%28dbg%29%281%29/76109
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20Tests%20%28dbg%29%281%29/76108

Original change's description:
> [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/1355880
> Reviewed-by: Timothy Dresser <tdresser@chromium.org>
> Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#612703}

TBR=tdresser@chromium.org,npm@chromium.org

Change-Id: I4b2456f4b9f44798b3157bea960d84f72a50a8bb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 841224, 843184
Reviewed-on: https://chromium-review.googlesource.com/c/1357404Reviewed-by: default avatarSebastien Seguin-Gagnon <sebsg@chromium.org>
Commit-Queue: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612790}
parent 82ec0997
...@@ -37,8 +37,9 @@ void EventTiming::WillDispatchEvent(const Event& event) { ...@@ -37,8 +37,9 @@ void EventTiming::WillDispatchEvent(const Event& event) {
// dispatched. // dispatched.
if ((performance_->ShouldBufferEventTiming() && if ((performance_->ShouldBufferEventTiming() &&
!performance_->IsEventTimingBufferFull()) || !performance_->IsEventTimingBufferFull()) ||
performance_->HasObserverFor(PerformanceEntry::kEvent) || performance_->HasObserverFor(PerformanceEntry::kEvent)
!performance_->FirstInputDetected()) { || (performance_->HasObserverFor(PerformanceEntry::kFirstInput)
&& !performance_->FirstInputDetected())) {
processing_start_ = CurrentTimeTicks(); processing_start_ = CurrentTimeTicks();
finished_will_dispatch_event_ = true; finished_will_dispatch_event_ = true;
} }
......
...@@ -340,7 +340,7 @@ void WindowPerformance::RegisterEventTiming(const AtomicString& event_type, ...@@ -340,7 +340,7 @@ void WindowPerformance::RegisterEventTiming(const AtomicString& event_type,
bool cancelable) { bool cancelable) {
DCHECK(origin_trials::EventTimingEnabled(GetExecutionContext())); DCHECK(origin_trials::EventTimingEnabled(GetExecutionContext()));
// |start_time| could be null in some tests that inject input. DCHECK(!start_time.is_null());
DCHECK(!processing_start.is_null()); DCHECK(!processing_start.is_null());
DCHECK(!processing_end.is_null()); DCHECK(!processing_end.is_null());
DCHECK_GE(processing_end, processing_start); DCHECK_GE(processing_end, processing_start);
...@@ -374,7 +374,7 @@ void WindowPerformance::ReportEventTimings(WebLayerTreeView::SwapResult result, ...@@ -374,7 +374,7 @@ void WindowPerformance::ReportEventTimings(WebLayerTreeView::SwapResult result,
for (const auto& entry : event_timings_) { for (const auto& entry : event_timings_) {
int duration_in_ms = std::ceil((end_time - entry->startTime()) / 8) * 8; int duration_in_ms = std::ceil((end_time - entry->startTime()) / 8) * 8;
entry->SetDuration(duration_in_ms); entry->SetDuration(duration_in_ms);
if (!first_input_timing_) { if (!first_input_detected_) {
if (entry->name() == "pointerdown") { if (entry->name() == "pointerdown") {
first_pointer_down_event_timing_ = first_pointer_down_event_timing_ =
PerformanceEventTiming::CreateFirstInputTiming(entry); PerformanceEventTiming::CreateFirstInputTiming(entry);
...@@ -413,6 +413,7 @@ void WindowPerformance::AddElementTiming(const AtomicString& name, ...@@ -413,6 +413,7 @@ void WindowPerformance::AddElementTiming(const AtomicString& name,
void WindowPerformance::DispatchFirstInputTiming( void WindowPerformance::DispatchFirstInputTiming(
PerformanceEventTiming* entry) { PerformanceEventTiming* entry) {
DCHECK(origin_trials::EventTimingEnabled(GetExecutionContext())); DCHECK(origin_trials::EventTimingEnabled(GetExecutionContext()));
first_input_detected_ = true;
if (!entry) if (!entry)
return; return;
...@@ -423,7 +424,8 @@ void WindowPerformance::DispatchFirstInputTiming( ...@@ -423,7 +424,8 @@ void WindowPerformance::DispatchFirstInputTiming(
} }
DCHECK(!first_input_timing_); DCHECK(!first_input_timing_);
first_input_timing_ = entry; if (ShouldBufferEventTiming())
first_input_timing_ = entry;
} }
void WindowPerformance::AddLayoutJankFraction(double jank_fraction) { void WindowPerformance::AddLayoutJankFraction(double jank_fraction) {
......
...@@ -71,7 +71,7 @@ class CORE_EXPORT WindowPerformance final : public Performance, ...@@ -71,7 +71,7 @@ class CORE_EXPORT WindowPerformance final : public Performance,
bool ShouldBufferEventTiming(); bool ShouldBufferEventTiming();
bool FirstInputDetected() const { return !!first_input_timing_; } bool FirstInputDetected() const { return first_input_detected_; }
// This method creates a PerformanceEventTiming and if needed creates a swap // This method creates a PerformanceEventTiming and if needed creates a swap
// promise to calculate the |duration| attribute when such promise is // promise to calculate the |duration| attribute when such promise is
...@@ -119,6 +119,10 @@ class CORE_EXPORT WindowPerformance final : public Performance, ...@@ -119,6 +119,10 @@ class CORE_EXPORT WindowPerformance final : public Performance,
// dispatch has been completed but the swap promise used to determine // dispatch has been completed but the swap promise used to determine
// |duration| has not been resolved. // |duration| has not been resolved.
HeapVector<Member<PerformanceEventTiming>> event_timings_; 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_; Member<PerformanceEventTiming> first_pointer_down_event_timing_;
mutable Member<PerformanceNavigation> navigation_; mutable Member<PerformanceNavigation> navigation_;
mutable Member<PerformanceTiming> timing_; mutable Member<PerformanceTiming> timing_;
......
...@@ -31,8 +31,8 @@ ...@@ -31,8 +31,8 @@
if (numEventsObserved >= 2) { if (numEventsObserved >= 2) {
assert_equals(performance.getEntriesByType('event').length, 0, assert_equals(performance.getEntriesByType('event').length, 0,
"There should be no buffered event entries."); "There should be no buffered event entries.");
assert_equals(performance.getEntriesByType('firstInput').length, 1, assert_equals(performance.getEntriesByType('firstInput').length, 0,
"There should be a buffered firstInput entry."); "There should be no buffered firstInput entries.");
// There should be 2 event entries and one firstInput entry. // There should be 2 event entries and one firstInput entry.
assert_equals(numEventsObserved, 2, assert_equals(numEventsObserved, 2,
"There should be 2 observed event entries."); "There should be 2 observed event entries.");
...@@ -42,8 +42,8 @@ ...@@ -42,8 +42,8 @@
} }
})).observe({ entryTypes: ['event', 'firstInput'] }); })).observe({ entryTypes: ['event', 'firstInput'] });
on_event(window, 'load', () => { on_event(window, 'load', () => {
clickAndBlockMain('button').then(() => { clickAndBlockMain('button').then(wait).then(() => {
clickAndBlockMain('button'); clickAndBlockMain('button').then(wait);
}); });
}); });
}, },
......
...@@ -62,9 +62,9 @@ registration are lost ...@@ -62,9 +62,9 @@ registration are lost
*/ */
async_test(function(t) { async_test(function(t) {
on_event(window, 'load', () => { on_event(window, 'load', () => {
clickAndBlockMain('button').then(wait).then(() => { clickAndBlockMain('button').then(() => {
startObserver(t); startObserver(t);
clickAndBlockMain('button'); clickAndBlockMain('button').then(wait);
processingStartMin = performance.now(); 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,9 +52,7 @@ let testFunction = function(session, t, fakeDeviceController) { ...@@ -52,9 +52,7 @@ let testFunction = function(session, t, fakeDeviceController) {
session.addEventListener("resetpose", onPoseReset, false); session.addEventListener("resetpose", onPoseReset, false);
// Trigger the reset pose. // Trigger the reset pose.
session.requestAnimationFrame(() => { session.requestAnimationFrame((time, xrFrame) => {});
session.requestAnimationFrame(() => {});
});
return eventPromise; return eventPromise;
}; };
......
...@@ -38,11 +38,12 @@ let testFunction = function(session, t, fakeDeviceController) { ...@@ -38,11 +38,12 @@ let testFunction = function(session, t, fakeDeviceController) {
return session.requestFrameOfReference("stage") return session.requestFrameOfReference("stage")
.then((frameOfRef) => new Promise((resolve, reject) => { .then((frameOfRef) => new Promise((resolve, reject) => {
function onFirstFrame(time, xrFrame) { function onFirstFrame(time, xrFrame) {
// On the first frame where the pose has been initialized, the stage // On the first frame, the stage should be using an emulated frame of
// should be using an emulated frame of reference because it has no // reference because it has no stageParameters yet. So the pose should
// stageParameters yet. So the pose should be ~1.5 meters off the floor. // be ~1.5 meters off the floor.
t.step( () => { t.step( () => {
let pose = xrFrame.getDevicePose(frameOfRef); let pose = xrFrame.getDevicePose(frameOfRef);
assert_not_equals(pose, null);
let poseMatrix = pose.poseModelMatrix; let poseMatrix = pose.poseModelMatrix;
assert_approx_equals(poseMatrix[12], 0.0, FLOAT_EPSILON); assert_approx_equals(poseMatrix[12], 0.0, FLOAT_EPSILON);
...@@ -50,10 +51,9 @@ let testFunction = function(session, t, fakeDeviceController) { ...@@ -50,10 +51,9 @@ let testFunction = function(session, t, fakeDeviceController) {
assert_approx_equals(poseMatrix[14], 0.0, FLOAT_EPSILON); assert_approx_equals(poseMatrix[14], 0.0, FLOAT_EPSILON);
fakeDeviceController.setStageTransform(VALID_STAGE_TRANSFORM); fakeDeviceController.setStageTransform(VALID_STAGE_TRANSFORM);
session.requestAnimationFrame(() => {
session.requestAnimationFrame(onFrame);
});
}); });
session.requestAnimationFrame(onFrame);
} }
function onFrame(time, xrFrame) { function onFrame(time, xrFrame) {
...@@ -70,9 +70,7 @@ let testFunction = function(session, t, fakeDeviceController) { ...@@ -70,9 +70,7 @@ let testFunction = function(session, t, fakeDeviceController) {
resolve(); resolve();
} }
session.requestAnimationFrame(() => { session.requestAnimationFrame(onFirstFrame);
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