Summary notes from email discussion. Affected tests include fast/events/touch/* and perhaps some under compositing/overflow.
Considering fast/events/touch/compositor-touch-hit-rects-global.html [ Crash ]
- values should be:
document: #document (0, 0, 785, 2000)
except none for overlay
- actual values are none
- we see a crash in ScrollingCoordinator::setTouchEventTargetRects as we can't get the
composited layer for the paint layer via enclosingLayerForPaintInvalidationCrossingFrameBoundaries()
- we either need to find alternate way to get the composited layer, or, rework to not require it somehow.
ScrollingCoordinator gathers touch event rects and maps them to the space of their containing GraphicsLayer. The core business logic is in accumulateDocumentTouchEventTargetRects; the rest of the code is about mapping the rects up to their containing GraphicsLayer.
A reasonable approach might be to:
1. Plumb these touch event rects to their containing PaintChunk
a. Before painting, compute LayerHitTestRects via a call to accumulateDocumentTouchEventTargetRects. Only update it under the same conditions under which ScrollingCoordinator::m_touchEventTargetRectsAreDirty becomes true.
a(1) Also invalidate paint for the containing LayoutObject if an event handler state changed on it.
b. When processing a new DrawingDisplayItem in PaintController::processNewItem, check to see if its DisplayItemClient is in LayerHitTestRects. If it is, expand the current PaintChunk's touchEventHitRegion to include the visual rect of the DrawingDisplayItem.
2. In PaintArtifactCompositor, copy all such touch event rects to the cc::Layer, similar to how the PaintChunk bounds determine the bounds of the cc::Layer created:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp?q=paintartifactcompositor.cpp&sq=package:chromium&dr&l=316
This approach requires invalidating paint when m_touchEventTargetRectsAreDirty becomes dirty, due to a(1). This means over-invalidating paint in cases when we add or remove an event listener, but otherwise don't mess with paint, for example via this call site:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/frame/EventHandlerRegistry.cpp?sq=package:chromium&dr=C&rcl=1481559468&l=257
But I think it's unavoidable given how SPv2 to re-play paint when changing hit testing parameters. Note that due to the way PaintController::generateChunkRasterInvalidationRects avoids invalidations that don't change anything, this should invalidate paint but not raster.
Another concern: I believe it is a common case that a site will register just one listener on the overall document, which with the proposal as outlined would mean invalidating paint on the whole document, which would be unfortunate but would only happen once on add, but probably in short order right after we already painted the document. Perhaps we could do something to optimize that case.
Comment 1 by wkorman@chromium.org
, Mar 28 2017