Paint the whole of a table-part background image behind the cells in a table

Paint the whole of a table-part background image behind the cells in a table

Fix the painting of background images on table parts so that the image is displayed
across the whole of the part. This fixes each of table-row-group, table-row,
table-column and table-column-group.

The fix is pretty simple in principle: just adjust the geometry we use to paint the image 
to reflect the cell's position relative to the table part from which the image originates.

Previously, we were just painting the image as though it was originating in each
cell. The one case left over is backgr_fixed-bg.html: this is still wrong and
I need to address it in a follow-up.

BUG=35697
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2600903002
Cr-Commit-Position: refs/heads/master@{#445283}
Committed: https://chromium.googlesource.com/chromium/src/+/949b36db0565e9a81fc50cbb1584d64d6090c147

[rhogan, 2016-12-26 21:01:13]

Description was changed from

==========
Paint the whole of an image behind the cells in a table

BUG=35697
==========

to

==========
Paint the whole of an image behind the cells in a table

BUG=35697
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[rhogan, 2016-12-26 21:01:20]

The CQ bit was checked by robhogan@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2016-12-26 21:01:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-26 22:38:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-26 22:38:26]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[rhogan, 2016-12-29 23:34:16]

The CQ bit was checked by robhogan@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2016-12-29 23:34:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-30 00:14:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-30 00:14:14]

Dry run: Try jobs failed on following builders:
  linux_layout_tests_slimming_paint_v2 on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_layout_te...)

[rhogan, 2016-12-31 12:03:55]

The CQ bit was checked by robhogan@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2016-12-31 12:04:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-31 13:16:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-31 13:16:04]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[rhogan, 2017-01-04 20:48:33]

The CQ bit was checked by robhogan@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-04 20:48:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-04 22:15:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-04 22:15:18]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[rhogan, 2017-01-07 18:56:30]

The CQ bit was checked by robhogan@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-07 18:56:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-07 20:19:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-07 20:19:04]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[rhogan, 2017-01-09 20:54:22]

The CQ bit was checked by robhogan@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-09 20:54:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[rhogan, 2017-01-09 21:03:00]

Description was changed from

==========
Paint the whole of an image behind the cells in a table

BUG=35697
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Paint the whole of a table-part background image behind the cells in a table

Fix the painting of background images on table parts so that the image is
displayed
across the whole of the part. This fixes each of table-row-group, table-row,
table-column and table-column-group.

Previously, we were just painting the image as though it was originating in each
cell. The one case left over is backgr_fixed-bg.html: this is still wrong and
I need to address it in a follow-up.

BUG=35697
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[rhogan, 2017-01-09 21:03:01]

robhogan@gmail.com changed reviewers:
+ pdr@chromium.org

[rhogan, 2017-01-09 22:01:32]

robhogan@gmail.com changed reviewers:
+ wangxianzhu@chromium.org

[pdr., 2017-01-09 22:58:25]

Xianzhu, would you be up for reviewing this one? You did quite a bit of work on
tables with atotic and might understand this better.

[commit-bot: I haz the power, 2017-01-09 23:04:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-09 23:04:15]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[rhogan, 2017-01-09 23:06:47]

Description was changed from

==========
Paint the whole of a table-part background image behind the cells in a table

Fix the painting of background images on table parts so that the image is
displayed
across the whole of the part. This fixes each of table-row-group, table-row,
table-column and table-column-group.

Previously, we were just painting the image as though it was originating in each
cell. The one case left over is backgr_fixed-bg.html: this is still wrong and
I need to address it in a follow-up.

BUG=35697
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Paint the whole of a table-part background image behind the cells in a table

Fix the painting of background images on table parts so that the image is
displayed
across the whole of the part. This fixes each of table-row-group, table-row,
table-column and table-column-group.

The fix is pretty simple in principle: just 
adjust the geometry we use to paint the image to
reflect the cell's position relative to the table 
part from which the image originates.

