Supports the invisible underline for native input fields.

Supports the invisible underline for native input fields.

Physical keyboard autocorrect feature needs to be launched in M43, which can sets composition with invisible underline.
For now, views::TextField doesn't respect the config in CompositionText for colors, background, etc.

This cl is to make the RenderText not render the underline if the CompositionText is configured with transparent color.

Note: this cl doesn't try to support background, colors, etc. as some TODOs like https://code.google.com/p/chromium/codesearch#chromium/src/ui/gfx/render_text.h&l=723

BUG=407576
TEST=Verified on clapper & linux_chromeos.

Committed: https://crrev.com/09036bd899e6b591fae4e9665247c78d26c14bab
Cr-Commit-Position: refs/heads/master@{#322942}

[Shu Chen, 2015-03-24 08:11:39]

shuchen@chromium.org changed reviewers:
+ msw@chromium.org

[Shu Chen, 2015-03-24 08:11:39]

Hi Mike, can you please help review this cl? Thanks

[msw, 2015-03-24 20:19:01]

https://codereview.chromium.org/1031533002/diff/40001/ui/views/controls/textf...
File ui/views/controls/textfield/textfield_model.cc (right):

https://codereview.chromium.org/1031533002/diff/40001/ui/views/controls/textf...
ui/views/controls/textfield/textfield_model.cc:568: if
(composition.underlines.size() > 0 && composition.underlines[0].color != 0)
nit: add a comment here to explain this behavior; it's not obvious.

https://codereview.chromium.org/1031533002/diff/40001/ui/views/controls/textf...
ui/views/controls/textfield/textfield_model.cc:618: void
TextfieldModel::ClearComposition() {
Shouldn't this also clear |composition_range_|?

[Shu Chen, 2015-03-26 04:22:14]

Thanks for your review!

https://codereview.chromium.org/1031533002/diff/40001/ui/views/controls/textf...
File ui/views/controls/textfield/textfield_model.cc (right):

https://codereview.chromium.org/1031533002/diff/40001/ui/views/controls/textf...
ui/views/controls/textfield/textfield_model.cc:568: if
(composition.underlines.size() > 0 && composition.underlines[0].color != 0)
On 2015/03/24 20:19:01, msw wrote:
> nit: add a comment here to explain this behavior; it's not obvious.

Done.

https://codereview.chromium.org/1031533002/diff/40001/ui/views/controls/textf...
ui/views/controls/textfield/textfield_model.cc:618: void
TextfieldModel::ClearComposition() {
On 2015/03/24 20:19:01, msw wrote:
> Shouldn't this also clear |composition_range_|?

Done.

[msw, 2015-03-26 20:22:48]

https://codereview.chromium.org/1031533002/diff/80001/ui/base/ime/composition...
File ui/base/ime/composition_text.cc (right):

https://codereview.chromium.org/1031533002/diff/80001/ui/base/ime/composition...
ui/base/ime/composition_text.cc:10:
underlines.push_back(CompositionUnderline());
Why do we need this change? It seems odd to add this underline to an empty
CompositionText object if it's just needed to accomodate
TextfieldModel::SetCompositionText's behavior at line 570: "if
(composition.underlines.size() > 0 && composition.underlines[0].color != 0)".

If we just want the textfield's composition range to be underlined by default,
unless the underline is specifically set to transparent, perhaps change the
condition in TextfieldModel::SetCompositionText to "if
(composition.underlines.empty() || composition.underlines[0].color != 0)"

https://codereview.chromium.org/1031533002/diff/80001/ui/views/controls/textf...
File ui/views/controls/textfield/textfield_model.cc (right):

https://codereview.chromium.org/1031533002/diff/80001/ui/views/controls/textf...
ui/views/controls/textfield/textfield_model.cc:568: // Don't render the
underline for composition which is configured as
nit: "Don't render transparent composition underlines."

[Shu Chen, 2015-03-30 03:56:21]

https://codereview.chromium.org/1031533002/diff/80001/ui/base/ime/composition...
File ui/base/ime/composition_text.cc (right):

https://codereview.chromium.org/1031533002/diff/80001/ui/base/ime/composition...
ui/base/ime/composition_text.cc:10:
underlines.push_back(CompositionUnderline());
On 2015/03/26 20:22:48, msw wrote:
> Why do we need this change? It seems odd to add this underline to an empty
> CompositionText object if it's just needed to accomodate
> TextfieldModel::SetCompositionText's behavior at line 570: "if
> (composition.underlines.size() > 0 && composition.underlines[0].color != 0)".
> 
> If we just want the textfield's composition range to be underlined by default,
> unless the underline is specifically set to transparent, perhaps change the
> condition in TextfieldModel::SetCompositionText to "if
> (composition.underlines.empty() || composition.underlines[0].color != 0)"

It was to make the tests can pass.
I've just checked the latest patchset that the test can still pass without this
hack.
So I just reverted this change. Thanks for catching!

https://codereview.chromium.org/1031533002/diff/80001/ui/views/controls/textf...
File ui/views/controls/textfield/textfield_model.cc (right):

https://codereview.chromium.org/1031533002/diff/80001/ui/views/controls/textf...
ui/views/controls/textfield/textfield_model.cc:568: // Don't render the
underline for composition which is configured as
On 2015/03/26 20:22:48, msw wrote:
> nit: "Don't render transparent composition underlines."

Done.

[msw, 2015-03-30 20:12:13]

lgtm

[Shu Chen, 2015-03-31 00:15:46]

The CQ bit was checked by shuchen@chromium.org

[commit-bot: I haz the power, 2015-03-31 00:16:40]

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

[commit-bot: I haz the power, 2015-03-31 01:39:45]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-03-31 01:41:28]

Patchset 6 (id:??) landed as
https://crrev.com/09036bd899e6b591fae4e9665247c78d26c14bab
Cr-Commit-Position: refs/heads/master@{#322942}