Diff
Modified: branches/safari-602-branch/LayoutTests/ChangeLog (208163 => 208164)
--- branches/safari-602-branch/LayoutTests/ChangeLog 2016-10-31 18:43:30 UTC (rev 208163)
+++ branches/safari-602-branch/LayoutTests/ChangeLog 2016-10-31 18:43:36 UTC (rev 208164)
@@ -1,3 +1,22 @@
+2016-10-31 Matthew Hanson <matthew_han...@apple.com>
+
+ Merge r208003. rdar://problem/28811878
+
+ 2016-10-25 Brent Fulgham <bfulg...@apple.com>
+
+ 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-31 Ryan Haddad <ryanhad...@apple.com>
Merge r207639. rdar://problem/29007088
Added: branches/safari-602-branch/LayoutTests/fast/layers/prevent-hit-test-during-layout-expected.txt (0 => 208164)
--- branches/safari-602-branch/LayoutTests/fast/layers/prevent-hit-test-during-layout-expected.txt (rev 0)
+++ branches/safari-602-branch/LayoutTests/fast/layers/prevent-hit-test-during-layout-expected.txt 2016-10-31 18:43:36 UTC (rev 208164)
@@ -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: branches/safari-602-branch/LayoutTests/fast/layers/prevent-hit-test-during-layout.html (0 => 208164)
--- branches/safari-602-branch/LayoutTests/fast/layers/prevent-hit-test-during-layout.html (rev 0)
+++ branches/safari-602-branch/LayoutTests/fast/layers/prevent-hit-test-during-layout.html 2016-10-31 18:43:36 UTC (rev 208164)
@@ -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: branches/safari-602-branch/LayoutTests/platform/efl/TestExpectations (208163 => 208164)
--- branches/safari-602-branch/LayoutTests/platform/efl/TestExpectations 2016-10-31 18:43:30 UTC (rev 208163)
+++ branches/safari-602-branch/LayoutTests/platform/efl/TestExpectations 2016-10-31 18:43:36 UTC (rev 208164)
@@ -560,6 +560,9 @@
# Need a new baseline for EFL port. It was added by r72173.
Bug(EFL) fast/css/line-height-determined-by-primary-font.html [ Failure ]
+# Only Mac has implemented DictionaryLookup
+fast/layers/prevent-hit-test-during-layout.html [ Skip ]
+
#////////////////////////////////////////////////////////////////////////////////////////
# CRASHES
#////////////////////////////////////////////////////////////////////////////////////////
Modified: branches/safari-602-branch/LayoutTests/platform/gtk/TestExpectations (208163 => 208164)
--- branches/safari-602-branch/LayoutTests/platform/gtk/TestExpectations 2016-10-31 18:43:30 UTC (rev 208163)
+++ branches/safari-602-branch/LayoutTests/platform/gtk/TestExpectations 2016-10-31 18:43:36 UTC (rev 208164)
@@ -1669,6 +1669,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: branches/safari-602-branch/LayoutTests/platform/ios-simulator/TestExpectations (208163 => 208164)
--- branches/safari-602-branch/LayoutTests/platform/ios-simulator/TestExpectations 2016-10-31 18:43:30 UTC (rev 208163)
+++ branches/safari-602-branch/LayoutTests/platform/ios-simulator/TestExpectations 2016-10-31 18:43:36 UTC (rev 208164)
@@ -2979,4 +2979,7 @@
webkit.org/b/163291 media/video-controls-visible-audio-only.html [ Failure ]
webkit.org/b/163291 media/video-fullscreeen-only-playback.html [ Failure ]
webkit.org/b/163291 media/video-play-audio-require-user-gesture.html [ Failure ]
-webkit.org/b/163291 media/video-play-require-user-gesture.html
\ No newline at end of file
+webkit.org/b/163291 media/video-play-require-user-gesture.html
+
+# Only Mac has implemented DictionaryLookup
+fast/layers/prevent-hit-test-during-layout.html [ Skip ]
Modified: branches/safari-602-branch/LayoutTests/platform/win/TestExpectations (208163 => 208164)
--- branches/safari-602-branch/LayoutTests/platform/win/TestExpectations 2016-10-31 18:43:30 UTC (rev 208163)
+++ branches/safari-602-branch/LayoutTests/platform/win/TestExpectations 2016-10-31 18:43:36 UTC (rev 208164)
@@ -3347,3 +3347,6 @@
# These tests hardcode platform-specific font aliases.
webkit.org/b/158649 fast/text/chinese-font-name-aliases.html [ ImageOnlyFailure ]
webkit.org/b/158649 fast/text/chinese-font-name-aliases-2.html [ ImageOnlyFailure ]
+
+# Only Mac has implemented DictionaryLookup
+fast/layers/prevent-hit-test-during-layout.html [ Skip ]
Modified: branches/safari-602-branch/Source/WebCore/ChangeLog (208163 => 208164)
--- branches/safari-602-branch/Source/WebCore/ChangeLog 2016-10-31 18:43:30 UTC (rev 208163)
+++ branches/safari-602-branch/Source/WebCore/ChangeLog 2016-10-31 18:43:36 UTC (rev 208164)
@@ -1,5 +1,44 @@
2016-10-31 Matthew Hanson <matthew_han...@apple.com>
+ Merge r208003. rdar://problem/28811878
+
+ 2016-10-27 Brent Fulgham <bfulg...@apple.com>
+
+ 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-31 Matthew Hanson <matthew_han...@apple.com>
+
Merge r206635 and r206637. rdar://problem/28718754
2016-10-28 Said Abou-Hallawa <sabouhall...@apple.com>
Modified: branches/safari-602-branch/Source/WebCore/dom/Document.cpp (208163 => 208164)
--- branches/safari-602-branch/Source/WebCore/dom/Document.cpp 2016-10-31 18:43:30 UTC (rev 208163)
+++ branches/safari-602-branch/Source/WebCore/dom/Document.cpp 2016-10-31 18:43:36 UTC (rev 208164)
@@ -1812,6 +1812,8 @@
void Document::scheduleStyleRecalc()
{
+ ASSERT(!m_renderView || !m_renderView->inHitTesting());
+
if (m_styleRecalcTimer.isActive() || inPageCache())
return;
Modified: branches/safari-602-branch/Source/WebCore/page/EventHandler.cpp (208163 => 208164)
--- branches/safari-602-branch/Source/WebCore/page/EventHandler.cpp 2016-10-31 18:43:30 UTC (rev 208163)
+++ branches/safari-602-branch/Source/WebCore/page/EventHandler.cpp 2016-10-31 18:43:36 UTC (rev 208164)
@@ -1136,9 +1136,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: branches/safari-602-branch/Source/WebCore/page/FrameView.cpp (208163 => 208164)
--- branches/safari-602-branch/Source/WebCore/page/FrameView.cpp 2016-10-31 18:43:30 UTC (rev 208163)
+++ branches/safari-602-branch/Source/WebCore/page/FrameView.cpp 2016-10-31 18:43:36 UTC (rev 208164)
@@ -2829,8 +2829,10 @@
return;
}
- if (RenderView* renderView = this->renderView())
+ if (auto* renderView = this->renderView()) {
+ ASSERT(!renderView->inHitTesting());
renderView->setNeedsLayout();
+ }
}
void FrameView::unscheduleRelayout()
Modified: branches/safari-602-branch/Source/WebCore/rendering/RenderView.cpp (208163 => 208164)
--- branches/safari-602-branch/Source/WebCore/rendering/RenderView.cpp 2016-10-31 18:43:30 UTC (rev 208163)
+++ branches/safari-602-branch/Source/WebCore/rendering/RenderView.cpp 2016-10-31 18:43:36 UTC (rev 208164)
@@ -52,6 +52,7 @@
#include "StyleInheritedData.h"
#include "TransformState.h"
#include <wtf/StackStats.h>
+#include <wtf/TemporaryChange.h>
namespace WebCore {
@@ -195,6 +196,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: branches/safari-602-branch/Source/WebCore/rendering/RenderView.h (208163 => 208164)
--- branches/safari-602-branch/Source/WebCore/rendering/RenderView.h 2016-10-31 18:43:30 UTC (rev 208163)
+++ branches/safari-602-branch/Source/WebCore/rendering/RenderView.h 2016-10-31 18:43:36 UTC (rev 208164)
@@ -1,6 +1,6 @@
/*
* Copyright (C) 1999 Lars Knoll (kn...@kde.org)
- * 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
@@ -252,6 +252,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;
@@ -369,6 +373,9 @@
bool m_hasSoftwareFilters;
bool m_usesFirstLineRules { false };
bool m_usesFirstLetterRules { false };
+#if !ASSERT_DISABLED
+ bool m_inHitTesting { false };
+#endif
HashSet<RenderElement*> m_renderersWithPausedImageAnimation;
HashSet<RenderElement*> m_visibleInViewportRenderers;
