Bug 179754 – firstLineBaseline called on non-laid out cell content during HTML table layout

• Component: Tables
• Status: NEW

[Frédéric Wang (:fredw)]

Created attachment 327053 [details]
Testcase

This was originally reported in bug 174131 for RenderMathMLScripts::firstLineBaseline which is the only implementation asserting !needsLayout(). However, it turns out that this not specific to MathML, so I'm opening it here so that people more familiar with HTML table layout can comment. Actually, the use of firstLineBaseline in MathML is really an implementation detail inherited from the old flexbox-based approach and we might change it in the future, see for example bug 155879.

Basically, RenderTableCell::layout() starts with the following line:

    int oldCellBaseline = cellBaselinePosition();
    layoutBlock(cellWidthChanged)

Then cellBaselinePosition() calls RenderBlockFlow::firstLineBaseline before the layout of the cell, which in turn may call RenderBlock::firstLineBaseline (if !childrenInline()) which will finally call firstLineBaseline on children. But these children are non laid out either.

Here are the firstLineBaseline implementations:

 find -type f -name '*.h' | xargs grep firstLineBaseline
./RenderBox.h:    virtual std::optional<int> firstLineBaseline() const { return std::optional<int>(); }
./RenderBlock.h:    std::optional<int> firstLineBaseline() const override;
./RenderTableSection.h:    std::optional<int> firstLineBaseline() const override;
./RenderTable.h:    std::optional<int> firstLineBaseline() const override;
./RenderMenuList.h:    std::optional<int> firstLineBaseline() const override { return RenderBlock::firstLineBaseline(); }
./RenderFlexibleBox.h:    std::optional<int> firstLineBaseline() const override;
./RenderTextControl.h:    std::optional<int> firstLineBaseline() const override { return RenderBlock::firstLineBaseline(); }
./RenderGrid.h:    std::optional<int> firstLineBaseline() const final;
./RenderBlockFlow.h:    std::optional<int> firstLineBaseline() const override;
./mathml/RenderMathMLOperator.h:    std::optional<int> firstLineBaseline() const final;
./mathml/RenderMathMLRow.h:    std::optional<int> firstLineBaseline() const override;
./mathml/RenderMathMLSpace.h:    std::optional<int> firstLineBaseline() const final;
./mathml/RenderMathMLToken.h:    std::optional<int> firstLineBaseline() const override;
./mathml/RenderMathMLFraction.h:    std::optional<int> firstLineBaseline() const final;
./mathml/RenderMathMLScripts.h:    std::optional<int> firstLineBaseline() const final;
./mathml/RenderMathMLPadded.h:    std::optional<int> firstLineBaseline() const final;
./mathml/RenderMathMLBlock.h:        return child.firstLineBaseline().value_or(child.logicalHeight());
./mathml/RenderMathMLBlock.h:    std::optional<int> firstLineBaseline() const final;

The attached testcase will hit many of these functions with needsLayout() == false.

The question is: is it OK to get firstLineBaseline on boxes that are non laid out yet? Maybe it's fine to return a dummy value for the initial "oldCellBaseline". Or maybe we should explicitly do an early return with a null optional (that's what happen for RenderFlexibleBox and RenderGrid implementations in the testcase) so that cellBaselinePosition() will use the fallback value instead of the wrong value? Maybe this special handling can just happen for RenderTableCell and all the other implementations should ASSERT(!needsLayout())? Any other idea?