Bug 65530 – ASSERT hit when frame flattening is on.

• Component: Frames
• Status: RESOLVED FIXED

[Yael]

Loading the URL http://www.w3schools.com/tags/tryit.asp?filename=tryhtml_iframe hits an ASSERT when frame flattening is on.
This issue was found by [e-mail] .
A patch is coming.

[Yi Shen]

Created attachment 107073 [details]
proposal fix

[Antonio Gomes]

Comment on attachment 107073 [details]
proposal fix

I r- it due to the lack of test.

[Yi Shen]

(In reply to comment #1)
> Created an attachment (id=107073) [details]
> proposal fix

In FrameView::layout(), when the frame flattening is on, and there is a parent view, it calls the parentView->layout() instead of doing the layout on current frameview. Ideally, calling the parentView->layout() eventually invokes the layout() on the current frameview.

However, there are two possibles that the current frameview won't get a chance to be layoutted. 1) The parentView's needsLayout() returns false when you call parentView->layout(); 2) The RenderIFrame::flattenFrame() returns false for some reasons, e.g. the size of the iframe is fixed. That's why the asset hits when loading some particular pages because the current frameview doesn't get layoutted.

The patch I made tries to fix this problem by checking the needsLayout() for current frameview when the recursive parent layout is done. If it still needs layout, then continue layouting the current frame as usual.

[Yi Shen]

