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

[root-layer-scrolls] Fix android fullscreen test

This patch fixes compositor-touch-hit-rects-fullscreen-video-controls
when run with root layer scrolls. This test was added for
crbug.com/372314. An explanation of that is helpful to understand the
fix here:

When Blink calculates the touch event handler rects it
has a special optimization when the document has a handler. In that
case, it doesn't bother checking anything else and places a content-
sized rect onto the document layer.

However, in the case of an Android fullscreen video, the compositor
detaches the document layer hierarchy and attaches the video and its
controls. Because of the optimization above, the compositor doesn't
bother calculating touch handler rects on these new layers, but because
the document layer is detached it seems as though there aren't any touch
handler regions at all. The fix in that bug was to disable the
optimization when a fullscreen video overlay is active to force
computing rects from the media control layers.

This test uses Internals::touchEventTargetLayerRects to calculate the
rects on the layer tree. This method uses
PaintLayerCompositor::RootGraphicsLayer as its starting point. Pre-RLS,
this would be the overflow controls host layer in the PLC. This was ok
because the document layer was connected to it's descendants and
disconnected when an overlay video was attached. With RLS,
RootGraphicsLayer returns explicitly the graphics layer for the
LayoutView which may not be attached at all. Thus the test was seeing
that a handler still existed on the document because that's where it
would start its search. The fix is to start the search from the painting
root which is above the PaintLayerCompositor and thus, wont descend into
the detached document layer.

I don't think this test is a good guard against the regression. Since
the document layer is detached we don't actually check that we correctly
calculate the touch rects on the video controls. It appears that they've
since been modified to not require touch handlers and the expectation
reflects that. As such, this test can't tell the difference between the
controls not having touch handlers and Blink failing to calculate touch
handler rects. I've filed https://crbug.com/818330 for this problem.

Bug: 816957
Change-Id: I088f960edb47af2d6d539e50a21b7e39067efb5f
Reviewed-on: https://chromium-review.googlesource.com/947052Reviewed-by: default avatarPhilip Rogers <pdr@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540694}
parent 1810d438
...@@ -3358,7 +3358,6 @@ crbug.com/807110 external/wpt/media-source/mediasource-sourcebuffer-mode-timesta ...@@ -3358,7 +3358,6 @@ crbug.com/807110 external/wpt/media-source/mediasource-sourcebuffer-mode-timesta
crbug.com/417782 paint/invalidation/window-resize/window-resize-vertical-writing-mode.html [ Crash ] crbug.com/417782 paint/invalidation/window-resize/window-resize-vertical-writing-mode.html [ Crash ]
crbug.com/417782 virtual/disable-spv175/paint/invalidation/window-resize/window-resize-vertical-writing-mode.html [ Skip ] crbug.com/417782 virtual/disable-spv175/paint/invalidation/window-resize/window-resize-vertical-writing-mode.html [ Skip ]
crbug.com/417782 [ Linux ] virtual/android/fullscreen/compositor-touch-hit-rects-fullscreen-video-controls.html [ Failure ]
# End of Root Layer Scrolling expectations # End of Root Layer Scrolling expectations
################################################################################ ################################################################################
......
...@@ -26,7 +26,7 @@ window.onload = function () { ...@@ -26,7 +26,7 @@ window.onload = function () {
waitForEvent(document, 'webkitfullscreenchange', function() { waitForEvent(document, 'webkitfullscreenchange', function() {
if (window.internals.runtimeFlags.forceOverlayFullscreenVideoEnabled) if (window.internals.runtimeFlags.forceOverlayFullscreenVideoEnabled)
consoleWrite("Should report another rect which is not on the document"); consoleWrite("Should report no rect or another rect which is not on the document");
else else
consoleWrite("Should keep rect on document"); consoleWrite("Should keep rect on document");
window.internals.forceCompositingUpdate(document); window.internals.forceCompositingUpdate(document);
......
...@@ -4,7 +4,7 @@ Should have single rect on document before fullscreen ...@@ -4,7 +4,7 @@ Should have single rect on document before fullscreen
handler: #document (0, 0, 800, 600) handler: #document (0, 0, 800, 600)
EVENT(webkitfullscreenchange) EVENT(webkitfullscreenchange)
Should report another rect which is not on the document Should report no rect or another rect which is not on the document
handler: no rects handler: no rects
END OF TEST END OF TEST
......
...@@ -4,7 +4,7 @@ Should have single rect on document before fullscreen ...@@ -4,7 +4,7 @@ Should have single rect on document before fullscreen
handler: #document (0, 0, 800, 600) handler: #document (0, 0, 800, 600)
EVENT(webkitfullscreenchange) EVENT(webkitfullscreenchange)
Should report another rect which is not on the document Should report no rect or another rect which is not on the document
handler: DIV (42, 3, 637, 24) handler: DIV (42, 3, 637, 24)
END OF TEST END OF TEST
......
...@@ -1976,7 +1976,10 @@ LayerRectList* Internals::touchEventTargetLayerRects( ...@@ -1976,7 +1976,10 @@ LayerRectList* Internals::touchEventTargetLayerRects(
if (auto* view = document->GetLayoutView()) { if (auto* view = document->GetLayoutView()) {
if (PaintLayerCompositor* compositor = view->Compositor()) { if (PaintLayerCompositor* compositor = view->Compositor()) {
if (GraphicsLayer* root_layer = compositor->RootGraphicsLayer()) { // Use the paint-root since we sometimes want to know about touch rects
// on layers outside the document hierarchy (e.g. when we replace the
// document with a video layer).
if (GraphicsLayer* root_layer = compositor->PaintRootGraphicsLayer()) {
LayerRectList* rects = LayerRectList::Create(); LayerRectList* rects = LayerRectList::Create();
AccumulateLayerRectList(compositor, root_layer, rects); AccumulateLayerRectList(compositor, root_layer, rects);
return rects; return rects;
......
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