Heap-buffer-overflow in void std::__final_insertion_sort.

Heap-buffer-overflow in void std::__final_insertion_sort.

It was getting crash while sorting rowspan cells. std::sort calls comparator
function 2 times with same value in reverse order when first call return
true for verification of result from second call which should be compulsory
false. But it was not there in our comparator function.

There was a mistake in comparator function. Cells should be sorted based
on row indexes only when one cell is not a subpart of another cell.

R=jchaffraix@chromium.org
BUG=296003
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=158347

[a.suchit, 2013-09-24 13:01:27]

With this fix, below exception should get fixed.

crbug.com/285703 [ Debug ]
fast/table/table-rowspan-crash-with-huge-rowspan-cells.html [ Crash Pass ]

But I did not updated yet file 'LayoutTests/TestExpectations' with it.

[Julien - ping for review, 2013-09-24 21:18:09]

> But I did not updated yet file 'LayoutTests/TestExpectations' with it.

Why don't we update the entry?

https://codereview.chromium.org/23874023/diff/1/LayoutTests/fast/table/table-...
File LayoutTests/fast/table/table-rowspan-crash-with-huge-rowspan-cells-2.html
(right):

https://codereview.chromium.org/23874023/diff/1/LayoutTests/fast/table/table-...
LayoutTests/fast/table/table-rowspan-crash-with-huge-rowspan-cells-2.html:3:
<script type="text/javascript"> if (window.testRunner) testRunner.dumpAsText();
</script>
Let's indent that like our code and remove the unneeded 'type' attribute:

<script>
if (window.testRunner)
    testRunner.dumpAsText();
</script>

https://codereview.chromium.org/23874023/diff/1/LayoutTests/fast/table/table-...
LayoutTests/fast/table/table-rowspan-crash-with-huge-rowspan-cells-2.html:6:
<h4>It should not crash.</h4>
For this test to PASS, it should not crash. (without the first half, it's not
totally obvious what is expected)

https://codereview.chromium.org/23874023/diff/1/LayoutTests/fast/table/table-...
LayoutTests/fast/table/table-rowspan-crash-with-huge-rowspan-cells-2.html:8: <td
rowspan=3>><td rowspan=3>><td rowspan=6>><td rowspan=4</td>
Could we change the content '>' into a more explicit content like row1column1
(rowspan=3), ...

Also let's indent this properly, and properly close the tags so that we don't
rely on the closing behavior from the parser.

[a.suchit, 2013-09-25 06:12:48]

I removed the below entry from Layout/TestExpectations. After this fix, below
test case should not crash.

crbug.com/285703 [ Debug ]
fast/table/table-rowspan-crash-with-huge-rowspan-cells.html [ Crash Pass ]

I have tried it locally many times but this test is not crashing any more.

https://codereview.chromium.org/23874023/diff/1/LayoutTests/fast/table/table-...
File LayoutTests/fast/table/table-rowspan-crash-with-huge-rowspan-cells-2.html
(right):

https://codereview.chromium.org/23874023/diff/1/LayoutTests/fast/table/table-...
LayoutTests/fast/table/table-rowspan-crash-with-huge-rowspan-cells-2.html:3:
<script type="text/javascript"> if (window.testRunner) testRunner.dumpAsText();
</script>
On 2013/09/24 21:18:09, Julien Chaffraix wrote:
> Let's indent that like our code and remove the unneeded 'type' attribute:
> 
> <script>
> if (window.testRunner)
>     testRunner.dumpAsText();
> </script>

Done.

https://codereview.chromium.org/23874023/diff/1/LayoutTests/fast/table/table-...
LayoutTests/fast/table/table-rowspan-crash-with-huge-rowspan-cells-2.html:6:
<h4>It should not crash.</h4>
On 2013/09/24 21:18:09, Julien Chaffraix wrote:
> For this test to PASS, it should not crash. (without the first half, it's not
> totally obvious what is expected)

Done.

https://codereview.chromium.org/23874023/diff/1/LayoutTests/fast/table/table-...
LayoutTests/fast/table/table-rowspan-crash-with-huge-rowspan-cells-2.html:8: <td
rowspan=3>><td rowspan=3>><td rowspan=6>><td rowspan=4</td>
On 2013/09/24 21:18:09, Julien Chaffraix wrote:
> Could we change the content '>' into a more explicit content like row1column1
> (rowspan=3), ...
> 
> Also let's indent this properly, and properly close the tags so that we don't
> rely on the closing behavior from the parser.

Done.

[Julien - ping for review, 2013-09-25 20:52:33]

lgtm

[commit-bot: I haz the power, 2013-09-25 20:52:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/a.suchit@samsung.com/23874023/13001

[commit-bot: I haz the power, 2013-09-26 01:21:11]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_layo...

[commit-bot: I haz the power, 2013-09-26 01:25:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/a.suchit@samsung.com/23874023/13001

[commit-bot: I haz the power, 2013-09-26 01:53:02]

Change committed as 158347