Skip to content

GitLab

T
tangled
Commit 5f1830ac authored by Tim Dresser's avatar Tim Dresser Committed by Commit Bot
Browse files
Options

Fix latency info termination. 

Previously, we incorrectly modified the trace ID of LatencyInfo
objects which were being coalesced. This caused their async events
to never terminate.

Now we don't modify the trace ID during coalescing.

Bug: 771165
Change-Id: I1e73b97579333b9be67886b4a93e53e8315d30cb
Reviewed-on: https://chromium-review.googlesource.com/700406Reviewed-by: default avatarDave Tapuska <dtapuska@chromium.org>
Commit-Queue: Timothy Dresser <tdresser@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509417}
parent b54d4836
Hide whitespace changes
Inline Side-by-side
Showing with 96 additions and 8 deletions
content/browser/renderer_host/input/mouse_latency_browsertest.cc
View file @ 5f1830ac
...@@ -15,11 +15,11 @@ ...@@ -15,11 +15,11 @@
#include "content/browser/renderer_host/input/synthetic_gesture.h" #include "content/browser/renderer_host/input/synthetic_gesture.h"
#include "content/browser/renderer_host/input/synthetic_gesture_controller.h" #include "content/browser/renderer_host/input/synthetic_gesture_controller.h"
#include "content/browser/renderer_host/input/synthetic_gesture_target.h" #include "content/browser/renderer_host/input/synthetic_gesture_target.h"
#include "content/browser/renderer_host/input/synthetic_smooth_move_gesture.h"
#include "content/browser/renderer_host/input/synthetic_tap_gesture.h" #include "content/browser/renderer_host/input/synthetic_tap_gesture.h"
#include "content/browser/renderer_host/render_widget_host_impl.h" #include "content/browser/renderer_host/render_widget_host_impl.h"
#include "content/browser/web_contents/web_contents_impl.h" #include "content/browser/web_contents/web_contents_impl.h"
#include "content/common/input/synthetic_gesture_params.h" #include "content/common/input/synthetic_gesture_params.h"
#include "content/common/input/synthetic_smooth_scroll_gesture_params.h"
#include "content/public/browser/render_view_host.h" #include "content/public/browser/render_view_host.h"
#include "content/public/browser/render_widget_host_view.h" #include "content/public/browser/render_widget_host_view.h"
#include "content/public/browser/tracing_controller.h" #include "content/public/browser/tracing_controller.h"
...@@ -30,7 +30,7 @@ ...@@ -30,7 +30,7 @@
namespace { namespace {
const char kMouseUpDownDataURL[] = const char kDataURL[] =
"data:text/html;charset=utf-8," "data:text/html;charset=utf-8,"
"<!DOCTYPE html>" "<!DOCTYPE html>"
"<html>" "<html>"
...@@ -38,6 +38,12 @@ const char kMouseUpDownDataURL[] = ...@@ -38,6 +38,12 @@ const char kMouseUpDownDataURL[] =
"<title>Mouse event trace events reported.</title>" "<title>Mouse event trace events reported.</title>"
"<script src=\"../../resources/testharness.js\"></script>" "<script src=\"../../resources/testharness.js\"></script>"
"<script src=\"../../resources/testharnessreport.js\"></script>" "<script src=\"../../resources/testharnessreport.js\"></script>"
"<script>"
" document.addEventListener('mousemove', () => {"
" var end = performance.now() + 20;"
" while(performance.now() < end);"
" });"
"</script>"
"<style>" "<style>"
"body {" "body {"
" height:3000px;" " height:3000px;"
...@@ -81,7 +87,7 @@ class MouseLatencyBrowserTest : public ContentBrowserTest { ...@@ -81,7 +87,7 @@ class MouseLatencyBrowserTest : public ContentBrowserTest {
protected: protected:
void LoadURL() { void LoadURL() {
const GURL data_url(kMouseUpDownDataURL); const GURL data_url(kDataURL);
NavigateToURL(shell(), data_url); NavigateToURL(shell(), data_url);
RenderWidgetHostImpl* host = GetWidgetHost(); RenderWidgetHostImpl* host = GetWidgetHost();
...@@ -107,6 +113,29 @@ class MouseLatencyBrowserTest : public ContentBrowserTest { ...@@ -107,6 +113,29 @@ class MouseLatencyBrowserTest : public ContentBrowserTest {
runner_->Run(); runner_->Run();
} }
// Generate mouse events drag from |position|.
void DoSyncCoalescedMoves(const gfx::PointF position,
const gfx::Vector2dF& delta1,
const gfx::Vector2dF& delta2) {
SyntheticSmoothMoveGestureParams params;
params.input_type = SyntheticSmoothMoveGestureParams::MOUSE_DRAG_INPUT;
params.start_point.SetPoint(position.x(), position.y());
params.distances.push_back(delta1);
params.distances.push_back(delta2);
std::unique_ptr<SyntheticSmoothMoveGesture> gesture(
new SyntheticSmoothMoveGesture(params));
GetWidgetHost()->QueueSyntheticGesture(
std::move(gesture),
base::BindOnce(&MouseLatencyBrowserTest::OnSyntheticGestureCompleted,
base::Unretained(this)));
// Runs until we get the OnSyntheticGestureCompleted callback
runner_ = base::MakeUnique<base::RunLoop>();
runner_->Run();
}
void StartTracing() { void StartTracing() {
base::trace_event::TraceConfig trace_config( base::trace_event::TraceConfig trace_config(
"{" "{"
...@@ -193,4 +222,54 @@ IN_PROC_BROWSER_TEST_F(MouseLatencyBrowserTest, ...@@ -193,4 +222,54 @@ IN_PROC_BROWSER_TEST_F(MouseLatencyBrowserTest,
"InputLatency::MouseUp", "InputLatency::MouseUp")); "InputLatency::MouseUp", "InputLatency::MouseUp"));
} }
// Ensures that LatencyInfo async slices are reported correctly for MouseMove
// events in the case where events are coalesced. (crbug.com/771165).
// Disabled on Android because we don't support synthetic mouse input on Android
// (crbug.com/723618).
#if defined(OS_ANDROID)
#define MAYBE_CoalescedMouseMovesCorrectlyTerminated \
DISABLED_CoalescedMouseMovesCorrectlyTerminated
#else
#define MAYBE_CoalescedMouseMovesCorrectlyTerminated \
CoalescedMouseMovesCorrectlyTerminated
#endif
IN_PROC_BROWSER_TEST_F(MouseLatencyBrowserTest,
MAYBE_CoalescedMouseMovesCorrectlyTerminated) {
LoadURL();
StartTracing();
DoSyncCoalescedMoves(gfx::PointF(100, 100), gfx::Vector2dF(150, 150),
gfx::Vector2dF(250, 250));
const base::Value& trace_data = StopTracing();
const base::DictionaryValue* trace_data_dict;
trace_data.GetAsDictionary(&trace_data_dict);
ASSERT_TRUE(trace_data.GetAsDictionary(&trace_data_dict));
const base::ListValue* traceEvents;
ASSERT_TRUE(trace_data_dict->GetList("traceEvents", &traceEvents));
std::map<std::string, int> trace_ids;
for (size_t i = 0; i < traceEvents->GetSize(); ++i) {
const base::DictionaryValue* traceEvent;
ASSERT_TRUE(traceEvents->GetDictionary(i, &traceEvent));
std::string name;
ASSERT_TRUE(traceEvent->GetString("name", &name));
if (name != "InputLatency::MouseMove")
continue;
std::string id;
if (traceEvent->GetString("id", &id))
++trace_ids[id];
}
for (auto i : trace_ids) {
// Each trace id should show up once for the begin, and once for the end.
EXPECT_EQ(2, i.second);
}
}
} // namespace content } // namespace content
ui/latency/latency_info.cc
View file @ 5f1830ac
...@@ -164,6 +164,12 @@ bool LatencyInfo::Verify(const std::vector<LatencyInfo>& latency_info, ...@@ -164,6 +164,12 @@ bool LatencyInfo::Verify(const std::vector<LatencyInfo>& latency_info,
void LatencyInfo::CopyLatencyFrom(const LatencyInfo& other, void LatencyInfo::CopyLatencyFrom(const LatencyInfo& other,
LatencyComponentType type) { LatencyComponentType type) {
// Don't clobber an existing trace_id_.
if (trace_id_ == -1) {
DCHECK(latency_components().empty());
trace_id_ = other.trace_id();
}
for (const auto& lc : other.latency_components()) { for (const auto& lc : other.latency_components()) {
if (lc.first.first == type) { if (lc.first.first == type) {
AddLatencyNumberWithTimestamp(lc.first.first, AddLatencyNumberWithTimestamp(lc.first.first,
...@@ -177,15 +183,19 @@ void LatencyInfo::CopyLatencyFrom(const LatencyInfo& other, ...@@ -177,15 +183,19 @@ void LatencyInfo::CopyLatencyFrom(const LatencyInfo& other,
expected_queueing_time_on_dispatch_ = expected_queueing_time_on_dispatch_ =
other.expected_queueing_time_on_dispatch_; other.expected_queueing_time_on_dispatch_;
trace_id_ = other.trace_id();
coalesced_ = other.coalesced(); coalesced_ = other.coalesced();
// TODO(tdresser): Ideally we'd copy |began_| here as well, but |began_| isn't // TODO(tdresser): Ideally we'd copy |began_| here as well, but |began_|
// very intuitive, and we can actually begin multiple times across copied // isn't very intuitive, and we can actually begin multiple times across
// events. // copied events.
terminated_ = other.terminated(); terminated_ = other.terminated();
} }
void LatencyInfo::AddNewLatencyFrom(const LatencyInfo& other) { void LatencyInfo::AddNewLatencyFrom(const LatencyInfo& other) {
// Don't clobber an existing trace_id_.
if (trace_id_ == -1) {
trace_id_ = other.trace_id();
}
for (const auto& lc : other.latency_components()) { for (const auto& lc : other.latency_components()) {
if (!FindLatency(lc.first.first, lc.first.second, NULL)) { if (!FindLatency(lc.first.first, lc.first.second, NULL)) {
AddLatencyNumberWithTimestamp(lc.first.first, AddLatencyNumberWithTimestamp(lc.first.first,
...@@ -199,7 +209,6 @@ void LatencyInfo::AddNewLatencyFrom(const LatencyInfo& other) { ...@@ -199,7 +209,6 @@ void LatencyInfo::AddNewLatencyFrom(const LatencyInfo& other) {
expected_queueing_time_on_dispatch_ = expected_queueing_time_on_dispatch_ =
other.expected_queueing_time_on_dispatch_; other.expected_queueing_time_on_dispatch_;
trace_id_ = other.trace_id();
coalesced_ = other.coalesced(); coalesced_ = other.coalesced();
// TODO(tdresser): Ideally we'd copy |began_| here as well, but |began_| isn't // TODO(tdresser): Ideally we'd copy |began_| here as well, but |began_| isn't
// very intuitive, and we can actually begin multiple times across copied // very intuitive, and we can actually begin multiple times across copied
......
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