Bug 151094

Summary: ASSERTION FAILED: m_isEngaged in WTF::Optional::value
Product: WebKit Reporter: Renata Hodovan <rhodovan.u-szeged>
Component: Layout and RenderingAssignee: Sergio Villar Senin <svillar>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, commit-queue, darin, esprehn+autocc, glenn, kondapallykalyan, simon.fraser, svillar, zalan
Priority: P2    
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=164366
Bug Depends on:    
Bug Blocks: 116980    
Attachments:
Description Flags
Test
none
Patch
none
Patch for landing none

Description Renata Hodovan 2015-11-10 07:29:50 PST
Created attachment 265176 [details]
Test

Load the attached test with debug MiniBrowser:

<style>
* {
    display: inline-flex;
    flex-direction: column-reverse;
    height: 0%;
}
</style>
<table>
    <caption>
        <h2></h2>
    </caption>
</table>


OS: Ubuntu 15.04 x86_64
Checked build: debug EFL
Checked version: 29ae33c


Backtrace:

ASSERTION FAILED: m_isEngaged
../../Source/WTF/wtf/Optional.h(153) : T& WTF::Optional<T>::value() [with T = WebCore::LayoutUnit]
1   0x7fd20c9bc89f WTFCrash
2   0x7fd213331afc WTF::Optional<WebCore::LayoutUnit>::value()
3   0x7fd21337675a WebCore::RenderFlexibleBox::adjustChildSizeForMinAndMax(WebCore::RenderBox&, WebCore::LayoutUnit)
4   0x7fd213376c8a WebCore::RenderFlexibleBox::computeNextFlexLine(WTF::Vector<WebCore::RenderBox*, 0ul, WTF::CrashOnOverflow, 16ul>&, WebCore::LayoutUnit&, double&, double&, WebCore::LayoutUnit&, bool&)
5   0x7fd21337578f WebCore::RenderFlexibleBox::layoutFlexItems(bool, WTF::Vector<WebCore::RenderFlexibleBox::LineContext, 0ul, WTF::CrashOnOverflow, 16ul>&)
6   0x7fd21337385f WebCore::RenderFlexibleBox::layoutBlock(bool, WebCore::LayoutUnit)
7   0x7fd2132b0558 WebCore::RenderBlock::layout()
8   0x7fd213283af5 WebCore::RenderElement::layoutIfNeeded()
9   0x7fd21330371a WebCore::RenderBlockFlow::layoutLineBoxes(bool, WebCore::LayoutUnit&, WebCore::LayoutUnit&)
10  0x7fd2132dea59 WebCore::RenderBlockFlow::layoutInlineChildren(bool, WebCore::LayoutUnit&, WebCore::LayoutUnit&)
11  0x7fd2132ddd9f WebCore::RenderBlockFlow::layoutBlock(bool, WebCore::LayoutUnit)
12  0x7fd21348141a WebCore::RenderTableCell::layout()
13  0x7fd21348ab2e WebCore::RenderTableRow::layout()
14  0x7fd213283af5 WebCore::RenderElement::layoutIfNeeded()
15  0x7fd21348d1b3 WebCore::RenderTableSection::layout()
16  0x7fd213283af5 WebCore::RenderElement::layoutIfNeeded()
17  0x7fd2134735fe WebCore::RenderTable::layout()
18  0x7fd213283af5 WebCore::RenderElement::layoutIfNeeded()
19  0x7fd213374477 WebCore::RenderFlexibleBox::computeMainAxisExtentForChild(WebCore::RenderBox&, WebCore::SizeType, WebCore::Length const&)
20  0x7fd21337660d WebCore::RenderFlexibleBox::adjustChildSizeForMinAndMax(WebCore::RenderBox&, WebCore::LayoutUnit)
21  0x7fd213376c8a WebCore::RenderFlexibleBox::computeNextFlexLine(WTF::Vector<WebCore::RenderBox*, 0ul, WTF::CrashOnOverflow, 16ul>&, WebCore::LayoutUnit&, double&, double&, WebCore::LayoutUnit&, bool&)
22  0x7fd21337578f WebCore::RenderFlexibleBox::layoutFlexItems(bool, WTF::Vector<WebCore::RenderFlexibleBox::LineContext, 0ul, WTF::CrashOnOverflow, 16ul>&)
23  0x7fd21337385f WebCore::RenderFlexibleBox::layoutBlock(bool, WebCore::LayoutUnit)
24  0x7fd2132b0558 WebCore::RenderBlock::layout()
25  0x7fd213283af5 WebCore::RenderElement::layoutIfNeeded()
26  0x7fd213374477 WebCore::RenderFlexibleBox::computeMainAxisExtentForChild(WebCore::RenderBox&, WebCore::SizeType, WebCore::Length const&)
27  0x7fd21337660d WebCore::RenderFlexibleBox::adjustChildSizeForMinAndMax(WebCore::RenderBox&, WebCore::LayoutUnit)
28  0x7fd213376c8a WebCore::RenderFlexibleBox::computeNextFlexLine(WTF::Vector<WebCore::RenderBox*, 0ul, WTF::CrashOnOverflow, 16ul>&, WebCore::LayoutUnit&, double&, double&, WebCore::LayoutUnit&, bool&)
29  0x7fd21337578f WebCore::RenderFlexibleBox::layoutFlexItems(bool, WTF::Vector<WebCore::RenderFlexibleBox::LineContext, 0ul, WTF::CrashOnOverflow, 16ul>&)
30  0x7fd21337385f WebCore::RenderFlexibleBox::layoutBlock(bool, WebCore::LayoutUnit)
31  0x7fd2132b0558 WebCore::RenderBlock::layout()
Error receiving IPC message on socket 40 in process 26076: Connection reset by peer
Aborted (core dumped)

Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x00007fd20c9bc8a4 in WTFCrash () at ../../Source/WTF/wtf/Assertions.cpp:321
321     *(int *)(uintptr_t)0xbbadbeef = 0;
#0  0x00007fd20c9bc8a4 in WTFCrash () at ../../Source/WTF/wtf/Assertions.cpp:321
#1  0x00007fd213331afc in WTF::Optional<WebCore::LayoutUnit>::value (this=0x7fff0288fcc0) at ../../Source/WTF/wtf/Optional.h:153
#2  0x00007fd21337675a in WebCore::RenderFlexibleBox::adjustChildSizeForMinAndMax (this=0x7fd1efbd0708, child=..., childSize=...) at ../../Source/WebCore/rendering/RenderFlexibleBox.cpp:878
#3  0x00007fd213376c8a in WebCore::RenderFlexibleBox::computeNextFlexLine (this=0x7fd1efbd0708, orderedChildren=..., preferredMainAxisExtent=..., totalFlexGrow=@0x7fff0288fe70: 0, totalWeightedFlexShrink=@0x7fff0288fe78: 0, minMaxAppliedMainAxisExtent=..., hasInfiniteLineLength=@0x7fff0288fe1f: true) at ../../Source/WebCore/rendering/RenderFlexibleBox.cpp:922
#4  0x00007fd21337578f in WebCore::RenderFlexibleBox::layoutFlexItems (this=0x7fd1efbd0708, relayoutChildren=false, lineContexts=...) at ../../Source/WebCore/rendering/RenderFlexibleBox.cpp:695
#5  0x00007fd21337385f in WebCore::RenderFlexibleBox::layoutBlock (this=0x7fd1efbd0708, relayoutChildren=false) at ../../Source/WebCore/rendering/RenderFlexibleBox.cpp:272
#6  0x00007fd2132b0558 in WebCore::RenderBlock::layout (this=0x7fd1efbd0708) at ../../Source/WebCore/rendering/RenderBlock.cpp:931
#7  0x00007fd213283af5 in WebCore::RenderElement::layoutIfNeeded (this=0x7fd1efbd0708) at ../../Source/WebCore/rendering/RenderElement.h:135
#8  0x00007fd21330371a in WebCore::RenderBlockFlow::layoutLineBoxes (this=0x7fd1efbd07d0, relayoutChildren=true, repaintLogicalTop=..., repaintLogicalBottom=...) at ../../Source/WebCore/rendering/RenderBlockLineLayout.cpp:1645
#9  0x00007fd2132dea59 in WebCore::RenderBlockFlow::layoutInlineChildren (this=0x7fd1efbd07d0, relayoutChildren=true, repaintLogicalTop=..., repaintLogicalBottom=...) at ../../Source/WebCore/rendering/RenderBlockFlow.cpp:651
#10 0x00007fd2132ddd9f in WebCore::RenderBlockFlow::layoutBlock (this=0x7fd1efbd07d0, relayoutChildren=true, pageLogicalHeight=...) at ../../Source/WebCore/rendering/RenderBlockFlow.cpp:483
#11 0x00007fd21348141a in WebCore::RenderTableCell::layout (this=0x7fd1efbd07d0) at ../../Source/WebCore/rendering/RenderTableCell.cpp:266
#12 0x00007fd21348ab2e in WebCore::RenderTableRow::layout (this=0x7fd1efba7da8) at ../../Source/WebCore/rendering/RenderTableRow.cpp:177
#13 0x00007fd213283af5 in WebCore::RenderElement::layoutIfNeeded (this=0x7fd1efba7da8) at ../../Source/WebCore/rendering/RenderElement.h:135
#14 0x00007fd21348d1b3 in WebCore::RenderTableSection::layout (this=0x7fd1efada5c8) at ../../Source/WebCore/rendering/RenderTableSection.cpp:409
#15 0x00007fd213283af5 in WebCore::RenderElement::layoutIfNeeded (this=0x7fd1efada5c8) at ../../Source/WebCore/rendering/RenderElement.h:135
#16 0x00007fd2134735fe in WebCore::RenderTable::layout (this=0x7fd1efaa6000) at ../../Source/WebCore/rendering/RenderTable.cpp:466
#17 0x00007fd213283af5 in WebCore::RenderElement::layoutIfNeeded (this=0x7fd1efaa6000) at ../../Source/WebCore/rendering/RenderElement.h:135
#18 0x00007fd213374477 in WebCore::RenderFlexibleBox::computeMainAxisExtentForChild (this=0x7fd1efbd0640, child=..., sizeType=WebCore::MinSize, size=...) at ../../Source/WebCore/rendering/RenderFlexibleBox.cpp:457
#19 0x00007fd21337660d in WebCore::RenderFlexibleBox::adjustChildSizeForMinAndMax (this=0x7fd1efbd0640, child=..., childSize=...) at ../../Source/WebCore/rendering/RenderFlexibleBox.cpp:872
#20 0x00007fd213376c8a in WebCore::RenderFlexibleBox::computeNextFlexLine (this=0x7fd1efbd0640, orderedChildren=..., preferredMainAxisExtent=..., totalFlexGrow=@0x7fff02890ca0: 0, totalWeightedFlexShrink=@0x7fff02890ca8: 0, minMaxAppliedMainAxisExtent=..., hasInfiniteLineLength=@0x7fff02890c4f: false) at ../../Source/WebCore/rendering/RenderFlexibleBox.cpp:922
#21 0x00007fd21337578f in WebCore::RenderFlexibleBox::layoutFlexItems (this=0x7fd1efbd0640, relayoutChildren=true, lineContexts=...) at ../../Source/WebCore/rendering/RenderFlexibleBox.cpp:695
#22 0x00007fd21337385f in WebCore::RenderFlexibleBox::layoutBlock (this=0x7fd1efbd0640, relayoutChildren=true) at ../../Source/WebCore/rendering/RenderFlexibleBox.cpp:272
#23 0x00007fd2132b0558 in WebCore::RenderBlock::layout (this=0x7fd1efbd0640) at ../../Source/WebCore/rendering/RenderBlock.cpp:931
#24 0x00007fd213283af5 in WebCore::RenderElement::layoutIfNeeded (this=0x7fd1efbd0640) at ../../Source/WebCore/rendering/RenderElement.h:135
#25 0x00007fd213374477 in WebCore::RenderFlexibleBox::computeMainAxisExtentForChild (this=0x7fd1efbd0190, child=..., sizeType=WebCore::MinSize, size=...) at ../../Source/WebCore/rendering/RenderFlexibleBox.cpp:457
#26 0x00007fd21337660d in WebCore::RenderFlexibleBox::adjustChildSizeForMinAndMax (this=0x7fd1efbd0190, child=..., childSize=...) at ../../Source/WebCore/rendering/RenderFlexibleBox.cpp:872
#27 0x00007fd213376c8a in WebCore::RenderFlexibleBox::computeNextFlexLine (this=0x7fd1efbd0190, orderedChildren=..., preferredMainAxisExtent=..., totalFlexGrow=@0x7fff02891240: 0, totalWeightedFlexShrink=@0x7fff02891248: 0, minMaxAppliedMainAxisExtent=..., hasInfiniteLineLength=@0x7fff028911ef: false) at ../../Source/WebCore/rendering/RenderFlexibleBox.cpp:922
#28 0x00007fd21337578f in WebCore::RenderFlexibleBox::layoutFlexItems (this=0x7fd1efbd0190, relayoutChildren=true, lineContexts=...) at ../../Source/WebCore/rendering/RenderFlexibleBox.cpp:695
#29 0x00007fd21337385f in WebCore::RenderFlexibleBox::layoutBlock (this=0x7fd1efbd0190, relayoutChildren=true) at ../../Source/WebCore/rendering/RenderFlexibleBox.cpp:272
#30 0x00007fd2132b0558 in WebCore::RenderBlock::layout (this=0x7fd1efbd0190) at ../../Source/WebCore/rendering/RenderBlock.cpp:931
#31 0x00007fd2132dee20 in WebCore::RenderBlockFlow::layoutBlockChild (this=0x7fd1efadd228, child=..., marginInfo=..., previousFloatLogicalBottom=..., maxFloatLogicalBottom=...) at ../../Source/WebCore/rendering/RenderBlockFlow.cpp:709
#32 0x00007fd2132de95f in WebCore::RenderBlockFlow::layoutBlockChildren (this=0x7fd1efadd228, relayoutChildren=true, maxFloatLogicalBottom=...) at ../../Source/WebCore/rendering/RenderBlockFlow.cpp:632
#33 0x00007fd2132dddc3 in WebCore::RenderBlockFlow::layoutBlock (this=0x7fd1efadd228, relayoutChildren=true, pageLogicalHeight=...) at ../../Source/WebCore/rendering/RenderBlockFlow.cpp:485
#34 0x00007fd2132b0558 in WebCore::RenderBlock::layout (this=0x7fd1efadd228) at ../../Source/WebCore/rendering/RenderBlock.cpp:931
#35 0x00007fd2134c2fa7 in WebCore::RenderView::layoutContent (this=0x7fd1efadd228, state=...) at ../../Source/WebCore/rendering/RenderView.cpp:253
#36 0x00007fd2134c369a in WebCore::RenderView::layout (this=0x7fd1efadd228) at ../../Source/WebCore/rendering/RenderView.cpp:378
#37 0x00007fd2130957f6 in WebCore::FrameView::layout (this=0x7fd1ef825540, allowSubtree=true) at ../../Source/WebCore/page/FrameView.cpp:1408
#38 0x00007fd212af2f3c in WebCore::Document::implicitClose (this=0x7fd1ef826a40) at ../../Source/WebCore/dom/Document.cpp:2704
#39 0x00007fd212f58827 in WebCore::FrameLoader::checkCallImplicitClose (this=0x7fd1efae4098) at ../../Source/WebCore/loader/FrameLoader.cpp:889
#40 0x00007fd212f5855e in WebCore::FrameLoader::checkCompleted (this=0x7fd1efae4098) at ../../Source/WebCore/loader/FrameLoader.cpp:835
#41 0x00007fd212f582ce in WebCore::FrameLoader::finishedParsing (this=0x7fd1efae4098) at ../../Source/WebCore/loader/FrameLoader.cpp:756
#42 0x00007fd212afcf6c in WebCore::Document::finishedParsing (this=0x7fd1ef826a40) at ../../Source/WebCore/dom/Document.cpp:4897
#43 0x00007fd213e68349 in WebCore::HTMLConstructionSite::finishedParsing (this=0x7fd1efafe6e0) at ../../Source/WebCore/html/parser/HTMLConstructionSite.cpp:403
#44 0x00007fd212e51064 in WebCore::HTMLTreeBuilder::finished (this=0x7fd1efafe6c0) at ../../Source/WebCore/html/parser/HTMLTreeBuilder.cpp:2937
#45 0x00007fd212e21708 in WebCore::HTMLDocumentParser::end (this=0x7fd1ef82e840) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:393
#46 0x00007fd212e217e1 in WebCore::HTMLDocumentParser::attemptToRunDeferredScriptsAndEnd (this=0x7fd1ef82e840) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:402
#47 0x00007fd212e204c3 in WebCore::HTMLDocumentParser::prepareToStopParsing (this=0x7fd1ef82e840) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:132
#48 0x00007fd212e21824 in WebCore::HTMLDocumentParser::attemptToEnd (this=0x7fd1ef82e840) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:414
#49 0x00007fd212e218db in WebCore::HTMLDocumentParser::finish (this=0x7fd1ef82e840) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:442
#50 0x00007fd212f43702 in WebCore::DocumentWriter::end (this=0x7fd1ef8249e0) at ../../Source/WebCore/loader/DocumentWriter.cpp:247
#51 0x00007fd212f2cfd8 in WebCore::DocumentLoader::finishedLoading (this=0x7fd1ef824940, finishTime=0) at ../../Source/WebCore/loader/DocumentLoader.cpp:437
#52 0x00007fd212f2cd36 in WebCore::DocumentLoader::notifyFinished (this=0x7fd1ef824940, resource=0x7fd1ef836000) at ../../Source/WebCore/loader/DocumentLoader.cpp:384
#53 0x00007fd212fd8143 in WebCore::CachedResource::checkNotify (this=0x7fd1ef836000) at ../../Source/WebCore/loader/cache/CachedResource.cpp:297
#54 0x00007fd212fd8252 in WebCore::CachedResource::finishLoading (this=0x7fd1ef836000) at ../../Source/WebCore/loader/cache/CachedResource.cpp:313
#55 0x00007fd212fd4446 in WebCore::CachedRawResource::finishLoading (this=0x7fd1ef836000, data=0x7fd1efbbb680) at ../../Source/WebCore/loader/cache/CachedRawResource.cpp:103
#56 0x00007fd212f9ce74 in WebCore::SubresourceLoader::didFinishLoading (this=0x7fd1ef82fa80, finishTime=0) at ../../Source/WebCore/loader/SubresourceLoader.cpp:372
#57 0x00007fd212f97953 in WebCore::ResourceLoader::didFinishLoading (this=0x7fd1ef82fa80, finishTime=0) at ../../Source/WebCore/loader/ResourceLoader.cpp:638
#58 0x00007fd21378b19a in WebCore::readCallback (asyncResult=0x255d1d0, data=0x7fd1efbbc680) at ../../Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:1341
#59 0x00007fd208fac5a6 in async_ready_callback_wrapper (source_object=0x24a49b0, res=0x255d1d0, user_data=0x7fd1efbbc680) at ginputstream.c:523
#60 0x00007fd208fd2b74 in g_task_return_now (task=0x255d1d0) at gtask.c:1077
#61 0x00007fd208fd2b99 in complete_in_idle_cb (task=0x255d1d0) at gtask.c:1086
#62 0x00007fd208a0aadd in g_main_dispatch (context=0x249eb50) at gmain.c:3064
#63 g_main_context_dispatch (context=context@entry=0x249eb50) at gmain.c:3663
#64 0x00007fd20a376e58 in _ecore_glib_select__locked (ecore_timeout=<optimized out>, efds=0x7fff028923b0, wfds=0x7fff02892330, rfds=0x7fff028922b0, ecore_fds=<optimized out>, ctx=<optimized out>) at lib/ecore/ecore_glib.c:172
#65 _ecore_glib_select (ecore_fds=<optimized out>, rfds=0x7fff028922b0, wfds=0x7fff02892330, efds=0x7fff028923b0, ecore_timeout=<optimized out>) at lib/ecore/ecore_glib.c:204
#66 0x00007fd20a37a4a4 in _ecore_main_select (timeout=9.532824124368238e-130) at lib/ecore/ecore_main.c:1459
#67 0x00007fd20a37aed4 in _ecore_main_loop_iterate_internal (once_only=once_only@entry=0) at lib/ecore/ecore_main.c:1893
#68 0x00007fd20a37afc7 in ecore_main_loop_begin () at lib/ecore/ecore_main.c:983
#69 0x00007fd20ca17795 in WTF::RunLoop::run () at ../../Source/WTF/wtf/efl/RunLoopEfl.cpp:49
#70 0x00007fd21287787d in WebKit::ChildProcessMain<WebKit::WebProcess, WebKit::WebProcessMain> (argc=2, argv=0x7fff028927e8) at ../../Source/WebKit2/Shared/unix/ChildProcessMain.h:61
#71 0x00007fd21287748b in WebKit::WebProcessMainUnix (argc=2, argv=0x7fff028927e8) at ../../Source/WebKit2/WebProcess/efl/WebProcessMainEfl.cpp:161
#72 0x00000000004008ea in main (argc=2, argv=0x7fff028927e8) at ../../Source/WebKit2/WebProcess/EntryPoint/unix/WebProcessMain.cpp:44
Comment 1 Sergio Villar Senin 2015-11-11 07:22:16 PST
Unable to reproduce it with ToT. Could you retry?
Comment 2 Sergio Villar Senin 2015-11-11 07:22:48 PST
(In reply to comment #1)
> Unable to reproduce it with ToT. Could you retry?

Sorry forget about this.
Comment 3 Sergio Villar Senin 2015-11-12 07:32:28 PST
Created attachment 265390 [details]
Patch
Comment 4 Darin Adler 2015-11-12 09:40:53 PST
Comment on attachment 265390 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=265390&action=review

> Source/WebCore/rendering/RenderBox.cpp:2919
> +    if (scrollsOverflowY && (!cell.style().logicalHeight().isAuto() || !cell.table()->style().logicalHeight().isAuto()))
> +        return true;
> +    return false;

Stylistically it seems we should just use return here rather than if (x) return true; return false;

> Source/WebCore/rendering/RenderBox.cpp:2960
> +                return tableCellShouldHaveZeroInitialSize(*cb, scrollsOverflowY()) ? Optional<LayoutUnit>() : Nullopt;

I find it really hard to understand Optional<LayoutUnit>() vs. Nullopt. I guess the first is a way of writing zero, but it really looks like null. It might be worth defining a local constant just to avoid that confusion.

> Source/WebCore/rendering/RenderBox.cpp:4669
> +        if (!skippedAutoHeightContainingBlock && !cb->hasOverrideLogicalContentHeight() && !tableCellShouldHaveZeroInitialSize(*cb, scrollsOverflowY))
> +            return false;
>          return true;

I think writing this as a return rather than if (x) return false; return true; would be better, do you agree?
Comment 5 Sergio Villar Senin 2015-11-13 01:16:21 PST
(In reply to comment #4)
> Comment on attachment 265390 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=265390&action=review
> 
> > Source/WebCore/rendering/RenderBox.cpp:2919
> > +    if (scrollsOverflowY && (!cell.style().logicalHeight().isAuto() || !cell.table()->style().logicalHeight().isAuto()))
> > +        return true;
> > +    return false;
> 
> Stylistically it seems we should just use return here rather than if (x)
> return true; return false;

Sure.

> 
> > Source/WebCore/rendering/RenderBox.cpp:2960
> > +                return tableCellShouldHaveZeroInitialSize(*cb, scrollsOverflowY()) ? Optional<LayoutUnit>() : Nullopt;
> 
> I find it really hard to understand Optional<LayoutUnit>() vs. Nullopt. I
> guess the first is a way of writing zero, but it really looks like null. It
> might be worth defining a local constant just to avoid that confusion.

It's just because LayoutUnit() is slightly more efficient than LayoutUnit(0).

> > Source/WebCore/rendering/RenderBox.cpp:4669
> > +        if (!skippedAutoHeightContainingBlock && !cb->hasOverrideLogicalContentHeight() && !tableCellShouldHaveZeroInitialSize(*cb, scrollsOverflowY))
> > +            return false;
> >          return true;
> 
> I think writing this as a return rather than if (x) return false; return
> true; would be better, do you agree?

Sure.
Comment 6 Sergio Villar Senin 2015-11-13 02:15:40 PST
Created attachment 265475 [details]
Patch for landing
Comment 7 WebKit Commit Bot 2015-11-13 03:13:43 PST
Comment on attachment 265475 [details]
Patch for landing

Clearing flags on attachment: 265475

Committed r192413: <http://trac.webkit.org/changeset/192413>
Comment 8 WebKit Commit Bot 2015-11-13 03:13:47 PST
All reviewed patches have been landed.  Closing bug.
Comment 9 Darin Adler 2015-11-13 09:30:50 PST
Comment on attachment 265475 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=265475&action=review

> Source/WebCore/rendering/RenderBox.cpp:2958
> +                return tableCellShouldHaveZeroInitialSize(*cb, scrollsOverflowY()) ? Optional<LayoutUnit>() : Nullopt;

If we want to keep the greater efficiency of LayoutUnit(), then I suggest we add constants somewhere in a header:

    zeroLayoutUnit
    zeroOptionalLayoutUnit

I think those will be easier to recognize as zero values than LayoutUnit() and Optional<LayoutUnit>() will be.
Comment 10 Alexey Proskuryakov 2016-11-03 16:44:08 PDT
This caused bug 164366.