Previously, we were just painting the image as though it was originating in each
cell. The one case left over is backgr_fixed-bg.html: this is still wrong and
I need to address it in a follow-up.

BUG=35697
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[Xianzhu, 2017-01-09 23:14:25]

On 2017/01/09 22:58:25, pdr. wrote:
> Xianzhu, would you be up for reviewing this one? You did quite a bit of work
on
> tables with atotic and might understand this better.

Sure.

[Xianzhu, 2017-01-09 23:18:52]

I think this CL is in the correct direction.

Before reviewing the code, I have some comments about the layout tests:
- I think the tests under mozilla or mozilla_expected_failures were imported so
we'd better not change them.
- Please add all new tests under fast/table, and give "*-2-*.html" more
meaningful names.

[rhogan, 2017-01-09 23:26:08]

On 2017/01/09 at 23:18:52, wangxianzhu wrote:
> I think this CL is in the correct direction.
> 
> Before reviewing the code, I have some comments about the layout tests:
> - I think the tests under mozilla or mozilla_expected_failures were imported
so we'd better not change them.
> - Please add all new tests under fast/table, and give "*-2-*.html" more
meaningful names.

Sure. Do you see what I'm doing with the tests though? Half of the test is
obscured from the viewport so I need to split them in order to make the hidden
half in each visible and test its content.

OK if I remove them from the Mozilla folder and rewrite or adapt them in
fast/table? Would that create copyright issues?

I also want to avoid making them retests. That would be a painful amount of
work.

[Xianzhu, 2017-01-09 23:35:37]

On 2017/01/09 23:26:08, rhogan wrote:
> On 2017/01/09 at 23:18:52, wangxianzhu wrote:
> > I think this CL is in the correct direction.
> > 
> > Before reviewing the code, I have some comments about the layout tests:
> > - I think the tests under mozilla or mozilla_expected_failures were imported
> so we'd better not change them.
> > - Please add all new tests under fast/table, and give "*-2-*.html" more
> meaningful names.
> 
> Sure. Do you see what I'm doing with the tests though? Half of the test is
> obscured from the viewport so I need to split them in order to make the hidden
> half in each visible and test its content.

SGTM.

> 
> OK if I remove them from the Mozilla folder and rewrite or adapt them in
> fast/table? Would that create copyright issues?
> 

I think this is OK.

> I also want to avoid making them retests. That would be a painful amount of
> work.

SGTM.

[rhogan, 2017-01-10 20:59:00]

The CQ bit was checked by robhogan@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-10 20:59:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[rhogan, 2017-01-10 20:59:58]

Description was changed from

==========
Paint the whole of a table-part background image behind the cells in a table

Fix the painting of background images on table parts so that the image is
displayed
across the whole of the part. This fixes each of table-row-group, table-row,
table-column and table-column-group.

The fix is pretty simple in principle: just 
adjust the geometry we use to paint the image to
reflect the cell's position relative to the table 
part from which the image originates.

Previously, we were just painting the image as though it was originating in each
cell. The one case left over is backgr_fixed-bg.html: this is still wrong and
I need to address it in a follow-up.

BUG=35697
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Paint the whole of a table-part background image behind the cells in a table

Fix the painting of background images on table parts so that the image is
displayed
across the whole of the part. This fixes each of table-row-group, table-row,
table-column and table-column-group.

The fix is pretty simple in principle: just adjust the geometry we use to paint
the image to reflect the cell's position relative to the table 
part from which the image originates.

Previously, we were just painting the image as though it was originating in each
cell. The one case left over is backgr_fixed-bg.html: this is still wrong and
I need to address it in a follow-up.

BUG=35697
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[rhogan, 2017-01-10 21:00:16]

Description was changed from

==========
Paint the whole of a table-part background image behind the cells in a table

Fix the painting of background images on table parts so that the image is
displayed
across the whole of the part. This fixes each of table-row-group, table-row,
table-column and table-column-group.