(In reply to comment #2)
> (From update of attachment 107073 [details])
> I r- it due to the lack of test.

Thanks for the review, Antonio. I do have a simple test page but it can't reproduce the problem every time, since there is a timing issue - calls layout() on a child frameview directly is trigged by frame loading. Do you know if there is a way to call layout() on a child frameview easily? thx

[Kenneth Rohde Christiansen]

Added Zalan as he has 2-3 not-yet upstreamed fixes to the frame flattening.

[Kenneth Rohde Christiansen]

(In reply to comment #5)
> Added Zalan as he has 2-3 not-yet upstreamed fixes to the frame flattening.

There are a few here, but you might want to look thru our branch, if you are fixing things related to frame flattening.

http://gitorious.org/+qtwebkit-webkit2-dev/webkit/qtwebkit-webkit2-dev/commit/de5b269e3b5524682f763ab6a8ec6c5751365bca
http://gitorious.org/+qtwebkit-webkit2-dev/webkit/qtwebkit-webkit2-dev/commit/dd52a398e7ecc30ec085e3f88541c7061cabe173

[Yael]

(In reply to comment #6)
> (In reply to comment #5)
> > Added Zalan as he has 2-3 not-yet upstreamed fixes to the frame flattening.
> 
> There are a few here, but you might want to look thru our branch, if you are fixing things related to frame flattening.
> 
> http://gitorious.org/+qtwebkit-webkit2-dev/webkit/qtwebkit-webkit2-dev/commit/de5b269e3b5524682f763ab6a8ec6c5751365bca
> http://gitorious.org/+qtwebkit-webkit2-dev/webkit/qtwebkit-webkit2-dev/commit/dd52a398e7ecc30ec085e3f88541c7061cabe173

The fixes in those 2 links are actually cherry-picking of fixes we did in webkit.org :)

[Kenneth Rohde Christiansen]

(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > Added Zalan as he has 2-3 not-yet upstreamed fixes to the frame flattening.
> > 
> > There are a few here, but you might want to look thru our branch, if you are fixing things related to frame flattening.
> > 
> > http://gitorious.org/+qtwebkit-webkit2-dev/webkit/qtwebkit-webkit2-dev/commit/de5b269e3b5524682f763ab6a8ec6c5751365bca
> > http://gitorious.org/+qtwebkit-webkit2-dev/webkit/qtwebkit-webkit2-dev/commit/dd52a398e7ecc30ec085e3f88541c7061cabe173
> 
> The fixes in those 2 links are actually cherry-picking of fixes we did in webkit.org :)

Oh great ;-) Anyway, I seem to remember that there are a few more from Zalan that I am pretty sure are not upstreamed. Something with 'position'. Maybe Zalan remembers or you can find it using git log.

[Martin Robinson]

The GTK+ bot his hitting this assertion when running fast/frames/flattening/iframe-flattening-nested.html after  http://trac.webkit.org/changeset/99361. I've confirmed that the patch here fixes the issue. Here is the assertion stack trace: 

#0  0x00002b73dfe5a323 in WebCore::FrameView::paintContents (this=0x48a2c5b0, p=0x7fff6db24870, rect=...) at ../../Source/WebCore/page/FrameView.cpp:2734
2734	    ASSERT(!needsLayout());

#0  0x00002b73dfe5a323 in WebCore::FrameView::paintContents (this=0x48a2c5b0, p=0x7fff6db24870, rect=...) at ../../Source/WebCore/page/FrameView.cpp:2734
#1  0x00002b73dff7461b in WebCore::ScrollView::paint (this=0x48a2c5b0, context=0x7fff6db24870, rect=...) at ../../Source/WebCore/platform/ScrollView.cpp:1047
#2  0x00002b73e01352f4 in WebCore::RenderWidget::paint (this=0x49ac6e48, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderWidget.cpp:290
#3  0x00002b73dffd5c97 in WebCore::InlineBox::paint (this=0x4b20d288, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/InlineBox.cpp:231
#4  0x00002b73dffdae1e in WebCore::InlineFlowBox::paint (this=0x4a84d448, paintInfo=..., paintOffset=..., lineTop=0, lineBottom=176) at ../../Source/WebCore/rendering/InlineFlowBox.cpp:1061
#5  0x00002b73e013b5b6 in WebCore::RootInlineBox::paint (this=0x4a84d448, paintInfo=..., paintOffset=..., lineTop=0, lineBottom=176) at ../../Source/WebCore/rendering/RootInlineBox.cpp:195
#6  0x00002b73e00c0ecb in WebCore::RenderLineBoxList::paint (this=0x4a00e898, renderer=0x4a00e7f8, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderLineBoxList.cpp:262
#7  0x00002b73dfffbdf2 in WebCore::RenderBlock::paintContents (this=0x4a00e7f8, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderBlock.cpp:2507
#8  0x00002b73dfffc672 in WebCore::RenderBlock::paintObject (this=0x4a00e7f8, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderBlock.cpp:2617
#9  0x00002b73dfffaed5 in WebCore::RenderBlock::paint (this=0x4a00e7f8, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderBlock.cpp:2362
#10 0x00002b73dfffc151 in WebCore::RenderBlock::paintChildren (this=0x49d4f368, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderBlock.cpp:2551
#11 0x00002b73dfffbe0b in WebCore::RenderBlock::paintContents (this=0x49d4f368, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderBlock.cpp:2509
#12 0x00002b73dfffc672 in WebCore::RenderBlock::paintObject (this=0x49d4f368, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderBlock.cpp:2617
#13 0x00002b73dfffaed5 in WebCore::RenderBlock::paint (this=0x49d4f368, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderBlock.cpp:2362
#14 0x00002b73dfffc151 in WebCore::RenderBlock::paintChildren (this=0x49d4f8c8, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderBlock.cpp:2551
#15 0x00002b73dfffbe0b in WebCore::RenderBlock::paintContents (this=0x49d4f8c8, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderBlock.cpp:2509
#16 0x00002b73dfffc672 in WebCore::RenderBlock::paintObject (this=0x49d4f8c8, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderBlock.cpp:2617
#17 0x00002b73dfffaed5 in WebCore::RenderBlock::paint (this=0x49d4f8c8, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderBlock.cpp:2362
#18 0x00002b73e00b29f3 in WebCore::RenderLayer::paintLayer (this=0x4a84c238, rootLayer=0x47d55a08, p=0x7fff6db24870, paintDirtyRect=..., paintBehavior=0, paintingRoot=0x0, region=0x0, overlapTestRequests=0x7fff6db22e90, paintFlags=0) at ../../Source/WebCore/rendering/RenderLayer.cpp:2806
#19 0x00002b73e00b2f57 in WebCore::RenderLayer::paintList (this=0x47d55a08, list=0x48a454b0, rootLayer=0x47d55a08, p=0x7fff6db24870, paintDirtyRect=..., paintBehavior=0, paintingRoot=0x0, region=0x0, overlapTestRequests=0x7fff6db22e90, paintFlags=0) at ../../Source/WebCore/rendering/RenderLayer.cpp:2865
#20 0x00002b73e00b2c51 in WebCore::RenderLayer::paintLayer (this=0x47d55a08, rootLayer=0x47d55a08, p=0x7fff6db24870, paintDirtyRect=..., paintBehavior=0, paintingRoot=0x0, region=0x0, overlapTestRequests=0x7fff6db22e90, paintFlags=0) at ../../Source/WebCore/rendering/RenderLayer.cpp:2827
#21 0x00002b73e00b1868 in WebCore::RenderLayer::paint (this=0x47d55a08, p=0x7fff6db24870, damageRect=..., paintBehavior=0, paintingRoot=0x0, region=0x0, paintFlags=0) at ../../Source/WebCore/rendering/RenderLayer.cpp:2549
#22 0x00002b73dfe5a55a in WebCore::FrameView::paintContents (this=0x48fe4c90, p=0x7fff6db24870, rect=...) at ../../Source/WebCore/page/FrameView.cpp:2770
#23 0x00002b73dff7461b in WebCore::ScrollView::paint (this=0x48fe4c90, context=0x7fff6db24870, rect=...) at ../../Source/WebCore/platform/ScrollView.cpp:1047
#24 0x00002b73e01352f4 in WebCore::RenderWidget::paint (this=0x4a84d2c8, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderWidget.cpp:290
#25 0x00002b73dffd5c97 in WebCore::InlineBox::paint (this=0x4a84d1d8, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/InlineBox.cpp:231
#26 0x00002b73dffdae1e in WebCore::InlineFlowBox::paint (this=0x49bb5cf8, paintInfo=..., paintOffset=..., lineTop=0, lineBottom=402) at ../../Source/WebCore/rendering/InlineFlowBox.cpp:1061
#27 0x00002b73e013b5b6 in WebCore::RootInlineBox::paint (this=0x49bb5cf8, paintInfo=..., paintOffset=..., lineTop=0, lineBottom=402) at ../../Source/WebCore/rendering/RootInlineBox.cpp:195
#28 0x00002b73e00c0ecb in WebCore::RenderLineBoxList::paint (this=0x47d56208, renderer=0x47d56168, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderLineBoxList.cpp:262
#29 0x00002b73dfffbdf2 in WebCore::RenderBlock::paintContents (this=0x47d56168, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderBlock.cpp:2507
#30 0x00002b73dfffc672 in WebCore::RenderBlock::paintObject (this=0x47d56168, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderBlock.cpp:2617
#31 0x00002b73dfffaed5 in WebCore::RenderBlock::paint (this=0x47d56168, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderBlock.cpp:2362
#32 0x00002b73dfffc151 in WebCore::RenderBlock::paintChildren (this=0x48360748, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderBlock.cpp:2551
#33 0x00002b73dfffbe0b in WebCore::RenderBlock::paintContents (this=0x48360748, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderBlock.cpp:2509
#34 0x00002b73dfffc672 in WebCore::RenderBlock::paintObject (this=0x48360748, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderBlock.cpp:2617
#35 0x00002b73dfffaed5 in WebCore::RenderBlock::paint (this=0x48360748, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderBlock.cpp:2362
#36 0x00002b73dfffc151 in WebCore::RenderBlock::paintChildren (this=0x48c5c5b8, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderBlock.cpp:2551
#37 0x00002b73dfffbe0b in WebCore::RenderBlock::paintContents (this=0x48c5c5b8, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderBlock.cpp:2509
#38 0x00002b73dfffc672 in WebCore::RenderBlock::paintObject (this=0x48c5c5b8, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderBlock.cpp:2617
#39 0x00002b73dfffaed5 in WebCore::RenderBlock::paint (this=0x48c5c5b8, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderBlock.cpp:2362
#40 0x00002b73e00b29f3 in WebCore::RenderLayer::paintLayer (this=0x49d4ff08, rootLayer=0x48275428, p=0x7fff6db24870, paintDirtyRect=..., paintBehavior=0, paintingRoot=0x0, region=0x0, overlapTestRequests=0x7fff6db24570, paintFlags=0) at ../../Source/WebCore/rendering/RenderLayer.cpp:2806
#41 0x00002b73e00b2f57 in WebCore::RenderLayer::paintList (this=0x48275428, list=0x4a84d390, rootLayer=0x48275428, p=0x7fff6db24870, paintDirtyRect=..., paintBehavior=0, paintingRoot=0x0, region=0x0, overlapTestRequests=0x7fff6db24570, paintFlags=0) at ../../Source/WebCore/rendering/RenderLayer.cpp:2865
#42 0x00002b73e00b2c51 in WebCore::RenderLayer::paintLayer (this=0x48275428, rootLayer=0x48275428, p=0x7fff6db24870, paintDirtyRect=..., paintBehavior=0, paintingRoot=0x0, region=0x0, overlapTestRequests=0x7fff6db24570, paintFlags=0) at ../../Source/WebCore/rendering/RenderLayer.cpp:2827
#43 0x00002b73e00b1868 in WebCore::RenderLayer::paint (this=0x48275428, p=0x7fff6db24870, damageRect=..., paintBehavior=0, paintingRoot=0x0, region=0x0, paintFlags=0) at ../../Source/WebCore/rendering/RenderLayer.cpp:2549
#44 0x00002b73dfe5a55a in WebCore::FrameView::paintContents (this=0x4a28de20, p=0x7fff6db24870, rect=...) at ../../Source/WebCore/page/FrameView.cpp:2770
#45 0x00002b73dff7461b in WebCore::ScrollView::paint (this=0x4a28de20, context=0x7fff6db24870, rect=...) at ../../Source/WebCore/platform/ScrollView.cpp:1047
#46 0x00002b73df691474 in WebKit::paintWebView (webView=0x870050, frame=0x89cb80, dirtyRegion=...) at ../../Source/WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp:515
#47 0x00002b73df691804 in WebKit::ChromeClient::paint (this=0x86eb80) at ../../Source/WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp:560
#48 0x00002b73df69098d in WebKit::repaintEverythingSoonTimeout (client=0x86eb80) at ../../Source/WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp:387
#49 0x00002b73e4926ddb in g_timeout_dispatch (source=0x4a08afa0, callback=<optimized out>, user_data=<optimized out>) at /tmp/buildd/glib2.0-2.28.6/./glib/gmain.c:3882
#50 0x00002b73e49254a3 in g_main_dispatch (context=0x7f3d70) at /tmp/buildd/glib2.0-2.28.6/./glib/gmain.c:2440
#51 g_main_context_dispatch (context=0x7f3d70) at /tmp/buildd/glib2.0-2.28.6/./glib/gmain.c:3013
#52 0x00002b73e4925c80 in g_main_context_iterate (context=0x7f3d70, block=1, dispatch=1, self=<optimized out>) at /tmp/buildd/glib2.0-2.28.6/./glib/gmain.c:3091
#53 0x00002b73e49262f2 in g_main_loop_run (loop=0x49d51110) at /tmp/buildd/glib2.0-2.28.6/./glib/gmain.c:3299
#54 0x00002b73e26d34cd in gtk_main () from /usr/lib/libgtk-3.so.0
#55 0x000000000042f295 in runTest (testPathOrURL=...) at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:708
#56 0x000000000042e8cd in runTestingServerLoop () at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:500
#57 0x0000000000430bfe in main (argc=2, argv=0x7fff6db25628) at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1204

[Philippe Normand]

*** Bug 72246 has been marked as a duplicate of this bug. ***

[Philippe Normand]

(In reply to comment #2)
> (From update of attachment 107073 [details])
> I r- it due to the lack of test.

Even if there's no new test that patches fixes 
fast/frames/flattening/iframe-flattening-nested.html and
fast/frames/flattening/iframe-flattening-fixed-width-and-height-no-scrolling.html

on GTK at least. Wouldn't that be a reason to accept this patch?

[zalan]

(In reply to comment #11)
> (In reply to comment #2)
> > (From update of attachment 107073 [details] [details])
> > I r- it due to the lack of test.
> 
> Even if there's no new test that patches fixes 
> fast/frames/flattening/iframe-flattening-nested.html and
> fast/frames/flattening/iframe-flattening-fixed-width-and-height-no-scrolling.html
> 
> on GTK at least. Wouldn't that be a reason to accept this patch?
This patch looks like a workaround for the problem where non-flattened iframe layout should not be reinitiated from the top. please check https://bugs.webkit.org/show_bug.cgi?id=80155 to see if the patch there fixes these failures.

[Philippe Normand]

(In reply to comment #12)
> (In reply to comment #11)
> > (In reply to comment #2)
> > > (From update of attachment 107073 [details] [details] [details])
> > > I r- it due to the lack of test.
> > 
> > Even if there's no new test that patches fixes 
> > fast/frames/flattening/iframe-flattening-nested.html and
> > fast/frames/flattening/iframe-flattening-fixed-width-and-height-no-scrolling.html
> > 
> > on GTK at least. Wouldn't that be a reason to accept this patch?
> This patch looks like a workaround for the problem where non-flattened iframe layout should not be reinitiated from the top. please check https://bugs.webkit.org/show_bug.cgi?id=80155 to see if the patch there fixes these failures.

Indeed it does fix them. They'll need rebaseline though.

*** This bug has been marked as a duplicate of bug 80155 ***

[Yael]

(In reply to comment #13)
> 
> *** This bug has been marked as a duplicate of bug 80155 ***

I don't think this is a duplicate of bug 80155. bug 80155 improves the situation, but the url above (http://www.w3schools.com/tags/tryit.asp?filename=tryhtml_iframe) still asserts.

[zalan]

(In reply to comment #14)
> (In reply to comment #13)
> > 
> > *** This bug has been marked as a duplicate of bug 80155 ***
> 
> I don't think this is a duplicate of bug 80155. bug 80155 improves the situation, but the url above (http://www.w3schools.com/tags/tryit.asp?filename=tryhtml_iframe) still asserts.

Correct, this looks a lot more like a duplicate of bug 81114. This w3school page has 3 level nested iframes where the bottommost includes blocking resources. It shows the same pattern of logs what I see with 81114's simple test case. https://bugs.webkit.org/show_bug.cgi?id=81114#c1

[Yi Shen]

(In reply to comment #15)
> (In reply to comment #14)
> > (In reply to comment #13)
> > > 
> > > *** This bug has been marked as a duplicate of bug 80155 ***
> > 
> > I don't think this is a duplicate of bug 80155. bug 80155 improves the situation, but the url above (http://www.w3schools.com/tags/tryit.asp?filename=tryhtml_iframe) still asserts.
> 
> Correct, this looks a lot more like a duplicate of bug 81114. This w3school page has 3 level nested iframes where the bottommost includes blocking resources. It shows the same pattern of logs what I see with 81114's simple test case. https://bugs.webkit.org/show_bug.cgi?id=81114#c1

Zalan, are you working on fixing this bug (or bug 81114).  As I remember, this issue can't be reproduced consistently on Mac build.

[zalan]

(In reply to comment #16)
> (In reply to comment #15)
> > (In reply to comment #14)
> > > (In reply to comment #13)
> > > > 
> > > > *** This bug has been marked as a duplicate of bug 80155 ***
> > > 
> > > I don't think this is a duplicate of bug 80155. bug 80155 improves the situation, but the url above (http://www.w3schools.com/tags/tryit.asp?filename=tryhtml_iframe) still asserts.
> > 
> > Correct, this looks a lot more like a duplicate of bug 81114. This w3school page has 3 level nested iframes where the bottommost includes blocking resources. It shows the same pattern of logs what I see with 81114's simple test case. https://bugs.webkit.org/show_bug.cgi?id=81114#c1
> 
> Zalan, are you working on fixing this bug (or bug 81114).  As I remember, this issue can't be reproduced consistently on Mac build.

Yes, I'll have a patch on it soon.

[zalan]

(In reply to comment #15)
> (In reply to comment #14)
> > (In reply to comment #13)
> > > 
> > > *** This bug has been marked as a duplicate of bug 80155 ***
> > 
> > I don't think this is a duplicate of bug 80155. bug 80155 improves the situation, but the url above (http://www.w3schools.com/tags/tryit.asp?filename=tryhtml_iframe) still asserts.
> 
> Correct, this looks a lot more like a duplicate of bug 81114. This w3school page has 3 level nested iframes where the bottommost includes blocking resources. It shows the same pattern of logs what I see with 81114's simple test case. https://bugs.webkit.org/show_bug.cgi?id=81114#c1

81114's patch won't fix this. it needs one more thing in addition to that patch.

[zalan]

fixed in http://trac.webkit.org/changeset/113091