Log Message
Merge 138717 > Fix incorrect assumption about in-flow descendants of inlines in touch event rect tracking > https://bugs.webkit.org/show_bug.cgi?id=105970 > > Reviewed by Simon Fraser. > > Source/WebCore: > > Correcting the touch event target rect accumulation code to no longer incorrectly assume that > non-block renderers held only normal-flow children. The updated code will always walk the > complete renderer sub-tree, but only do the work of calculating the absolute rect when the > child won't necessarily fall inside its parent (floating, positioned, or transformed). > > Tests: platform/chromium/fast/events/touch/compositor-touch-hit-rects.html updated to catch bug. > > * page/scrolling/ScrollingCoordinator.cpp: > (WebCore::accumulateRendererTouchEventTargetRects): Walk all renderer sub-trees. Also keeping > track of the last added parent container rect to avoid adding redundant rectangles. > (WebCore::accumulateDocumentEventTargetRects): Avoiding adding empty rects. > > LayoutTests: > > * platform/chromium-linux/platform/chromium/fast/events/touch/compositor-touch-hit-rects-expected.txt: Updating expectations > * platform/chromium/fast/events/touch/compositor-touch-hit-rects.html: Updating test to check previously failing case where > an inline with a touch handler contains a non-normal-flow child. Also, fixing the test since it was incorrectly duplicated. > * platform/chromium/TestExpectations: >
[email protected] Review URL: https://codereview.chromium.org/11753015
Modified Paths
- branches/chromium/1364/LayoutTests/platform/chromium/TestExpectations
- branches/chromium/1364/LayoutTests/platform/chromium/fast/events/touch/compositor-touch-hit-rects.html
- branches/chromium/1364/LayoutTests/platform/chromium-win/platform/chromium/fast/events/touch/compositor-touch-hit-rects-expected.txt
- branches/chromium/1364/Source/WebCore/page/scrolling/ScrollingCoordinator.cpp
Diff
Modified: branches/chromium/1364/LayoutTests/platform/chromium/TestExpectations (138738 => 138739)
--- branches/chromium/1364/LayoutTests/platform/chromium/TestExpectations 2013-01-03 22:05:35 UTC (rev 138738)
+++ branches/chromium/1364/LayoutTests/platform/chromium/TestExpectations 2013-01-03 22:06:22 UTC (rev 138739)
@@ -4012,6 +4012,7 @@
# Needs baselines
Bug(thakis) [ Android Release ] fast/forms/zoomed-controls.html [ ImageOnlyFailure ]
+Bug(leviw) [ Android Win Mac Linux ] platform/chromium/fast/events/touch/compositor-touch-hit-rects.html [ Failure ]
# Flaky on win7 since at least r162410
webkit.org/b/99886 [ Win7 ] fast/dom/shadow/input-with-validation.html [ ImageOnlyFailure Pass ]
Modified: branches/chromium/1364/LayoutTests/platform/chromium/fast/events/touch/compositor-touch-hit-rects.html (138738 => 138739)
--- branches/chromium/1364/LayoutTests/platform/chromium/fast/events/touch/compositor-touch-hit-rects.html 2013-01-03 22:05:35 UTC (rev 138738)
+++ branches/chromium/1364/LayoutTests/platform/chromium/fast/events/touch/compositor-touch-hit-rects.html 2013-01-03 22:06:22 UTC (rev 138739)
@@ -54,112 +54,15 @@
<div>causes a</div>
continuation</b>
</div>
-</div>
-<script>
-if (!window.testRunner)
- return;
-
-window.testRunner.dumpAsText();
-
-function listener() { }
-
-function log(msg) {
- var span = document.createElement("span");
- document.getElementById("console").appendChild(span);
- span.innerHTML = msg + '<br />';
-}
-
-function sortRects(a, b) {
- return a.top - b.top;
-}
-
-function logRects(id) {
- element = document.getElementById(id);
- element.addEventListener('touchstart', listener, false);
- rects = window.internals.touchEventTargetClientRects(document);
- var sortedRects = new Array();
- for (var i = 0; i < rects.length; ++i)
- sortedRects[i] = rects[i];
- sortedRects.sort(sortRects);
- for (var i = 0; i < rects.length; ++i)
- log(id + "[" + i + "]: (" + sortedRects[i].left + ", " + sortedRects[i].top + ", " + sortedRects[i].width + ", " + sortedRects[i].height + ")");
- element.removeEventListener('touchstart', listener, false);
-}
-
-logRects("normalFlow");
-logRects("absoluteChildContainer");
-logRects("relativeChildContainer");
-logRects("overhangingContainer");
-logRects("transformedChildContainer");
-logRects("continuation");
-
-var testContainer = document.getElementById("tests");
-testContainer.parentNode.removeChild(testContainer);
-
-</script>
-</body>
-<!DOCTYPE html>
-<html>
-<head>
-<style>
-#transformedChild {
- -webkit-transform: rotate3d(0.2, 1, 0, 35grad);
-}
-#absoluteChild {
- position: absolute;
- top: 300px;
-}
-#relativeChild {
- position: relative;
- top: 200px;
-}
-#overhangingContainer {
- height: 10px;
-}
-#overhangingFloatingChild {
- width: 100px;
- float: left;
-}
-#tests {
- font: 10px Ahem;
-}
-</style>
-</head>
-<body>
-<p id="description">This tests verifies the hit test regions given to the compositor. It can only be run in DumpRenderTree.
-The outputted rects should cover the hit test regions of all the listed elements.</p>
-<div id="console"></div>
-
-<div id="tests">
- <div id="normalFlow">
- Lorem ipsum
- <span>sum</span>.
- </div>
- <div id="absoluteChildContainer">
- Lorem ipsum
- <span id="absoluteChild">Absolute child</span>
- </div>
- <div id="relativeChildContainer">
- Lorem ipsum
- <span id="relativeChild">Relative child</span>
- </div>
- <div id="overhangingContainer">
- <div id="overhangingFloatingChild">Overhanging float overhanging float overhanging float overhanging float</div>
- </div>
- <div id="transformedChildContainer">
- <div id="transformedChild">Transformed</div>
- </div>
<div>
- <b id="continuation">This b tag
- <div>causes a</div>
- continuation</b>
+ <span id="inlineAbsoluteChildContainer">
+ Inline with absolute child.
+ <span id="absoluteChild">Absolute child in inline.</span>
+ </span>
</div>
</div>
<script>
-if (!window.testRunner)
- return;
-window.testRunner.dumpAsText();
function listener() { }
@@ -186,15 +89,23 @@
element.removeEventListener('touchstart', listener, false);
}
-logRects("normalFlow");
-logRects("absoluteChildContainer");
-logRects("relativeChildContainer");
-logRects("overhangingContainer");
-logRects("transformedChildContainer");
-logRects("continuation");
+function runTest() {
+ if (!window.testRunner)
+ return;
-var testContainer = document.getElementById("tests");
-testContainer.parentNode.removeChild(testContainer);
+ window.testRunner.dumpAsText();
+ logRects("normalFlow");
+ logRects("absoluteChildContainer");
+ logRects("relativeChildContainer");
+ logRects("overhangingContainer");
+ logRects("transformedChildContainer");
+ logRects("continuation");
+ logRects("inlineAbsoluteChildContainer");
+ var testContainer = document.getElementById("tests");
+ testContainer.parentNode.removeChild(testContainer);
+}
+
+runTest();
</script>
</body>
Modified: branches/chromium/1364/LayoutTests/platform/chromium-win/platform/chromium/fast/events/touch/compositor-touch-hit-rects-expected.txt (138738 => 138739)
--- branches/chromium/1364/LayoutTests/platform/chromium-win/platform/chromium/fast/events/touch/compositor-touch-hit-rects-expected.txt 2013-01-03 22:05:35 UTC (rev 138738)
+++ branches/chromium/1364/LayoutTests/platform/chromium-win/platform/chromium/fast/events/touch/compositor-touch-hit-rects-expected.txt 2013-01-03 22:06:22 UTC (rev 138739)
@@ -1,45 +1,15 @@
This tests verifies the hit test regions given to the compositor. It can only be run in DumpRenderTree. The outputted rects should cover the hit test regions of all the listed elements.
normalFlow[0]: (8, 72, 784, 10)
-normalFlow[1]: (8, 72, 784, 10)
-normalFlow[2]: (128, 72, 30, 10)
-normalFlow[3]: (8, 72, 784, 10)
-absoluteChildContainer[0]: (8, 162, 784, 10)
-absoluteChildContainer[1]: (8, 162, 784, 10)
-absoluteChildContainer[2]: (8, 162, 784, 10)
-absoluteChildContainer[3]: (118, 300, 140, 10)
-absoluteChildContainer[4]: (118, 300, 140, 10)
-relativeChildContainer[0]: (8, 272, 784, 10)
-relativeChildContainer[1]: (8, 272, 784, 10)
-relativeChildContainer[2]: (8, 272, 784, 10)
-relativeChildContainer[3]: (128, 472, 140, 10)
-overhangingContainer[0]: (8, 362, 784, 10)
-overhangingContainer[1]: (8, 362, 110, 80)
-overhangingContainer[2]: (8, 362, 110, 80)
-transformedChildContainer[0]: (62, 421, 661, 32)
-transformedChildContainer[1]: (62, 421, 661, 32)
-transformedChildContainer[2]: (8, 432, 769, 10)
-continuation[0]: (108, 502, 110, 10)
-normalFlow[0]: (8, 544, 769, 10)
-normalFlow[1]: (8, 544, 769, 10)
-normalFlow[2]: (128, 544, 30, 10)
-normalFlow[3]: (8, 544, 769, 10)
-absoluteChildContainer[0]: (118, 300, 140, 10)
+absoluteChildContainer[0]: (8, 102, 784, 10)
absoluteChildContainer[1]: (118, 300, 140, 10)
-absoluteChildContainer[2]: (8, 634, 769, 10)
-absoluteChildContainer[3]: (8, 634, 769, 10)
-absoluteChildContainer[4]: (8, 634, 769, 10)
-relativeChildContainer[0]: (8, 744, 769, 10)
-relativeChildContainer[1]: (8, 744, 769, 10)
-relativeChildContainer[2]: (8, 744, 769, 10)
-relativeChildContainer[3]: (128, 944, 140, 10)
-overhangingContainer[0]: (8, 834, 769, 10)
-overhangingContainer[1]: (8, 834, 110, 80)
-overhangingContainer[2]: (8, 834, 110, 80)
-transformedChildContainer[0]: (62, 893, 661, 32)
-transformedChildContainer[1]: (62, 893, 661, 32)
-transformedChildContainer[2]: (8, 904, 769, 10)
-continuation[0]: (108, 974, 110, 10)
-This tests verifies the hit test regions given to the compositor. It can only be run in DumpRenderTree. The outputted rects should cover the hit test regions of all the listed elements.
+relativeChildContainer[0]: (8, 152, 784, 10)
+relativeChildContainer[1]: (128, 352, 140, 10)
+overhangingContainer[0]: (8, 202, 784, 10)
+overhangingContainer[1]: (8, 202, 110, 80)
+transformedChildContainer[0]: (63, 240, 674, 34)
+transformedChildContainer[1]: (8, 252, 784, 10)
+continuation[0]: (108, 302, 100, 10)
+inlineAbsoluteChildContainer[0]: (378, 300, 250, 10)
+inlineAbsoluteChildContainer[1]: (108, 352, 270, 10)
-
Modified: branches/chromium/1364/Source/WebCore/page/scrolling/ScrollingCoordinator.cpp (138738 => 138739)
--- branches/chromium/1364/Source/WebCore/page/scrolling/ScrollingCoordinator.cpp 2013-01-03 22:05:35 UTC (rev 138738)
+++ branches/chromium/1364/Source/WebCore/page/scrolling/ScrollingCoordinator.cpp 2013-01-03 22:06:22 UTC (rev 138739)
@@ -179,15 +179,20 @@
}
#if ENABLE(TOUCH_EVENT_TRACKING)
-static void accumulateRendererTouchEventTargetRects(Vector<IntRect>& rects, const RenderObject* renderer)
+static void accumulateRendererTouchEventTargetRects(Vector<IntRect>& rects, const RenderObject* renderer, const IntRect& parentRect = IntRect())
{
- // FIXME: This method is O(N^2) as it walks the tree to the root for every renderer. RenderGeometryMap would fix this.
- rects.append(enclosingIntRect(renderer->clippedOverflowRectForRepaint(0)));
- if (renderer->isRenderBlock()) {
- const RenderBlock* block = toRenderBlock(renderer);
- for (RenderObject* child = block->firstChild(); child; child = child->nextSibling())
- accumulateRendererTouchEventTargetRects(rects, child);
+ IntRect adjustedParentRect = parentRect;
+ if (parentRect.isEmpty() || renderer->isFloating() || renderer->isPositioned() || renderer->hasTransform()) {
+ // FIXME: This method is O(N^2) as it walks the tree to the root for every renderer. RenderGeometryMap would fix this.
+ IntRect r = enclosingIntRect(renderer->clippedOverflowRectForRepaint(0));
+ if (!r.isEmpty() && !parentRect.contains(r)) {
+ rects.append(r);
+ adjustedParentRect = r;
+ }
}
+
+ for (RenderObject* child = renderer->firstChild(); child; child = child->nextSibling())
+ accumulateRendererTouchEventTargetRects(rects, child, adjustedParentRect);
}
static void accumulateDocumentEventTargetRects(Vector<IntRect>& rects, const Document* document)
@@ -203,8 +208,11 @@
continue;
if (touchTarget == document) {
- if (RenderView* view = document->renderView())
- rects.append(enclosingIntRect(view->clippedOverflowRectForRepaint(0)));
+ if (RenderView* view = document->renderView()) {
+ IntRect r = enclosingIntRect(view->clippedOverflowRectForRepaint(0));
+ if (!r.isEmpty())
+ rects.append(r);
+ }
return;
}
_______________________________________________ webkit-changes mailing list [email protected] http://lists.webkit.org/mailman/listinfo/webkit-changes
