Bug 81114 - ASSERT(!needsLayout()) in FrameView::paintContents() with multiple nested iframes, when frame flattening is on
Summary: ASSERT(!needsLayout()) in FrameView::paintContents() with multiple nested ifr...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zalan
URL: http://www.iol.ie/~mariah/dbrooks/del...
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-14 08:41 PDT by zalan
Modified: 2012-04-03 14:35 PDT (History)
6 users (show)

See Also:


Attachments
Patch (6.92 KB, patch)
2012-03-26 11:43 PDT, zalan
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-01 (9.80 MB, application/zip)
2012-03-26 12:56 PDT, WebKit Review Bot
no flags Details
Patch (7.66 KB, patch)
2012-03-27 08:09 PDT, zalan
no flags Details | Formatted Diff | Diff
Patch (8.08 KB, patch)
2012-04-03 08:52 PDT, zalan
koivisto: review+
koivisto: commit-queue-
Details | Formatted Diff | Diff
Patch (8.41 KB, patch)
2012-04-03 13:57 PDT, zalan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description zalan 2012-03-14 08:41:13 PDT
In case of multiple nested iframes, when the bottom most iframe has a blocking resource and there's a call to paint the tree, before the blocking resource arrives, FrameView::paintContens() asserts with !needsLayout().

this behavior can be seen at www.iol.ie/~mariah/dbrooks/delon.html when clicking on Tag Board link.

simplified test case:
<html><body><iframe src="data:text/html,<html><body><iframe src='http://dl.dropbox.com/u/36138203/cbox.html'></iframe></body></html>"></iframe></body></html>
where cbox.html is a simple page with a blocking resource.
<html><head><script src="notfound.js" type="text/javascript"></script></head><body></body></html>
Comment 1 zalan 2012-03-14 09:43:12 PDT
steps leading to assertion:

1, When the bottommost iframes loads, FrameView/RenderView are constructed and get marked dirty. (markContainingBlocksForLayout )
2, Parser stops at the script element.
3, Viewport requests painting.
5, updateLayoutAndStyleIfNeededRecursive(mainframe) is called to clean the render tree recursively before performing painting.
4, At this point only the newly constructed, bottommost FrameView is dirty. Both its parent and its parent's parent (mainframe) are clean (due to previous layouts)
5, layout() is called on the bottommost FrameView, which in turn re-initiates the layout flow from the topmost FrameView and expects that the layouting will reach itself.
6, Since parent FrameViews are clean, layouting stops at the topmost frame.
7, Bottommost FrameView's layout returns with dirty tree.

No assertion happens:
1, When the blocking resource is not present and parsing hits <body>.
HTMLBodyElement::insertedIntoDocument() schedules relayout on the current (bottommost) FrameView, which in turn marks its parent dirty.

2, When there's one less nested iframe.
The layout works accidentally in this case. At step 6, when layout(mainframe) performs post layout tasks, it calls updateWidgetPositions(), which eventually calls layout on direct (dirty)child frames. So getting the bottommost tree clean does not happen as the result of the normal layout flow (FrameView(mainframe)->RenderView->...->RenderIFrame->FrameView(child)), but as the result of perform post layout task.
Comment 2 zalan 2012-03-26 11:43:57 PDT
Created attachment 133856 [details]
Patch
Comment 3 WebKit Review Bot 2012-03-26 12:56:15 PDT
Comment on attachment 133856 [details]
Patch

Attachment 133856 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12145013

New failing tests:
http/tests/misc/iframe-flattening-3level-nesting-with-blocking-resource.html
Comment 4 WebKit Review Bot 2012-03-26 12:56:22 PDT
Created attachment 133871 [details]
Archive of layout-test-results from ec2-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 5 zalan 2012-03-27 08:09:22 PDT
Created attachment 134066 [details]
Patch
Comment 6 zalan 2012-03-27 08:10:45 PDT
"CONSOLE MESSAGE: line 7: Uncaught TypeError: Object [object Object] has no method 'setFrameFlatteningEnabled'"
chromium-ews failed on setFrameFlatteningEnabled(). Added to skip list similarly to all the other frame flattening tests.
Comment 7 zalan 2012-04-03 08:52:12 PDT
Created attachment 135337 [details]
Patch
Comment 8 Antti Koivisto 2012-04-03 12:28:55 PDT
Comment on attachment 135337 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=135337&action=review

> Source/WebCore/ChangeLog:9
> +        When frame flattening is on, childframes try to mark ancestors dirty to be able to restart
> +        layout from mainframe. When marking ancestor fails, childframe layout needs to continue normally.

You should explain what the case where marking ancestors fails is and why it is not an error.

> LayoutTests/platform/chromium/test_expectations.txt:724
>  WONTFIX SKIP : fast/frames/flattening = TIMEOUT
> +WONTFIX SKIP : http/tests/misc/iframe-flattening-3level-nesting-with-blocking-resource.html = TIMEOUT

Please check that other platforms are ok with this when landing.
Comment 9 zalan 2012-04-03 13:47:02 PDT
(In reply to comment #8)
> (From update of attachment 135337 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=135337&action=review
> 
> > Source/WebCore/ChangeLog:9
> > +        When frame flattening is on, childframes try to mark ancestors dirty to be able to restart
> > +        layout from mainframe. When marking ancestor fails, childframe layout needs to continue normally.
> 
> You should explain what the case where marking ancestors fails is and why it is not an error.
> 
> > LayoutTests/platform/chromium/test_expectations.txt:724
> >  WONTFIX SKIP : fast/frames/flattening = TIMEOUT
> > +WONTFIX SKIP : http/tests/misc/iframe-flattening-3level-nesting-with-blocking-resource.html = TIMEOUT
> 
> Please check that other platforms are ok with this when landing.
other platforms do implement layoutTestController.setFrameFlatteningEnabled
Comment 10 zalan 2012-04-03 13:57:29 PDT
Created attachment 135418 [details]
Patch
Comment 11 WebKit Review Bot 2012-04-03 14:35:25 PDT
Comment on attachment 135418 [details]
Patch

Clearing flags on attachment: 135418

Committed r113091: <http://trac.webkit.org/changeset/113091>
Comment 12 WebKit Review Bot 2012-04-03 14:35:43 PDT
All reviewed patches have been landed.  Closing bug.