Commit 4a243ad4 authored by Daniel Libby's avatar Daniel Libby Committed by Commit Bot

Remove histogram for initial gesture scroll recognition latency

Revert the code changes from crrev.com/c/1590960 and mark the histogram
as obsolete. We were investigating how modifying the slop region
affected this metric, and have seen data showing that, as expected, the
metric goes proportional to the decrease in slop distance.

Bug: 988762
Change-Id: I802aad4f80b718d163f4b536f3ee66e7a90b3526
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1875367Reviewed-by: default avatarNavid Zolghadr <nzolghadr@chromium.org>
Reviewed-by: default avatarIlya Sherman <isherman@chromium.org>
Commit-Queue: Daniel Libby <dlibby@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#708793}
parent 4be24bf9
......@@ -41520,6 +41520,9 @@ uploading your change for review.
<histogram name="Event.Scroll.TouchGestureLatency" units="ms"
expires_after="M79">
<obsolete>
Deprecated as of 10/2019 due to no further need of data.
</obsolete>
<owner>nzolghadr@chromium.org</owner>
<owner>dlibby@microsoft.com</owner>
<summary>
......@@ -10,7 +10,6 @@
#include "base/auto_reset.h"
#include "base/macros.h"
#include "base/metrics/histogram_macros.h"
#include "base/trace_event/trace_event.h"
#include "ui/events/event_constants.h"
#include "ui/events/gesture_detection/gesture_configuration.h"
......@@ -327,16 +326,14 @@ class GestureProvider::GestureListenerImpl : public ScaleGestureListener,
float raw_distance_y) override {
float distance_x = raw_distance_x;
float distance_y = raw_distance_y;
base::TimeTicks original_event_timestamp;
if (!scroll_event_sent_ && e2.GetPointerCount() < 3) {
// Remove the touch slop region from the first scroll event to avoid a
// jump. Touch slop isn't used for scroll gestures with greater than 2
// pointers down, in those cases we don't subtract the slop.
std::pair<gfx::Vector2dF, base::TimeTicks> slop_delta_with_timestamp =
gfx::Vector2dF delta =
ComputeFirstScrollDelta(e1, e2, secondary_pointer_down);
distance_x = slop_delta_with_timestamp.first.x();
distance_y = slop_delta_with_timestamp.first.y();
original_event_timestamp = slop_delta_with_timestamp.second;
distance_x = delta.x();
distance_y = delta.y();
}
snap_scroll_controller_.UpdateSnapScrollMode(distance_x, distance_y);
......@@ -351,15 +348,6 @@ class GestureProvider::GestureListenerImpl : public ScaleGestureListener,
return true;
if (!scroll_event_sent_) {
if (!original_event_timestamp.is_null()) {
// Calculate the latency (and log once per gesture-based scroll
// initiation) by determining the amount of time it took from when
// the pointer down event happened until now (when we've recognized
// that it should be a scroll).
UMA_HISTOGRAM_TIMES("Event.Scroll.TouchGestureLatency",
base::TimeTicks::Now() - original_event_timestamp);
}
// Note that scroll start hints are in distance traveled, where
// scroll deltas are in the opposite direction.
GestureEventDetails scroll_details(ET_GESTURE_SCROLL_BEGIN, -distance_x,
......@@ -722,7 +710,7 @@ class GestureProvider::GestureListenerImpl : public ScaleGestureListener,
// for the first time, scroll delta is adjusted.
// The new deltas are calculated for each pointer individually,
// and the final scroll delta is the average over all delta values.
std::pair<gfx::Vector2dF, base::TimeTicks> ComputeFirstScrollDelta(
gfx::Vector2dF ComputeFirstScrollDelta(
const MotionEvent& ev1,
const MotionEvent& ev2,
const MotionEvent& secondary_pointer_down) {
......@@ -731,7 +719,6 @@ class GestureProvider::GestureListenerImpl : public ScaleGestureListener,
DCHECK(ev2.GetPointerCount() < 3);
gfx::Vector2dF delta(0, 0);
base::TimeTicks original_timestamp;
for (size_t i = 0; i < ev2.GetPointerCount(); i++) {
const int pointer_id = ev2.GetPointerId(i);
const MotionEvent* source_pointer_down_event =
......@@ -748,13 +735,9 @@ class GestureProvider::GestureListenerImpl : public ScaleGestureListener,
float dx = source_pointer_down_event->GetX(source_index) - ev2.GetX(i);
float dy = source_pointer_down_event->GetY(source_index) - ev2.GetY(i);
delta += SubtractSlopRegion(dx, dy);
if (original_timestamp.is_null()) {
original_timestamp = source_pointer_down_event->GetEventTime();
}
}
delta.Scale(1.0 / ev2.GetPointerCount());
return std::make_pair(delta, original_timestamp);
return delta;
}
const GestureProvider::Config config_;
......
......@@ -724,38 +724,6 @@ TEST_F(GestureProviderTest, NoTapAfterScrollBegins) {
EXPECT_FALSE(HasReceivedGesture(ET_GESTURE_LONG_TAP));
}
TEST_F(GestureProviderTest, ScrollBeginLatencyUMA) {
auto histogram_tester = std::make_unique<base::HistogramTester>();
base::TimeTicks event_time = base::TimeTicks::Now();
// Send in a DOWN and MOVE that exceeds the slop threshold, and validate
// that the UMA fires.
MockMotionEvent event =
ObtainMotionEvent(event_time, MotionEvent::Action::DOWN);
EXPECT_TRUE(gesture_provider_->OnTouchEvent(event));
EXPECT_EQ(ET_GESTURE_TAP_DOWN, GetMostRecentGestureEventType());
EXPECT_EQ(1, GetMostRecentGestureEvent().details.touch_points());
const float touch_slop = GetTouchSlop();
event =
ObtainMotionEvent(event_time + kOneMicrosecond, MotionEvent::Action::MOVE,
kFakeCoordX + 2 * touch_slop, kFakeCoordY);
EXPECT_TRUE(gesture_provider_->OnTouchEvent(event));
EXPECT_EQ(ET_GESTURE_SCROLL_UPDATE, GetMostRecentGestureEventType());
base::TimeTicks complete_time = base::TimeTicks::Now();
std::vector<base::Bucket> buckets =
histogram_tester->GetAllSamples("Event.Scroll.TouchGestureLatency");
EXPECT_EQ(buckets.size(), 1ULL);
EXPECT_LE(buckets.front().min, (complete_time - event_time).InMilliseconds());
event = ObtainMotionEvent(event_time + kOneSecond, MotionEvent::Action::UP,
kFakeCoordX + 2 * touch_slop, kFakeCoordY);
EXPECT_TRUE(gesture_provider_->OnTouchEvent(event));
EXPECT_EQ(ET_GESTURE_SCROLL_END, GetMostRecentGestureEventType());
}
TEST_F(GestureProviderTest, DoubleTap) {
base::TimeTicks event_time = base::TimeTicks::Now();
......@@ -1024,7 +992,6 @@ TEST_F(GestureProviderTest, SlopRegionCheckOnTwoFingerScroll) {
EnableTwoFingerTap(kMaxTwoFingerTapSeparation, base::TimeDelta());
const float scaled_touch_slop = GetTouchSlop();
auto histogram_tester = std::make_unique<base::HistogramTester>();
base::TimeTicks event_time = base::TimeTicks::Now();
MockMotionEvent event =
......@@ -1072,10 +1039,6 @@ TEST_F(GestureProviderTest, SlopRegionCheckOnTwoFingerScroll) {
EXPECT_EQ(ET_GESTURE_SCROLL_UPDATE, GetReceivedGesture(3).type());
EXPECT_EQ(ET_GESTURE_SCROLL_END, GetReceivedGesture(4).type());
EXPECT_EQ(5U, GetReceivedGestureCount());
// Even when there are two pointers, we should only log the gesture latency
// one time.
histogram_tester->ExpectTotalCount("Event.Scroll.TouchGestureLatency", 1);
}
// This test simulates cases like (crbug.com/704426) in which some of the events
......
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