On Oct 19, 2010, at 2:07 PM, Alex Milowski wrote:

On Tue, Oct 19, 2010 at 11:29 AM, David Hyatt <hyatt@apple.com> wrote:
(1) Make sure any layout methods you call do setNeedsLayout(false) at the end of them.
(2) Look for any early returns in any of your layout methods, since maybe you did an early return causing the setNeedsLayout(false) to be missed.
(3) Make sure you aren't dirtying a child for a re-layout without immediately doing that re-layout, e.g., don't call setChildNeedsLayout(true, false) on some child and then bail without doing a layout.

While this is helpful, the current code (in the patch) follows these
principles (except when RenderBlock::layout() is called last and so
setNeedsLayout(false) is already done).  The problem I have is an
*ancestor* is marked as having a child needing layout during the
layout process.  When then MathML layout finishes, the MathML
rendering objects do not need layout but the parent is marked with
m_normalChildNeedsLayout set to true.


Ok, just speculating from eyeballing the code....  I think layoutInlineChildren should do setNeedsLayout(false) on inlines when the end of the inline is encountered rather than the start of it.

The iteration order is start of inline -> contents of inline -> end of inline, and we do setNeedsLayout(false) at the start of the inline.  If the contents of the inline end up causing a dirtying up the chain, then this will not be caught and cleared.

    else if (o->isText() || (o->isRenderInline() && !endOfInline)) {
                if (fullLayout || o->selfNeedsLayout())
                    dirtyLineBoxesForRenderer(o, fullLayout);
                o->setNeedsLayout(false);
                if (!o->isText())
                    toRenderInline(o)->invalidateVerticalPosition(); // FIXME: Should do better here and not always invalidate everything.
            }

In that code above I think !endOfInline should maybe just become endOfInline instead.

dave
(hyatt@apple.com)