The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b62bd420664564c0b417521a60a54c35fc3daaa2

commit b62bd420664564c0b417521a60a54c35fc3daaa2
Author: Robert Flack <flackr@chromium.org>
Date: Wed Jul 25 14:54:42 2018

Remove use of layer ids from LayerTreeHostImpl scroll methods.

LayerTreeHostImpl::ScrollLayerTo and
LayerTreeHostImpl::GetScrollOffsetForLayer looked up the cc::LayerImpl
from the active tree by layer_id, however the caller knows the element
id and this allows us to more directly look up the scroll node and ask
the scroll tree directly. This moves us towards not needing layer ids in
cc.

Bug: 865973
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ic279a95592071bbd93cd4c239a5a738b6041d292
Reviewed-on: https://chromium-review.googlesource.com/1145130
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577887}
[modify] https://crrev.com/b62bd420664564c0b417521a60a54c35fc3daaa2/cc/input/input_handler.h
[modify] https://crrev.com/b62bd420664564c0b417521a60a54c35fc3daaa2/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/b62bd420664564c0b417521a60a54c35fc3daaa2/cc/trees/layer_tree_host_impl.h
[modify] https://crrev.com/b62bd420664564c0b417521a60a54c35fc3daaa2/cc/trees/property_tree.cc
[modify] https://crrev.com/b62bd420664564c0b417521a60a54c35fc3daaa2/cc/trees/property_tree.h
[modify] https://crrev.com/b62bd420664564c0b417521a60a54c35fc3daaa2/ui/compositor/compositor.cc
[modify] https://crrev.com/b62bd420664564c0b417521a60a54c35fc3daaa2/ui/compositor/compositor.h
[modify] https://crrev.com/b62bd420664564c0b417521a60a54c35fc3daaa2/ui/compositor/layer.cc
[modify] https://crrev.com/b62bd420664564c0b417521a60a54c35fc3daaa2/ui/events/blink/input_handler_proxy_unittest.cc
[modify] https://crrev.com/b62bd420664564c0b417521a60a54c35fc3daaa2/ui/views/controls/scroll_view_unittest.cc

Philip, in the review you suggested removing FindActiveTreeLayerById, but there are still a few places that look up layers by ids that seem not to be obviously removable[1]. Do you think that none of these should be necessary? If so, do you think we need to maintain a complete element id to layer map (currently only track scroll layers) instead or do you expect that we should never need to look up layers?

[1] For example,
- NotifyTileStateChanged looks up the LayerImpl in order to call NotifyTileStateChanged which adds to the LayerImpl's damage_rect_.
- RenderSurfaceImpl::MaskLayer is used to find the mask layer to append quads to it.
- ComputeViewportSelectionBound tests against the bounds of the selected layer (looked up by layer id).

I'm also not sure if every Layer has an element_id, given the various checks for layer id: https://cs.chromium.org/search/?q=if%5C+.*inputs_%5C.element_id&type=cs and https://cs.chromium.org/search/?q=if.*layer%5C-%5C%3Eelement_id%5C(%5C)&type=cs

I'm sorry for sending you down that path. I was wrong and FindActiveTreeLayerById is fine. It is LayerByElementId that we should be getting rid of.