First of all, I think this bug affects all the platforms/ports, since it was detected while looking into the bug #118595 which affects all the ports enabling SUBPIXEL_LAYOUT flag by default. Secondly, The bug is not reproducible if the SATURATED_LAYOUT_ARITHMETIC flag is enabled, at least for the WebKitGtk+ port. Finally, when debugging the test provided by the bug #118595, I found out that the max-height CSS property is handled by the WebCore::CSSPrimitiveValue::computeLength<WebCore::Length> method, which, when using SUBPIXEL_LAYOUT, the property value of 2000000000px is passed through the MathExtras::clampTo<float> function with [min, max] = [-33554430, 33554429]. At some point in the layout process, the RenderBox::constrainLogicalHeightByMinMax method is called, causing the arithmetic overflow when adjusting the box borders at RenderBox::adjustBorderBoxLogicalHeightForBoxSizing while computing the height + bordersPlusPadding operation, being height the style()->logicalMaxHeight(). Clearly, the SATURATED_LAYOUT_ARITHMETIC support will avoid this overflow, since the "+" operator uses the "saturatedAddition", but I guess it would be also possible to protect the adjustBorderBoxLogicalHeightForBoxSizing method to avoid the problem. However, I tend to think that using SUBPIXEL_LAYOUT without the SATURATED_LAYOUT_ARITHMETIC is a risk in terms of this kind of arithmetic overflow.
Created attachment 207763 [details] Test to verify the issue.
Created attachment 207772 [details] Patch
I would very much prefer to attempt a general fix, like always use SATURATED_LAYOUT_ARITHMETIC whenever ENABLE_SUBPIXEL_LAYOUT is enabled.
Created attachment 215492 [details] Patch
Created attachment 215493 [details] Patch
Comment on attachment 215493 [details] Patch Attachment 215493 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/17628157
Comment on attachment 215493 [details] Patch Attachment 215493 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/17578145
Created attachment 215495 [details] Patch
Comment on attachment 215495 [details] Patch Attachment 215495 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/17558153
Comment on attachment 215495 [details] Patch Attachment 215495 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/17598157
Created attachment 215496 [details] Patch
(In reply to comment #11) > Created an attachment (id=215496) [details] > Patch Fix for bug #123931 introduced regressions on the GTK port because the port enables subpixel layout while still disabling the saturated layout arithmetic. Landing this patch is the ideal fix, so any reviews would be greatly appreciated.
Comment on attachment 215496 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=215496&action=review > Source/WTF/wtf/FeatureDefines.h:294 > +#if !defined(ENABLE_SATURATED_LAYOUT_ARITHMETIC) > +#define ENABLE_SATURATED_LAYOUT_ARITHMETIC 1 > +#endif > + Let's just enable it via build-webkit and in SetupWebKitFeatures.m4 and avoiding changing this file for now. Hopefully we can remove the flag shortly. > LayoutTests/fast/css/max-height-huge-value-expected.html:2 > +<html lang="en"> Do you need the lang tag here? > LayoutTests/fast/css/max-height-huge-value-expected.html:10 > + width: 100px; > + border: 2px solid blue; > + overflow: hidden; > + max-height: 20000000px; > + } Bit of a nit, but this indentation is irregular. > LayoutTests/platform/gtk/TestExpectations:-1384 > -webkit.org/b/118595 fast/regions/auto-size/autoheight-correct-region-for-lines-2.html [ ImageOnlyFailure ] > - Does this test already cover the failure. If so, you may not need a new test.
Created attachment 218970 [details] Patch Removed new test and applied suggested changes.
Comment on attachment 218970 [details] Patch Clearing flags on attachment: 218970 Committed r160441: <http://trac.webkit.org/changeset/160441>
All reviewed patches have been landed. Closing bug.