Commit 39a4e54d authored by David Bokan's avatar David Bokan Committed by Commit Bot

Revert "Unregister cc scrollbar when registering new one"

This reverts commit 8448024f.

Reason for revert: This likely introduced a container-overflow: crbug.com/742520

Original change's description:
> Unregister cc scrollbar when registering new one
> 
> We previously DCHECKed that we could never register a new scrollbar
> against a scroll layer that already has a scrollbar. i.e. We expected
> that the old scrollbar would be unregistered prior to registering the
> new one.
> 
> RootScroller can break this assumption on Android. On Android, the
> global rootScroller (i.e. OuterViewport) is prevented from creating
> its own scrollbars, instead inheriting the VisualViewport-created
> scrollbars. In other words, the VisualViewport creates a pair of
> scrollbars and they get re-assigned to whatever the current outer
> viewport is.
> 
> This means that when we change the global rootScroller (for context see
> core/page/scrolling/README.md), the previous rootScroller now creates
> its own scrollbars and the VisualViewport scrollbars get registered to
> the new rootScroller. This then creates a race between moving the
> VisualViewport scrollbar registration and new scrollbar registration on
> the old rootScroller.
> 
> Long-term, the outer viewport shouldn't own the scrollbars but for now
> it doesn't hurt to just unregister the scrollbar if we notice the
> scroller already has scrollbars registered.
> 
> Bug: 739738
> Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
> Change-Id: I99e8a95881e7dad19c0318a661da80f441b97b19
> Reviewed-on: https://chromium-review.googlesource.com/562018
> Reviewed-by: Weiliang Chen <weiliangc@chromium.org>
> Commit-Queue: David Bokan <bokan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#486086}

TBR=bokan@chromium.org,weiliangc@chromium.org

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

Bug: 739738, 742520, 742678
Change-Id: I1134e17dcb04a3a2cc50f07ecbc0fc79cc74f344
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Reviewed-on: https://chromium-review.googlesource.com/574547
Commit-Queue: David Bokan <bokan@chromium.org>
Reviewed-by: default avatarDavid Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487082}
parent e37f8f73
...@@ -1656,20 +1656,15 @@ void LayerTreeImpl::RegisterScrollbar(ScrollbarLayerImplBase* scrollbar_layer) { ...@@ -1656,20 +1656,15 @@ void LayerTreeImpl::RegisterScrollbar(ScrollbarLayerImplBase* scrollbar_layer) {
return; return;
auto& scrollbar_ids = element_id_to_scrollbar_layer_ids_[scroll_element_id]; auto& scrollbar_ids = element_id_to_scrollbar_layer_ids_[scroll_element_id];
int& scrollbar_layer_id = scrollbar_layer->orientation() == HORIZONTAL if (scrollbar_layer->orientation() == HORIZONTAL) {
? scrollbar_ids.horizontal DCHECK_EQ(scrollbar_ids.horizontal, Layer::INVALID_ID)
: scrollbar_ids.vertical; << "Existing scrollbar should have been unregistered.";
scrollbar_ids.horizontal = scrollbar_layer->id();
// We used to DCHECK this was not the case but this can occur on Android: as } else {
// the visual viewport supplies scrollbars for the outer viewport, if the DCHECK_EQ(scrollbar_ids.vertical, Layer::INVALID_ID)
// outer viewport is changed, we race between updating the visual viewport << "Existing scrollbar should have been unregistered.";
// scrollbars and registering new scrollbars on the old outer viewport. It'd scrollbar_ids.vertical = scrollbar_layer->id();
// be nice if we could fix this to be cleaner but its harmless to just }
// unregister here.
if (scrollbar_layer_id != Layer::INVALID_ID)
UnregisterScrollbar(scrollbar_layer);
scrollbar_layer_id = scrollbar_layer->id();
if (IsActiveTree() && scrollbar_layer->is_overlay_scrollbar() && if (IsActiveTree() && scrollbar_layer->is_overlay_scrollbar() &&
scrollbar_layer->GetScrollbarAnimator() != scrollbar_layer->GetScrollbarAnimator() !=
......
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