| Summary: | Crash in RenderFlexibleBox::computeInnerFlexBaseSizeForChild due to nullopt optional | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> | ||||||||||
| Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | ajuma, bfulgham, eugenebut, ews-feeder, koivisto, product-security, sdefresne, simon.fraser, webkit-bug-importer, zalan | ||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||
| Version: | WebKit Nightly Build | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=222684 | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Ryosuke Niwa
2020-02-11 17:10:49 PST
Created attachment 390470 [details]
Test case (unreduced)
Crashes inside RenderFlexibleBox::computeInnerFlexBaseSizeForChild method, because computeMainAxisExtentForChild returns Optional without value: if (mainAxisLengthIsDefinite(child, flexBasis)) return std::max(0_lu, computeMainAxisExtentForChild(child, MainOrPreferredSize, flexBasis).value()); This is the only Optional::value call inside the file. All other calls are Optional::valueOr. Perhaps calling valueOr() would be the right fix, but I will also try to figure out why returned Optional does not have a value. Created attachment 396033 [details]
Test case (reduced)
Here is what happens: 1. computeInnerFlexBaseSizeForChild is called for HTMLObjectElement child 2. mainAxisLengthIsDefinite returns true, because child's flex basis is percent and flexible box has definite height 3. computeMainAxisExtentForChild calls computeContentLogicalHeight which returns null, so computeMainAxisExtentForChild returns null if #2 and #3 are correct, then calling valueOr(0) on computeMainAxisExtentForChild's result seems to be the right fix. I don't think I have enough knowledge to argue that #2 and #3 are correct though. Chrome/Blink sizes this as fit-content instead of max-content: https://chromium-review.googlesource.com/c/chromium/src/+/1769536/12/third_party/blink/renderer/core/layout/layout_flexible_box.cc I will test if the same approach works in WebKit next week. (In reply to Eugene But from comment #5) > Chrome/Blink sizes this as fit-content instead of max-content: > > https://chromium-review.googlesource.com/c/chromium/src/+/1769536/12/ > third_party/blink/renderer/core/layout/layout_flexible_box.cc > > I will test if the same approach works in WebKit next week. Thanks for the test reduction! I briefly looked into it and this might be one of the unspecified cases where there's a circular dependency between the child and the containing block sizing (e.g. shrink to fit vs. percentage width) In that case a .valueOr(0) would be sufficient. Will look into it a bit more tomorrow. (In reply to zalan from comment #6) > (In reply to Eugene But from comment #5) > > Chrome/Blink sizes this as fit-content instead of max-content: > > > > https://chromium-review.googlesource.com/c/chromium/src/+/1769536/12/ > > third_party/blink/renderer/core/layout/layout_flexible_box.cc > > > > I will test if the same approach works in WebKit next week. > Thanks for the test reduction! I briefly looked into it and this might be > one of the unspecified cases where there's a circular dependency between the > child and the containing block sizing (e.g. shrink to fit vs. percentage > width) In that case a .valueOr(0) would be sufficient. Will look into it a > bit more tomorrow. <style> .flexBox { display: -webkit-flex; width: min-content; writing-mode: vertical-lr; -webkit-flex-direction: column-reverse; } .flexItem { flex: 0 0 50%; text-anchor: middle; } </style> <div class=flexBox><input class=flexItem><div class=flexItem></div></div> We seems to set the m_hasDefiniteHeight to true incorrectly when laying out the <input> (since input reports horizontal writing mode here and RenderBox::computePercentageLogicalHeight returns with a non-null value (the percentage height is resolved against the containing block width here). Now when we get to the <div> we believe the axis length is set (height in this case) so we should be fine with resolving the percentage hight -but we are really not because the containing block's height is not computed yet (again, the width is). Created attachment 396340 [details]
Patch
Comment on attachment 396340 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=396340&action=review > Source/WebCore/ChangeLog:10 > + because perpendicular boxes's height are resolved against the containing block's width. a perpendicular box's height is resolved > Source/WebCore/rendering/RenderFlexibleBox.cpp:771 > + // Perpendicular children's height are resolved against the containing block's width which is not the main axis. The height of a perpendicular child is resolved Created attachment 396341 [details]
Patch
Committed r260055: <https://trac.webkit.org/changeset/260055> All reviewed patches have been landed. Closing bug and clearing flags on attachment 396341 [details]. |