Diff
Modified: branches/safari-600.8-branch/LayoutTests/ChangeLog (186631 => 186632)
--- branches/safari-600.8-branch/LayoutTests/ChangeLog 2015-07-09 21:03:34 UTC (rev 186631)
+++ branches/safari-600.8-branch/LayoutTests/ChangeLog 2015-07-09 21:03:39 UTC (rev 186632)
@@ -1,5 +1,46 @@
2015-07-09 Matthew Hanson <matthew_han...@apple.com>
+ Merge r186577. rdar://problem/21533109
+
+ 2015-07-08 Lucas Forschler <lforsch...@apple.com>
+
+ Merge r186165. rdar://problem/21533207
+
+ 2015-06-30 Zalan Bujtas <za...@apple.com>
+
+ 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-07-09 Matthew Hanson <matthew_han...@apple.com>
+
Merge r186575. rdar://problem/21716377
2015-07-08 Lucas Forschler <lforsch...@apple.com>
Added: branches/safari-600.8-branch/LayoutTests/fast/frames/flattening/hittest-iframe-while-style-changes-crash-expected.txt (0 => 186632)
--- branches/safari-600.8-branch/LayoutTests/fast/frames/flattening/hittest-iframe-while-style-changes-crash-expected.txt (rev 0)
+++ branches/safari-600.8-branch/LayoutTests/fast/frames/flattening/hittest-iframe-while-style-changes-crash-expected.txt 2015-07-09 21:03:39 UTC (rev 186632)
@@ -0,0 +1 @@
+Pass if no crash or assert in debug.
Added: branches/safari-600.8-branch/LayoutTests/fast/frames/flattening/hittest-iframe-while-style-changes-crash.html (0 => 186632)
--- branches/safari-600.8-branch/LayoutTests/fast/frames/flattening/hittest-iframe-while-style-changes-crash.html (rev 0)
+++ branches/safari-600.8-branch/LayoutTests/fast/frames/flattening/hittest-iframe-while-style-changes-crash.html 2015-07-09 21:03:39 UTC (rev 186632)
@@ -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: branches/safari-600.8-branch/Source/WebCore/ChangeLog (186631 => 186632)
--- branches/safari-600.8-branch/Source/WebCore/ChangeLog 2015-07-09 21:03:34 UTC (rev 186631)
+++ branches/safari-600.8-branch/Source/WebCore/ChangeLog 2015-07-09 21:03:39 UTC (rev 186632)
@@ -1,5 +1,54 @@
2015-07-09 Matthew Hanson <matthew_han...@apple.com>
+ Merge r186577. rdar://problem/21533109
+
+ 2015-07-08 Lucas Forschler <lforsch...@apple.com>
+
+ Merge r186165. rdar://problem/21533207
+
+ 2015-06-30 Zalan Bujtas <za...@apple.com>
+
+ 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-07-09 Matthew Hanson <matthew_han...@apple.com>
+
Merge r186575. rdar://problem/21716377
2015-07-08 Lucas Forschler <lforsch...@apple.com>
Modified: branches/safari-600.8-branch/Source/WebCore/page/FrameView.cpp (186631 => 186632)
--- branches/safari-600.8-branch/Source/WebCore/page/FrameView.cpp 2015-07-09 21:03:34 UTC (rev 186631)
+++ branches/safari-600.8-branch/Source/WebCore/page/FrameView.cpp 2015-07-09 21:03:39 UTC (rev 186632)
@@ -1090,6 +1090,9 @@
if (isInLayout())
return;
+ if (layoutDisallowed())
+ return;
+
// Protect the view from being deleted during layout (in recalcStyle).
Ref<FrameView> protect(*this);
@@ -3582,9 +3585,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: branches/safari-600.8-branch/Source/WebCore/page/FrameView.h (186631 => 186632)
--- branches/safari-600.8-branch/Source/WebCore/page/FrameView.h 2015-07-09 21:03:34 UTC (rev 186631)
+++ branches/safari-600.8-branch/Source/WebCore/page/FrameView.h 2015-07-09 21:03:39 UTC (rev 186632)
@@ -350,6 +350,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
void updateLayoutAndStyleIfNeededRecursive();
@@ -690,6 +694,7 @@
unsigned m_deferSetNeedsLayouts;
bool m_setNeedsLayoutWasDeferred;
+ int m_layoutDisallowed { 0 };
RefPtr<Node> m_nodeToDraw;
PaintBehavior m_paintBehavior;
Modified: branches/safari-600.8-branch/Source/WebCore/rendering/RenderView.cpp (186631 => 186632)
--- branches/safari-600.8-branch/Source/WebCore/rendering/RenderView.cpp 2015-07-09 21:03:34 UTC (rev 186631)
+++ branches/safari-600.8-branch/Source/WebCore/rendering/RenderView.cpp 2015-07-09 21:03:39 UTC (rev 186632)
@@ -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 @@
bool RenderView::hitTest(const HitTestRequest& request, const HitTestLocation& location, HitTestResult& result)
{
+ FrameFlatteningLayoutDisallower disallower(frameView());
+
if (layer()->hitTest(request, location, result))
return true;
