MacViews: Don't set window's alpha to 0 if window is already visible.

MacViews: Don't set window's alpha to 0 if window is already visible.

Currently repeated calls to Show() a Widget cause a flash. This is because of
flag initial_visibility_suppressed_ in BridgedNativeWidget which was added in
r417863 to show some dialogs with an alpha value of 0 initially, till the
frames from the compositor arrive.

See http://crbug.com/623950#c18

To fix the problem this patch adds an additional check, so this logic would be
applied only if window isn't already visible.

BUG=654961
Committed: https://crrev.com/2b3fd8fa7e745eda9e69f49190f372aba03687d7
Cr-Commit-Position: refs/heads/master@{#424974}

[atimoxin, 2016-10-11 15:31:47]

Description was changed from

==========
Do SetVisibilityState() only if state is really changed.

This patch fixes problem described in
https://bugs.chromium.org/p/chromium/issues/detail?id=623950#c18

BUG=623950
==========

to

==========
Do SetVisibilityState() only if state is really changed.

This patch fixes problem described in
https://bugs.chromium.org/p/chromium/issues/detail?id=623950#c18

BUG=623950
==========

[atimoxin, 2016-10-11 15:31:47]

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

[tapted, 2016-10-11 23:05:16]

https://codereview.chromium.org/2407153004/diff/1/ui/views/cocoa/bridged_nati...
File ui/views/cocoa/bridged_native_widget.mm (right):

https://codereview.chromium.org/2407153004/diff/1/ui/views/cocoa/bridged_nati...
ui/views/cocoa/bridged_native_widget.mm:1: // Copyright 2014 The Chromium
Authors. All rights reserved.
for the CL description, make sure you follow the guide at
https://www.chromium.org/developers/contributing-code#TOC-Writing-change-list...

That is, you need to say the current behaviour and give rationale for the change
(the bug link helps a lot, but the change description still needs to be
self-contained). You also need to describe the fix.

E.g., you could say

MacViews: <describe change>

Currently repeated calls to Show() a Widget cause a flash. This is because
<reason>. See http://crbug.com/623950#c18

To fix <describe fix>.

BUG=623950

https://codereview.chromium.org/2407153004/diff/1/ui/views/cocoa/bridged_nati...
ui/views/cocoa/bridged_native_widget.mm:593: [NSApp
activateIgnoringOtherApps:YES];
This line (and the line above) need to happen regardless of the visibility
state. That is, a call to Show() should always raise the window to the top of
the z-order stack, even if it's already visible. I think we just need to modify
the condition on line 584 instead

[atimoxin, 2016-10-12 12:29:21]

Description was changed from

==========
Do SetVisibilityState() only if state is really changed.

This patch fixes problem described in
https://bugs.chromium.org/p/chromium/issues/detail?id=623950#c18

BUG=623950
==========

to

==========
MacViews: Don't set window's alpha to 0 if window is already visible.

Currently repeated calls to Show() a Widget cause a flash. This is because of
flag initial_visibility_suppressed_ in BridgedNativeWidget which was added in
r417863 to show some dialogs with an alpha value of 0 initially, till the
frames from the compositor arrive.

See http://crbug.com/623950#c18

To fix the problem this patch adds an additional check, so this logic would be
applied only if window isn't already visible.

BUG=654961
==========

[atimoxin, 2016-10-12 12:43:07]

On 2016/10/11 23:05:16, tapted wrote:
>
https://codereview.chromium.org/2407153004/diff/1/ui/views/cocoa/bridged_nati...
> File ui/views/cocoa/bridged_native_widget.mm (right):
> 
>
https://codereview.chromium.org/2407153004/diff/1/ui/views/cocoa/bridged_nati...
> ui/views/cocoa/bridged_native_widget.mm:1: // Copyright 2014 The Chromium
> Authors. All rights reserved.
> for the CL description, make sure you follow the guide at
>
https://www.chromium.org/developers/contributing-code#TOC-Writing-change-list...
> 
> That is, you need to say the current behaviour and give rationale for the
change
> (the bug link helps a lot, but the change description still needs to be
> self-contained). You also need to describe the fix.
> 
> E.g., you could say
> 
> MacViews: <describe change>
> 
> Currently repeated calls to Show() a Widget cause a flash. This is because
> <reason>. See http://crbug.com/623950#c18
> 
> To fix <describe fix>.
> 
> BUG=623950
> 
>
https://codereview.chromium.org/2407153004/diff/1/ui/views/cocoa/bridged_nati...
> ui/views/cocoa/bridged_native_widget.mm:593: [NSApp
> activateIgnoringOtherApps:YES];
> This line (and the line above) need to happen regardless of the visibility
> state. That is, a call to Show() should always raise the window to the top of
> the z-order stack, even if it's already visible. I think we just need to
modify
> the condition on line 584 instead

Thank you!

I fixed the CL's description and moved check to more appropriate place.

[tapted, 2016-10-13 00:20:02]

lgtm

[atimoxin, 2016-10-13 06:50:37]

The CQ bit was checked by atimoxin@yandex-team.ru to run a CQ dry run

[commit-bot: I haz the power, 2016-10-13 06:50:55]

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-10-13 07:16:21]

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

[commit-bot: I haz the power, 2016-10-13 07:16:22]

Dry run: This issue passed the CQ dry run.

[atimoxin, 2016-10-13 07:25:02]

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

[commit-bot: I haz the power, 2016-10-13 07:25:14]

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-10-13 07:33:38]

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

[commit-bot: I haz the power, 2016-10-13 07:33:39]

Failed to apply the patch.
On branch working_branch
Your branch is up-to-date with 'origin/refs/pending/heads/master'.
nothing to commit, working tree clean

[commit-bot: I haz the power, 2016-10-13 07:33:44]

Description was changed from

==========
MacViews: Don't set window's alpha to 0 if window is already visible.

Currently repeated calls to Show() a Widget cause a flash. This is because of
flag initial_visibility_suppressed_ in BridgedNativeWidget which was added in
r417863 to show some dialogs with an alpha value of 0 initially, till the
frames from the compositor arrive.

See http://crbug.com/623950#c18

To fix the problem this patch adds an additional check, so this logic would be
applied only if window isn't already visible.

BUG=654961
==========

to

==========
MacViews: Don't set window's alpha to 0 if window is already visible.

Currently repeated calls to Show() a Widget cause a flash. This is because of
flag initial_visibility_suppressed_ in BridgedNativeWidget which was added in
r417863 to show some dialogs with an alpha value of 0 initially, till the
frames from the compositor arrive.

See http://crbug.com/623950#c18

To fix the problem this patch adds an additional check, so this logic would be
applied only if window isn't already visible.

BUG=654961

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

[commit-bot: I haz the power, 2016-10-13 07:33:44]

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