Commit 14f2111f authored by Findit's avatar Findit

Revert "Add explicit flag for compositor scrollbar injected gestures"

This reverts commit d9a56afc.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 669086 as the
culprit for flakes in the build cycles as shown on:
https://analysis.chromium.org/p/chromium/flake-portal/analysis/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vZDlhNTZhZmNiZGY5ODUwYmMzOWJiM2VkYjU2ZDA3ZDExYTFlYjJiMgw

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.chromiumos/linux-chromeos-rel/25818

Sample Failed Step: content_browsertests on Ubuntu-16.04

Sample Flaky Test: ScrollLatencyScrollbarBrowserTest.ScrollbarThumbDragLatency

Original change's description:
> Add explicit flag for compositor scrollbar injected gestures
> 
> The original change to enable scrollbar latency for the composited
> scrollbars incorrectly used an existing member to try and determine
> whether a GestureScrollUpdate was the first one in an injected sequence
> or not. is_first_gesture_scroll_update_ was incorrect because it is only
> updated when input is actually dispatched to InputHandlerProxy, and the
> flag is cleared for all GSUs before the location where it was being
> read.
> 
> This bug was missed because of incorrect tests. The
> VerifyRecordedSamplesForHistogram method doesn't actually assert or
> expect anything - the return value must be inspected.
> 
> As part of fixing up the tests, I made a few other changes to get them
> passing consistently across all platforms:
> - turn on main thread scrollbar injection feature (in case it's ever
>   turned off we don't want the tests to start failing)
> - enable mock scrollbars
> - disable smooth scrolling
> - don't run scrollbar tests on Android
> 
> The composited scrollbar button test is disabled due to a bug in how
> the mock theme reports its button sizes, which throws off the region
> detection in ScrollbarLayerImplBase::IdentifyScrollbarPart (filed
> crbug.com/974063 for this issue).
> 
> Change-Id: Ie1a762a5f6ecc264d22f0256db68f141fc76b950
> 
> Bug: 954007
> Change-Id: Ib258e08e083e79da90ba2e4e4216e4879cf00cf7
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1652741
> Commit-Queue: Daniel Libby <dlibby@microsoft.com>
> Reviewed-by: David Bokan <bokan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#669086}


Change-Id: Icc743e48fa740fe27f0cb0cfa21b209a696f518c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 954007
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1660114
Cr-Commit-Position: refs/heads/master@{#669150}
parent a7237f2e
......@@ -174,7 +174,6 @@ InputHandlerProxy::InputHandlerProxy(cc::InputHandler* input_handler,
current_overscroll_params_(nullptr),
has_ongoing_compositor_scroll_or_pinch_(false),
is_first_gesture_scroll_update_(false),
last_injected_gesture_was_begin_(false),
tick_clock_(base::DefaultTickClock::GetInstance()),
snap_fling_controller_(std::make_unique<cc::SnapFlingController>(this)),
compositor_touch_action_enabled_(
......@@ -312,10 +311,15 @@ void InputHandlerProxy::DispatchSingleInputEvent(
current_overscroll_params_.reset();
blink::WebGestureEvent::Type type = event_with_callback->event().GetType();
if (type == blink::WebGestureEvent::kGestureScrollUpdate) {
EnsureScrollUpdateLatencyComponent(
&monitored_latency_info, event_with_callback->event().TimeStamp());
}
InputHandlerProxy::EventDisposition disposition = RouteToTypeSpecificHandler(
event_with_callback->event(), original_latency_info);
blink::WebGestureEvent::Type type = event_with_callback->event().GetType();
switch (type) {
case blink::WebGestureEvent::kGestureScrollBegin:
is_first_gesture_scroll_update_ = true;
......@@ -361,6 +365,32 @@ void InputHandlerProxy::DispatchSingleInputEvent(
std::move(current_overscroll_params_));
}
// Scroll updates injected from within the renderer process will not have a
// scroll update component, since those are added to the latency info
// in the browser process before being dispatched to the renderer.
void InputHandlerProxy::EnsureScrollUpdateLatencyComponent(
LatencyInfo* monitored_latency_info,
base::TimeTicks original_timestamp) {
// Currently we only expect LatencyInfo's of type SCROLLBAR to get into
// this state, but there are a few exceptions (i.e. GestureScrollUpdates
// that are coalesced with GesturePinchUpdates via
// CompositorThreadEventQueue::Queue/CoalesceScrollAndPinch).
// TODO(dlibby): Update that codepath and turn this into a DCHECK when there
// is a missing scroll update component.
if (monitored_latency_info->source_event_type() !=
ui::SourceEventType::SCROLLBAR)
return;
// Add a scroll update component to the latency info if one doesn't exist,
// based on whether or not this is the first scroll update we've seen in a
// gesture sequence.
monitored_latency_info->AddLatencyNumberWithTimestamp(
(is_first_gesture_scroll_update_)
? ui::INPUT_EVENT_LATENCY_FIRST_SCROLL_UPDATE_ORIGINAL_COMPONENT
: ui::INPUT_EVENT_LATENCY_SCROLL_UPDATE_ORIGINAL_COMPONENT,
original_timestamp, 1);
}
void InputHandlerProxy::DispatchQueuedInputEvents() {
// Calling |NowTicks()| is expensive so we only want to do it once.
base::TimeTicks now = tick_clock_->NowTicks();
......@@ -406,24 +436,6 @@ void InputHandlerProxy::InjectScrollbarGestureScroll(
DCHECK(!scrollbar_latency_info.FindLatency(
ui::INPUT_EVENT_LATENCY_RENDERING_SCHEDULED_IMPL_COMPONENT, nullptr));
if (type == WebInputEvent::Type::kGestureScrollBegin) {
last_injected_gesture_was_begin_ = true;
} else {
if (type == WebInputEvent::Type::kGestureScrollUpdate) {
// For injected GSUs, add a scroll update component to the latency info
// so that it is properly classified as a scroll. If the last injected
// gesture was a GSB, then this GSU is the first scroll update - mark
// the LatencyInfo as such.
scrollbar_latency_info.AddLatencyNumberWithTimestamp(
(last_injected_gesture_was_begin_)
? ui::INPUT_EVENT_LATENCY_FIRST_SCROLL_UPDATE_ORIGINAL_COMPONENT
: ui::INPUT_EVENT_LATENCY_SCROLL_UPDATE_ORIGINAL_COMPONENT,
original_timestamp, 1);
}
last_injected_gesture_was_begin_ = false;
}
std::unique_ptr<EventWithCallback> gesture_event_with_callback_update =
std::make_unique<EventWithCallback>(
std::move(web_scoped_gesture_event), scrollbar_latency_info,
......
......@@ -187,6 +187,12 @@ class InputHandlerProxy : public cc::InputHandlerClient,
bool* is_touching_scrolling_layer,
cc::TouchAction* white_listed_touch_action);
// Scroll updates injected from within the renderer process will not have a
// scroll update component, since those are added to the latency info
// in the browser process before being dispatched to the renderer.
void EnsureScrollUpdateLatencyComponent(LatencyInfo* monitored_latency_info,
base::TimeTicks original_timestamp);
InputHandlerProxyClient* client_;
cc::InputHandler* input_handler_;
......@@ -229,13 +235,6 @@ class InputHandlerProxy : public cc::InputHandlerClient,
bool has_ongoing_compositor_scroll_or_pinch_;
bool is_first_gesture_scroll_update_;
// Whether the last injected scroll gesture was a GestureScrollBegin. Used to
// determine which GestureScrollUpdate is the first in a gesture sequence for
// latency classification. This is separate from
// |is_first_gesture_scroll_update_| and is used to determine which type of
// latency component should be added for injected GestureScrollUpdates.
bool last_injected_gesture_was_begin_;
const base::TickClock* tick_clock_;
std::unique_ptr<cc::SnapFlingController> snap_fling_controller_;
......
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