Commit 666b8c5d authored by David Bokan's avatar David Bokan Committed by Commit Bot

[Refactor] Remove redundant scrolling state

Since https://crrev.com/c/1980984 has decoupled latching from scroll
animation state, we can now simply use the latched node for sending of
scrollend and overscroll events to DOM. We simply require that the
latched node be preserved past the GSE so that in the next commit we can
pass it along for dispatching the scrollend event.

This CL removes |scroll_animating_latched_element_id_| (which was
actually unused since the previous CL) and
|scroll_animating_overscroll_target_element_id_|. The latter was
previously needed since ScrollAnimated would clear the latched node if
an animation wasn't created (e.g. at the extent). Since the latch is now
maintained, it can be used in its place.

This CL renames |last_scroller_element_id_| to |last_latched_scroller_|
and makes it match the |CurrentlyScrollingNode|, with the only exception
being that it gets cleared only after the first commit after the
CurrentlyScrollingNode is cleared.

Bug: 940508
Change-Id: I2d0c63bd97c3bc1a9bd0158a08b3352b09bfb1f8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1981195
Commit-Queue: David Bokan <bokan@chromium.org>
Reviewed-by: default avatarPhilip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#728278}
parent fefd69fe
...@@ -4112,11 +4112,6 @@ InputHandler::ScrollStatus LayerTreeHostImpl::ScrollAnimatedBegin( ...@@ -4112,11 +4112,6 @@ InputHandler::ScrollStatus LayerTreeHostImpl::ScrollAnimatedBegin(
// this does not currently go through the scroll customization machinery // this does not currently go through the scroll customization machinery
// that ScrollBy uses for non-animated wheel scrolls. // that ScrollBy uses for non-animated wheel scrolls.
scroll_status = ScrollBegin(scroll_state, WHEEL); scroll_status = ScrollBegin(scroll_state, WHEEL);
if (scroll_status.thread == SCROLL_ON_IMPL_THREAD) {
scroll_animating_latched_element_id_ = CurrentlyScrollingNode()->element_id;
scroll_animating_overscroll_target_element_id_ =
CurrentlyScrollingNode()->element_id;
}
return scroll_status; return scroll_status;
} }
...@@ -4267,7 +4262,6 @@ void LayerTreeHostImpl::ScrollAnimated(const gfx::Point& viewport_point, ...@@ -4267,7 +4262,6 @@ void LayerTreeHostImpl::ScrollAnimated(const gfx::Point& viewport_point,
did_scroll_x_for_scroll_gesture_ |= scrolled.x() != 0; did_scroll_x_for_scroll_gesture_ |= scrolled.x() != 0;
did_scroll_y_for_scroll_gesture_ |= scrolled.y() != 0; did_scroll_y_for_scroll_gesture_ |= scrolled.y() != 0;
if (scrolled == pending_delta) { if (scrolled == pending_delta) {
scroll_animating_latched_element_id_ = scroll_node->element_id;
TRACE_EVENT_INSTANT0("cc", "Viewport scroll animated", TRACE_EVENT_INSTANT0("cc", "Viewport scroll animated",
TRACE_EVENT_SCOPE_THREAD); TRACE_EVENT_SCOPE_THREAD);
animation_created = true; animation_created = true;
...@@ -4277,7 +4271,6 @@ void LayerTreeHostImpl::ScrollAnimated(const gfx::Point& viewport_point, ...@@ -4277,7 +4271,6 @@ void LayerTreeHostImpl::ScrollAnimated(const gfx::Point& viewport_point,
if (ScrollAnimationCreate(scroll_node, delta, delayed_by)) { if (ScrollAnimationCreate(scroll_node, delta, delayed_by)) {
did_scroll_x_for_scroll_gesture_ |= delta.x() != 0; did_scroll_x_for_scroll_gesture_ |= delta.x() != 0;
did_scroll_y_for_scroll_gesture_ |= delta.y() != 0; did_scroll_y_for_scroll_gesture_ |= delta.y() != 0;
scroll_animating_latched_element_id_ = scroll_node->element_id;
TRACE_EVENT_INSTANT0("cc", "created scroll animation", TRACE_EVENT_INSTANT0("cc", "created scroll animation",
TRACE_EVENT_SCOPE_THREAD); TRACE_EVENT_SCOPE_THREAD);
animation_created = true; animation_created = true;
...@@ -4583,8 +4576,7 @@ void LayerTreeHostImpl::LatchToScroller(ScrollState* scroll_state, ...@@ -4583,8 +4576,7 @@ void LayerTreeHostImpl::LatchToScroller(ScrollState* scroll_state,
TRACE_EVENT_SCOPE_THREAD, "isNull", TRACE_EVENT_SCOPE_THREAD, "isNull",
scroll_node ? false : true); scroll_node ? false : true);
active_tree_->SetCurrentlyScrollingNode(scroll_node); active_tree_->SetCurrentlyScrollingNode(scroll_node);
last_scroller_element_id_ = last_latched_scroller_ = scroll_node ? scroll_node->element_id : ElementId();
scroll_node ? scroll_node->element_id : ElementId();
} }
bool LayerTreeHostImpl::CanConsumeDelta(const ScrollNode& scroll_node, bool LayerTreeHostImpl::CanConsumeDelta(const ScrollNode& scroll_node,
...@@ -5149,27 +5141,12 @@ std::unique_ptr<ScrollAndScaleSet> LayerTreeHostImpl::ProcessScrollDeltas() { ...@@ -5149,27 +5141,12 @@ std::unique_ptr<ScrollAndScaleSet> LayerTreeHostImpl::ProcessScrollDeltas() {
scroll_info->overscroll_delta = overscroll_delta_for_main_thread_; scroll_info->overscroll_delta = overscroll_delta_for_main_thread_;
overscroll_delta_for_main_thread_ = gfx::Vector2dF(); overscroll_delta_for_main_thread_ = gfx::Vector2dF();
if (scroll_gesture_did_end_) { // Use the |last_latched_scroller_| rather than the |CurrentlyScrollingNode|
// When the scrolling has finished send the element id of the last node that // since the latter may be cleared by a GSE before we've committed these
// has scrolled. // values to the main thread.
scroll_info->scroll_latched_element_id = last_scroller_element_id_; scroll_info->scroll_latched_element_id = last_latched_scroller_;
last_scroller_element_id_ = ElementId(); if (scroll_gesture_did_end_)
} else { last_latched_scroller_ = ElementId();
// In scroll animation path CurrentlyScrollingNode does not exist during
// overscrolling. Use the explicitly stored overscroll target instead.
scroll_info->scroll_latched_element_id =
scroll_animating_overscroll_target_element_id_;
scroll_animating_overscroll_target_element_id_ = ElementId();
if (!scroll_info->scroll_latched_element_id) {
// In non-animating scroll path the overscroll target is always the
// CurrentlyScrollingNode.
ScrollNode* node =
active_tree_->property_trees()->scroll_tree.CurrentlyScrollingNode();
scroll_info->scroll_latched_element_id =
node ? node->element_id : ElementId();
}
}
if (browser_controls_manager()) { if (browser_controls_manager()) {
scroll_info->browser_controls_constraint = scroll_info->browser_controls_constraint =
......
...@@ -1182,26 +1182,6 @@ class CC_EXPORT LayerTreeHostImpl : public InputHandler, ...@@ -1182,26 +1182,6 @@ class CC_EXPORT LayerTreeHostImpl : public InputHandler,
std::unique_ptr<PendingTreeRasterDurationHistogramTimer> std::unique_ptr<PendingTreeRasterDurationHistogramTimer>
pending_tree_raster_duration_timer_; pending_tree_raster_duration_timer_;
// The element id of the scroll node to which scroll animations must latch.
// This gets reset at ScrollAnimatedBegin, and updated the first time that a
// scroll animation is created in ScrollAnimated. We need to use element ids
// instead of node ids since they are stable across the property tree update
// in SetPropertyTrees.
ElementId scroll_animating_latched_element_id_;
// In scroll animation path CurrentlyScrollingNode does not exist when there
// is no ongoing scroll animation (e.g. during overscrolling). In this case we
// need to explicitly store the element id of the overscroll event target. The
// overscroll event target is either the element that scroll animation is
// latched to (scroll_animating_latched_element_id_) when any scrolling has
// happened during the current scroll sequence or the last element in the
// scroll chain when no scrolling has happened during the current scroll
// sequence. TODO(input-dev): Decouple CurrentlyScrollingNode life cycle from
// scroll animation life cycle to use CurrentlyScrollingNode instead of both
// scroll_animating_latched_element_id_ and
// scroll_animating_overscroll_target_element_id_. https://crbug.com/940508
ElementId scroll_animating_overscroll_target_element_id_;
// If a scroll snap is being animated, then the value of this will be the // If a scroll snap is being animated, then the value of this will be the
// element id(s) of the target(s). Otherwise, the ids will be invalid. // element id(s) of the target(s). Otherwise, the ids will be invalid.
// At the end of a scroll animation, the target should be set as the scroll // At the end of a scroll animation, the target should be set as the scroll
...@@ -1271,8 +1251,10 @@ class CC_EXPORT LayerTreeHostImpl : public InputHandler, ...@@ -1271,8 +1251,10 @@ class CC_EXPORT LayerTreeHostImpl : public InputHandler,
bool scroll_gesture_did_end_; bool scroll_gesture_did_end_;
// Set in ScrollBegin and outlives the currently scrolling node so it can be // Set in ScrollBegin and outlives the currently scrolling node so it can be
// used to send the scrollend DOM event when scrolling has happened on CC. // used to send the scrollend and overscroll DOM events from the main thread
ElementId last_scroller_element_id_; // when scrolling occurs on the compositor thread. This value is cleared at
// the first commit after a GSE.
ElementId last_latched_scroller_;
// Scroll animation can finish either before or after GSE arrival. // Scroll animation can finish either before or after GSE arrival.
// deferred_scroll_end_ is set when the GSE has arrvied before scroll // deferred_scroll_end_ is set when the GSE has arrvied before scroll
......
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