Bug 141688

Summary: FrameView::layoutTimerFired() should update style if needed before doing layout
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: Layout and RenderingAssignee: Simon Fraser (smfr) <simon.fraser>
Status: REOPENED ---    
Severity: Normal CC: commit-queue, kling, simon.fraser, zalan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 142368    
Bug Blocks:    
Attachments:
Description Flags
Patch none

Description Simon Fraser (smfr) 2015-02-16 18:41:01 PST 
It's possible for the style recalc timer to get scheduled after the layout timer (both delay 0). The layout timer fires first, so we do layout, but then the style recalc timer fires and we do style recalc and layout again.

This happens in fast/canvas/webgl/arraybuffer-transfer-of-control.html and likely other content.
Comment 1 Simon Fraser (smfr) 2015-02-17 16:31:42 PST 
Fixing this does reduce the number of layouts slightly, but not the total time spent in layout.
Comment 2 Simon Fraser (smfr) 2015-02-28 14:04:34 PST 
Created attachment 247613 [details]
Patch
Comment 3 Andreas Kling 2015-02-28 14:52:03 PST 
Comment on attachment 247613 [details]
Patch

r=me
This does indeed seem totally reasonable.
Comment 4 WebKit Commit Bot 2015-02-28 17:04:27 PST 
Comment on attachment 247613 [details]
Patch

Clearing flags on attachment: 247613

Committed r180846: <http://trac.webkit.org/changeset/180846>
Comment 5 WebKit Commit Bot 2015-02-28 17:04:40 PST 
All reviewed patches have been landed.  Closing bug.
Comment 6 WebKit Commit Bot 2015-03-05 15:54:56 PST 
Re-opened since this is blocked by bug 142368
Comment 7 Simon Fraser (smfr) 2015-03-06 17:38:31 PST 
Fix for the regression:

diff --git a/Source/WebCore/page/FrameView.cpp b/Source/WebCore/page/FrameView.cpp
index 229c8a910a042486c9991e4785d8a8dd8058d711..484d90ca1f002b43c55abdb76ea17d65b9f395a2 100644
--- a/Source/WebCore/page/FrameView.cpp
+++ b/Source/WebCore/page/FrameView.cpp
@@ -2440,6 +2440,12 @@ void FrameView::layoutTimerFired()
     if (!frame().document()->ownerElement())
         printf("Layout timer fired at %lld\n", frame().document()->elapsedTime().count());
 #endif
+
+    {
+        TemporaryChange<bool> layoutSchedulingDisabler(m_layoutSchedulingEnabled, false);
+        frame().document()->updateStyleIfNeeded();
+    }
+
     layout();
 }
 
Needs testcase.