Introduce paintFragmentWithPhase(), to simplify paint*ForFragments().

Introduce paintFragmentWithPhase(), to simplify paint*ForFragments().

There was some ~duplicated code in those methods, pertaining to clipping
and setting up a PaintInfo and calling renderer::paint().

R=chrishtr@chromium.org

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

[mstensho (USE GERRIT), 2014-11-05 13:55:51]

The reason for this change is that I want to add some code for multicol in this
area (more about that later), and I don't want to duplicate that code as well.
But I think this clean-up is a worthwhile thing to do, regardless of that, no?

This CL depends on https://codereview.chromium.org/706603002/ being accepted.

[chrishtr, 2014-11-06 02:53:35]

Nice cleanup.

Looks good modulo the other CL.

https://codereview.chromium.org/704753002/diff/1/Source/core/paint/LayerPaint...
File Source/core/paint/LayerPainter.cpp (left):

https://codereview.chromium.org/704753002/diff/1/Source/core/paint/LayerPaint...
Source/core/paint/LayerPainter.cpp:811: // FIXME: Eventually we will collect the
region from the fragment itself instead of just from the paint info.
Is this comment bogus?

https://codereview.chromium.org/704753002/diff/1/Source/core/paint/LayerPaint...
File Source/core/paint/LayerPainter.cpp (right):

https://codereview.chromium.org/704753002/diff/1/Source/core/paint/LayerPaint...
Source/core/paint/LayerPainter.cpp:728: for (const LayerFragment& fragment:
layerFragments) {
for (const auto& fragment : layerFragments)

should also work.

[mstensho (USE GERRIT), 2014-11-06 14:11:31]

https://codereview.chromium.org/704753002/diff/1/Source/core/paint/LayerPaint...
File Source/core/paint/LayerPainter.cpp (left):

https://codereview.chromium.org/704753002/diff/1/Source/core/paint/LayerPaint...
Source/core/paint/LayerPainter.cpp:811: // FIXME: Eventually we will collect the
region from the fragment itself instead of just from the paint info.
On 2014/11/06 02:53:35, chrishtr wrote:
> Is this comment bogus?

Relics from CSS regions support. It stems from 351d52a91a74ff86b6 in WebKit
(r144318), February 2013. There's a region parameter to PaintInfo() in that
commit, which doesn't exist in Blink. FIXMEs still present in WebKit, although
the region parameter is now gone there.

Anyway, doesn't apply to us.

https://codereview.chromium.org/704753002/diff/1/Source/core/paint/LayerPaint...
File Source/core/paint/LayerPainter.cpp (right):

https://codereview.chromium.org/704753002/diff/1/Source/core/paint/LayerPaint...
Source/core/paint/LayerPainter.cpp:728: for (const LayerFragment& fragment:
layerFragments) {
On 2014/11/06 02:53:35, chrishtr wrote:
> for (const auto& fragment : layerFragments)
> 
> should also work.

Done.

[chrishtr, 2014-11-06 18:22:36]

https://codereview.chromium.org/704753002/diff/1/Source/core/paint/LayerPaint...
File Source/core/paint/LayerPainter.cpp (left):

https://codereview.chromium.org/704753002/diff/1/Source/core/paint/LayerPaint...
Source/core/paint/LayerPainter.cpp:811: // FIXME: Eventually we will collect the
region from the fragment itself instead of just from the paint info.
On 2014/11/06 14:11:31, mstensho wrote:
> On 2014/11/06 02:53:35, chrishtr wrote:
> > Is this comment bogus?
> 
> Relics from CSS regions support. It stems from 351d52a91a74ff86b6 in WebKit
> (r144318), February 2013. There's a region parameter to PaintInfo() in that
> commit, which doesn't exist in Blink. FIXMEs still present in WebKit, although
> the region parameter is now gone there.
> 
> Anyway, doesn't apply to us.

So remove?

[mstensho (USE GERRIT), 2014-11-06 19:08:25]

https://codereview.chromium.org/704753002/diff/1/Source/core/paint/LayerPaint...
File Source/core/paint/LayerPainter.cpp (left):

https://codereview.chromium.org/704753002/diff/1/Source/core/paint/LayerPaint...
Source/core/paint/LayerPainter.cpp:811: // FIXME: Eventually we will collect the
region from the fragment itself instead of just from the paint info.
On 2014/11/06 18:22:36, chrishtr wrote:
> On 2014/11/06 14:11:31, mstensho wrote:
> > On 2014/11/06 02:53:35, chrishtr wrote:
> > > Is this comment bogus?
> > 
> > Relics from CSS regions support. It stems from 351d52a91a74ff86b6 in WebKit
> > (r144318), February 2013. There's a region parameter to PaintInfo() in that
> > commit, which doesn't exist in Blink. FIXMEs still present in WebKit,
although
> > the region parameter is now gone there.
> > 
> > Anyway, doesn't apply to us.
> 
> So remove?

But I did remove it. :)

[chrishtr, 2014-11-06 21:20:37]

On 2014/11/06 at 19:08:25, mstensho wrote:
>
https://codereview.chromium.org/704753002/diff/1/Source/core/paint/LayerPaint...
> File Source/core/paint/LayerPainter.cpp (left):
> 
>
https://codereview.chromium.org/704753002/diff/1/Source/core/paint/LayerPaint...
> Source/core/paint/LayerPainter.cpp:811: // FIXME: Eventually we will collect
the region from the fragment itself instead of just from the paint info.
> On 2014/11/06 18:22:36, chrishtr wrote:
> > On 2014/11/06 14:11:31, mstensho wrote:
> > > On 2014/11/06 02:53:35, chrishtr wrote:
> > > > Is this comment bogus?
> > > 
> > > Relics from CSS regions support. It stems from 351d52a91a74ff86b6 in
WebKit
> > > (r144318), February 2013. There's a region parameter to PaintInfo() in
that
> > > commit, which doesn't exist in Blink. FIXMEs still present in WebKit,
although
> > > the region parameter is now gone there.
> > > 
> > > Anyway, doesn't apply to us.
> > 
> > So remove?
> 
> But I did remove it. :)

Ah. My bad.

[chrishtr, 2014-11-06 21:20:42]

lgtm

[mstensho (USE GERRIT), 2014-11-06 21:29:42]

The CQ bit was checked by mstensho@opera.com

[commit-bot: I haz the power, 2014-11-06 21:30:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/704753002/40001

[commit-bot: I haz the power, 2014-11-06 22:47:48]

Committed patchset #3 (id:40001) as 184936