Make computeLogicalHeight virtual and updateLogicalHeight non-virtual.

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Tony Chang]

In bug 94982, the code for computeLogicalHeight was made const and put int a non-virtual function.  This got rid of some hacks where we would store the old height, call computeLogicalHeight, then restore the old height.

However, in the new code, it's hard to use computeLogicalHeight because it's not virtual so if you call it on a pointer and your pointer overrides updateLogicalHeight (e.g., it's an iframe), it'll do the wrong thing.  See RenderBlock::computeBlockPreferredLogicalWidths for an example of where we would like to use childBox->computeLogicalHeight(..), but it crashes because sometimes childBox is a RenderView, which overrides updateLogicalHeight().

I think long term, we want to make computeLogicalHeight virtual and move code from the subclass updateLogicalHeight into it and then make updateLogicalHeight non-virtual.

[Tony Chang]

computeLogicalHeight is now virtual, however, I can't make updateLogicalHeight virtual because of RenderMultiColumnSet, which sets a member variable during updateLogicalHeight.

It looks like that variable is the same as logicalHeight right now, but the comments suggest that there is a future use for it.  I'd like to remove RenderMultiColumnSet::m_computedColumnHeight.

[Dave Hyatt]

Yeah, you're not going to be able to remove this. It's going to be needed.

[Morten Stenshorne]

FYI: the RenderMultiColumnSet::updateLogicalHeight() override is going away with the fix for https://bugs.webkit.org/show_bug.cgi?id=116033 , but in the meantime (after this bug was reported) RenderRegion::updateLogicalHeight() has appeared.

[Rob Buis]

Created attachment 462358 [details]
Patch

[Tim Nguyen (:ntim)]

r=me

[EWS]

Committed 254564@main (c096c71bf576): <https://commits.webkit.org/254564@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 462358 [details].

[Radar WebKit Bug Importer]

<rdar://problem/100036927>