- Revision
- 110738
- Author
- [email protected]
- Date
- 2012-03-14 13:22:55 -0700 (Wed, 14 Mar 2012)
Log Message
Frame flattening ASSERT(!needsLayout()) in FrameView::paintContents()
https://bugs.webkit.org/show_bug.cgi?id=80155
Patch by Zalan Bujtas <[email protected]> on 2012-03-14
Reviewed by Antti Koivisto.
Source/WebCore:
This patch ensures that an iframe only schedules and calls parent's layout,
when it is going to be flattened. Non-flattened iframe does not affect
parent's layout, so normal layout flow applies. isInSubframeLayoutWithFrameFlattening()
function has been added to test whether a particular child frame is changing
parent's layout. This function also ensures that scheduleRelayout() and layout()
are in sync of checking againts frame flattening.
Test: fast/frames/flattening/iframe-flattening-fixed-width-and-height-no-scrolling-with-js-forced-layout.html
* page/FrameView.cpp:
(WebCore::FrameView::avoidScrollbarCreation):
(WebCore::FrameView::layout):
(WebCore::FrameView::scheduleRelayout):
(WebCore::FrameView::isInChildFrameWithFrameFlattening):
(WebCore):
(WebCore::FrameView::updateLayoutAndStyleIfNeededRecursive):
* page/FrameView.h:
(FrameView):
* rendering/RenderIFrame.h:
(RenderIFrame):
(WebCore::RenderIFrame::renderName):
LayoutTests:
* fast/frames/flattening/iframe-flattening-fixed-width-and-height-no-scrolling-with-js-forced-layout-expected.txt: Added.
* fast/frames/flattening/iframe-flattening-fixed-width-and-height-no-scrolling-with-js-forced-layout.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (110737 => 110738)
--- trunk/LayoutTests/ChangeLog 2012-03-14 20:07:42 UTC (rev 110737)
+++ trunk/LayoutTests/ChangeLog 2012-03-14 20:22:55 UTC (rev 110738)
@@ -1,3 +1,13 @@
+2012-03-14 Zalan Bujtas <[email protected]>
+
+ Frame flattening ASSERT(!needsLayout()) in FrameView::paintContents()
+ https://bugs.webkit.org/show_bug.cgi?id=80155
+
+ Reviewed by Antti Koivisto.
+
+ * fast/frames/flattening/iframe-flattening-fixed-width-and-height-no-scrolling-with-js-forced-layout-expected.txt: Added.
+ * fast/frames/flattening/iframe-flattening-fixed-width-and-height-no-scrolling-with-js-forced-layout.html: Added.
+
2012-03-14 Ojan Vafai <[email protected]>
Add Lion failures in order to green the lion bot.
Added: trunk/LayoutTests/fast/frames/flattening/iframe-flattening-fixed-width-and-height-no-scrolling-with-js-forced-layout-expected.txt (0 => 110738)
--- trunk/LayoutTests/fast/frames/flattening/iframe-flattening-fixed-width-and-height-no-scrolling-with-js-forced-layout-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/frames/flattening/iframe-flattening-fixed-width-and-height-no-scrolling-with-js-forced-layout-expected.txt 2012-03-14 20:22:55 UTC (rev 110738)
@@ -0,0 +1,3 @@
+This test verifies that nested iframes do not cause assertion failure, when _javascript_ is forcing layout. PASS if no assert in debug.
+
+
Added: trunk/LayoutTests/fast/frames/flattening/iframe-flattening-fixed-width-and-height-no-scrolling-with-js-forced-layout.html (0 => 110738)
--- trunk/LayoutTests/fast/frames/flattening/iframe-flattening-fixed-width-and-height-no-scrolling-with-js-forced-layout.html (rev 0)
+++ trunk/LayoutTests/fast/frames/flattening/iframe-flattening-fixed-width-and-height-no-scrolling-with-js-forced-layout.html 2012-03-14 20:22:55 UTC (rev 110738)
@@ -0,0 +1,33 @@
+<html>
+<head>
+ <script type="text/_javascript_">
+ if (window.layoutTestController) {
+ layoutTestController.waitUntilDone();
+ layoutTestController.dumpAsText();
+ layoutTestController.setFrameFlatteningEnabled(true);
+ }
+
+ function test()
+ {
+ setTimeout(function() {
+ if (window.layoutTestController)
+ layoutTestController.notifyDone();
+ }, 0);
+ }
+ </script>
+</head>
+<body _onload_="test()">
+ <div>
+ <p>This test verifies that nested iframes do not cause assertion failure, when _javascript_ is forcing layout. PASS if no assert in debug.</p>
+ </div>
+ <iframe src=""
+ <style>body { background-color: green; }</style>
+ <body>
+ <iframe src='' scrolling='no' height='50px' width='50px'></iframe>
+ <script>document.body.offsetHeight</script>
+ </body>" scrolling="no" height="100px" width="100px"></iframe>
+ <script>document.body.offsetHeight</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (110737 => 110738)
--- trunk/Source/WebCore/ChangeLog 2012-03-14 20:07:42 UTC (rev 110737)
+++ trunk/Source/WebCore/ChangeLog 2012-03-14 20:22:55 UTC (rev 110738)
@@ -1,3 +1,32 @@
+2012-03-14 Zalan Bujtas <[email protected]>
+
+ Frame flattening ASSERT(!needsLayout()) in FrameView::paintContents()
+ https://bugs.webkit.org/show_bug.cgi?id=80155
+
+ Reviewed by Antti Koivisto.
+
+ This patch ensures that an iframe only schedules and calls parent's layout,
+ when it is going to be flattened. Non-flattened iframe does not affect
+ parent's layout, so normal layout flow applies. isInSubframeLayoutWithFrameFlattening()
+ function has been added to test whether a particular child frame is changing
+ parent's layout. This function also ensures that scheduleRelayout() and layout()
+ are in sync of checking againts frame flattening.
+
+ Test: fast/frames/flattening/iframe-flattening-fixed-width-and-height-no-scrolling-with-js-forced-layout.html
+
+ * page/FrameView.cpp:
+ (WebCore::FrameView::avoidScrollbarCreation):
+ (WebCore::FrameView::layout):
+ (WebCore::FrameView::scheduleRelayout):
+ (WebCore::FrameView::isInChildFrameWithFrameFlattening):
+ (WebCore):
+ (WebCore::FrameView::updateLayoutAndStyleIfNeededRecursive):
+ * page/FrameView.h:
+ (FrameView):
+ * rendering/RenderIFrame.h:
+ (RenderIFrame):
+ (WebCore::RenderIFrame::renderName):
+
2012-03-14 Anders Carlsson <[email protected]>
Don't cap the scroll position if layout happens when a FrameView's overhangAmount is non-zero
Modified: trunk/Source/WebCore/page/FrameView.cpp (110737 => 110738)
--- trunk/Source/WebCore/page/FrameView.cpp 2012-03-14 20:07:42 UTC (rev 110737)
+++ trunk/Source/WebCore/page/FrameView.cpp 2012-03-14 20:22:55 UTC (rev 110738)
@@ -56,6 +56,7 @@
#include "RenderArena.h"
#include "RenderEmbeddedObject.h"
#include "RenderFullScreen.h"
+#include "RenderIFrame.h"
#include "RenderLayer.h"
#include "RenderPart.h"
#include "RenderScrollbar.h"
@@ -436,7 +437,7 @@
ASSERT(m_frame);
// with frame flattening no subframe can have scrollbars
- // but we also cannot turn scrollbars of as we determine
+ // but we also cannot turn scrollbars off as we determine
// our flattening policy using that.
if (!m_frame->ownerElement())
@@ -891,17 +892,19 @@
if (m_inLayout)
return;
- bool inSubframeLayoutWithFrameFlattening = parent() && m_frame->settings() && m_frame->settings()->frameFlatteningEnabled();
+ bool inChildFrameLayoutWithFrameFlattening = isInChildFrameWithFrameFlattening();
- if (inSubframeLayoutWithFrameFlattening) {
- if (parent()->isFrameView()) {
- FrameView* parentView = static_cast<FrameView*>(parent());
- if (!parentView->m_nestedLayoutCount) {
- while (parentView->parent() && parentView->parent()->isFrameView())
- parentView = static_cast<FrameView*>(parentView->parent());
- parentView->layout(allowSubtree);
- return;
- }
+ if (inChildFrameLayoutWithFrameFlattening) {
+ FrameView* parentView = parentFrameView();
+ if (parentView && !parentView->m_nestedLayoutCount) {
+ while (parentView->parentFrameView())
+ parentView = parentView->parentFrameView();
+
+ parentView->layout(allowSubtree);
+
+ RenderObject* root = m_layoutRoot ? m_layoutRoot : m_frame->document()->renderer();
+ ASSERT_UNUSED(root, !root->needsLayout());
+ return;
}
}
@@ -940,7 +943,7 @@
{
TemporaryChange<bool> changeSchedulingEnabled(m_layoutSchedulingEnabled, false);
- if (!m_nestedLayoutCount && !m_inSynchronousPostLayout && m_postLayoutTasksTimer.isActive() && !inSubframeLayoutWithFrameFlattening) {
+ if (!m_nestedLayoutCount && !m_inSynchronousPostLayout && m_postLayoutTasksTimer.isActive() && !inChildFrameLayoutWithFrameFlattening) {
// This is a new top-level layout. If there are any remaining tasks from the previous
// layout, finish them now.
m_inSynchronousPostLayout = true;
@@ -1118,7 +1121,7 @@
if (!m_postLayoutTasksTimer.isActive()) {
if (!m_inSynchronousPostLayout) {
- if (inSubframeLayoutWithFrameFlattening) {
+ if (inChildFrameLayoutWithFrameFlattening) {
if (RenderView* root = rootRenderer(this))
root->updateWidgetPositions();
} else {
@@ -1129,7 +1132,7 @@
}
}
- if (!m_postLayoutTasksTimer.isActive() && (needsLayout() || m_inSynchronousPostLayout || inSubframeLayoutWithFrameFlattening)) {
+ if (!m_postLayoutTasksTimer.isActive() && (needsLayout() || m_inSynchronousPostLayout || inChildFrameLayoutWithFrameFlattening)) {
// If we need layout or are already in a synchronous call to postLayoutTasks(),
// defer widget updates and event dispatch until after we return. postLayoutTasks()
// can make us need to update again, and we can get stuck in a nasty cycle unless
@@ -1962,12 +1965,10 @@
if (!m_frame->document()->shouldScheduleLayout())
return;
- // When frame flattening is enabled, the contents of the frame affects layout of the parent frames.
+ // When frame flattening is enabled, the contents of the frame could affect the layout of the parent frames.
// Also invalidate parent frame starting from the owner element of this frame.
- if (m_frame->settings() && m_frame->settings()->frameFlatteningEnabled() && m_frame->ownerRenderer()) {
- if (m_frame->ownerElement()->hasTagName(iframeTag) || m_frame->ownerElement()->hasTagName(frameTag))
- m_frame->ownerRenderer()->setNeedsLayout(true, true);
- }
+ if (isInChildFrameWithFrameFlattening() && m_frame->ownerRenderer())
+ m_frame->ownerRenderer()->setNeedsLayout(true, true);
int delay = m_frame->document()->minimumLayoutDelay();
if (m_layoutTimer.isActive() && m_delayedLayout && !delay)
@@ -2801,6 +2802,25 @@
return 0;
}
+bool FrameView::isInChildFrameWithFrameFlattening()
+{
+ if (!parent() || !m_frame->ownerElement() || !m_frame->settings() || !m_frame->settings()->frameFlatteningEnabled())
+ return false;
+
+ // Frame flattening applies when the owner element is either in a frameset or
+ // an iframe with flattening parameters.
+ if (m_frame->ownerElement()->hasTagName(iframeTag)) {
+ RenderIFrame* iframeRenderer = toRenderIFrame(m_frame->ownerElement()->renderPart());
+
+ if (iframeRenderer->flattenFrame())
+ return true;
+
+ } else if (m_frame->ownerElement()->hasTagName(frameTag))
+ return true;
+
+ return false;
+}
+
void FrameView::updateControlTints()
{
// This is called when control tints are changed from aqua/graphite to clear and vice versa.
@@ -3008,6 +3028,11 @@
// updateLayoutAndStyleIfNeededRecursive is called when we need to make sure style and layout are up-to-date before
// painting, so we need to flush out any deferred repaints too.
flushDeferredRepaints();
+
+ // When frame flattening is on, child frame can mark parent frame dirty. In such case, child frame
+ // needs to call layout on parent frame recursively.
+ // This assert ensures that parent frames are clean, when child frames finished updating layout and style.
+ ASSERT(!needsLayout());
}
void FrameView::flushDeferredRepaints()
Modified: trunk/Source/WebCore/page/FrameView.h (110737 => 110738)
--- trunk/Source/WebCore/page/FrameView.h 2012-03-14 20:07:42 UTC (rev 110737)
+++ trunk/Source/WebCore/page/FrameView.h 2012-03-14 20:22:55 UTC (rev 110738)
@@ -406,6 +406,8 @@
FrameView* parentFrameView() const;
+ bool isInChildFrameWithFrameFlattening();
+
virtual AXObjectCache* axObjectCache() const;
void notifyWidgetsInAllFrames(WidgetNotification);
Modified: trunk/Source/WebCore/rendering/RenderIFrame.h (110737 => 110738)
--- trunk/Source/WebCore/rendering/RenderIFrame.h 2012-03-14 20:07:42 UTC (rev 110737)
+++ trunk/Source/WebCore/rendering/RenderIFrame.h 2012-03-14 20:22:55 UTC (rev 110738)
@@ -34,6 +34,8 @@
public:
explicit RenderIFrame(Element*);
+ bool flattenFrame();
+
private:
virtual void computeLogicalHeight();
virtual void computeLogicalWidth();
@@ -44,8 +46,6 @@
virtual const char* renderName() const { return "RenderPartObject"; } // Lying for now to avoid breaking tests
- bool flattenFrame();
-
};
inline RenderIFrame* toRenderIFrame(RenderObject* object)
