Fix for ASSERTION FAILURE contentSize >=0

Fix for ASSERTION FAILURE contentSize >=0

The codeflow here is as follows:
adjustChildSizeForMinAndMax() -> computeMainAxisExtentForChild() ->
computeLogicalWidthUsing() -> computeIntrinsicLogicalWidthUsing() -> 
computeIntrinsicLogicalWidths()

In the final function we set the minPreferredLogicalWidth as
minPreferredLogicalWidth = child->minPreferredLogicalWidth()

Here, minPreferredLogicalWidth() returns a negative value, so
minPreferredLogicalWidth is negative and we add this negative number to
minLogicalWidth which does not make sense and makes minLogicalWidth negative.
Hence, adding a check here not to add negative values (as they do not make
sense). This fixes the assertion failure.

BUG=492678

[Sunil Ratnu, 2015-05-27 16:05:53]

sunil.ratnu@samsung.com changed reviewers:
+ cbiesinger@chromium.org, ojan@chromium.org, tony@chromium.org

[Sunil Ratnu, 2015-05-27 16:05:54]

Hi,

Please take a look.

Thanks,
Sunil

https://codereview.chromium.org/1154113004/diff/20001/LayoutTests/css3/flexbo...
File LayoutTests/css3/flexbox/content-size-crash-expected.txt (right):

https://codereview.chromium.org/1154113004/diff/20001/LayoutTests/css3/flexbo...
LayoutTests/css3/flexbox/content-size-crash-expected.txt:18: description('This
test passes if it does not crash.');
Ideally these lines should not have been here. But whenever we load the test
case html, then the code present between the <style> tags is also rendered on
the screen i.e. the whole code is dumped onto the screen. Should we mark that as
a separate bug? And until the style part is being rendered onto the screen, we
can not keep this testcase free from the above code. Any suggestions?

[cbiesinger, 2015-05-27 16:50:20]

not lgtm

https://codereview.chromium.org/1154113004/diff/20001/LayoutTests/css3/flexbo...
File LayoutTests/css3/flexbox/content-size-crash-expected.txt (right):

https://codereview.chromium.org/1154113004/diff/20001/LayoutTests/css3/flexbo...
LayoutTests/css3/flexbox/content-size-crash-expected.txt:18: description('This
test passes if it does not crash.');
On 2015/05/27 16:05:54, Sunil Ratnu wrote:
> Ideally these lines should not have been here. But whenever we load the test
> case html, then the code present between the <style> tags is also rendered on
> the screen i.e. the whole code is dumped onto the screen. Should we mark that
as
> a separate bug? And until the style part is being rendered onto the screen, we
> can not keep this testcase free from the above code. Any suggestions?

That text is only there because of * { display: flex; }. Maybe you can limit
that more?

https://codereview.chromium.org/1154113004/diff/20001/Source/core/layout/Layo...
File Source/core/layout/LayoutFlexibleBox.cpp (right):

https://codereview.chromium.org/1154113004/diff/20001/Source/core/layout/Layo...
Source/core/layout/LayoutFlexibleBox.cpp:120: // We add minPreferredLogicalWidth
only if is positive. Adding negative widths does not make sense.
minPreferredLogicalWidth should not be negative. Why is it?

I feel like I saw another CL that tried to fix something like that... Ah: That
was https://codereview.chromium.org/343173003/, which is now committed. It seems
that a similar change needs to be applied to whichever LayoutObject returns a
negative number here.

[cbiesinger, 2015-05-27 21:22:53]

Turns out the immediate cause is this:
(gdb) p child->borderAndPaddingLogicalWidth()
$2 = -62.5px

So LayoutBlock::computePreferredLogicalWidths will turn this negative. However,
this value should not be negative either. Apparently the 84% padding gets
computed to a negative value:
(gdb) p child->paddingRight()
$7 = -62.5px

due to:
(gdb) p child->containingBlockLogicalWidthForContent()
$8 = -74.4219px

Gotta stop my investigation here, but hopefully that helps!