[webkit-dev] Focus Crash Relating to MathML

[David Hyatt]

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

> On Tue, Oct 19, 2010 at 11:29 AM, David Hyatt <hyatt at 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 at apple.com)

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20101019/dbb485bf/attachment.html>