views::Label: Enable text selection related tests on Mac.

views::Label: Enable text selection related tests on Mac.

Currently text selection related tests for Labels are not run on Mac since it
uses RenderTextMac by default. Enable these tests on Mac, by explicitly using
RenderTextHarfBuzz which supports text selection. To do this, create a new test
fixture class called LabelSelectionTest and append the kEnableHarfBuzzRenderText
switch to the list of command line switches.

BUG=630365
Committed: https://crrev.com/d52fcdcee551a2f4e77c1f7223eb73f6d85c210b
Cr-Commit-Position: refs/heads/master@{#431107}

[karandeepb, 2016-11-09 06:52:26]

Description was changed from

==========
Labels: Enable text selection related tests on Mac.
==========

to

==========
views::Label: Enable text selection related tests on Mac.

Currently text selection related tests for Labels are not run on Mac since it
uses RenderTextMac by default. Enable these tests on Mac, by explicitly using
RenderTextHarfBuzz which supports text selection. To do this, create a new test 
fixture class called LabelSelectionTest and append the kEnableHarfBuzzRenderText

switch to the list of command line switches.

BUG=630365
==========

[karandeepb, 2016-11-09 06:54:42]

The CQ bit was checked by karandeepb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-09 06:54:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-09 08:19:56]

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

[commit-bot: I haz the power, 2016-11-09 08:19:57]

Dry run: This issue passed the CQ dry run.

[karandeepb, 2016-11-09 08:38:29]

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

[karandeepb, 2016-11-09 08:38:31]

PTAL sky@. Did not think of this earlier.

[sky, 2016-11-09 16:23:10]

LGTM - I'm ok with this, but if there if you there isn't a good reason not to
promote the function command line switch setting to LabelTest.

https://codereview.chromium.org/2486963003/diff/20001/ui/views/controls/label...
File ui/views/controls/label_unittest.cc (right):

https://codereview.chromium.org/2486963003/diff/20001/ui/views/controls/label...
ui/views/controls/label_unittest.cc:80: // On Mac, by default RenderTextMac is
used for labels which does not
Is there a reason not to promoted this to LabelTest::SetUp and have it apply to
all?

[karandeepb, 2016-11-10 00:00:03]

PTAL sky@.

https://codereview.chromium.org/2486963003/diff/20001/ui/views/controls/label...
File ui/views/controls/label_unittest.cc (right):

https://codereview.chromium.org/2486963003/diff/20001/ui/views/controls/label...
ui/views/controls/label_unittest.cc:80: // On Mac, by default RenderTextMac is
used for labels which does not
On 2016/11/09 16:23:10, sky wrote:
> Is there a reason not to promoted this to LabelTest::SetUp and have it apply
to
> all?

For non text selection related tests, I wanted to have test coverage for
RenderTextMac as well, which is used currently. 

Alternatively, I can change LabelTest to use parameterized tests to test both
the underlying RenderText implementations for non text selection related tests.
However, since we don't actually use RTHB for Labels currently, this may not be
necessary.

[sky, 2016-11-10 00:35:00]

On 2016/11/10 00:00:03, karandeepb wrote:
> PTAL sky@.
> 
>
https://codereview.chromium.org/2486963003/diff/20001/ui/views/controls/label...
> File ui/views/controls/label_unittest.cc (right):
> 
>
https://codereview.chromium.org/2486963003/diff/20001/ui/views/controls/label...
> ui/views/controls/label_unittest.cc:80: // On Mac, by default RenderTextMac is
> used for labels which does not
> On 2016/11/09 16:23:10, sky wrote:
> > Is there a reason not to promoted this to LabelTest::SetUp and have it apply
> to
> > all?
> 
> For non text selection related tests, I wanted to have test coverage for
> RenderTextMac as well, which is used currently. 
> 
> Alternatively, I can change LabelTest to use parameterized tests to test both
> the underlying RenderText implementations for non text selection related
tests.
> However, since we don't actually use RTHB for Labels currently, this may not
be
> necessary.

It doesn't look like you uploaded a new patch. I was fine with the most recent
patch (patchset 2) so if you feel that is best land it.

[karandeepb, 2016-11-10 00:36:14]

The CQ bit was checked by karandeepb@chromium.org

[commit-bot: I haz the power, 2016-11-10 00:36:52]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-10 00:42:23]

Description was changed from

==========
views::Label: Enable text selection related tests on Mac.

Currently text selection related tests for Labels are not run on Mac since it
uses RenderTextMac by default. Enable these tests on Mac, by explicitly using
RenderTextHarfBuzz which supports text selection. To do this, create a new test 
fixture class called LabelSelectionTest and append the kEnableHarfBuzzRenderText

switch to the list of command line switches.

BUG=630365
==========

to

==========
views::Label: Enable text selection related tests on Mac.

Currently text selection related tests for Labels are not run on Mac since it
uses RenderTextMac by default. Enable these tests on Mac, by explicitly using
RenderTextHarfBuzz which supports text selection. To do this, create a new test 
fixture class called LabelSelectionTest and append the kEnableHarfBuzzRenderText

switch to the list of command line switches.

BUG=630365
==========

[commit-bot: I haz the power, 2016-11-10 00:42:23]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-10 00:45:29]

Description was changed from

==========
views::Label: Enable text selection related tests on Mac.

Currently text selection related tests for Labels are not run on Mac since it
uses RenderTextMac by default. Enable these tests on Mac, by explicitly using
RenderTextHarfBuzz which supports text selection. To do this, create a new test 
fixture class called LabelSelectionTest and append the kEnableHarfBuzzRenderText

switch to the list of command line switches.

BUG=630365
==========

to

==========
views::Label: Enable text selection related tests on Mac.

Currently text selection related tests for Labels are not run on Mac since it
uses RenderTextMac by default. Enable these tests on Mac, by explicitly using
RenderTextHarfBuzz which supports text selection. To do this, create a new test
fixture class called LabelSelectionTest and append the kEnableHarfBuzzRenderText
switch to the list of command line switches.

BUG=630365

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

[commit-bot: I haz the power, 2016-11-10 00:45:30]

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