[Webkit-unassigned] [Bug 125306] New: Invalidating two disjoint elements' relayouts triggers full relayout

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=125306

           Summary: Invalidating two disjoint elements' relayouts triggers
                    full relayout
           Product: WebKit
           Version: 528+ (Nightly build)
          Platform: All
               URL: http://www.russellmcc.com/private/abslayoutwebkit/test
                    .html
        OS/Version: Unspecified
            Status: NEW
          Severity: Normal
          Priority: P2
         Component: New Bugs
        AssignedTo: webkit-unassigned at lists.webkit.org
        ReportedBy: russell.mcclellan at gmail.com


We ran into this while developing a web app with a complicated layout.  Performing a full layout can take 10s or 100s of ms due to the complexity of the layout.  This bug totally cripples the mobile user experience during drags that affect multiple elements without using one of the workarounds described below.

If you invalidate the layouts two elements where neither are ancestors of the other, without triggering a layout in between, Webkit will decide that the whole document's layout is invalid.  In many cases, this is a very bad performance decision.  For example, see the linked page: clicking the "click here" button will change the text on the two absolutely-positioned divs.  As a web developer, I might expect this to cause a relayout of each of those divs and nothing else.  Instead, the entire document has a relayout event (as can been seen from the web inspector's timeline view), even though almost all of the document is *not* invalid.

As a web developer, I came up with two different workarounds (perhaps there are more): you can force a synchronous layout by querying something like the layoutHeight after invalidating the layout of a simple element (this is what the "or here" button in the test page does to prevent a full-document reflow).  Another option would be to institute a policy of always deferring functions that might affect the layout to their own stack by using setTimeout with zero timeout (webkit seems to always do a layout when the javascript stack clears, but I didn't look at the source to confirm this).  Neither of these workarounds is really a natural coding style.

I couldn't resist poking around in the source to find where this logic occurs: it seems to me that the culprit is in WebCore:page/FrameView.cpp's  FrameView::scheduleRelayoutOfSubtree function, which updates a "m_layoutRoot" variable which represents the RenderElement to start the layout from the next time a layout is triggered.  If there's already a layout scheduled, and it can't find an ancestor relationship between the new invalid element and the existing m_layoutRoot, it decides to do a full relayout.

I had two ideas of approaches that might be better than the status quo.  Please take these suggestions with a grain of salt as I'm not too familiar with the WebKit codebase - I'm just thinking out loud.  If someone familiar with this stuff thinks these are good ideas and would be willing to hold my hand a little bit I'd be happy to try to contribute either.

A somewhat complex approach that may be much more performant would be to replace "m_layoutRoot" with an "m_layoutRoots" an array of requested layout reflows, and then scheduleRelayoutOfSubtree would just add the subtree into m_layoutRoots.  Then, in layout, go through each of the m_layoutRoots in order, making sure to remove from the array any element  that happens to be layed out before it's turn (e.g.,  if it was a descendent of a previous member of m_layoutRoots).

Another approach that perhaps would be better than the current situation and would perhaps be easier to code would be, instead of triggering a full layout, synchronously layout the pending m_layoutRoot right there in the scheduleRelayoutOfSubtree function.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.