- Revision
- 210195
- Author
- [email protected]
- Date
- 2016-12-28 02:37:07 -0800 (Wed, 28 Dec 2016)
Log Message
Merge r208003 - Prevent hit tests from being performed on an invalid render tree
https://bugs.webkit.org/show_bug.cgi?id=163877
<rdar://problem/28675761>
Reviewed by Simon Fraser.
Source/WebCore:
Changeset r200971 added code to ensure that layout is up-to-date before hit testing, but did
so only for the main frame. It was still possible to enter cross-frame hit testing with a
subframe needing style recalc. In that situation, the subframe's updateLayout() would get
called, which could trigger a compositing change that marked the parent frame as needing style
recalc. A subsequent layout on the parent frame (for example by hit testing traversing into
a second subframe) could then mutate the parent frame's layer tree while hit testing was
traversing it.
This patch modifies the hit test logic to ensure that a recursive layout is performed so that
we always perform hit tests on a clean set of frames. It also adds some assertions to warn
us if we encounter this invalid state.
Tested by fast/layers/prevent-hit-test-during-layout.html.
* dom/Document.cpp:
(WebCore::Document::scheduleStyleRecalc): Assert that we are not hit testing
during style recalculation.
* page/EventHandler.cpp:
(WebCore::EventHandler::hitTestResultAtPoint): Ensure that we have a clean render tree
when hit testing.
* page/FrameView.cpp:
(WebCore::FrameView::setNeedsLayout): Assert that we are not in the process of hit testing
when we schedule a layout.
* rendering/RenderView.cpp:
(WebCore::RenderView::hitTest): Mark RenderView as in an active hit test.
* rendering/RenderView.h:
LayoutTests:
* fast/layers/prevent-hit-test-during-layout-expected.txt: Added.
* fast/layers/prevent-hit-test-during-layout.html: Added.
* platform/efl/TestExpectations: Skip on this platform.
* platform/gtk/TestExpectations: Skip on this platform.
* platform/ios-simulator/TestExpectations: Skip on this platform.
* platform/win/TestExpectations: Skip on this platform.
Modified Paths
Added Paths
Diff
Modified: releases/WebKitGTK/webkit-2.14/LayoutTests/ChangeLog (210194 => 210195)
--- releases/WebKitGTK/webkit-2.14/LayoutTests/ChangeLog 2016-12-28 10:30:06 UTC (rev 210194)
+++ releases/WebKitGTK/webkit-2.14/LayoutTests/ChangeLog 2016-12-28 10:37:07 UTC (rev 210195)
@@ -1,3 +1,18 @@
+2016-10-25 Brent Fulgham <[email protected]>
+
+ Prevent hit tests from being performed on an invalid render tree
+ https://bugs.webkit.org/show_bug.cgi?id=163877
+ <rdar://problem/28675761>
+
+ Reviewed by Simon Fraser.
+
+ * fast/layers/prevent-hit-test-during-layout-expected.txt: Added.
+ * fast/layers/prevent-hit-test-during-layout.html: Added.
+ * platform/efl/TestExpectations: Skip on this platform.
+ * platform/gtk/TestExpectations: Skip on this platform.
+ * platform/ios-simulator/TestExpectations: Skip on this platform.
+ * platform/win/TestExpectations: Skip on this platform.
+
2016-10-26 Zalan Bujtas <[email protected]>
Ignore out-of-flow siblings when searching for a spanner candidate.
Added: releases/WebKitGTK/webkit-2.14/LayoutTests/fast/layers/prevent-hit-test-during-layout-expected.txt (0 => 210195)
--- releases/WebKitGTK/webkit-2.14/LayoutTests/fast/layers/prevent-hit-test-during-layout-expected.txt (rev 0)
+++ releases/WebKitGTK/webkit-2.14/LayoutTests/fast/layers/prevent-hit-test-during-layout-expected.txt 2016-12-28 10:37:07 UTC (rev 210195)
@@ -0,0 +1,11 @@
+This tests makes sure that hit tests are not processed while laying out the page.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
+
+This test passes if it did not crash.
Added: releases/WebKitGTK/webkit-2.14/LayoutTests/fast/layers/prevent-hit-test-during-layout.html (0 => 210195)
--- releases/WebKitGTK/webkit-2.14/LayoutTests/fast/layers/prevent-hit-test-during-layout.html (rev 0)
+++ releases/WebKitGTK/webkit-2.14/LayoutTests/fast/layers/prevent-hit-test-during-layout.html 2016-12-28 10:37:07 UTC (rev 210195)
@@ -0,0 +1,56 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <script src=""
+ <script>
+ var fixedDiv;
+ var target;
+
+ if (window.testRunner) {
+ description("This tests makes sure that hit tests are not processed while laying out the page.");
+ testRunner.waitUntilDone();
+ }
+
+ function triggerHitTest() {
+ var spanRects = target.getClientRects();
+
+ if (spanRects.count > 1) {
+ document.body.innerHTML += "<br/>ERROR: More than one rect for hit word."
+ testRunner.notifyDone();
+ return;
+ }
+
+ var rect = spanRects[0];
+ var x = rect.left + rect.width / 2;
+ var y = rect.top + rect.height / 2;
+
+ // This internal test call is used because it is guaranteed to trigger
+ // a recursive hit test. Dictionary lookup itself is not required for this test.
+ var lookupRange = internals.rangeForDictionaryLookupAtLocation(x, y);
+
+ document.body.innerHTML += "<br/>This test passes if it did not crash.";
+ testRunner.notifyDone();
+ }
+
+ function runTest() {
+ fixedDiv = window.frames['fixedFrame'].document.getElementById('fixedDiv');
+ target = document.getElementById('target');
+
+ setTimeout(function() {
+ fixedDiv.style.position = "relative";
+ triggerHitTest();
+ }, 0);
+ }
+ </script>
+</head>
+<body _onload_="runTest()">
+ <div>
+ <iframe id="fixedFrame" src=''>
+ </iframe>
+ <iframe id="target" src=''></iframe>
+ <iframe src=''></iframe>
+ </div>
+ <script src=""
+</body>
+</html>
\ No newline at end of file
Modified: releases/WebKitGTK/webkit-2.14/LayoutTests/platform/efl/TestExpectations (210194 => 210195)
--- releases/WebKitGTK/webkit-2.14/LayoutTests/platform/efl/TestExpectations 2016-12-28 10:30:06 UTC (rev 210194)
+++ releases/WebKitGTK/webkit-2.14/LayoutTests/platform/efl/TestExpectations 2016-12-28 10:37:07 UTC (rev 210195)
@@ -569,6 +569,9 @@
security/contentSecurityPolicy/plugins-types-blocks-youtube-plugin-replacement-without-mime-type.html [ Skip ]
security/contentSecurityPolicy/plugins-types-blocks-youtube-plugin-replacement.html [ Skip ]
+# Only Mac has implemented DictionaryLookup
+fast/layers/prevent-hit-test-during-layout.html [ Skip ]
+
#////////////////////////////////////////////////////////////////////////////////////////
# CRASHES
#////////////////////////////////////////////////////////////////////////////////////////
Modified: releases/WebKitGTK/webkit-2.14/LayoutTests/platform/gtk/TestExpectations (210194 => 210195)
--- releases/WebKitGTK/webkit-2.14/LayoutTests/platform/gtk/TestExpectations 2016-12-28 10:30:06 UTC (rev 210194)
+++ releases/WebKitGTK/webkit-2.14/LayoutTests/platform/gtk/TestExpectations 2016-12-28 10:37:07 UTC (rev 210195)
@@ -1713,6 +1713,9 @@
webkit.org/b/152247 fast/forms/input-appearance-spinbutton.html [ Skip ]
webkit.org/b/152247 fast/forms/listbox-scrollbar-hit-test.html [ Skip ]
+# Only Mac has implemented DictionaryLookup
+fast/layers/prevent-hit-test-during-layout.html [ Skip ]
+
#////////////////////////////////////////////////////////////////////////////////////////
# End of tests with architecture-specific results
#////////////////////////////////////////////////////////////////////////////////////////
Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/ChangeLog (210194 => 210195)
--- releases/WebKitGTK/webkit-2.14/Source/WebCore/ChangeLog 2016-12-28 10:30:06 UTC (rev 210194)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/ChangeLog 2016-12-28 10:37:07 UTC (rev 210195)
@@ -1,3 +1,38 @@
+2016-10-27 Brent Fulgham <[email protected]>
+
+ Prevent hit tests from being performed on an invalid render tree
+ https://bugs.webkit.org/show_bug.cgi?id=163877
+ <rdar://problem/28675761>
+
+ Reviewed by Simon Fraser.
+
+ Changeset r200971 added code to ensure that layout is up-to-date before hit testing, but did
+ so only for the main frame. It was still possible to enter cross-frame hit testing with a
+ subframe needing style recalc. In that situation, the subframe's updateLayout() would get
+ called, which could trigger a compositing change that marked the parent frame as needing style
+ recalc. A subsequent layout on the parent frame (for example by hit testing traversing into
+ a second subframe) could then mutate the parent frame's layer tree while hit testing was
+ traversing it.
+
+ This patch modifies the hit test logic to ensure that a recursive layout is performed so that
+ we always perform hit tests on a clean set of frames. It also adds some assertions to warn
+ us if we encounter this invalid state.
+
+ Tested by fast/layers/prevent-hit-test-during-layout.html.
+
+ * dom/Document.cpp:
+ (WebCore::Document::scheduleStyleRecalc): Assert that we are not hit testing
+ during style recalculation.
+ * page/EventHandler.cpp:
+ (WebCore::EventHandler::hitTestResultAtPoint): Ensure that we have a clean render tree
+ when hit testing.
+ * page/FrameView.cpp:
+ (WebCore::FrameView::setNeedsLayout): Assert that we are not in the process of hit testing
+ when we schedule a layout.
+ * rendering/RenderView.cpp:
+ (WebCore::RenderView::hitTest): Mark RenderView as in an active hit test.
+ * rendering/RenderView.h:
+
2016-10-26 Zalan Bujtas <[email protected]>
Ignore out-of-flow siblings when searching for a spanner candidate.
Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/dom/Document.cpp (210194 => 210195)
--- releases/WebKitGTK/webkit-2.14/Source/WebCore/dom/Document.cpp 2016-12-28 10:30:06 UTC (rev 210194)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/dom/Document.cpp 2016-12-28 10:37:07 UTC (rev 210195)
@@ -1820,6 +1820,8 @@
void Document::scheduleStyleRecalc()
{
+ ASSERT(!m_renderView || !m_renderView->inHitTesting());
+
if (m_styleRecalcTimer.isActive() || inPageCache())
return;
Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/page/EventHandler.cpp (210194 => 210195)
--- releases/WebKitGTK/webkit-2.14/Source/WebCore/page/EventHandler.cpp 2016-12-28 10:30:06 UTC (rev 210194)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/page/EventHandler.cpp 2016-12-28 10:37:07 UTC (rev 210195)
@@ -1144,9 +1144,11 @@
unsigned nonNegativePaddingWidth = std::max<LayoutUnit>(0, padding.width()).toUnsigned();
unsigned nonNegativePaddingHeight = std::max<LayoutUnit>(0, padding.height()).toUnsigned();
+
// We should always start hit testing a clean tree.
- if (m_frame.document())
- m_frame.document()->updateLayoutIgnorePendingStylesheets();
+ if (auto* frameView = m_frame.view())
+ frameView->updateLayoutAndStyleIfNeededRecursive();
+
HitTestResult result(point, nonNegativePaddingHeight, nonNegativePaddingWidth, nonNegativePaddingHeight, nonNegativePaddingWidth);
RenderView* renderView = m_frame.contentRenderer();
if (!renderView)
Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/page/FrameView.cpp (210194 => 210195)
--- releases/WebKitGTK/webkit-2.14/Source/WebCore/page/FrameView.cpp 2016-12-28 10:30:06 UTC (rev 210194)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/page/FrameView.cpp 2016-12-28 10:37:07 UTC (rev 210195)
@@ -2839,8 +2839,10 @@
return;
}
- if (RenderView* renderView = this->renderView())
+ if (auto* renderView = this->renderView()) {
+ ASSERT(!renderView->inHitTesting());
renderView->setNeedsLayout();
+ }
}
void FrameView::unscheduleRelayout()
Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/rendering/RenderView.cpp (210194 => 210195)
--- releases/WebKitGTK/webkit-2.14/Source/WebCore/rendering/RenderView.cpp 2016-12-28 10:30:06 UTC (rev 210194)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/rendering/RenderView.cpp 2016-12-28 10:37:07 UTC (rev 210195)
@@ -52,6 +52,7 @@
#include "StyleInheritedData.h"
#include "TransformState.h"
#include <wtf/StackStats.h>
+#include <wtf/TemporaryChange.h>
namespace WebCore {
@@ -183,6 +184,10 @@
bool RenderView::hitTest(const HitTestRequest& request, const HitTestLocation& location, HitTestResult& result)
{
document().updateLayout();
+
+#if !ASSERT_DISABLED
+ TemporaryChange<bool> hitTestRestorer { m_inHitTesting, true };
+#endif
FrameFlatteningLayoutDisallower disallower(frameView());
Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/rendering/RenderView.h (210194 => 210195)
--- releases/WebKitGTK/webkit-2.14/Source/WebCore/rendering/RenderView.h 2016-12-28 10:30:06 UTC (rev 210194)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/rendering/RenderView.h 2016-12-28 10:37:07 UTC (rev 210195)
@@ -1,6 +1,6 @@
/*
* Copyright (C) 1999 Lars Knoll ([email protected])
- * Copyright (C) 2006, 2015 Apple Inc.
+ * Copyright (C) 2006, 2015-2016 Apple Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
@@ -250,6 +250,10 @@
const HashSet<const RenderBox*>& boxesWithScrollSnapCoordinates() { return m_boxesWithScrollSnapCoordinates; }
#endif
+#if !ASSERT_DISABLED
+ bool inHitTesting() const { return m_inHitTesting; }
+#endif
+
protected:
void mapLocalToContainer(const RenderLayerModelObject* repaintContainer, TransformState&, MapCoordinatesFlags, bool* wasFixed) const override;
const RenderObject* pushMappingToContainer(const RenderLayerModelObject* ancestorToStopAt, RenderGeometryMap&) const override;
@@ -360,6 +364,9 @@
bool m_hasSoftwareFilters { false };
bool m_usesFirstLineRules { false };
bool m_usesFirstLetterRules { false };
+#if !ASSERT_DISABLED
+ bool m_inHitTesting { false };
+#endif
HashSet<RenderElement*> m_renderersWithPausedImageAnimation;
HashSet<RenderElement*> m_visibleInViewportRenderers;
