Skip to content

Commit

Permalink
Only log event latency histograms for whitelisted events.
Browse files Browse the repository at this point in the history
Previously, we were logging them for all events but only had
a subset of those documented in histograms.xml, which means
the others were using memory, bandwidth and were being sent
up without anyone using them. This CL makes the code only
log the ones that are documented.

Bug: 828188
Change-Id: Ib165ced4f8a8395e5b0928adbcb2affad8ad8bad
Reviewed-on: https://chromium-review.googlesource.com/c/1443026
Commit-Queue: Alexei Svitkine <[email protected]>
Reviewed-by: Navid Zolghadr <[email protected]>
Cr-Commit-Position: refs/heads/master@{#627723}
  • Loading branch information
asvitkine-chromium authored and Commit Bot committed Jan 31, 2019
1 parent bb926bc commit bddec50
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 67 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,14 @@ void RenderWidgetHostLatencyTracker::ComputeInputLatencyHistograms(

std::string event_name = WebInputEvent::GetName(type);

if (latency.source_event_type() == ui::SourceEventType::KEY_PRESS)
if (latency.source_event_type() == ui::SourceEventType::KEY_PRESS) {
event_name = "KeyPress";
} else if (event_name != "TouchEnd" && event_name != "TouchMove" &&
event_name != "TouchStart") {
// Only log events we care about (that are documented in histograms.xml),
// to avoid using memory and bandwidth for metrics that are not important.
return;
}

std::string default_action_status =
action_prevented ? "DefaultPrevented" : "DefaultAllowed";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1185,72 +1185,6 @@ TEST_F(RenderWidgetHostLatencyTrackerTest,
ElementsAre());
}

// Some touch input histograms aren't reported for multi-finger touch. Other
// input modalities shouldn't be impacted by there being an active multi-finger
// touch gesture.
TEST_F(RenderWidgetHostLatencyTrackerTest, WheelDuringMultiFingerTouch) {
SyntheticWebTouchEvent touch_event;
InputEventAckState ack_state = INPUT_EVENT_ACK_STATE_NOT_CONSUMED;

{
// First touch start.
ui::LatencyInfo latency;
latency.set_source_event_type(ui::SourceEventType::TOUCH);
touch_event.PressPoint(1, 1);
tracker()->OnInputEvent(touch_event, &latency);
tracker()->OnInputEventAck(touch_event, &latency, ack_state);
}

{
// Second touch start.
ui::LatencyInfo latency;
latency.set_source_event_type(ui::SourceEventType::TOUCH);
touch_event.PressPoint(1, 1);
tracker()->OnInputEvent(touch_event, &latency);
tracker()->OnInputEventAck(touch_event, &latency, ack_state);
}

{
// Wheel event.
ui::LatencyInfo latency;
latency.set_source_event_type(ui::SourceEventType::WHEEL);
// These numbers are sensitive to where the histogram buckets are.
int timestamps_ms[] = {11, 25, 35};
auto wheel_event = SyntheticWebMouseWheelEventBuilder::Build(
blink::WebMouseWheelEvent::kPhaseChanged);
tracker()->OnInputEvent(touch_event, &latency);

ui::LatencyInfo fake_latency;
fake_latency.set_trace_id(kTraceEventId);
fake_latency.set_source_event_type(ui::SourceEventType::TOUCH);
fake_latency.AddLatencyNumberWithTimestamp(
ui::INPUT_EVENT_LATENCY_BEGIN_RWH_COMPONENT,
base::TimeTicks() + base::TimeDelta::FromMilliseconds(timestamps_ms[0]),
1);

fake_latency.AddLatencyNumberWithTimestamp(
ui::INPUT_EVENT_LATENCY_RENDERER_MAIN_COMPONENT,
base::TimeTicks() + base::TimeDelta::FromMilliseconds(timestamps_ms[1]),
1);

fake_latency.AddLatencyNumberWithTimestamp(
ui::INPUT_EVENT_LATENCY_ACK_RWH_COMPONENT,
base::TimeTicks() + base::TimeDelta::FromMilliseconds(timestamps_ms[2]),
1);

// Call ComputeInputLatencyHistograms directly to avoid OnInputEventAck
// overwriting components.
tracker()->ComputeInputLatencyHistograms(wheel_event.GetType(),
fake_latency, ack_state);

tracker()->OnInputEventAck(wheel_event, &latency, ack_state);
}

EXPECT_THAT(histogram_tester().GetAllSamples(
"Event.Latency.QueueingTime.MouseWheelDefaultAllowed"),
ElementsAre(Bucket(14, 1)));
}

TEST_F(RenderWidgetHostLatencyTrackerTest, TouchpadPinchEvents) {
ui::LatencyInfo latency;
latency.set_trace_id(kTraceEventId);
Expand Down

0 comments on commit bddec50

Please sign in to comment.