Commit 1d9f1be4 authored by Mohsen Izadi's avatar Mohsen Izadi Committed by Commit Bot

Fix end-to-end EventLatency metrics for key events

Currently, we do not report end-to-end EventLatency metrics for
WebInputEvent's of type kRawKeyDown and kChar. An example is when a user
enters a text in an input field. The reason is that the function that
converts WebInputEvent::Type to ui::EventType return ET_UNKNOWN for
these two specific event types. It should return ET_KEY_PRESSED.

Bug: 1054007
Change-Id: I701a71c43dc2f2b13ee20607b0950858155398e3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2085597
Commit-Queue: Mohsen Izadi <mohsen@chromium.org>
Reviewed-by: default avatarSadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760358}
parent 3760e692
...@@ -51,6 +51,9 @@ const char* EventMetrics::GetTypeName() const { ...@@ -51,6 +51,9 @@ const char* EventMetrics::GetTypeName() const {
case ui::ET_MOUSEWHEEL: case ui::ET_MOUSEWHEEL:
return "MouseWheel"; return "MouseWheel";
case ui::ET_KEY_PRESSED: case ui::ET_KEY_PRESSED:
// TODO(crbug/1071645): Currently, all ET_KEY_PRESSED events are reported
// under EventLatency.KeyPressed histogram. This includes both key-down
// and key-char events. Consider reporting them separately.
return "KeyPressed"; return "KeyPressed";
case ui::ET_KEY_RELEASED: case ui::ET_KEY_RELEASED:
return "KeyReleased"; return "KeyReleased";
......
...@@ -54,6 +54,8 @@ class EventLatencyBrowserTest : public ContentBrowserTest { ...@@ -54,6 +54,8 @@ class EventLatencyBrowserTest : public ContentBrowserTest {
void FocusButton() const { ASSERT_TRUE(ExecJs(shell(), "focusButton()")); } void FocusButton() const { ASSERT_TRUE(ExecJs(shell(), "focusButton()")); }
void FocusInput() const { ASSERT_TRUE(ExecJs(shell(), "focusInput()")); }
void StartAnimations() const { void StartAnimations() const {
ASSERT_TRUE(ExecJs(shell(), "startAnimations()")); ASSERT_TRUE(ExecJs(shell(), "startAnimations()"));
} }
...@@ -74,8 +76,8 @@ IN_PROC_BROWSER_TEST_F(EventLatencyBrowserTest, KeyPressOnButton) { ...@@ -74,8 +76,8 @@ IN_PROC_BROWSER_TEST_F(EventLatencyBrowserTest, KeyPressOnButton) {
->GetRootWindow()); ->GetRootWindow());
// Press and release the space key. Since the button on the test page is // Press and release the space key. Since the button on the test page is
// focused, this should change the visuals of the button and generate a // focused, this should change the visuals of the button and generate
// compositor frame with appropriate event latency metrics. // compositor frames with appropriate event latency metrics.
generator.PressKey(ui::VKEY_SPACE, 0); generator.PressKey(ui::VKEY_SPACE, 0);
generator.ReleaseKey(ui::VKEY_SPACE, 0); generator.ReleaseKey(ui::VKEY_SPACE, 0);
RunUntilInputProcessed(GetWidgetHost()); RunUntilInputProcessed(GetWidgetHost());
...@@ -83,6 +85,17 @@ IN_PROC_BROWSER_TEST_F(EventLatencyBrowserTest, KeyPressOnButton) { ...@@ -83,6 +85,17 @@ IN_PROC_BROWSER_TEST_F(EventLatencyBrowserTest, KeyPressOnButton) {
FetchHistogramsFromChildProcesses(); FetchHistogramsFromChildProcesses();
base::HistogramTester::CountsMap expected_counts = { base::HistogramTester::CountsMap expected_counts = {
{"EventLatency.KeyPressed.BrowserToRendererCompositor", 1},
{"EventLatency.KeyPressed.BeginImplFrameToSendBeginMainFrame", 1},
{"EventLatency.KeyPressed.SendBeginMainFrameToCommit", 1},
{"EventLatency.KeyPressed.Commit", 1},
{"EventLatency.KeyPressed.EndCommitToActivation", 1},
{"EventLatency.KeyPressed.Activation", 1},
{"EventLatency.KeyPressed.EndActivateToSubmitCompositorFrame", 1},
{"EventLatency.KeyPressed."
"SubmitCompositorFrameToPresentationCompositorFrame",
1},
{"EventLatency.KeyPressed.TotalLatency", 1},
{"EventLatency.KeyReleased.BrowserToRendererCompositor", 1}, {"EventLatency.KeyReleased.BrowserToRendererCompositor", 1},
{"EventLatency.KeyReleased.BeginImplFrameToSendBeginMainFrame", 1}, {"EventLatency.KeyReleased.BeginImplFrameToSendBeginMainFrame", 1},
{"EventLatency.KeyReleased.SendBeginMainFrameToCommit", 1}, {"EventLatency.KeyReleased.SendBeginMainFrameToCommit", 1},
...@@ -115,8 +128,8 @@ IN_PROC_BROWSER_TEST_F(EventLatencyBrowserTest, KeyPressOnButtonWithAnimation) { ...@@ -115,8 +128,8 @@ IN_PROC_BROWSER_TEST_F(EventLatencyBrowserTest, KeyPressOnButtonWithAnimation) {
->GetRootWindow()); ->GetRootWindow());
// Press and release the space key. Since the button on the test page is // Press and release the space key. Since the button on the test page is
// focused, this should change the visuals of the button and generate a // focused, this should change the visuals of the button and generate
// compositor frame with appropriate event latency metrics. // compositor frames with appropriate event latency metrics.
generator.PressKey(ui::VKEY_SPACE, 0); generator.PressKey(ui::VKEY_SPACE, 0);
generator.ReleaseKey(ui::VKEY_SPACE, 0); generator.ReleaseKey(ui::VKEY_SPACE, 0);
RunUntilInputProcessed(GetWidgetHost()); RunUntilInputProcessed(GetWidgetHost());
...@@ -124,6 +137,17 @@ IN_PROC_BROWSER_TEST_F(EventLatencyBrowserTest, KeyPressOnButtonWithAnimation) { ...@@ -124,6 +137,17 @@ IN_PROC_BROWSER_TEST_F(EventLatencyBrowserTest, KeyPressOnButtonWithAnimation) {
FetchHistogramsFromChildProcesses(); FetchHistogramsFromChildProcesses();
base::HistogramTester::CountsMap expected_counts = { base::HistogramTester::CountsMap expected_counts = {
{"EventLatency.KeyPressed.BrowserToRendererCompositor", 1},
{"EventLatency.KeyPressed.BeginImplFrameToSendBeginMainFrame", 1},
{"EventLatency.KeyPressed.SendBeginMainFrameToCommit", 1},
{"EventLatency.KeyPressed.Commit", 1},
{"EventLatency.KeyPressed.EndCommitToActivation", 1},
{"EventLatency.KeyPressed.Activation", 1},
{"EventLatency.KeyPressed.EndActivateToSubmitCompositorFrame", 1},
{"EventLatency.KeyPressed."
"SubmitCompositorFrameToPresentationCompositorFrame",
1},
{"EventLatency.KeyPressed.TotalLatency", 1},
{"EventLatency.KeyReleased.BrowserToRendererCompositor", 1}, {"EventLatency.KeyReleased.BrowserToRendererCompositor", 1},
{"EventLatency.KeyReleased.BeginImplFrameToSendBeginMainFrame", 1}, {"EventLatency.KeyReleased.BeginImplFrameToSendBeginMainFrame", 1},
{"EventLatency.KeyReleased.SendBeginMainFrameToCommit", 1}, {"EventLatency.KeyReleased.SendBeginMainFrameToCommit", 1},
...@@ -140,4 +164,48 @@ IN_PROC_BROWSER_TEST_F(EventLatencyBrowserTest, KeyPressOnButtonWithAnimation) { ...@@ -140,4 +164,48 @@ IN_PROC_BROWSER_TEST_F(EventLatencyBrowserTest, KeyPressOnButtonWithAnimation) {
testing::ContainerEq(expected_counts)); testing::ContainerEq(expected_counts));
} }
// Tests that entering a character in a textbox leads to appropriate event
// latency metrics being reported even though the page has an animation running.
IN_PROC_BROWSER_TEST_F(EventLatencyBrowserTest, KeyPressInInputWithAnimation) {
base::HistogramTester histogram_tester;
ASSERT_NO_FATAL_FAILURE(LoadTestPage());
StartAnimations();
FocusInput();
ui::test::EventGenerator generator(shell()
->web_contents()
->GetRenderWidgetHostView()
->GetNativeView()
->GetRootWindow());
// Enter a character into the focused textbox. This should generate compositor
// frames with appropriate event latency metrics.
generator.PressKey(ui::VKEY_A, 0);
generator.ReleaseKey(ui::VKEY_A, 0);
RunUntilInputProcessed(GetWidgetHost());
FetchHistogramsFromChildProcesses();
// TODO(crbug/1071645): Since this is this first key-press after the textbox
// is focused, there would be two reports, one for the RawKeyDown that causes
// some style changes (due to :focus-visible behavior) and one for the Char
// that inserts the actual character. These should be reported separately.
base::HistogramTester::CountsMap expected_counts = {
{"EventLatency.KeyPressed.BrowserToRendererCompositor", 2},
{"EventLatency.KeyPressed.BeginImplFrameToSendBeginMainFrame", 2},
{"EventLatency.KeyPressed.SendBeginMainFrameToCommit", 2},
{"EventLatency.KeyPressed.Commit", 2},
{"EventLatency.KeyPressed.EndCommitToActivation", 2},
{"EventLatency.KeyPressed.Activation", 2},
{"EventLatency.KeyPressed.EndActivateToSubmitCompositorFrame", 2},
{"EventLatency.KeyPressed."
"SubmitCompositorFrameToPresentationCompositorFrame",
2},
{"EventLatency.KeyPressed.TotalLatency", 2},
};
EXPECT_THAT(histogram_tester.GetTotalCountsForPrefix("EventLatency."),
testing::ContainerEq(expected_counts));
}
} // namespace content } // namespace content
...@@ -36,12 +36,14 @@ ...@@ -36,12 +36,14 @@
<body> <body>
<button id="button">Press here with space</button> <button id="button">Press here with space</button>
<input id="input" placeholder="Type something here">
<div> <div>
<div id="raf-box"></div> <div id="raf-box"></div>
<div id="main-box"></div> <div id="main-box"></div>
</div> </div>
<script> <script>
var button = document.getElementById('button'); var button = document.getElementById('button');
var input = document.getElementById('input');
var rafBox = document.getElementById('raf-box'); var rafBox = document.getElementById('raf-box');
var mainBox = document.getElementById('main-box'); var mainBox = document.getElementById('main-box');
...@@ -52,6 +54,10 @@ ...@@ -52,6 +54,10 @@
button.focus(); button.focus();
}; };
var focusInput = function() {
input.focus();
};
var rafAnimate = function(time) { var rafAnimate = function(time) {
if (rafStart === null) if (rafStart === null)
rafStart = time; rafStart = time;
......
...@@ -1018,13 +1018,13 @@ EventType WebEventTypeToEventType(WebInputEvent::Type type) { ...@@ -1018,13 +1018,13 @@ EventType WebEventTypeToEventType(WebInputEvent::Type type) {
case WebInputEvent::kMouseWheel: case WebInputEvent::kMouseWheel:
return ET_MOUSEWHEEL; return ET_MOUSEWHEEL;
case WebInputEvent::kRawKeyDown: case WebInputEvent::kRawKeyDown:
return ET_UNKNOWN; return ET_KEY_PRESSED;
case WebInputEvent::kKeyDown: case WebInputEvent::kKeyDown:
return ET_KEY_PRESSED; return ET_KEY_PRESSED;
case WebInputEvent::kKeyUp: case WebInputEvent::kKeyUp:
return ET_KEY_RELEASED; return ET_KEY_RELEASED;
case WebInputEvent::kChar: case WebInputEvent::kChar:
return ET_UNKNOWN; return ET_KEY_PRESSED;
case WebInputEvent::kGestureScrollBegin: case WebInputEvent::kGestureScrollBegin:
return ET_GESTURE_SCROLL_BEGIN; return ET_GESTURE_SCROLL_BEGIN;
case WebInputEvent::kGestureScrollEnd: case WebInputEvent::kGestureScrollEnd:
......
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