Do not SchedulePaint() inside views::Label::OnPaint()

Do not SchedulePaint() inside views::Label::OnPaint()

View::SchedulePaint() shouldn't be called inside an OnPaint() method. At
best it wastes computation doing a follow-up, redundant paint. On Mac
it's causing a weird interaction with transparent backgrounds for
strings of particular lengths on retina screens.

For performance, MaybeBuildRenderTextLines() is always deferred until a
Paint. It calls RecalculateColors() to apply colors to the lines it
newly creates, and that schedules a paint. But
MaybeBuildRenderTextLines() just needs to apply the colors that have
already been calculated.

So, to fix, split RecalculateColors() into the color calculation and
ApplyTextColors(), which can be called from OnPaint().

BUG=604092
TEST=On a retina-screen Mac with a fresh profile, navigate (e.g. to
chrome://version), then press Backspace, The "Press <key> to go back"
popup should appear and it should have a consistent, transparent
background.

Committed: https://crrev.com/2b40c5ca82b21f503cc7cc75ac101f1d2b710fc1
Cr-Commit-Position: refs/heads/master@{#400273}

[tapted, 2016-06-14 08:12:28]

Description was changed from

==========
Do not schedule a Paint inside Label::OnPaint()

BUG=604092
==========

to

==========
Do not SchedulePaint() inside views::Label::OnPaint()

View::SchedulePaint() shouldn't be called inside an OnPaint() method. At
best it wastes computation doing an immediate, redundant paint. On Mac
it's causing a weird interaction with transparent backgrounds for
strings of particular lengths on retina screens.

views::Label does lots of caching and deferred building of RenderText
instances (since that is slow). So to fix, feed a bool |in_paint|
through MaybeBuildRenderTextLines() to suppress the SchedulePaint() in
Label::RecalculateColors().

BUG=604092
TEST=On a retina-screen Mac with a fresh profile, navigate (e.g. to
chrome://version), then press Backspace, Press <key> to go back should
appear and it should have a consistent, transparent background.
==========

[tapted, 2016-06-14 08:12:28]

tapted@chromium.org changed reviewers:
+ sky@chromium.org

[tapted, 2016-06-14 11:07:09]

Description was changed from

==========
Do not SchedulePaint() inside views::Label::OnPaint()

View::SchedulePaint() shouldn't be called inside an OnPaint() method. At
best it wastes computation doing an immediate, redundant paint. On Mac
it's causing a weird interaction with transparent backgrounds for
strings of particular lengths on retina screens.

views::Label does lots of caching and deferred building of RenderText
instances (since that is slow). So to fix, feed a bool |in_paint|
through MaybeBuildRenderTextLines() to suppress the SchedulePaint() in
Label::RecalculateColors().

BUG=604092
TEST=On a retina-screen Mac with a fresh profile, navigate (e.g. to
chrome://version), then press Backspace, Press <key> to go back should
appear and it should have a consistent, transparent background.
==========

to

==========
Do not SchedulePaint() inside views::Label::OnPaint()

View::SchedulePaint() shouldn't be called inside an OnPaint() method. At
best it wastes computation doing a follow-up, redundant paint. On Mac
it's causing a weird interaction with transparent backgrounds for
strings of particular lengths on retina screens.

For performance, MaybeBuildRenderTextLines() is always deferred until a
Paint. It calls RecalculateColors() to apply colors to the lines in
newly creates, but really it just needs to apply the colors that have
already been calculated.

So, to fix, split RecalculateColors() into the color calculation and
ApplyTextColors(), which can be called from OnPaint().

BUG=604092
TEST=On a retina-screen Mac with a fresh profile, navigate (e.g. to
chrome://version), then press Backspace, Press <key> to go back should
appear and it should have a consistent, transparent background.
==========

[tapted, 2016-06-14 11:08:19]

Description was changed from

==========
Do not SchedulePaint() inside views::Label::OnPaint()

View::SchedulePaint() shouldn't be called inside an OnPaint() method. At
best it wastes computation doing a follow-up, redundant paint. On Mac
it's causing a weird interaction with transparent backgrounds for
strings of particular lengths on retina screens.

For performance, MaybeBuildRenderTextLines() is always deferred until a
Paint. It calls RecalculateColors() to apply colors to the lines in
newly creates, but really it just needs to apply the colors that have
already been calculated.

So, to fix, split RecalculateColors() into the color calculation and
ApplyTextColors(), which can be called from OnPaint().

BUG=604092
TEST=On a retina-screen Mac with a fresh profile, navigate (e.g. to
chrome://version), then press Backspace, Press <key> to go back should
appear and it should have a consistent, transparent background.
==========

to

==========
Do not SchedulePaint() inside views::Label::OnPaint()

View::SchedulePaint() shouldn't be called inside an OnPaint() method. At
best it wastes computation doing a follow-up, redundant paint. On Mac
it's causing a weird interaction with transparent backgrounds for
strings of particular lengths on retina screens.

For performance, MaybeBuildRenderTextLines() is always deferred until a
Paint. It calls RecalculateColors() to apply colors to the lines in
newly creates, at that schedules a paint. But
MaybeBuildRenderTextLines() just needs to apply the colors that have
already been calculated.

So, to fix, split RecalculateColors() into the color calculation and
ApplyTextColors(), which can be called from OnPaint().

BUG=604092
TEST=On a retina-screen Mac with a fresh profile, navigate (e.g. to
chrome://version), then press Backspace, Press <key> to go back should
appear and it should have a consistent, transparent background.
==========

[tapted, 2016-06-14 11:08:46]

Description was changed from

==========
Do not SchedulePaint() inside views::Label::OnPaint()

View::SchedulePaint() shouldn't be called inside an OnPaint() method. At
best it wastes computation doing a follow-up, redundant paint. On Mac
it's causing a weird interaction with transparent backgrounds for
strings of particular lengths on retina screens.

For performance, MaybeBuildRenderTextLines() is always deferred until a
Paint. It calls RecalculateColors() to apply colors to the lines in
newly creates, at that schedules a paint. But
MaybeBuildRenderTextLines() just needs to apply the colors that have
already been calculated.

So, to fix, split RecalculateColors() into the color calculation and
ApplyTextColors(), which can be called from OnPaint().

BUG=604092
TEST=On a retina-screen Mac with a fresh profile, navigate (e.g. to
chrome://version), then press Backspace, Press <key> to go back should
appear and it should have a consistent, transparent background.
==========

to

==========
Do not SchedulePaint() inside views::Label::OnPaint()

View::SchedulePaint() shouldn't be called inside an OnPaint() method. At
best it wastes computation doing a follow-up, redundant paint. On Mac
it's causing a weird interaction with transparent backgrounds for
strings of particular lengths on retina screens.

For performance, MaybeBuildRenderTextLines() is always deferred until a
Paint. It calls RecalculateColors() to apply colors to the lines in
newly creates, and that schedules a paint. But
MaybeBuildRenderTextLines() just needs to apply the colors that have
already been calculated.

So, to fix, split RecalculateColors() into the color calculation and
ApplyTextColors(), which can be called from OnPaint().

BUG=604092
TEST=On a retina-screen Mac with a fresh profile, navigate (e.g. to
chrome://version), then press Backspace, Press <key> to go back should
appear and it should have a consistent, transparent background.
==========

[tapted, 2016-06-14 11:09:36]

Description was changed from

==========
Do not SchedulePaint() inside views::Label::OnPaint()

View::SchedulePaint() shouldn't be called inside an OnPaint() method. At
best it wastes computation doing a follow-up, redundant paint. On Mac
it's causing a weird interaction with transparent backgrounds for
strings of particular lengths on retina screens.

For performance, MaybeBuildRenderTextLines() is always deferred until a
Paint. It calls RecalculateColors() to apply colors to the lines in
newly creates, and that schedules a paint. But
MaybeBuildRenderTextLines() just needs to apply the colors that have
already been calculated.

So, to fix, split RecalculateColors() into the color calculation and
ApplyTextColors(), which can be called from OnPaint().

BUG=604092
TEST=On a retina-screen Mac with a fresh profile, navigate (e.g. to
chrome://version), then press Backspace, Press <key> to go back should
appear and it should have a consistent, transparent background.
==========

to

==========
Do not SchedulePaint() inside views::Label::OnPaint()

View::SchedulePaint() shouldn't be called inside an OnPaint() method. At
best it wastes computation doing a follow-up, redundant paint. On Mac
it's causing a weird interaction with transparent backgrounds for
strings of particular lengths on retina screens.

For performance, MaybeBuildRenderTextLines() is always deferred until a
Paint. It calls RecalculateColors() to apply colors to the lines in
newly creates, and that schedules a paint. But
MaybeBuildRenderTextLines() just needs to apply the colors that have
already been calculated.

So, to fix, split RecalculateColors() into the color calculation and
ApplyTextColors(), which can be called from OnPaint().

BUG=604092
TEST=On a retina-screen Mac with a fresh profile, navigate (e.g. to
chrome://version), then press Backspace, The "Press <key> to go back"
popup should appear and it should have a consistent, transparent
background.
==========

[tapted, 2016-06-14 11:14:17]

Description was changed from

==========
Do not SchedulePaint() inside views::Label::OnPaint()

View::SchedulePaint() shouldn't be called inside an OnPaint() method. At
best it wastes computation doing a follow-up, redundant paint. On Mac
it's causing a weird interaction with transparent backgrounds for
strings of particular lengths on retina screens.

For performance, MaybeBuildRenderTextLines() is always deferred until a
Paint. It calls RecalculateColors() to apply colors to the lines in
newly creates, and that schedules a paint. But
MaybeBuildRenderTextLines() just needs to apply the colors that have
already been calculated.

So, to fix, split RecalculateColors() into the color calculation and
ApplyTextColors(), which can be called from OnPaint().

BUG=604092
TEST=On a retina-screen Mac with a fresh profile, navigate (e.g. to
chrome://version), then press Backspace, The "Press <key> to go back"
popup should appear and it should have a consistent, transparent
background.
==========

to

==========
Do not SchedulePaint() inside views::Label::OnPaint()

View::SchedulePaint() shouldn't be called inside an OnPaint() method. At
best it wastes computation doing a follow-up, redundant paint. On Mac
it's causing a weird interaction with transparent backgrounds for
strings of particular lengths on retina screens.

For performance, MaybeBuildRenderTextLines() is always deferred until a
Paint. It calls RecalculateColors() to apply colors to the lines it
newly creates, and that schedules a paint. But
MaybeBuildRenderTextLines() just needs to apply the colors that have
already been calculated.

So, to fix, split RecalculateColors() into the color calculation and
ApplyTextColors(), which can be called from OnPaint().

BUG=604092
TEST=On a retina-screen Mac with a fresh profile, navigate (e.g. to
chrome://version), then press Backspace, The "Press <key> to go back"
popup should appear and it should have a consistent, transparent
background.
==========

[tapted, 2016-06-14 11:15:54]

Hi sky, please take a look

https://codereview.chromium.org/2065003002/diff/40001/ui/views/controls/label.cc
File ui/views/controls/label.cc (right):

https://codereview.chromium.org/2065003002/diff/40001/ui/views/controls/label...
ui/views/controls/label.cc:360: MaybeBuildRenderTextLines();
This was the call triggering a second paint

https://codereview.chromium.org/2065003002/diff/40001/ui/views/controls/label...
ui/views/controls/label.cc:490: MaybeBuildRenderTextLines();
There's a call here as well, but GetFocusBounds() is only called in OnPaint.
(the only other call is in GetDisplayTextForTesting())

[sky, 2016-06-16 15:45:58]

LGTM

[tapted, 2016-06-16 21:02:31]

The CQ bit was checked by tapted@chromium.org

[commit-bot: I haz the power, 2016-06-16 21:03:15]

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

[commit-bot: I haz the power, 2016-06-16 21:58:11]

Description was changed from

==========
Do not SchedulePaint() inside views::Label::OnPaint()

View::SchedulePaint() shouldn't be called inside an OnPaint() method. At
best it wastes computation doing a follow-up, redundant paint. On Mac
it's causing a weird interaction with transparent backgrounds for
strings of particular lengths on retina screens.

For performance, MaybeBuildRenderTextLines() is always deferred until a
Paint. It calls RecalculateColors() to apply colors to the lines it
newly creates, and that schedules a paint. But
MaybeBuildRenderTextLines() just needs to apply the colors that have
already been calculated.

So, to fix, split RecalculateColors() into the color calculation and
ApplyTextColors(), which can be called from OnPaint().

BUG=604092
TEST=On a retina-screen Mac with a fresh profile, navigate (e.g. to
chrome://version), then press Backspace, The "Press <key> to go back"
popup should appear and it should have a consistent, transparent
background.
==========

to

==========
Do not SchedulePaint() inside views::Label::OnPaint()

View::SchedulePaint() shouldn't be called inside an OnPaint() method. At
best it wastes computation doing a follow-up, redundant paint. On Mac
it's causing a weird interaction with transparent backgrounds for
strings of particular lengths on retina screens.

For performance, MaybeBuildRenderTextLines() is always deferred until a
Paint. It calls RecalculateColors() to apply colors to the lines it
newly creates, and that schedules a paint. But
MaybeBuildRenderTextLines() just needs to apply the colors that have
already been calculated.

So, to fix, split RecalculateColors() into the color calculation and
ApplyTextColors(), which can be called from OnPaint().

BUG=604092
TEST=On a retina-screen Mac with a fresh profile, navigate (e.g. to
chrome://version), then press Backspace, The "Press <key> to go back"
popup should appear and it should have a consistent, transparent
background.
==========

[commit-bot: I haz the power, 2016-06-16 21:58:12]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-06-16 22:00:40]

Description was changed from

==========
Do not SchedulePaint() inside views::Label::OnPaint()

View::SchedulePaint() shouldn't be called inside an OnPaint() method. At
best it wastes computation doing a follow-up, redundant paint. On Mac
it's causing a weird interaction with transparent backgrounds for
strings of particular lengths on retina screens.

For performance, MaybeBuildRenderTextLines() is always deferred until a
Paint. It calls RecalculateColors() to apply colors to the lines it
newly creates, and that schedules a paint. But
MaybeBuildRenderTextLines() just needs to apply the colors that have
already been calculated.

So, to fix, split RecalculateColors() into the color calculation and
ApplyTextColors(), which can be called from OnPaint().

BUG=604092
TEST=On a retina-screen Mac with a fresh profile, navigate (e.g. to
chrome://version), then press Backspace, The "Press <key> to go back"
popup should appear and it should have a consistent, transparent
background.
==========

to

==========
Do not SchedulePaint() inside views::Label::OnPaint()

View::SchedulePaint() shouldn't be called inside an OnPaint() method. At
best it wastes computation doing a follow-up, redundant paint. On Mac
it's causing a weird interaction with transparent backgrounds for
strings of particular lengths on retina screens.

For performance, MaybeBuildRenderTextLines() is always deferred until a
Paint. It calls RecalculateColors() to apply colors to the lines it
newly creates, and that schedules a paint. But
MaybeBuildRenderTextLines() just needs to apply the colors that have
already been calculated.

So, to fix, split RecalculateColors() into the color calculation and
ApplyTextColors(), which can be called from OnPaint().

BUG=604092
TEST=On a retina-screen Mac with a fresh profile, navigate (e.g. to
chrome://version), then press Backspace, The "Press <key> to go back"
popup should appear and it should have a consistent, transparent
background.

Committed: https://crrev.com/2b40c5ca82b21f503cc7cc75ac101f1d2b710fc1
Cr-Commit-Position: refs/heads/master@{#400273}
==========

[commit-bot: I haz the power, 2016-06-16 22:00:41]

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