Commit 2ca01526 authored by Sahel Sharify's avatar Sahel Sharify Committed by Commit Bot

Do not coalesce wheel with phaseBegan to wheel with phaseEnded.

If no element in the scrolling chain is not scrollable in the given direction
(delta_hints) on GSB we latch to the viewport. This is not user friendly on
platforms that do not have any scroll affordance animations (e.g. Windows)
since if the user changes the scrolling direction and an element in the chain
is actually scrollable in the new direction, the user expects it to scroll and
has no idea that we have latched to the (possibly unscrollable) viewport for
the rest of the scroll sequence. To address the issue for timer based wheel
scroll sequence we end the sequence and start a new one upon arrival of a wheel
event with different delta directions if no GSU has been consumed since the
beginning of the sequence. This cl ensures that the GSE/GSB generated for
breaking the latching seqeunce are not coalesced with each other.

Bug: 882998
Change-Id: Icf703c935475fffd69b899c59abc03ec1b548784
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1524703Reviewed-by: default avatarNavid Zolghadr <nzolghadr@chromium.org>
Commit-Queue: Sahel Sharify <sahel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#641719}
parent 9018deba
...@@ -315,17 +315,9 @@ bool HaveConsistentPhase(const WebMouseWheelEvent& event_to_coalesce, ...@@ -315,17 +315,9 @@ bool HaveConsistentPhase(const WebMouseWheelEvent& event_to_coalesce,
} }
if (event.has_synthetic_phase) { if (event.has_synthetic_phase) {
// Synthetic phase information is added based on a timer in // It is alright to coalesce a wheel event with synthetic phaseChanged to
// MouseWheelPhaseHandler. This information is for simulating scroll // its previous one with synthetic phaseBegan.
// sequences when the beginning and end of scrolls are not available. It is return (event.phase == WebMouseWheelEvent::kPhaseBegan &&
// alright to coalesce an event with synthetic phaseBegan to its previous
// event with synthetic phaseEnded since these phase values don't correspond
// with real start and end of the scroll sequences.
// It is also alright to coalesce a wheel event with synthetic phaseChanged
// to its previous one with synthetic phaseBegan.
return (event.phase == WebMouseWheelEvent::kPhaseEnded &&
event_to_coalesce.phase == WebMouseWheelEvent::kPhaseBegan) ||
(event.phase == WebMouseWheelEvent::kPhaseBegan &&
event_to_coalesce.phase == WebMouseWheelEvent::kPhaseChanged); event_to_coalesce.phase == WebMouseWheelEvent::kPhaseChanged);
} }
return false; return false;
...@@ -375,19 +367,12 @@ void Coalesce(const WebMouseWheelEvent& event_to_coalesce, ...@@ -375,19 +367,12 @@ void Coalesce(const WebMouseWheelEvent& event_to_coalesce,
MergeDispatchTypes(old_dispatch_type, event_to_coalesce.dispatch_type); MergeDispatchTypes(old_dispatch_type, event_to_coalesce.dispatch_type);
if (event_to_coalesce.has_synthetic_phase && if (event_to_coalesce.has_synthetic_phase &&
event_to_coalesce.phase != old_phase) { event_to_coalesce.phase != old_phase) {
if (event_to_coalesce.phase == WebMouseWheelEvent::kPhaseBegan) {
// Coalesce a wheel event with synthetic phase began with a wheel event
// with synthetic phase ended.
DCHECK_EQ(WebMouseWheelEvent::kPhaseEnded, old_phase);
event->phase = WebMouseWheelEvent::kPhaseChanged;
} else {
// Coalesce a wheel event with synthetic phase changed to a wheel event // Coalesce a wheel event with synthetic phase changed to a wheel event
// with synthetic phase began. // with synthetic phase began.
DCHECK_EQ(WebMouseWheelEvent::kPhaseChanged, event_to_coalesce.phase); DCHECK_EQ(WebMouseWheelEvent::kPhaseChanged, event_to_coalesce.phase);
DCHECK_EQ(WebMouseWheelEvent::kPhaseBegan, old_phase); DCHECK_EQ(WebMouseWheelEvent::kPhaseBegan, old_phase);
event->phase = WebMouseWheelEvent::kPhaseBegan; event->phase = WebMouseWheelEvent::kPhaseBegan;
} }
}
} }
bool CanCoalesce(const WebTouchEvent& event_to_coalesce, bool CanCoalesce(const WebTouchEvent& event_to_coalesce,
......
...@@ -182,21 +182,24 @@ TEST(BlinkEventUtilTest, WebMouseWheelEventCoalescing) { ...@@ -182,21 +182,24 @@ TEST(BlinkEventUtilTest, WebMouseWheelEventCoalescing) {
coalesced_event.phase = blink::WebMouseWheelEvent::kPhaseEnded; coalesced_event.phase = blink::WebMouseWheelEvent::kPhaseEnded;
EXPECT_FALSE(CanCoalesce(event_to_be_coalesced, coalesced_event)); EXPECT_FALSE(CanCoalesce(event_to_be_coalesced, coalesced_event));
// With timer based wheel scroll latching, we break the latching sequence on
// direction change when all prior GSU events in the current sequence are
// ignored. To do so we dispatch the pending wheel event with phaseEnded and
// the first wheel event in the opposite direction will have phaseBegan. The
// GSB generated from this wheel event will cause a new hittesting. To make
// sure that a GSB will actually get created we should not coalesce the wheel
// event with synthetic kPhaseBegan to one with synthetic kPhaseEnded.
event_to_be_coalesced.has_synthetic_phase = true; event_to_be_coalesced.has_synthetic_phase = true;
coalesced_event.has_synthetic_phase = true; coalesced_event.has_synthetic_phase = true;
EXPECT_TRUE(CanCoalesce(event_to_be_coalesced, coalesced_event)); EXPECT_FALSE(CanCoalesce(event_to_be_coalesced, coalesced_event));
Coalesce(event_to_be_coalesced, &coalesced_event);
EXPECT_EQ(blink::WebMouseWheelEvent::kPhaseChanged, coalesced_event.phase);
EXPECT_EQ(7, coalesced_event.delta_x);
EXPECT_EQ(9, coalesced_event.delta_y);
event_to_be_coalesced.phase = blink::WebMouseWheelEvent::kPhaseChanged; event_to_be_coalesced.phase = blink::WebMouseWheelEvent::kPhaseChanged;
coalesced_event.phase = blink::WebMouseWheelEvent::kPhaseBegan; coalesced_event.phase = blink::WebMouseWheelEvent::kPhaseBegan;
EXPECT_TRUE(CanCoalesce(event_to_be_coalesced, coalesced_event)); EXPECT_TRUE(CanCoalesce(event_to_be_coalesced, coalesced_event));
Coalesce(event_to_be_coalesced, &coalesced_event); Coalesce(event_to_be_coalesced, &coalesced_event);
EXPECT_EQ(blink::WebMouseWheelEvent::kPhaseBegan, coalesced_event.phase); EXPECT_EQ(blink::WebMouseWheelEvent::kPhaseBegan, coalesced_event.phase);
EXPECT_EQ(10, coalesced_event.delta_x); EXPECT_EQ(7, coalesced_event.delta_x);
EXPECT_EQ(13, coalesced_event.delta_y); EXPECT_EQ(9, coalesced_event.delta_y);
event_to_be_coalesced.resending_plugin_id = 3; event_to_be_coalesced.resending_plugin_id = 3;
EXPECT_FALSE(CanCoalesce(event_to_be_coalesced, coalesced_event)); EXPECT_FALSE(CanCoalesce(event_to_be_coalesced, coalesced_event));
......
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