Move InlineTextBox::paint guts into blink::TextPainter.

Move InlineTextBox::paint guts into blink::TextPainter.

This makes it a little clearer than when we have 12-argument functions.

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

[jbroman, 2014-09-03 21:44:49]

jbroman@chromium.org changed reviewers:
+ fmalita@chromium.org, schenney@chromium.org

[jbroman, 2014-09-03 21:44:49]

I think this is a net win for making InlineTextBox's painting a little more
rational.

I've got a patch that starts to use text blobs for caching that I found a little
easier to reason about this way.

Do y'all also think this is cleaner?

(Presumably because it impairs inlining to move this to another translation
unit, there is a minor performance impact, but it seems to be <1%, so I think
it's worth it.)

[Stephen Chennney, 2014-09-04 15:26:49]

It certainly makes it clearer, but I would go the extra step and move the
painting commands for combined text to yet another method. Then we have all the
drawing commands that occur for a particular on-screen entity appearing in an
isolated method.

[Stephen Chennney, 2014-09-04 15:27:28]

https://codereview.chromium.org/538663002/diff/1/Source/core/rendering/TextPa...
File Source/core/rendering/TextPainter.cpp (right):

https://codereview.chromium.org/538663002/diff/1/Source/core/rendering/TextPa...
Source/core/rendering/TextPainter.cpp:60: if (m_combinedText) {
Make this block a distinct method too.

[jbroman, 2014-09-04 19:57:07]

On 2014/09/04 15:26:49, Stephen Chenney wrote:
> It certainly makes it clearer, but I would go the extra step and move the
> painting commands for combined text to yet another method. Then we have all
the
> drawing commands that occur for a particular on-screen entity appearing in an
> isolated method.

I'm not sure I understand correctly. Do you mean you want a paint(...) method
and a paintForCombinedText(RenderCombineText*, ...)?
And *in addition* the latter should call paintEmphasisMarkForCombinedText(...)?

[Stephen Chennney, 2014-09-04 20:35:36]

On 2014/09/04 19:57:07, jbroman wrote:
> On 2014/09/04 15:26:49, Stephen Chenney wrote:
> > It certainly makes it clearer, but I would go the extra step and move the
> > painting commands for combined text to yet another method. Then we have all
> the
> > drawing commands that occur for a particular on-screen entity appearing in
an
> > isolated method.
> 
> I'm not sure I understand correctly. Do you mean you want a paint(...) method
> and a paintForCombinedText(RenderCombineText*, ...)?
> And *in addition* the latter should call
paintEmphasisMarkForCombinedText(...)?

I just mean create "TextPainter::paintCombinedText" (it can be private) and call
it from "paint" inside that "if (m_combinedText)"

[jbroman, 2014-09-04 21:16:52]

https://codereview.chromium.org/538663002/diff/1/Source/core/rendering/TextPa...
File Source/core/rendering/TextPainter.cpp (right):

https://codereview.chromium.org/538663002/diff/1/Source/core/rendering/TextPa...
Source/core/rendering/TextPainter.cpp:60: if (m_combinedText) {
On 2014/09/04 15:27:28, Stephen Chenney wrote:
> Make this block a distinct method too.

Done.

[Stephen Chennney, 2014-09-05 13:40:53]

The CQ bit was checked by schenney@chromium.org

[Stephen Chennney, 2014-09-05 13:40:54]

lgtm

[commit-bot: I haz the power, 2014-09-05 13:41:56]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jbroman@chromium.org/538663002/20001

[commit-bot: I haz the power, 2014-09-05 13:46:51]

Committed patchset #2 (id:20001) as 181471