- Revision
- 186436
- Author
- [email protected]
- Date
- 2015-07-07 03:49:00 -0700 (Tue, 07 Jul 2015)
Log Message
Merge r186165 - Frame flattening: Hit-testing an iframe could end up destroying the associated inline tree context.
https://bugs.webkit.org/show_bug.cgi?id=146447
rdar://problem/20613501
Reviewed by Simon Fraser.
This patch ensures that the render tree associated with the document on which
the hit-test is initiated does not get laid out, unless it was directly mutated prior to the hittest.
Hit-test requirements:
1. A clean the render tree before hit-testing gets propagated to the renderers.
Document::updateLayout() ensures it by calling both updateStyleIfNeeded() and layout() not only on the current tree, but also
on the ancestors if needed.
2. No render tree mutation while hit-testing the renderers.
When an iframe is being hit-tested, this hit-test could bubble down to the child frame's render view.
In order to ensure #1, we call Document::updateLayout() on the current (subframe) document.
If updateStyleIfNeeded() mutates the render tree, we mark it dirty for layout(). However frame flattening also
marks the parent renderer (RenderIFrame) dirty.
While calling layout() to clean the current render tree, we end up laying out the parent tree too.
Laying out the parent tree could end up destroying the inline tree context from where the
hittest just bubbled down. (InlineFlowBox -> RenderWidget -> RenderView).
This patch protects the render tree from such unintentional inline tree mutation during hittesting.
After the initial layout we set a layout disallow flag on the frame view to defer subsequent layouts.
This patch only changes behavior when frame flattening is enabled, but in future we may always want to enable this.
Source/WebCore:
Test: fast/frames/flattening/hittest-iframe-while-style-changes-crash.html
* page/FrameView.cpp:
(WebCore::FrameView::layout):
(WebCore::FrameView::startLayoutAtMainFrameViewIfNeeded): Deleted. -> Assertion in no longer valid.
* page/FrameView.h:
* rendering/RenderView.cpp:
(WebCore::FrameFlatteningLayoutDisallower::FrameFlatteningLayoutDisallower):
(WebCore::FrameFlatteningLayoutDisallower::~FrameFlatteningLayoutDisallower):
(WebCore::RenderView::hitTest): Protect the render tree from subsequent layouts.
LayoutTests:
* fast/frames/flattening/hittest-iframe-while-style-changes-crash-expected.txt: Added.
* fast/frames/flattening/hittest-iframe-while-style-changes-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: releases/WebKitGTK/webkit-2.8/LayoutTests/ChangeLog (186435 => 186436)
--- releases/WebKitGTK/webkit-2.8/LayoutTests/ChangeLog 2015-07-07 10:42:06 UTC (rev 186435)
+++ releases/WebKitGTK/webkit-2.8/LayoutTests/ChangeLog 2015-07-07 10:49:00 UTC (rev 186436)
@@ -1,3 +1,36 @@
+2015-06-30 Zalan Bujtas <[email protected]>
+
+ Frame flattening: Hit-testing an iframe could end up destroying the associated inline tree context.
+ https://bugs.webkit.org/show_bug.cgi?id=146447
+ rdar://problem/20613501
+
+ Reviewed by Simon Fraser.
+
+ This patch ensures that the render tree associated with the document on which
+ the hit-test is initiated does not get laid out, unless it was directly mutated prior to the hittest.
+
+ Hit-test requirements:
+ 1. A clean the render tree before hit-testing gets propagated to the renderers.
+ Document::updateLayout() ensures it by calling both updateStyleIfNeeded() and layout() not only on the current tree, but also
+ on the ancestors if needed.
+
+ 2. No render tree mutation while hit-testing the renderers.
+
+ When an iframe is being hit-tested, this hit-test could bubble down to the child frame's render view.
+ In order to ensure #1, we call Document::updateLayout() on the current (subframe) document.
+ If updateStyleIfNeeded() mutates the render tree, we mark it dirty for layout(). However frame flattening also
+ marks the parent renderer (RenderIFrame) dirty.
+ While calling layout() to clean the current render tree, we end up laying out the parent tree too.
+ Laying out the parent tree could end up destroying the inline tree context from where the
+ hittest just bubbled down. (InlineFlowBox -> RenderWidget -> RenderView).
+
+ This patch protects the render tree from such unintentional inline tree mutation during hittesting.
+ After the initial layout we set a layout disallow flag on the frame view to defer subsequent layouts.
+ This patch only changes behavior when frame flattening is enabled, but in future we may always want to enable this.
+
+ * fast/frames/flattening/hittest-iframe-while-style-changes-crash-expected.txt: Added.
+ * fast/frames/flattening/hittest-iframe-while-style-changes-crash.html: Added.
+
2015-06-28 Chris Dumez <[email protected]>
Crash: com.apple.WebKit.WebContent at com.apple.WebCore: WebCore::CachedFrameBase::restore + 333
Added: releases/WebKitGTK/webkit-2.8/LayoutTests/fast/frames/flattening/hittest-iframe-while-style-changes-crash-expected.txt (0 => 186436)
--- releases/WebKitGTK/webkit-2.8/LayoutTests/fast/frames/flattening/hittest-iframe-while-style-changes-crash-expected.txt (rev 0)
+++ releases/WebKitGTK/webkit-2.8/LayoutTests/fast/frames/flattening/hittest-iframe-while-style-changes-crash-expected.txt 2015-07-07 10:49:00 UTC (rev 186436)
@@ -0,0 +1 @@
+Pass if no crash or assert in debug.
Added: releases/WebKitGTK/webkit-2.8/LayoutTests/fast/frames/flattening/hittest-iframe-while-style-changes-crash.html (0 => 186436)
--- releases/WebKitGTK/webkit-2.8/LayoutTests/fast/frames/flattening/hittest-iframe-while-style-changes-crash.html (rev 0)
+++ releases/WebKitGTK/webkit-2.8/LayoutTests/fast/frames/flattening/hittest-iframe-while-style-changes-crash.html 2015-07-07 10:49:00 UTC (rev 186436)
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>This tests that hittesting an iframe when frame flattening is on does not crash.</title>
+<script>
+if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+}
+
+if (window.internals)
+ internals.settings.setFrameFlatteningEnabled(true);
+
+function runTest() {
+ setTimeout(function() {
+ document.getElementById('clickonthis').contentDocument.getElementById('foo').style.display = "none";
+ if (window.internals)
+ internals.nodesFromRect(document, 100, 100, 0, 0, 0, 0, false, false, true);
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }, 0);
+}
+</script>
+<body>
+Pass if no crash or assert in debug.
+<iframe _onload_="runTest()" id=clickonthis src="" id=foo>foobar</div>">
+</body>
Modified: releases/WebKitGTK/webkit-2.8/Source/WebCore/ChangeLog (186435 => 186436)
--- releases/WebKitGTK/webkit-2.8/Source/WebCore/ChangeLog 2015-07-07 10:42:06 UTC (rev 186435)
+++ releases/WebKitGTK/webkit-2.8/Source/WebCore/ChangeLog 2015-07-07 10:49:00 UTC (rev 186436)
@@ -1,3 +1,44 @@
+2015-06-30 Zalan Bujtas <[email protected]>
+
+ Frame flattening: Hit-testing an iframe could end up destroying the associated inline tree context.
+ https://bugs.webkit.org/show_bug.cgi?id=146447
+ rdar://problem/20613501
+
+ Reviewed by Simon Fraser.
+
+ This patch ensures that the render tree associated with the document on which
+ the hit-test is initiated does not get laid out, unless it was directly mutated prior to the hittest.
+
+ Hit-test requirements:
+ 1. A clean the render tree before hit-testing gets propagated to the renderers.
+ Document::updateLayout() ensures it by calling both updateStyleIfNeeded() and layout() not only on the current tree, but also
+ on the ancestors if needed.
+
+ 2. No render tree mutation while hit-testing the renderers.
+
+ When an iframe is being hit-tested, this hit-test could bubble down to the child frame's render view.
+ In order to ensure #1, we call Document::updateLayout() on the current (subframe) document.
+ If updateStyleIfNeeded() mutates the render tree, we mark it dirty for layout(). However frame flattening also
+ marks the parent renderer (RenderIFrame) dirty.
+ While calling layout() to clean the current render tree, we end up laying out the parent tree too.
+ Laying out the parent tree could end up destroying the inline tree context from where the
+ hittest just bubbled down. (InlineFlowBox -> RenderWidget -> RenderView).
+
+ This patch protects the render tree from such unintentional inline tree mutation during hittesting.
+ After the initial layout we set a layout disallow flag on the frame view to defer subsequent layouts.
+ This patch only changes behavior when frame flattening is enabled, but in future we may always want to enable this.
+
+ Test: fast/frames/flattening/hittest-iframe-while-style-changes-crash.html
+
+ * page/FrameView.cpp:
+ (WebCore::FrameView::layout):
+ (WebCore::FrameView::startLayoutAtMainFrameViewIfNeeded): Deleted. -> Assertion in no longer valid.
+ * page/FrameView.h:
+ * rendering/RenderView.cpp:
+ (WebCore::FrameFlatteningLayoutDisallower::FrameFlatteningLayoutDisallower):
+ (WebCore::FrameFlatteningLayoutDisallower::~FrameFlatteningLayoutDisallower):
+ (WebCore::RenderView::hitTest): Protect the render tree from subsequent layouts.
+
2015-06-28 Chris Dumez <[email protected]>
Crash: com.apple.WebKit.WebContent at com.apple.WebCore: WebCore::CachedFrameBase::restore + 333
Modified: releases/WebKitGTK/webkit-2.8/Source/WebCore/page/FrameView.cpp (186435 => 186436)
--- releases/WebKitGTK/webkit-2.8/Source/WebCore/page/FrameView.cpp 2015-07-07 10:42:06 UTC (rev 186435)
+++ releases/WebKitGTK/webkit-2.8/Source/WebCore/page/FrameView.cpp 2015-07-07 10:49:00 UTC (rev 186436)
@@ -1123,6 +1123,9 @@
if (isInLayout())
return;
+ if (layoutDisallowed())
+ return;
+
// Protect the view from being deleted during layout (in recalcStyle).
Ref<FrameView> protect(*this);
@@ -3729,9 +3732,6 @@
parentView = parentView->parentFrameView();
parentView->layout(allowSubtree);
-
- RenderElement* root = m_layoutRoot ? m_layoutRoot : frame().document()->renderView();
- ASSERT_UNUSED(root, !root->needsLayout());
}
void FrameView::updateControlTints()
Modified: releases/WebKitGTK/webkit-2.8/Source/WebCore/page/FrameView.h (186435 => 186436)
--- releases/WebKitGTK/webkit-2.8/Source/WebCore/page/FrameView.h 2015-07-07 10:42:06 UTC (rev 186435)
+++ releases/WebKitGTK/webkit-2.8/Source/WebCore/page/FrameView.h 2015-07-07 10:49:00 UTC (rev 186436)
@@ -358,6 +358,10 @@
bool isInChildFrameWithFrameFlattening() const;
+ void startDisallowingLayout() { ++m_layoutDisallowed; }
+ void endDisallowingLayout() { ASSERT(m_layoutDisallowed > 0); --m_layoutDisallowed; }
+ bool layoutDisallowed() const { return m_layoutDisallowed; }
+
static double currentPaintTimeStamp() { return sCurrentPaintTimeStamp; } // returns 0 if not painting
WEBCORE_EXPORT void updateLayoutAndStyleIfNeededRecursive();
@@ -716,6 +720,7 @@
unsigned m_deferSetNeedsLayouts;
bool m_setNeedsLayoutWasDeferred;
+ int m_layoutDisallowed { 0 };
RefPtr<Node> m_nodeToDraw;
PaintBehavior m_paintBehavior;
Modified: releases/WebKitGTK/webkit-2.8/Source/WebCore/rendering/RenderView.cpp (186435 => 186436)
--- releases/WebKitGTK/webkit-2.8/Source/WebCore/rendering/RenderView.cpp 2015-07-07 10:42:06 UTC (rev 186435)
+++ releases/WebKitGTK/webkit-2.8/Source/WebCore/rendering/RenderView.cpp 2015-07-07 10:49:00 UTC (rev 186436)
@@ -54,6 +54,26 @@
namespace WebCore {
+struct FrameFlatteningLayoutDisallower {
+ FrameFlatteningLayoutDisallower(FrameView& frameView)
+ : m_frameView(frameView)
+ , m_disallowLayout(frameView.frame().settings().frameFlatteningEnabled())
+ {
+ if (m_disallowLayout)
+ m_frameView.startDisallowingLayout();
+ }
+
+ ~FrameFlatteningLayoutDisallower()
+ {
+ if (m_disallowLayout)
+ m_frameView.endDisallowingLayout();
+ }
+
+private:
+ FrameView& m_frameView;
+ bool m_disallowLayout { false };
+};
+
struct SelectionIterator {
RenderObject* m_current;
Vector<RenderMultiColumnSpannerPlaceholder*> m_spannerStack;
@@ -176,6 +196,8 @@
{
document().updateLayout();
+ FrameFlatteningLayoutDisallower disallower(frameView());
+
if (layer()->hitTest(request, location, result))
return true;
