Commit e47b748a authored by Lan Wei's avatar Lan Wei Committed by Commit Bot

Update hover when scroll snap happens at the snap point

For the case that the snap point is the same as the scroll position, we
set the snap_at_gesture_scroll_end flag to be true, but there is no
scroll snap animation happen, We pass a callback of setting hover
state dirty to snap function, and when the snap point is the same as
the scroll position, the callback will be executed to update the hover
state right away.

Majid's CL to change SnapCoordinator::PerformSnapping to return true
when the snap point is the same as the scroll position:
https://chromium-review.googlesource.com/c/chromium/src/+/1769044

Bug: 877132
Change-Id: I4cfc1bf03ee980b4b68e74aad79a8bec6a906da1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1790259
Commit-Queue: Lan Wei <lanwei@chromium.org>
Reviewed-by: default avatarNavid Zolghadr <nzolghadr@chromium.org>
Reviewed-by: default avatarMajid Valipour <majidvp@chromium.org>
Reviewed-by: default avatarDavid Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707990}
parent e9b6e11c
......@@ -1889,6 +1889,75 @@ TEST_F(EventHandlerSimTest,
EXPECT_TRUE(target2->IsHovered());
}
TEST_F(EventHandlerSimTest,
TestUpdateHoverAfterMainThreadScrollAtSnapPointAtBeginFrame) {
ScopedUpdateHoverAtBeginFrameForTest scoped_feature(true);
WebView().MainFrameWidget()->Resize(WebSize(800, 600));
SimRequest request("https://example.com/test.html", "text/html");
LoadURL("https://example.com/test.html");
request.Complete(R"HTML(
<!DOCTYPE html>
<style>
div {
position: absolute;
}
#scroller {
width: 500px;
height: 500px;
overflow: scroll;
scroll-snap-type: both mandatory;
border: solid black 5px;
}
.target:hover {
background-color: red;
}
.target {
width: 200px;
height: 500px;
scroll-snap-align: start;
background-color: blue;
}
</style>
<body>
<div id="scroller">
<div class="target" id="target1" style="left: 0px; top: 0px;"></div>
<div class="target" id="target2" style="left: 0px; top: 500px;"></div>
</div>
</body>
)HTML");
Compositor().BeginFrame();
// Set mouse position and active web view.
InitializeMousePositionAndActivateView(150, 150);
Compositor().BeginFrame();
Element* const scroller = GetDocument().getElementById("scroller");
Element* target1 = GetDocument().getElementById("target1");
Element* target2 = GetDocument().getElementById("target2");
EXPECT_TRUE(target1->IsHovered());
EXPECT_FALSE(target2->IsHovered());
// Send scroll gesture events which will cause scroll happen in main thread.
// The hover state will be marked dirty when the scroll lands exactly on a
// snap point.
ScrollableArea* scrollable_area =
scroller->GetLayoutBox()->GetScrollableArea();
ASSERT_EQ(0, scrollable_area->GetScrollOffset().Height());
constexpr float delta_y = 500;
InjectScrollFromGestureEvents(
scrollable_area->GetCompositorElementId().GetStableId(), 0, delta_y);
ASSERT_EQ(500, scrollable_area->GetScrollOffset().Height());
EXPECT_TRUE(target1->IsHovered());
EXPECT_FALSE(target2->IsHovered());
// The hover effect on targets is updated after the next begin frame.
Compositor().BeginFrame();
ASSERT_EQ(500, scrollable_area->GetScrollOffset().Height());
EXPECT_FALSE(target1->IsHovered());
EXPECT_TRUE(target2->IsHovered());
}
TEST_F(EventHandlerSimTest, LargeCustomCursorIntersectsViewport) {
WebView().MainFrameWidget()->Resize(WebSize(800, 600));
SimRequest request("https://example.com/test.html", "text/html");
......
......@@ -684,7 +684,6 @@ WebInputEventResult ScrollManager::HandleGestureScrollEnd(
const WebGestureEvent& gesture_event) {
TRACE_EVENT0("input", "ScrollManager::handleGestureScrollEnd");
Node* node = scroll_gesture_handling_node_;
bool snap_at_gesture_scroll_end = false;
if (node && node->GetLayoutObject()) {
// If the GSE is for a scrollable area that has an in-progress animation,
......@@ -722,28 +721,33 @@ WebInputEventResult ScrollManager::HandleGestureScrollEnd(
ScrollState::Create(std::move(scroll_state_data));
CustomizedScroll(*scroll_state);
snap_at_gesture_scroll_end = SnapAtGestureScrollEnd(gesture_event);
// We add a callback to set the hover state dirty and send a scroll end
// event when the scroll ends without snap or the snap point is the same as
// the scroll position.
base::ScopedClosureRunner callback(WTF::Bind(
[](WeakPersistent<LocalFrame> local_frame,
WeakPersistent<ScrollManager> scroll_manager) {
if (RuntimeEnabledFeatures::UpdateHoverAtBeginFrameEnabled() &&
local_frame) {
local_frame->GetEventHandler().MarkHoverStateDirty();
}
Node* scroll_end_target = scroll_manager->GetScrollEventTarget();
if (RuntimeEnabledFeatures::OverscrollCustomizationEnabled() &&
scroll_end_target) {
scroll_end_target->GetDocument().EnqueueScrollEndEventForNode(
scroll_end_target);
}
},
WrapWeakPersistent(&(frame_->LocalFrameRoot())),
WrapWeakPersistent(this)));
SnapAtGestureScrollEnd(gesture_event, std::move(callback));
NotifyScrollPhaseEndForCustomizedScroll();
if (RuntimeEnabledFeatures::OverscrollCustomizationEnabled() &&
!snap_at_gesture_scroll_end) {
if (Node* scroll_end_target = GetScrollEventTarget()) {
scroll_end_target->GetDocument().EnqueueScrollEndEventForNode(
scroll_end_target);
}
}
}
ClearGestureScrollState();
// If we are performing a snap at the scrollend gesture, we should update
// hover state dirty at the end of the programmatic scroll animation caused
// by the snap, and we should avoid marking the hover state dirty here.
if (!snap_at_gesture_scroll_end &&
RuntimeEnabledFeatures::UpdateHoverAtBeginFrameEnabled()) {
frame_->LocalFrameRoot().GetEventHandler().MarkHoverStateDirty();
}
return WebInputEventResult::kNotHandled;
}
......@@ -757,7 +761,9 @@ ScrollOffset GetScrollDirection(FloatSize delta) {
return delta.ShrunkTo(FloatSize(1, 1)).ExpandedTo(FloatSize(-1, -1));
}
bool ScrollManager::SnapAtGestureScrollEnd(const WebGestureEvent& end_event) {
bool ScrollManager::SnapAtGestureScrollEnd(
const WebGestureEvent& end_event,
base::ScopedClosureRunner on_finish) {
if (!previous_gesture_scrolled_node_ ||
(!did_scroll_x_for_scroll_gesture_ && !did_scroll_y_for_scroll_gesture_))
return false;
......@@ -787,11 +793,13 @@ bool ScrollManager::SnapAtGestureScrollEnd(const WebGestureEvent& end_event) {
// limit the miscalculation to 1px.
ScrollOffset scroll_direction =
GetScrollDirection(last_scroll_delta_for_scroll_gesture_);
return scrollable_area->SnapForDirection(scroll_direction);
return scrollable_area->SnapForDirection(scroll_direction,
std::move(on_finish));
}
return scrollable_area->SnapAtCurrentPosition(
did_scroll_x_for_scroll_gesture_, did_scroll_y_for_scroll_gesture_);
did_scroll_x_for_scroll_gesture_, did_scroll_y_for_scroll_gesture_,
std::move(on_finish));
}
bool ScrollManager::GetSnapFlingInfo(
......
......@@ -7,6 +7,7 @@
#include <memory>
#include "base/callback_helpers.h"
#include "base/macros.h"
#include "cc/input/snap_fling_controller.h"
#include "third_party/blink/public/platform/web_input_event_result.h"
......@@ -141,7 +142,8 @@ class CORE_EXPORT ScrollManager : public GarbageCollected<ScrollManager>,
WebGestureEvent SynthesizeGestureScrollBegin(
const WebGestureEvent& update_event);
bool SnapAtGestureScrollEnd(const WebGestureEvent& end_event);
bool SnapAtGestureScrollEnd(const WebGestureEvent& end_event,
base::ScopedClosureRunner callback);
void NotifyScrollPhaseBeginForCustomizedScroll(const ScrollState&);
void NotifyScrollPhaseEndForCustomizedScroll();
......
......@@ -864,27 +864,33 @@ void ScrollableArea::SnapAfterScrollbarScrolling(
orientation == kVerticalScrollbar);
}
bool ScrollableArea::SnapAtCurrentPosition(bool scrolled_x, bool scrolled_y) {
bool ScrollableArea::SnapAtCurrentPosition(
bool scrolled_x,
bool scrolled_y,
base::ScopedClosureRunner on_finish) {
FloatPoint current_position = ScrollPosition();
return SnapForEndPosition(current_position, scrolled_x, scrolled_y);
return SnapForEndPosition(current_position, scrolled_x, scrolled_y,
std::move(on_finish));
}
bool ScrollableArea::SnapForEndPosition(const FloatPoint& end_position,
bool scrolled_x,
bool scrolled_y) {
bool scrolled_y,
base::ScopedClosureRunner on_finish) {
std::unique_ptr<cc::SnapSelectionStrategy> strategy =
cc::SnapSelectionStrategy::CreateForEndPosition(
gfx::ScrollOffset(end_position), scrolled_x, scrolled_y);
return PerformSnapping(*strategy);
return PerformSnapping(*strategy, std::move(on_finish));
}
bool ScrollableArea::SnapForDirection(const ScrollOffset& delta) {
bool ScrollableArea::SnapForDirection(const ScrollOffset& delta,
base::ScopedClosureRunner on_finish) {
FloatPoint current_position = ScrollPosition();
std::unique_ptr<cc::SnapSelectionStrategy> strategy =
cc::SnapSelectionStrategy::CreateForDirection(
gfx::ScrollOffset(current_position),
gfx::ScrollOffset(delta.Width(), delta.Height()));
return PerformSnapping(*strategy);
return PerformSnapping(*strategy, std::move(on_finish));
}
bool ScrollableArea::SnapForEndAndDirection(const ScrollOffset& delta) {
......@@ -896,8 +902,8 @@ bool ScrollableArea::SnapForEndAndDirection(const ScrollOffset& delta) {
return PerformSnapping(*strategy);
}
bool ScrollableArea::PerformSnapping(
const cc::SnapSelectionStrategy& strategy) {
bool ScrollableArea::PerformSnapping(const cc::SnapSelectionStrategy& strategy,
base::ScopedClosureRunner on_finish) {
base::Optional<FloatPoint> snap_point = GetSnapPosition(strategy);
if (!snap_point)
return false;
......@@ -905,7 +911,8 @@ bool ScrollableArea::PerformSnapping(
CancelScrollAnimation();
CancelProgrammaticScrollAnimation();
SetScrollOffset(ScrollPositionToOffset(snap_point.value()),
kProgrammaticScroll, kScrollBehaviorSmooth);
kProgrammaticScroll, kScrollBehaviorSmooth,
on_finish.Release());
return true;
}
......
......@@ -26,6 +26,7 @@
#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_SCROLL_SCROLLABLE_AREA_H_
#define THIRD_PARTY_BLINK_RENDERER_CORE_SCROLL_SCROLLABLE_AREA_H_
#include "base/callback_helpers.h"
#include "cc/input/scroll_snap_data.h"
#include "third_party/blink/public/platform/web_color_scheme.h"
#include "third_party/blink/renderer/core/core_export.h"
......@@ -147,14 +148,24 @@ class CORE_EXPORT ScrollableArea : public GarbageCollectedMixin {
// SnapForEndAndDirection() return true if snapping was performed, and false
// otherwise. Note that this does not necessarily mean that any scrolling was
// performed as a result e.g., if we are already at the snap point.
// The scroll callback parameter is used to set the hover state dirty and
// send a scroll end event when the scroll ends without snap or the snap
// point is the same as the scroll position.
//
// SnapAtCurrentPosition() calls SnapForEndPosition() with the current
// scroll position.
bool SnapAtCurrentPosition(bool scrolled_x, bool scrolled_y);
bool SnapForEndPosition(const FloatPoint& end_position,
bool scrolled_x,
bool scrolled_y);
bool SnapForDirection(const ScrollOffset& delta);
bool SnapAtCurrentPosition(
bool scrolled_x,
bool scrolled_y,
base::ScopedClosureRunner on_finish = base::ScopedClosureRunner());
bool SnapForEndPosition(
const FloatPoint& end_position,
bool scrolled_x,
bool scrolled_y,
base::ScopedClosureRunner on_finish = base::ScopedClosureRunner());
bool SnapForDirection(
const ScrollOffset& delta,
base::ScopedClosureRunner on_finish = base::ScopedClosureRunner());
bool SnapForEndAndDirection(const ScrollOffset& delta);
base::Optional<FloatPoint> GetSnapPosition(
......@@ -517,7 +528,9 @@ class CORE_EXPORT ScrollableArea : public GarbageCollectedMixin {
virtual float PixelStep(ScrollbarOrientation) const;
// Returns true if a snap point was found.
bool PerformSnapping(const cc::SnapSelectionStrategy& strategy);
bool PerformSnapping(
const cc::SnapSelectionStrategy& strategy,
base::ScopedClosureRunner on_finish = base::ScopedClosureRunner());
mutable Member<ScrollAnimatorBase> scroll_animator_;
mutable Member<ProgrammaticScrollAnimator> programmatic_scroll_animator_;
......
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