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/562018Reviewed-by: Weiliang Chen <weiliangc@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486086}

cc/trees/layer_tree_impl.cc

@@ -1657,15 +1657,20 @@ void LayerTreeImpl::RegisterScrollbar(ScrollbarLayerImplBase* scrollbar_layer) {
     return;
 
   auto& scrollbar_ids = element_id_to_scrollbar_layer_ids_[scroll_element_id];
-  if (scrollbar_layer->orientation() == HORIZONTAL) {
-    DCHECK_EQ(scrollbar_ids.horizontal, Layer::INVALID_ID)
-        << "Existing scrollbar should have been unregistered.";
-    scrollbar_ids.horizontal = scrollbar_layer->id();
-  } else {
-    DCHECK_EQ(scrollbar_ids.vertical, Layer::INVALID_ID)
-        << "Existing scrollbar should have been unregistered.";
-    scrollbar_ids.vertical = scrollbar_layer->id();
-  }
+  int& scrollbar_layer_id = scrollbar_layer->orientation() == HORIZONTAL
+                                ? scrollbar_ids.horizontal
+                                : scrollbar_ids.vertical;
+
+  // We used to DCHECK this was not the case but this can occur on Android: as
+  // the visual viewport supplies scrollbars for the outer viewport, if the
+  // outer viewport is changed, we race between updating the visual viewport
+  // 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);
+
+  scrollbar_layer_id = scrollbar_layer->id();
 
   if (IsActiveTree() && scrollbar_layer->is_overlay_scrollbar() &&
       scrollbar_layer->GetScrollbarAnimator() !=