MacViews: Implement NativeWidgetMac::SetOpacity function.

MacViews: Implement NativeWidgetMac::SetOpacity function.

BUG=454698
TEST=views_unittests --gtest_filter="NativeWidgetMacTest.ChangeOpacity"

Committed: https://crrev.com/c3801c2a06a3c72c5c34c1907db535afb129716f
Cr-Commit-Position: refs/heads/master@{#370348}

[Andrey Kraynov, 2016-01-15 11:03:56]

iceman@yandex-team.ru changed reviewers:
+ tapted@chromium.org

[Andrey Kraynov, 2016-01-15 11:03:56]

Hi!

This is an extracted part of my previous CL
(https://codereview.chromium.org/1533883002/) related to MacViews.
As you pointed, I tried to implement SetOpacity function using setAlphaValue.
Also I was able to write a quite straightforward unit-test for
BridgedNativeWidget.
Can we continue to discuss that approach?
Is it possible to write a cross-platform unit test for Widgets opacity?
Thanks in advance!

[Andrey Kraynov, 2016-01-15 11:04:59]

Description was changed from

==========
MacViews: Implement BridgedNativeWidget::SetOpacity function.

BUG=415024
TEST=views_unittests --gtest_filter="NativeWidgetMacTest.ChangeOpacity"
==========

to

==========
MacViews: Implement NativeWidgetMac::SetOpacity function.

BUG=415024
TEST=views_unittests --gtest_filter="NativeWidgetMacTest.ChangeOpacity"
==========

[tapted, 2016-01-18 00:35:15]

lgtm after a couple of nits are addressed - thanks!

On 2016/01/15 11:03:56, Andrey Kraynov wrote:
> This is an extracted part of my previous CL
> (https://codereview.chromium.org/1533883002/) related to MacViews.
> As you pointed, I tried to implement SetOpacity function using setAlphaValue.
> Also I was able to write a quite straightforward unit-test for
> BridgedNativeWidget.
> Can we continue to discuss that approach?
> Is it possible to write a cross-platform unit test for Widgets opacity?

This looks good. For a cross-platform test, we could implement something like
WidgetTest::GetNativeWindowOpacity(gfx::NativeWindow), but the test is nice and
simple, and I think there is too much variance across platforms for it to be
useful.

https://codereview.chromium.org/1590803003/diff/1/ui/views/widget/native_widg...
File ui/views/widget/native_widget_mac.mm (right):

https://codereview.chromium.org/1590803003/diff/1/ui/views/widget/native_widg...
ui/views/widget/native_widget_mac.mm:1: // Copyright 2014 The Chromium Authors.
All rights reserved.
CL description: the BUG= should be 454698

https://codereview.chromium.org/1590803003/diff/1/ui/views/widget/native_widg...
File ui/views/widget/native_widget_mac_unittest.mm (right):

https://codereview.chromium.org/1590803003/diff/1/ui/views/widget/native_widg...
ui/views/widget/native_widget_mac_unittest.mm:1097: auto old_opacity =
[ns_window alphaValue];
nit: auto->CGFloat (auto is allowed "when it helps readability" which doesn't
really apply here)

https://codereview.chromium.org/1590803003/diff/1/ui/views/widget/native_widg...
ui/views/widget/native_widget_mac_unittest.mm:1100: EXPECT_EQ(0xAA / 255.0,
[ns_window alphaValue]);
nit: EXPECT_DOUBLE_EQ

(or try EXPECT_FLOAT_EQ if that fails, but I think that would only be on 32-bit
which is no longer ever true for Mac)

[Andrey Kraynov, 2016-01-18 09:40:21]

Description was changed from

==========
MacViews: Implement NativeWidgetMac::SetOpacity function.

BUG=415024
TEST=views_unittests --gtest_filter="NativeWidgetMacTest.ChangeOpacity"
==========

to

==========
MacViews: Implement NativeWidgetMac::SetOpacity function.

BUG= 454698
TEST=views_unittests --gtest_filter="NativeWidgetMacTest.ChangeOpacity"
==========

[Andrey Kraynov, 2016-01-18 09:43:54]

Fixed, thanks!

https://codereview.chromium.org/1590803003/diff/1/ui/views/widget/native_widg...
File ui/views/widget/native_widget_mac.mm (right):

https://codereview.chromium.org/1590803003/diff/1/ui/views/widget/native_widg...
ui/views/widget/native_widget_mac.mm:1: // Copyright 2014 The Chromium Authors.
All rights reserved.
On 2016/01/18 00:35:14, tapted wrote:
> CL description: the BUG= should be 454698

Done.

https://codereview.chromium.org/1590803003/diff/1/ui/views/widget/native_widg...
File ui/views/widget/native_widget_mac_unittest.mm (right):

https://codereview.chromium.org/1590803003/diff/1/ui/views/widget/native_widg...
ui/views/widget/native_widget_mac_unittest.mm:1097: auto old_opacity =
[ns_window alphaValue];
On 2016/01/18 00:35:14, tapted wrote:
> nit: auto->CGFloat (auto is allowed "when it helps readability" which doesn't
> really apply here)

Done.

https://codereview.chromium.org/1590803003/diff/1/ui/views/widget/native_widg...
ui/views/widget/native_widget_mac_unittest.mm:1100: EXPECT_EQ(0xAA / 255.0,
[ns_window alphaValue]);
On 2016/01/18 00:35:14, tapted wrote:
> nit: EXPECT_DOUBLE_EQ
> 
> (or try EXPECT_FLOAT_EQ if that fails, but I think that would only be on
32-bit
> which is no longer ever true for Mac)

Done.

[Andrey Kraynov, 2016-01-19 09:45:23]

Hi!

Sorry to bother you, but could you take a look at the final version of this CL,
please?
I prefer not to press the commit button without an explicit approvement =)
Thanks!

[tapted, 2016-01-19 20:21:16]

No worries. It's all good :) lgtm

[Andrey Kraynov, 2016-01-20 06:31:10]

The CQ bit was checked by iceman@yandex-team.ru

[commit-bot: I haz the power, 2016-01-20 06:33:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1590803003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1590803003/20001

[commit-bot: I haz the power, 2016-01-20 08:52:31]

Description was changed from

==========
MacViews: Implement NativeWidgetMac::SetOpacity function.

BUG= 454698
TEST=views_unittests --gtest_filter="NativeWidgetMacTest.ChangeOpacity"
==========

to

==========
MacViews: Implement NativeWidgetMac::SetOpacity function.

BUG= 454698
TEST=views_unittests --gtest_filter="NativeWidgetMacTest.ChangeOpacity"
==========

[commit-bot: I haz the power, 2016-01-20 08:52:32]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-20 08:53:24]

Description was changed from

==========
MacViews: Implement NativeWidgetMac::SetOpacity function.

BUG= 454698
TEST=views_unittests --gtest_filter="NativeWidgetMacTest.ChangeOpacity"
==========

to

==========
MacViews: Implement NativeWidgetMac::SetOpacity function.

BUG= 454698
TEST=views_unittests --gtest_filter="NativeWidgetMacTest.ChangeOpacity"

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

[commit-bot: I haz the power, 2016-01-20 08:53:25]

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