The fix is pretty simple in principle: just adjust the geometry we use to paint
the image to reflect the cell's position relative to the table 
part from which the image originates.

Previously, we were just painting the image as though it was originating in each
cell. The one case left over is backgr_fixed-bg.html: this is still wrong and
I need to address it in a follow-up.

BUG=35697
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Paint the whole of a table-part background image behind the cells in a table

Fix the painting of background images on table parts so that the image is
displayed
across the whole of the part. This fixes each of table-row-group, table-row,
table-column and table-column-group.

The fix is pretty simple in principle: just adjust the geometry we use to paint
the image 
to reflect the cell's position relative to the table part from which the image
originates.

Previously, we were just painting the image as though it was originating in each
cell. The one case left over is backgr_fixed-bg.html: this is still wrong and
I need to address it in a follow-up.

BUG=35697
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[commit-bot: I haz the power, 2017-01-10 22:29:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-10 22:29:50]

Dry run: Try jobs failed on following builders:
  linux_layout_tests_slimming_paint_v2 on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_layout_te...)

[rhogan, 2017-01-15 14:20:51]

> > I also want to avoid making them retests. That would be a painful amount of
> > work.
> 
> SGTM.

This is ready for another look!

[Xianzhu, 2017-01-15 20:45:58]

This is a nice fix to the old bug. Thanks!

Only some minor comments.

https://codereview.chromium.org/2600903002/diff/120001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/fast/table/backgr_border-table-2.html
(right):

https://codereview.chromium.org/2600903002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/table/backgr_border-table-2.html:1:
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN">
Do the "*-2.html" increase test coverage? If not I would suggest to omit them.

https://codereview.chromium.org/2600903002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/BackgroundImageGeometry.cpp (right):

