Bug 76154 – A Frame with frame flattening can be stuck in a state in which performPostLayoutTasks() is never executed

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Benjamin Poulain]

For <rdar://problem/10363387>.

With Frame Flattening, performPostLayoutTasks() is always executed asynchronously.

If FrameView::unscheduleRelayout() is executed when a pending post layout task timer exists, performPostLayoutTasks() will never be executed for this frame.

Here is the sequence of events
1) there is a layout, and since inSubframeLayoutWithFrameFlattening == true, m_hasPendingPostLayoutTasks becomes true and a post layout is scheduled
2) there is a FrameView::unscheduleRelayout(), which kill the timer, and leave m_hasPendingPostLayoutTasks == true
so m_hasPendingPostLayoutTasks == true, and in the common case that would get processed in the next layout() but here we only execute the post layout on timer
3) all the next layouts skip the postLayout() tasks because m_hasPendingPostLayoutTasks == true

[Benjamin Poulain]

Created attachment 122183 [details]
Patch

[Benjamin Poulain]

I have trouble making a test for this. I do not have yet a reliable way to trigger FrameView::unscheduleRelayout() after a layout() but before the timer.

I would appreciate if you could already check if this looks correct.

[Simon Fraser (smfr)]

Comment on attachment 122183 [details]
Patch

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

I think Beth should look at this.

> Source/WebCore/ChangeLog:14
> +        This patch revert the handling of the postLayoutTasks to its state prior to r66552.

Presumably without regressing the bug that r66552 fixed? Can you say more here about how you managed that?

[Benjamin Poulain]

> > Source/WebCore/ChangeLog:14
> > +        This patch revert the handling of the postLayoutTasks to its state prior to r66552.
> 
> Presumably without regressing the bug that r66552 fixed? Can you say more here about how you managed that?

This do not revert r66552, just revert a detail of the patch: the change from the timer to a bool to manage the state.

[Benjamin Poulain]

Some more context: r66552 changed the postLayoutTasks from the timer to a bool, and stopped the timer on unschedudeRelayout. That change makes sense.

But then, r77988 change the way the postLayoutTasks are performed, and that is when everything went to hell.

[Antti Koivisto]

Comment on attachment 122183 [details]
Patch

Looks good to me and reducing complexity here a bit is nice (assuming the earlier problematic cases are still handled somehow).

[Beth Dakin]

Comment on attachment 122183 [details]
Patch

Well if Antti's on board, so am I ;-)

Looks good. And since I desperately can't remember why I added m_hasPendingPostLayoutTasks, I can't really object to its removal. I still think you should test the original reproducibility steps of that bug that I was fixing when I added it, but r=me.

[Benjamin Poulain]

Committed r104874: <http://trac.webkit.org/changeset/104874>