[webkit-dev] Focus Crash Relating to MathML

[David Hyatt]

Also, if your pattern of code in a layout method is

(1) Call base class RenderBlock::layout
(2) Do other stuff that might cause dirtying

Then you should really bulletproof that code by adding

(1) Call base class RenderBlock::layout
(2) Do a setChildNeedsLayout(true, false) on yourself just to make yourself already dirty.
(3) Do other stuff that might cause dirtying
(4) Do a setNeedsLayout(false)

We don't really have a good setup for calling base class layout methods... technically you should stay dirty throughout the lifetime of your own layout method, but the base class method will mark you as "clean."  We should come up with something better at some point, but for now I think if you just dirty for the rest of the code you want to run and then mark yourself clean at the end, you'd stop the problem as well.

dave

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.
> 
> This only becomes a problem when the parent of the RenderMathMLMath
> rendering object is a RenderInline instance as the a RenderBlock will
> call setNeedsLayout(false) on itself at the very end of layout.  To
> me, although I have yet to confirm this, it seems like
> setNeedsLayout(false) is called during the layout of the inline flow
> from RenderBlock::layoutInlineChildren() on the RenderInline instance
> and then the RenderInline is marked with a child needing layout.
> Unfortunately, none of the above suggestions are going to fix that.
> 
> I think the call to destroyLeftoverChildren() is also something we
> should reconsider.  In my very simple example, this is what is causing
> the RenderInline instance to be marked with a child needing layout as
> it causes a traversal through the ancestors.  I know why it is there
> but it doesn't necessarily seem like the right way (or place) to
> reorganize the operator stacking.
> 
> -- 
> --Alex Milowski
> "The excellence of grammar as a guide is proportional to the paucity of the
> inflexions, i.e. to the degree of analysis effected by the language
> considered."
> 
> Bertrand Russell in a footnote of Principles of Mathematics
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev