Use RenderText directly to draw tooltip

Use RenderText directly to draw tooltip to imporove format and performance.

This replaces the old CL (https://codereview.chromium.org/340543004/). This runs faster and doesn't require the label caching CL. (for the example in 370226, this is 20x faster on arm. ~60ms total vs ~3ms total)

I'll look into switching to label once I get the performance data with label after label caching is enabled.

This depends on two CLs
1) https://codereview.chromium.org/916423002/  Tooltip Cleanup
2) https://codereview.chromium.org/915383003/  RTL format problem.

but this can be reviewed independently.

BUG=370226, 451853
Committed: https://crrev.com/3e0cc51a03d65bf057742ac0f1f5e0c8b633bda5
Cr-Commit-Position: refs/heads/master@{#317590}

[oshima, 2015-02-12 21:14:23]

Patchset #2 (id:20001) has been deleted

[oshima, 2015-02-12 21:14:26]

Patchset #1 (id:1) has been deleted

[oshima, 2015-02-12 21:27:35]

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

[sky, 2015-02-19 00:07:56]

How come you're not converting to Label directly?

https://codereview.chromium.org/924433002/diff/60001/ui/views/corewm/tooltip_...
File ui/views/corewm/tooltip_aura.cc (right):

https://codereview.chromium.org/924433002/diff/60001/ui/views/corewm/tooltip_...
ui/views/corewm/tooltip_aura.cc:99: gfx::RenderText* render_text_;
scoped_ptr

[oshima, 2015-02-19 05:50:26]

On 2015/02/19 00:07:56, sky wrote:
> How come you're not converting to Label directly?

Because label's mutiline is super slow. Mukai is currently working on
label refactoring, which include better multi-line support, but it won't be
in 42.

We'll look into switching to label, once his change is landed, and its
performance is evaluated.

> 
>
https://codereview.chromium.org/924433002/diff/60001/ui/views/corewm/tooltip_...
> File ui/views/corewm/tooltip_aura.cc (right):
> 
>
https://codereview.chromium.org/924433002/diff/60001/ui/views/corewm/tooltip_...
> ui/views/corewm/tooltip_aura.cc:99: gfx::RenderText* render_text_;
> scoped_ptr

Done.

[sky, 2015-02-19 17:05:33]

Are tooltips really a slow spot for performance?

On Wed, Feb 18, 2015 at 9:50 PM,  <oshima@chromium.org> wrote:
> On 2015/02/19 00:07:56, sky wrote:
>>
>> How come you're not converting to Label directly?
>
>
> Because label's mutiline is super slow. Mukai is currently working on
> label refactoring, which include better multi-line support, but it won't be
> in 42.
>
> We'll look into switching to label, once his change is landed, and its
> performance is evaluated.
>
>
>
>
https://codereview.chromium.org/924433002/diff/60001/ui/views/corewm/tooltip_...
>>
>> File ui/views/corewm/tooltip_aura.cc (right):
>
>
>
>
https://codereview.chromium.org/924433002/diff/60001/ui/views/corewm/tooltip_...
>>
>> ui/views/corewm/tooltip_aura.cc:99: gfx::RenderText* render_text_;
>> scoped_ptr
>
>
> Done.
>
>
> https://codereview.chromium.org/924433002/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Jun Mukai, 2015-02-19 19:08:33]

On 2015/02/19 17:05:33, sky wrote:
> Are tooltips really a slow spot for performance?

Yes. oshima wrote an analysis as
https://docs.google.com/a/chromium.org/document/d/1iuemUhAMAxlSxybb8dKdYVx6V7...
The real problem is multiline-wrapping which tooltip relies on.
Our current multiline-wrapping is really inefficient unfortunately, because it
invokes gfx::Canvas::GetStringWidth() for too many times (and
gfx::Canvas::GetStringWidth() internally creates RenderText instance and
computes the text size by EnsureLayout()). To avoid this inefficiency, first
create RenderText() instance, compute the layout information, and then decide
the line breaking position based on the precomputed layout data.
I believe this is what oshima is trying to do in this CL.

[sky, 2015-02-19 19:15:11]

Ok, LGTM

[oshima, 2015-02-20 04:41:44]

The CQ bit was checked by oshima@chromium.org

[commit-bot: I haz the power, 2015-02-20 04:42:14]

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

[commit-bot: I haz the power, 2015-02-20 05:45:18]

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

[commit-bot: I haz the power, 2015-02-20 05:45:19]

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

[oshima, 2015-02-21 05:51:33]

The CQ bit was checked by oshima@chromium.org

[oshima, 2015-02-21 05:51:34]

The CQ bit was unchecked by oshima@chromium.org

[oshima, 2015-02-21 15:42:40]

New patchsets have been uploaded after l-g-t-m from sky@chromium.org

[oshima, 2015-02-23 16:34:35]

FYI: I set set_owned_by_client(), as it's used to be.

[oshima, 2015-02-23 16:34:40]

The CQ bit was checked by oshima@chromium.org

[commit-bot: I haz the power, 2015-02-23 16:34:53]

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

[commit-bot: I haz the power, 2015-02-23 16:38:17]

Committed patchset #4 (id:100001)

[commit-bot: I haz the power, 2015-02-23 16:38:49]

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