https://codereview.chromium.org/2600903002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/BackgroundImageGeometry.cpp:326: static
void expandToTableColumnGroup(const LayoutTableCell* cell,
s/*/&/

https://codereview.chromium.org/2600903002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/BackgroundImageGeometry.cpp:332: :
&LayoutTableCell::nextCell;
Wdyt using the following:

  auto siblingCell = ... (replacing line 330)
  (the type of this siblingCell is method pointer)

then use
  for (const auto* sibling = (cell.*siblingCell)(); sibling; sibling =
(sibling->*siblingCell)())

This also avoids const_cast.

You could also still use std::function but should add 'const' to avoid
const_cast below.

https://codereview.chromium.org/2600903002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/BackgroundImageGeometry.cpp:356: 
DCHECK(positioningBox.isLayoutTableCol());

https://codereview.chromium.org/2600903002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/BackgroundImageGeometry.cpp:365: if
(positioningBox.isLayoutTableCol() &&
Remove 'positioningBox.isLayoutTableCol() &&'.

https://codereview.chromium.org/2600903002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/BackgroundImageGeometry.cpp:371:
DCHECK(positioningBox.isLayoutTableCol() &&
Remove 'positioningBox.isLayoutTableCol() &&'.

https://codereview.chromium.org/2600903002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/BackgroundImageGeometry.cpp:393: 
Ditto comments as in getOffsetForCell().

https://codereview.chromium.org/2600903002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/BackgroundImageGeometry.cpp:439: ?
static_cast<const LayoutBoxModelObject&>(*rootBox)
s/static_cast<const LayoutBoxModelObject&>(...)/toLayoutBoxModelObject(...)/

https://codereview.chromium.org/2600903002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/BackgroundImageGeometry.cpp:441: ?
static_cast<const LayoutBoxModelObject&>(*backgroundObject)
Ditto.

https://codereview.chromium.org/2600903002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/BackgroundImageGeometry.cpp:500: :
paintRect.size()) -
I doubt if the name getPositionAreaSizeForCell() is accurate, because for
cellUsingContainerBackground case, the whole statement is like:
  positioningAreaSize = getPositioningAreaSizeForCell() - LayoutSize(left +
right, top + bottom);

Perhaps making line 497-500 a separate statement helps to make clear what it
means.

https://codereview.chromium.org/2600903002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/BackgroundImageGeometry.h (right):

https://codereview.chromium.org/2600903002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/BackgroundImageGeometry.h:30: const
LayoutObject*,
Please give the parameter a name because the method is public and the
parameter's meaning is not clear just based on its type.

[rhogan, 2017-01-16 20:06:28]

On 2017/01/15 at 20:45:58, wangxianzhu wrote:
> This is a nice fix to the old bug. Thanks!
> 
> Only some minor comments.
Did everything except the following:
>
https://codereview.chromium.org/2600903002/diff/120001/third_party/WebKit/Lay...
> third_party/WebKit/LayoutTests/fast/table/backgr_border-table-2.html:1:
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN">
> Do the "*-2.html" increase test coverage? If not I would suggest to omit them.
They do, they are the border-collapsed version of the tests and the behaviour is
different.

>
https://codereview.chromium.org/2600903002/diff/120001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/paint/BackgroundImageGeometry.cpp:439: ?
static_cast<const LayoutBoxModelObject&>(*rootBox)
> s/static_cast<const LayoutBoxModelObject&>(...)/toLayoutBoxModelObject(...)/
> 
toLayoutBoxModelObject() doesn't work inside inline '? .. :' the compiler thinks
it's returning a void.

[rhogan, 2017-01-16 20:14:45]

The CQ bit was checked by robhogan@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-16 20:15:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-16 21:03:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-16 21:03:18]

Dry run: Try jobs failed on following builders:
  linux_layout_tests_slimming_paint_v2 on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_layout_te...)

[rhogan, 2017-01-16 21:11:51]

The CQ bit was checked by robhogan@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-16 21:12:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[rhogan, 2017-01-16 21:36:43]

The CQ bit was checked by robhogan@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-16 21:36:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-16 22:39:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-16 22:39:28]

Dry run: Try jobs failed on following builders:
  linux_layout_tests_slimming_paint_v2 on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_layout_te...)

[Xianzhu, 2017-01-17 18:21:19]

https://codereview.chromium.org/2600903002/diff/150001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/BackgroundImageGeometry.cpp (right):

https://codereview.chromium.org/2600903002/diff/150001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/BackgroundImageGeometry.cpp:440: : obj;
I found why toLayoutBoxModelObject for the first one doesn't work: because the
cast is unnecessary. Removing the cast work for me on Linux.

The second cast should be toLayoutBoxModelObject().

https://codereview.chromium.org/2600903002/diff/150001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/BackgroundImageGeometry.cpp:486:
rootBox->location();
Not your fault, can you fix the above two lines?

485 s/,$/;/
486 d

[Xianzhu, 2017-01-17 18:22:21]

On 2017/01/16 20:06:28, rhogan wrote:
>
https://codereview.chromium.org/2600903002/diff/120001/third_party/WebKit/Lay...
> > third_party/WebKit/LayoutTests/fast/table/backgr_border-table-2.html:1:
> <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN">
> > Do the "*-2.html" increase test coverage? If not I would suggest to omit
them.
> They do, they are the border-collapsed version of the tests and the behaviour
is
> different.
> 

Can you rename the *-2.html files to *-collapsed-border.html?

[Xianzhu, 2017-01-17 18:22:58]

LGTM with the comments in #55 and #56 addressed.

[rhogan, 2017-01-19 20:54:09]

The CQ bit was checked by robhogan@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-19 20:54:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-19 21:50:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-19 21:50:17]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[rhogan, 2017-01-19 22:28:23]

On 2017/01/17 at 18:22:58, wangxianzhu wrote:
> LGTM with the comments in #55 and #56 addressed.

Thanks wangxianzhu - I'm not getting a scrollbar on the slimming paint png
results. Is that expected?

https://storage.googleapis.com/chromium-layout-test-archives/linux_layout_tes...

[Xianzhu, 2017-01-19 23:05:56]

On 2017/01/19 22:28:23, rhogan wrote:
> On 2017/01/17 at 18:22:58, wangxianzhu wrote:
> > LGTM with the comments in #55 and #56 addressed.
> 
> Thanks wangxianzhu - I'm not getting a scrollbar on the slimming paint png
> results. Is that expected?
> 
>
https://storage.googleapis.com/chromium-layout-test-archives/linux_layout_tes...

I believe this is an existing issue of spv2. You can add failure expectations
(with 'Bug(none)') in LayoutTests/FlagExpectations/enable-slimming-paint-v2.

[pdr., 2017-01-20 06:29:36]

On 2017/01/19 at 23:05:56, wangxianzhu wrote:
> On 2017/01/19 22:28:23, rhogan wrote:
> > On 2017/01/17 at 18:22:58, wangxianzhu wrote:
> > > LGTM with the comments in #55 and #56 addressed.
> > 
> > Thanks wangxianzhu - I'm not getting a scrollbar on the slimming paint png
> > results. Is that expected?
> > 
> >
https://storage.googleapis.com/chromium-layout-test-archives/linux_layout_tes...
> 
> I believe this is an existing issue of spv2. You can add failure expectations
(with 'Bug(none)') in LayoutTests/FlagExpectations/enable-slimming-paint-v2.

Yeah, this is definitely expected. It's my top priority this quarter.

[rhogan, 2017-01-21 00:10:53]

The CQ bit was checked by robhogan@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-21 00:11:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-21 02:05:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-21 02:05:08]

Dry run: This issue passed the CQ dry run.

[rhogan, 2017-01-21 08:14:13]

The CQ bit was checked by robhogan@gmail.com

[rhogan, 2017-01-21 08:14:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
wangxianzhu@chromium.org
Link to the patchset: https://codereview.chromium.org/2600903002/#ps170001
(title: "Paint the whole of an image behind the cells in a table")

[commit-bot: I haz the power, 2017-01-21 08:14:30]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-21 08:29:52]

CQ is committing da patch.

Bot data: {"patchset_id": 170001, "attempt_start_ts": 1484986453733220,
"parent_rev": "1ef4789838348d41635517ea25426ab1f9e75941", "commit_rev":
"949b36db0565e9a81fc50cbb1584d64d6090c147"}

[commit-bot: I haz the power, 2017-01-21 08:30:35]

Description was changed from

==========
Paint the whole of a table-part background image behind the cells in a table

Fix the painting of background images on table parts so that the image is
displayed
across the whole of the part. This fixes each of table-row-group, table-row,
table-column and table-column-group.

The fix is pretty simple in principle: just adjust the geometry we use to paint
the image 
to reflect the cell's position relative to the table part from which the image
originates.

Previously, we were just painting the image as though it was originating in each
cell. The one case left over is backgr_fixed-bg.html: this is still wrong and
I need to address it in a follow-up.

BUG=35697
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Paint the whole of a table-part background image behind the cells in a table

Fix the painting of background images on table parts so that the image is
displayed
across the whole of the part. This fixes each of table-row-group, table-row,
table-column and table-column-group.

The fix is pretty simple in principle: just adjust the geometry we use to paint
the image 
to reflect the cell's position relative to the table part from which the image
originates.

Previously, we were just painting the image as though it was originating in each
cell. The one case left over is backgr_fixed-bg.html: this is still wrong and
I need to address it in a follow-up.

BUG=35697
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2600903002
Cr-Commit-Position: refs/heads/master@{#445283}
Committed:
https://chromium.googlesource.com/chromium/src/+/949b36db0565e9a81fc50cbb1584...
==========

[commit-bot: I haz the power, 2017-01-21 08:30:38]

Committed patchset #12 (id:170001) as
https://chromium.googlesource.com/chromium/src/+/949b36db0565e9a81fc50cbb1584...