Commit ec9c793c authored by David Bokan's avatar David Bokan Committed by Commit Bot

Reland "Unregister cc scrollbar when registering new one"

This was previously landed in:
https://chromium-review.googlesource.com/c/562018/

but was reverted in:
https://chromium-review.googlesource.com/c/574547/

due to a a Clusterfuzz bug. Turns out that calling UnregisterScrollbar
can erase the references we're holding in scrollbar_ids and
scrollbar_layer_id so we have to refresh those.

Bug: 739738
Change-Id: Ic80018212b1370be6a33a39add19da6848b59fb6
Reviewed-on: https://chromium-review.googlesource.com/578428Reviewed-by: default avatarWeiliang Chen <weiliangc@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488030}
parent f5285289
...@@ -1646,17 +1646,29 @@ void LayerTreeImpl::RegisterScrollbar(ScrollbarLayerImplBase* scrollbar_layer) { ...@@ -1646,17 +1646,29 @@ void LayerTreeImpl::RegisterScrollbar(ScrollbarLayerImplBase* scrollbar_layer) {
if (!scroll_element_id) if (!scroll_element_id)
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];
if (scrollbar_layer->orientation() == HORIZONTAL) { int* scrollbar_layer_id = scrollbar_layer->orientation() == HORIZONTAL
DCHECK_EQ(scrollbar_ids.horizontal, Layer::INVALID_ID) ? &scrollbar_ids->horizontal
<< "Existing scrollbar should have been unregistered."; : &scrollbar_ids->vertical;
scrollbar_ids.horizontal = scrollbar_layer->id();
} else { // We used to DCHECK this was not the case but this can occur on Android: as
DCHECK_EQ(scrollbar_ids.vertical, Layer::INVALID_ID) // the visual viewport supplies scrollbars for the outer viewport, if the
<< "Existing scrollbar should have been unregistered."; // outer viewport is changed, we race between updating the visual viewport
scrollbar_ids.vertical = scrollbar_layer->id(); // scrollbars and registering new scrollbars on the old outer viewport. It'd
// 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);
// The scrollbar_ids could have been erased above so get it again.
scrollbar_ids = &element_id_to_scrollbar_layer_ids_[scroll_element_id];
scrollbar_layer_id = scrollbar_layer->orientation() == HORIZONTAL
? &scrollbar_ids->horizontal
: &scrollbar_ids->vertical;
} }
*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() !=
LayerTreeSettings::NO_ANIMATOR) { LayerTreeSettings::NO_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