Bug 141688 - FrameView::layoutTimerFired() should update style if needed before doing layout
Summary: FrameView::layoutTimerFired() should update style if needed before doing layout
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords:
Depends on: 142368
Blocks:
  Show dependency treegraph
 
Reported: 2015-02-16 18:41 PST by Simon Fraser (smfr)
Modified: 2015-03-06 17:38 PST (History)
4 users (show)

See Also:


Attachments
Patch (1.69 KB, patch)
2015-02-28 14:04 PST, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.