Move line painting to BlockFlowPainter.

Move line painting to BlockFlowPainter.

Removed some unnecessary includes, since I had to modify the list of includes
anyway.

Also got rid of LayoutBlock(Flow)::paintFloats() in the process.

BUG=302024
Committed: https://crrev.com/e3ee937939aeee83e1908a995dc32d8ef7c5c3d2
Cr-Commit-Position: refs/heads/master@{#394581}

[mstensho (USE GERRIT), 2016-05-18 15:12:33]

mstensho@opera.com changed reviewers:
+ chrishtr@chromium.org, wangxianzhu@chromium.org

[mstensho (USE GERRIT), 2016-05-18 15:12:34]

https://codereview.chromium.org/1993713002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/BlockPainter.cpp (right):

https://codereview.chromium.org/1993713002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/BlockPainter.cpp:179:
m_layoutBlock.paintContentsAndFloats(contentsPaintInfo, paintOffset);
Not sure how you prefer this. We could just as well have both paintContents()
and paintFloats() as virtual methods on LayoutBlock. Or we could just have
paintContents(), and have the LayoutBlockFlow override do floats as well. Floats
are LayoutBlockFlow-specific anyway, and other parts of the code may very well
just consider them to be "content".

[chrishtr, 2016-05-18 16:50:42]

https://codereview.chromium.org/1993713002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/BlockPainter.cpp (right):

https://codereview.chromium.org/1993713002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/BlockPainter.cpp:179:
m_layoutBlock.paintContentsAndFloats(contentsPaintInfo, paintOffset);
On 2016/05/18 at 15:12:34, mstensho wrote:
> Not sure how you prefer this. We could just as well have both paintContents()
and paintFloats() as virtual methods on LayoutBlock. Or we could just have
paintContents(), and have the LayoutBlockFlow override do floats as well. Floats
are LayoutBlockFlow-specific anyway, and other parts of the code may very well
just consider them to be "content".

It would be good to avoid having a method related to painting floats on
LayoutBlock. What do you think of:

if (isLayoutBlockFlow(m_layoutBlock))
  toLayoutBlockFlow(m_layoutBlock).paintFloats(...)


This at least localizes the differences and seems pretty clear.

[mstensho (USE GERRIT), 2016-05-18 18:51:08]

https://codereview.chromium.org/1993713002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/BlockPainter.cpp (right):

https://codereview.chromium.org/1993713002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/BlockPainter.cpp:179:
m_layoutBlock.paintContentsAndFloats(contentsPaintInfo, paintOffset);
On 2016/05/18 16:50:42, chrishtr wrote:
> On 2016/05/18 at 15:12:34, mstensho wrote:
> > Not sure how you prefer this. We could just as well have both
paintContents()
> and paintFloats() as virtual methods on LayoutBlock. Or we could just have
> paintContents(), and have the LayoutBlockFlow override do floats as well.
Floats
> are LayoutBlockFlow-specific anyway, and other parts of the code may very well
> just consider them to be "content".
> 
> It would be good to avoid having a method related to painting floats on
> LayoutBlock. What do you think of:
> 
> if (isLayoutBlockFlow(m_layoutBlock))
>   toLayoutBlockFlow(m_layoutBlock).paintFloats(...)
> 
> 
> This at least localizes the differences and seems pretty clear.

Hmm... is it okay to initialize a BlockFlowPainter from BlockPainter? If so, the
patch can be simplified by a lot, instead of spaghettiing in and out of layout
and paint code. :) Please take a look.

[chrishtr, 2016-05-18 21:11:32]

lgtm

Definitely what you did is ok, nice. There are other examples of this kind of
thing elsewhere.
In general it's better to avoid having virtuals and jumping back and forth. This
makes paint
code clearer, faster and more self-contained.

[mstensho (USE GERRIT), 2016-05-18 21:34:28]

The CQ bit was checked by mstensho@opera.com

[mstensho (USE GERRIT), 2016-05-18 21:34:57]

The CQ bit was unchecked by mstensho@opera.com

[commit-bot: I haz the power, 2016-05-18 21:35:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1993713002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1993713002/20001

[mstensho (USE GERRIT), 2016-05-18 21:35:53]

Description was changed from

==========
Move line painting to BlockFlowPainter.

Removed some unnecessary includes, since I had to modify the list of includes
anyway.

BUG=302024
==========

to

==========
Move line painting to BlockFlowPainter.

Removed some unnecessary includes, since I had to modify the list of includes
anyway.

Also got rid of LayoutBlock(Flow)::paintFloats() in the process.

BUG=302024
==========

[mstensho (USE GERRIT), 2016-05-18 21:35:58]

The CQ bit was checked by mstensho@opera.com

[commit-bot: I haz the power, 2016-05-18 21:36:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1993713002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1993713002/20001

[commit-bot: I haz the power, 2016-05-18 22:55:58]

Description was changed from

==========
Move line painting to BlockFlowPainter.

Removed some unnecessary includes, since I had to modify the list of includes
anyway.

Also got rid of LayoutBlock(Flow)::paintFloats() in the process.

BUG=302024
==========

to

==========
Move line painting to BlockFlowPainter.

Removed some unnecessary includes, since I had to modify the list of includes
anyway.

Also got rid of LayoutBlock(Flow)::paintFloats() in the process.

BUG=302024
==========

[commit-bot: I haz the power, 2016-05-18 22:55:59]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-05-18 22:57:32]

Description was changed from

==========
Move line painting to BlockFlowPainter.

Removed some unnecessary includes, since I had to modify the list of includes
anyway.

Also got rid of LayoutBlock(Flow)::paintFloats() in the process.

BUG=302024
==========

to

==========
Move line painting to BlockFlowPainter.

Removed some unnecessary includes, since I had to modify the list of includes
anyway.

Also got rid of LayoutBlock(Flow)::paintFloats() in the process.

BUG=302024

Committed: https://crrev.com/e3ee937939aeee83e1908a995dc32d8ef7c5c3d2
Cr-Commit-Position: refs/heads/master@{#394581}
==========

[commit-bot: I haz the power, 2016-05-18 22:57:33]

Patchset 2 (id:??) landed as
https://crrev.com/e3ee937939aeee83e1908a995dc32d8ef7c5c3d2
Cr-Commit-Position: refs/heads/master@{#394581}