|Summary:||A Frame with frame flattening can be stuck in a state in which performPostLayoutTasks() is never executed|
|Product:||WebKit||Reporter:||Benjamin Poulain <benjamin>|
|Component:||Layout and Rendering||Assignee:||Benjamin Poulain <benjamin>|
|Severity:||Normal||CC:||bdakin, ddkilzer, eric.carlson, hyatt, kenneth, koivisto, max.hong.shen, simon.fraser, yael, zalan|
|Version:||528+ (Nightly build)|
Description Benjamin Poulain 2012-01-12 00:23:39 PST
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
Comment 2 Benjamin Poulain 2012-01-12 00:48:57 PST
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.
Comment 3 Simon Fraser (smfr) 2012-01-12 09:59:31 PST
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?
Comment 4 Benjamin Poulain 2012-01-12 11:42:38 PST
> > 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.
Comment 5 Benjamin Poulain 2012-01-12 11:48:29 PST
Comment 6 Antti Koivisto 2012-01-12 13:00:16 PST
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).
Comment 7 Beth Dakin 2012-01-12 14:24:17 PST
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.