Currently it's this logic is inlined in the function which makes it longer than it should be. Also the naming doesn't help understanding what is going on in the function at the moment. Patch forthcoming.
Created attachment 130956 [details] Proposed refactoring.
Comment on attachment 130956 [details] Proposed refactoring. View in context: https://bugs.webkit.org/attachment.cgi?id=130956&action=review In terms of correctness, this looks fine to me. > Source/WebCore/rendering/RenderTableSection.cpp:475 > + if (extraLogicalHeight > 0 && m_rowPos[totalRows]) { nit: maybe make this an early out for extra clarity. > Source/WebCore/rendering/RenderTableSection.cpp:479 > + int tot = m_rowPos[totalRows]; > + int add = 0; > + int prev = m_rowPos[0]; style nit: I realize you're just moving this code, but don't use abbreviations like "tot" and "prev". > Source/WebCore/rendering/RenderTableSection.cpp:499 > + if (m_rowPos[totalRows] || !nextSibling()) { nit: maybe an early return here too. > Source/WebCore/rendering/RenderTableSection.cpp:511 > + remainingExtraLogicalHeight = distributeExtraLogicalHeightToPercentRows(remainingExtraLogicalHeight, totalPercent); > + remainingExtraLogicalHeight = distributeExtraLogicalHeightToAutoRows(remainingExtraLogicalHeight, autoRowsCount); The previous code had checks for totalPercent == 0 and autoRowsCount == 0, which avoided a needless iteration through all of the rows. Is that worth keeping?
Comment on attachment 130956 [details] Proposed refactoring. View in context: https://bugs.webkit.org/attachment.cgi?id=130956&action=review >> Source/WebCore/rendering/RenderTableSection.cpp:511 >> + remainingExtraLogicalHeight = distributeExtraLogicalHeightToAutoRows(remainingExtraLogicalHeight, autoRowsCount); > > The previous code had checks for totalPercent == 0 and autoRowsCount == 0, which avoided a needless iteration through all of the rows. Is that worth keeping? Yes, autoRowsCount == 0 is handled inside distributeExtraLogicalHeightToAutoRows, however I forgot to keep the totalPercent check while cleaning the code. As a side note, a table usually doesn't have a lot of sections AFAICT but it's better not to slow down in case it does.
Created attachment 131069 [details] Same change, nits removed.
Comment on attachment 131069 [details] Same change, nits removed. Looks good to me!
Comment on attachment 131069 [details] Same change, nits removed. Clearing flags on attachment: 131069 Committed r110336: <http://trac.webkit.org/changeset/110336>
All reviewed patches have been landed. Closing bug.