Crash when inner and outer spanning cells end indexes are same.

Crash when inner and outer spanning cells end indexes are same.

Position of the cells below spanning cells is not correct So Assert is failing.

When inner and outer spanning cells end indexes are same than lower rows
positions was updating based on changed height by outer spanning cells
but total changed height should be changed height by (inner spanning cell
 + outer spanning cell).

R=jchaffraix@chromium.org

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=155581

[suchit.agrawal, 2013-08-02 16:06:43]

In the last patch, one test case was not covered. When inner and outer spanning
cells end indexes are same than lower rows positions was updating based on
changed height by outer spanning cells but total changed height should be
changed height by (inner spanning cell + outer spanning cell).

[Julien - ping for review, 2013-08-02 23:45:04]

Please fix the description (1 line descriptions should be for trivial changes
and this change is not one).

[Julien - ping for review, 2013-08-05 22:21:10]

lgtm

https://codereview.chromium.org/21909002/diff/9001/Source/core/rendering/Rend...
File Source/core/rendering/RenderTableSection.cpp (right):

https://codereview.chromium.org/21909002/diff/9001/Source/core/rendering/Rend...
Source/core/rendering/RenderTableSection.cpp:458: m_rowPos[rowIndex + rowSpan]
-= extraHeightToPropagate;
This line seems dumb: can't we just patch the distribute functions to not touch
the |rowIndex + rowSpan|'s row instead?

[suchit.agrawal, 2013-08-05 23:33:55]

https://codereview.chromium.org/21909002/diff/9001/Source/core/rendering/Rend...
File Source/core/rendering/RenderTableSection.cpp (right):

https://codereview.chromium.org/21909002/diff/9001/Source/core/rendering/Rend...
Source/core/rendering/RenderTableSection.cpp:458: m_rowPos[rowIndex + rowSpan]
-= extraHeightToPropagate;
On 2013/08/05 22:21:10, Julien Chaffraix wrote:
> This line seems dumb: can't we just patch the distribute functions to not
touch
> the |rowIndex + rowSpan|'s row instead?

|extraHeightToPropagate| is calculated based on the position changed of
|rowIndex + rowSpan|.
This is last row of the spanning cell. we does not know how much extra height
would be applicable for this row which would be calculated by only distributed
functions only.
And meaning of distributed functions would be changes if we escape one row in
all row to distribute height. Some ASSERTs also used there which will not allow
it.
Even here, we don't know that which distributed function given the new height to
this row.

Or we can do following :

int originalBeforePosition = m_rowPos[rowIndex + rowSpan]; above can be changed
based on condition 
if (rowIndex + rowSpan == lastRowIndex + lastRowSpan)
originalBeforePosition = m_rowPos[rowIndex + rowSpan] - extraHeightToPropagate;
else
originalBeforePosition = m_rowPos[rowIndex + rowSpan];

But this will not work in below case :
where A rowspan value is 3 or more and B rowspan value is 2.

+---+---+
|   |   |
|   +---+---+
| A |   |   |
|   | B +---+
|   |   |   |
+---+---+---+

So I think, we need to stick on it. If you have any suggestions please let me
know. I will try my best to change it. Thanks

[commit-bot: I haz the power, 2013-08-06 04:12:04]

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

[commit-bot: I haz the power, 2013-08-06 04:12:08]

Can't process patch for file
LayoutTests/platform/linux/tables/mozilla/core/bloomberg-expected.png.
Binary file support is temporarilly disabled due to a bug. Please commit blindly
the binary files first then commit the source change as a separate CL. Sorry for
the annoyance.

[commit-bot: I haz the power, 2013-08-06 04:58:36]

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

[commit-bot: I haz the power, 2013-08-06 06:22:42]

Change committed as 155581