Skip to content

Commit

Permalink
Deprecate Event.Latency.OS.* for Event.Latency.OS2.*
Browse files Browse the repository at this point in the history
The new metric is correct on Windows, has a wider range so that fewer
samples land in the overflow bucket, and has a millisecond granularity
instead of microsecond.

Also removes the Event.Latency.OS_WIN.* metrics that were used to
validate this code.

Bug: 1189656
Change-Id: I555459596c3ed4f47a874ed27c943f254e230577
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2840587
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Brian White <bcwhite@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Joe Mason <joenotcharles@chromium.org>
Cr-Commit-Position: refs/heads/master@{#878451}
  • Loading branch information
JoeNotCharlesGoogle authored and Chromium LUCI CQ committed May 3, 2021
1 parent de33e9b commit 548a93b
Show file tree
Hide file tree
Showing 17 changed files with 378 additions and 378 deletions.
5 changes: 2 additions & 3 deletions content/browser/android/content_ui_event_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "ui/events/android/key_event_android.h"
#include "ui/events/android/motion_event_android.h"
#include "ui/events/base_event_utils.h"
#include "ui/events/event_utils.h"

using base::android::AttachCurrentThread;
using base::android::JavaParamRef;
Expand Down Expand Up @@ -99,9 +100,7 @@ void ContentUiEventHandler::SendMouseWheelEvent(
base::TimeTicks current_time = ui::EventTimeForNow();
base::TimeTicks event_time =
base::TimeTicks() + base::TimeDelta::FromMilliseconds(time_ms);
base::TimeDelta delta = current_time - event_time;
UMA_HISTOGRAM_CUSTOM_COUNTS("Event.Latency.OS.MOUSE_WHEEL",
delta.InMicroseconds(), 1, 1000000, 50);
ComputeEventLatencyOS(ui::ET_MOUSEWHEEL, event_time, current_time);
ui::MotionEventAndroid::Pointer pointer(
0, x, y, 0.0f /* touch_major */, 0.0f /* touch_minor */, 0.0f, 0.0f, 0);

Expand Down
20 changes: 10 additions & 10 deletions content/browser/renderer_host/render_widget_host_view_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@
#include "ui/events/blink/blink_features.h"
#include "ui/events/blink/did_overscroll_params.h"
#include "ui/events/blink/web_input_event_traits.h"
#include "ui/events/event_utils.h"
#include "ui/events/gesture_detection/gesture_provider_config_helper.h"
#include "ui/gfx/android/view_configuration.h"
#include "ui/gfx/codec/jpeg_codec.h"
Expand Down Expand Up @@ -2304,25 +2305,24 @@ void RenderWidgetHostViewAndroid::OnStylusSelectTap(base::TimeTicks time,
void RenderWidgetHostViewAndroid::ComputeEventLatencyOSTouchHistograms(
const ui::MotionEvent& event) {
base::TimeTicks event_time = event.GetEventTime();
base::TimeDelta delta = base::TimeTicks::Now() - event_time;
base::TimeTicks current_time = base::TimeTicks::Now();
ui::EventType event_type;
switch (event.GetAction()) {
case ui::MotionEvent::Action::DOWN:
case ui::MotionEvent::Action::POINTER_DOWN:
UMA_HISTOGRAM_CUSTOM_COUNTS("Event.Latency.OS.TOUCH_PRESSED",
delta.InMicroseconds(), 1, 1000000, 50);
return;
event_type = ui::ET_TOUCH_PRESSED;
break;
case ui::MotionEvent::Action::MOVE:
UMA_HISTOGRAM_CUSTOM_COUNTS("Event.Latency.OS.TOUCH_MOVED",
delta.InMicroseconds(), 1, 1000000, 50);
return;
event_type = ui::ET_TOUCH_MOVED;
break;
case ui::MotionEvent::Action::UP:
case ui::MotionEvent::Action::POINTER_UP:
UMA_HISTOGRAM_CUSTOM_COUNTS("Event.Latency.OS.TOUCH_RELEASED",
delta.InMicroseconds(), 1, 1000000, 50);
return;
event_type = ui::ET_TOUCH_RELEASED;
break;
default:
return;
}
ui::ComputeEventLatencyOS(event_type, event_time, current_time);
}

void RenderWidgetHostViewAndroid::CreateOverscrollControllerIfPossible() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1697,7 +1697,7 @@ void SendEndPinchEvent() {
base::HistogramTester histogram_tester;

// Send an initial wheel event for scrolling by 3 lines.
// Verify that Event.Latency.OS.MOUSE_WHEEL histogram is computed properly.
// Verify that Event.Latency.OS2.MOUSE_WHEEL histogram is computed properly.
NSEvent* wheelEvent = MockScrollWheelEventWithPhase(@selector(phaseBegan),3);
[rwhv_mac_->GetInProcessNSView() scrollWheel:wheelEvent];

Expand All @@ -1708,6 +1708,7 @@ void SendEndPinchEvent() {
blink::mojom::InputEventResultState::kConsumed);

histogram_tester.ExpectTotalCount("Event.Latency.OS.MOUSE_WHEEL", 1);
histogram_tester.ExpectTotalCount("Event.Latency.OS2.MOUSE_WHEEL", 1);
}

// This test verifies that |selected_text_| is updated accordingly with
Expand Down
33 changes: 29 additions & 4 deletions tools/metrics/histograms/histograms_xml/event/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -380,13 +380,14 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
</summary>
</histogram>

<histogram name="Event.Latency.OS" units="microseconds"
expires_after="2021-12-01">
<histogram name="Event.Latency.OS" units="microseconds" expires_after="M92">
<owner>flackr@chromium.org</owner>
<owner>input-dev@chromium.org</owner>
<summary>
Time between input event received by OS and sent to Chrome, recorded for
each event when it is received by Chrome.
each event when it is received by Chrome. This is obsoleted by
Event.Latency.OS2 and should be removed after they've been run side-by-side
for M92.

Warning: This metric may include reports from clients with low-resolution
clocks (i.e. on Windows, ref. |TimeTicks::IsHighResolution()|). Such reports
Expand All @@ -396,6 +397,21 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
</summary>
</histogram>

<histogram name="Event.Latency.OS2" units="ms" expires_after="2021-12-01">
<owner>flackr@chromium.org</owner>
<owner>input-dev@chromium.org</owner>
<summary>
Time between input event received by OS and sent to Chrome, recorded for
each event when it is received by Chrome.

Warning: On Windows, the timestamps of most events come from a clock with a
several msec resolution so will have an abnormal distribution even when
TimeTicks::IsHighResolution returns true. The exception is touch events
which usually come from the high-resolution clock when
TimeTicks::IsHighResolution returns true.
</summary>
</histogram>

<histogram name="Event.Latency.OS_NO_VALIDATION.POSITIVE" units="ms"
expires_after="2021-08-09">
<owner>sullivan@chromium.org</owner>
Expand All @@ -404,14 +420,17 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
For Mac, a version of Event.Latency.OS that has the positive values it would
contain if ValidateEventTimeClock() were not called on the timestamps.

This metric is intended to debug http://crbug.com/1039833, where very high
This metric is intended to debug http://crbug.com/1039833 where very high
numbers are seen for PageLoad.InputDelay3. The units of this metric are the
same as PageLoad.InputDelay3 for consistency while debugging.
</summary>
</histogram>

<histogram name="Event.Latency.OS_WIN.HIGH_RES" units="ms"
expires_after="2021-12-01">
<obsolete>
Removed 2021-04-22 - replaced by Event.Latency.OS2.
</obsolete>
<owner>joenotcharles@chromium.org</owner>
<owner>catan-team@chromium.org</owner>
<summary>
Expand All @@ -434,6 +453,9 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.

<histogram name="Event.Latency.OS_WIN.LOW_RES" units="ms"
expires_after="2021-12-01">
<obsolete>
Removed 2021-04-22 - replaced by Event.Latency.OS2.
</obsolete>
<owner>joenotcharles@chromium.org</owner>
<owner>catan-team@chromium.org</owner>
<summary>
Expand All @@ -456,6 +478,9 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.

<histogram name="Event.Latency.OS_WIN_IS_VALID" enum="Boolean"
expires_after="2021-12-01">
<obsolete>
Removed 2021-04-22 - replaced by Event.Latency.OS2.
</obsolete>
<owner>joenotcharles@chromium.org</owner>
<owner>catan-team@chromium.org</owner>
<summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6333,11 +6333,24 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
<suffix name="TOUCH_PRESSED" label=""/>
<suffix name="TOUCH_RELEASED" label=""/>
<affected-histogram name="Event.Latency.OS"/>
<affected-histogram name="Event.Latency.OS2"/>
<affected-histogram name="Event.Latency.OS_NO_VALIDATION.NEGATIVE"/>
<affected-histogram name="Event.Latency.OS_NO_VALIDATION.POSITIVE"/>
<affected-histogram name="Event.Latency.OS_WIN.HIGH_RES"/>
<affected-histogram name="Event.Latency.OS_WIN.LOW_RES"/>
<affected-histogram name="Event.Latency.OS_WIN_IS_VALID"/>
<affected-histogram name="Event.Latency.OS_WIN.HIGH_RES">
<obsolete>
Removed 2021-04-22 - replaced by Event.Latency.OS2.
</obsolete>
</affected-histogram>
<affected-histogram name="Event.Latency.OS_WIN.LOW_RES">
<obsolete>
Removed 2021-04-22 - replaced by Event.Latency.OS2.
</obsolete>
</affected-histogram>
<affected-histogram name="Event.Latency.OS_WIN_IS_VALID">
<obsolete>
Removed 2021-04-22 - replaced by Event.Latency.OS2.
</obsolete>
</affected-histogram>
</histogram_suffixes>

<histogram_suffixes name="EventLatencyPinchTypes" separator=".">
Expand Down
5 changes: 5 additions & 0 deletions ui/events/cocoa/events_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ int EventFlagsFromNative(const PlatformEvent& event) {
return timestamp;
}

base::TimeTicks EventLatencyTimeFromNative(const PlatformEvent& native_event,
base::TimeTicks current_time) {
return EventTimeFromNative(native_event);
}

gfx::PointF EventLocationFromNative(const PlatformEvent& native_event) {
NSWindow* window = [native_event window];
NSPoint location = [native_event locationInWindow];
Expand Down
Loading

0 comments on commit 548a93b

Please sign in to comment.