Commit 5dd16566 authored by David Bokan's avatar David Bokan Committed by Commit Bot

Revert "Do not coalesce wheel with phaseBegan to wheel with phaseEnded."

This reverts commit 2ca01526.

Reason for revert: Speculative revert for https://crbug.com/978980

Original change's description:
> 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/+/1524703
> Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org>
> Commit-Queue: Sahel Sharify <sahel@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#641719}

TBR=nzolghadr@chromium.org,sahel@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 882998,978980
Change-Id: I85799b6e6eace029b4c2ef00e142f035ac9e8e4c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1688230Reviewed-by: default avatarDavid Bokan <bokan@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#674615}
parent 810d97a5
...@@ -317,9 +317,17 @@ bool HaveConsistentPhase(const WebMouseWheelEvent& event_to_coalesce, ...@@ -317,9 +317,17 @@ bool HaveConsistentPhase(const WebMouseWheelEvent& event_to_coalesce,
} }
if (event.has_synthetic_phase) { if (event.has_synthetic_phase) {
// It is alright to coalesce a wheel event with synthetic phaseChanged to // Synthetic phase information is added based on a timer in
// its previous one with synthetic phaseBegan. // MouseWheelPhaseHandler. This information is for simulating scroll
return (event.phase == WebMouseWheelEvent::kPhaseBegan && // sequences when the beginning and end of scrolls are not available. It is
// 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;
...@@ -369,11 +377,18 @@ void Coalesce(const WebMouseWheelEvent& event_to_coalesce, ...@@ -369,11 +377,18 @@ 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) {
// Coalesce a wheel event with synthetic phase changed to a wheel event if (event_to_coalesce.phase == WebMouseWheelEvent::kPhaseBegan) {
// with synthetic phase began. // Coalesce a wheel event with synthetic phase began with a wheel event
DCHECK_EQ(WebMouseWheelEvent::kPhaseChanged, event_to_coalesce.phase); // with synthetic phase ended.
DCHECK_EQ(WebMouseWheelEvent::kPhaseBegan, old_phase); DCHECK_EQ(WebMouseWheelEvent::kPhaseEnded, old_phase);
event->phase = WebMouseWheelEvent::kPhaseBegan; event->phase = WebMouseWheelEvent::kPhaseChanged;
} else {
// Coalesce a wheel event with synthetic phase changed to a wheel event
// with synthetic phase began.
DCHECK_EQ(WebMouseWheelEvent::kPhaseChanged, event_to_coalesce.phase);
DCHECK_EQ(WebMouseWheelEvent::kPhaseBegan, old_phase);
event->phase = WebMouseWheelEvent::kPhaseBegan;
}
} }
} }
......
...@@ -252,24 +252,21 @@ TEST(BlinkEventUtilTest, WebMouseWheelEventCoalescing) { ...@@ -252,24 +252,21 @@ 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_FALSE(CanCoalesce(event_to_be_coalesced, coalesced_event)); EXPECT_TRUE(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(7, coalesced_event.delta_x); EXPECT_EQ(10, coalesced_event.delta_x);
EXPECT_EQ(9, coalesced_event.delta_y); EXPECT_EQ(13, 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